From d028b5672e07a7724e9b11fdaa500518966f6c5f Mon Sep 17 00:00:00 2001 From: augier <christophe@c-henry.fr> Date: Sat, 24 Oct 2015 16:15:33 -0700 Subject: [PATCH] Fix remarks --- .../authorizations_controller.rb | 28 ++++++++----------- .../api/openid_connect/clients_controller.rb | 4 +-- .../api/openid_connect/authorization.rb | 12 ++++---- app/models/api/openid_connect/id_token.rb | 2 +- .../api/openid_connect/o_auth_application.rb | 4 +-- app/presenters/user_application_presenter.rb | 3 +- app/serializers/user_info_serializer.rb | 2 +- .../endpoint_start_point.rb | 2 +- .../invalid_redirect_uri.rb | 2 +- .../invalid_sector_identifier_uri.rb | 2 +- .../subject_identifier_creator.rb | 4 +-- 11 files changed, 30 insertions(+), 35 deletions(-) rename lib/api/openid_connect/{exception => error}/invalid_redirect_uri.rb (90%) rename lib/api/openid_connect/{exception => error}/invalid_sector_identifier_uri.rb (90%) diff --git a/app/controllers/api/openid_connect/authorizations_controller.rb b/app/controllers/api/openid_connect/authorizations_controller.rb index 2f1b605656..0c7eb78a2d 100644 --- a/app/controllers/api/openid_connect/authorizations_controller.rb +++ b/app/controllers/api/openid_connect/authorizations_controller.rb @@ -55,7 +55,7 @@ module Api def handle_prompt(prompt, auth) if prompt.include? "select_account" handle_params_error("account_selection_required", - "There is no support for choosing among multiple accounts") + "There is no support for choosing among multiple accounts") elsif prompt.include? "none" handle_prompt_none(prompt, auth) elsif prompt.include?("login") && logged_in_before?(60) @@ -92,9 +92,9 @@ module Api return unless claims_json claims_array = claims_json["userinfo"].try(:keys) return unless claims_array - claims = claims_array.join(" ") req = build_rack_request - req.update_param("scope", req[:scope] + " " + claims) + claims = claims_array.unshift(req[:scope]).join(" ") + req.update_param("scope", claims) end def logged_in_before?(seconds) @@ -111,16 +111,16 @@ module Api process_authorization_consent("true") else handle_params_error("interaction_required", - "The Authentication Request cannot be completed without end-user interaction") + "The Authentication Request cannot be completed without end-user interaction") end else handle_params_error("invalid_request", - "The 'none' value cannot be used with any other prompt value") + "The 'none' value cannot be used with any other prompt value") end end def handle_start_point_response(endpoint) - _status, header, response = *endpoint.call(request.env) + _status, header, response = endpoint.call(request.env) if response.redirect? redirect_to header["Location"] else @@ -129,10 +129,10 @@ module Api end def save_params_and_render_consent_form(endpoint) - @o_auth_application, @response_type, @redirect_uri, @scopes = *[ - endpoint.o_auth_application, endpoint.response_type, - endpoint.redirect_uri, endpoint.scopes - ] + @o_auth_application = endpoint.o_auth_application + @response_type = endpoint.response_type + @redirect_uri = endpoint.redirect_uri + @scopes = endpoint.scopes save_request_parameters @app = UserApplicationPresenter.new @o_auth_application, @scopes render :new @@ -157,7 +157,7 @@ module Api end def handle_confirmation_endpoint_response(endpoint) - _status, header, _response = *endpoint.call(request.env) + _status, header, _response = endpoint.call(request.env) delete_authorization_session_variables redirect_to header["Location"] end @@ -188,11 +188,7 @@ module Api end def response_type_as_space_seperated_values - if session[:response_type].respond_to?(:map) - session[:response_type].join(" ") - else - session[:response_type] - end + [*session[:response_type]].join(" ") end def handle_params_error(error, error_description) diff --git a/app/controllers/api/openid_connect/clients_controller.rb b/app/controllers/api/openid_connect/clients_controller.rb index d1f56dabeb..38eabc92cb 100644 --- a/app/controllers/api/openid_connect/clients_controller.rb +++ b/app/controllers/api/openid_connect/clients_controller.rb @@ -6,11 +6,11 @@ module Api end rescue_from OpenIDConnect::ValidationFailed, - ActiveRecord::RecordInvalid, Api::OpenidConnect::Exception::InvalidSectorIdentifierUri do |e| + ActiveRecord::RecordInvalid, Api::OpenidConnect::Error::InvalidSectorIdentifierUri do |e| validation_fail_as_json(e) end - rescue_from Api::OpenidConnect::Exception::InvalidRedirectUri do |e| + rescue_from Api::OpenidConnect::Error::InvalidRedirectUri do |e| validation_fail_redirect_uri(e) end diff --git a/app/models/api/openid_connect/authorization.rb b/app/models/api/openid_connect/authorization.rb index cff556fa77..ea73cfb79e 100644 --- a/app/models/api/openid_connect/authorization.rb +++ b/app/models/api/openid_connect/authorization.rb @@ -4,9 +4,8 @@ module Api belongs_to :user belongs_to :o_auth_application - validates :user, presence: true + validates :user, presence: true, uniqueness: {scope: :o_auth_application} validates :o_auth_application, presence: true - validates :user, uniqueness: {scope: :o_auth_application} validate :validate_scope_names serialize :scopes, JSON @@ -38,8 +37,7 @@ module Api def create_code SecureRandom.hex(32).tap do |code| - self.code = code - save + update!(code: code) end end @@ -52,13 +50,13 @@ module Api end def self.find_by_client_id_and_user(client_id, user) - app = Api::OpenidConnect::OAuthApplication.find_by(client_id: client_id) + app = Api::OpenidConnect::OAuthApplication.where(client_id: client_id) find_by(o_auth_application: app, user: user) end def self.find_by_refresh_token(client_id, refresh_token) - Api::OpenidConnect::Authorization.joins(:o_auth_application).find_by( - o_auth_applications: {client_id: client_id}, refresh_token: refresh_token) + app = Api::OpenidConnect::OAuthApplication.where(client_id: client_id) + find_by(o_auth_application: app, refresh_token: refresh_token) end def self.use_code(code) diff --git a/app/models/api/openid_connect/id_token.rb b/app/models/api/openid_connect/id_token.rb index b6d3303760..92a0244c8b 100644 --- a/app/models/api/openid_connect/id_token.rb +++ b/app/models/api/openid_connect/id_token.rb @@ -39,7 +39,7 @@ module Api end def build_sub - Api::OpenidConnect::SubjectIdentifierCreator.createSub(authorization) + Api::OpenidConnect::SubjectIdentifierCreator.create(authorization) end end end diff --git a/app/models/api/openid_connect/o_auth_application.rb b/app/models/api/openid_connect/o_auth_application.rb index 0004a93ad2..e42b7fb518 100644 --- a/app/models/api/openid_connect/o_auth_application.rb +++ b/app/models/api/openid_connect/o_auth_application.rb @@ -56,7 +56,7 @@ module Api redirect_uris = attributes[:redirect_uris] sector_identifier_uri_includes_redirect_uris = (redirect_uris - sector_identifier_uri_json).empty? return if sector_identifier_uri_includes_redirect_uris - raise Api::OpenidConnect::Exception::InvalidSectorIdentifierUri.new + raise Api::OpenidConnect::Error::InvalidSectorIdentifierUri.new end def check_redirect_uris(attributes) @@ -64,7 +64,7 @@ module Api uri_array = redirect_uris.map {|uri| URI(uri) } any_uri_contains_fragment = uri_array.any? {|uri| !uri.fragment.nil? } return unless any_uri_contains_fragment - raise Api::OpenidConnect::Exception::InvalidRedirectUri.new + raise Api::OpenidConnect::Error::InvalidRedirectUri.new end def supported_metadata diff --git a/app/presenters/user_application_presenter.rb b/app/presenters/user_application_presenter.rb index 7930d0808a..ad147626a6 100644 --- a/app/presenters/user_application_presenter.rb +++ b/app/presenters/user_application_presenter.rb @@ -43,6 +43,7 @@ class UserApplicationPresenter def url client_redirect = URI(@app.redirect_uris[0]) - "#{client_redirect.scheme}://#{client_redirect.host}" + client_redirect.path = "/" + client_redirect.to_s end end diff --git a/app/serializers/user_info_serializer.rb b/app/serializers/user_info_serializer.rb index 7e75206936..4ef29f3b7c 100644 --- a/app/serializers/user_info_serializer.rb +++ b/app/serializers/user_info_serializer.rb @@ -3,7 +3,7 @@ class UserInfoSerializer < ActiveModel::Serializer def sub auth = serialization_options[:authorization] - Api::OpenidConnect::SubjectIdentifierCreator.createSub(auth) + Api::OpenidConnect::SubjectIdentifierCreator.create(auth) end def name diff --git a/lib/api/openid_connect/authorization_point/endpoint_start_point.rb b/lib/api/openid_connect/authorization_point/endpoint_start_point.rb index 87fd005fce..007a2c592c 100644 --- a/lib/api/openid_connect/authorization_point/endpoint_start_point.rb +++ b/lib/api/openid_connect/authorization_point/endpoint_start_point.rb @@ -16,7 +16,7 @@ module Api def replace_profile_scope_with_specific_claims(req) profile_claims = %w(sub aud name nickname profile picture) - scopes_as_claims = req.scope.map {|scope| scope == "profile" ? profile_claims : [scope] }.flatten!.uniq + scopes_as_claims = req.scope.flat_map {|scope| scope == "profile" ? profile_claims : [scope] }.uniq req.update_param("scope", scopes_as_claims) end diff --git a/lib/api/openid_connect/exception/invalid_redirect_uri.rb b/lib/api/openid_connect/error/invalid_redirect_uri.rb similarity index 90% rename from lib/api/openid_connect/exception/invalid_redirect_uri.rb rename to lib/api/openid_connect/error/invalid_redirect_uri.rb index a407505b4d..2cb5e3894d 100644 --- a/lib/api/openid_connect/exception/invalid_redirect_uri.rb +++ b/lib/api/openid_connect/error/invalid_redirect_uri.rb @@ -1,6 +1,6 @@ module Api module OpenidConnect - module Exception + module Error class InvalidRedirectUri < ::ArgumentError def initialize super "Redirect uri contains fragment" diff --git a/lib/api/openid_connect/exception/invalid_sector_identifier_uri.rb b/lib/api/openid_connect/error/invalid_sector_identifier_uri.rb similarity index 90% rename from lib/api/openid_connect/exception/invalid_sector_identifier_uri.rb rename to lib/api/openid_connect/error/invalid_sector_identifier_uri.rb index 6383aee22b..2431e1445c 100644 --- a/lib/api/openid_connect/exception/invalid_sector_identifier_uri.rb +++ b/lib/api/openid_connect/error/invalid_sector_identifier_uri.rb @@ -1,6 +1,6 @@ module Api module OpenidConnect - module Exception + module Error class InvalidSectorIdentifierUri < ::ArgumentError def initialize super "Invalid sector identifier uri" diff --git a/lib/api/openid_connect/subject_identifier_creator.rb b/lib/api/openid_connect/subject_identifier_creator.rb index 3c39e93a1a..b6a771fa0c 100644 --- a/lib/api/openid_connect/subject_identifier_creator.rb +++ b/lib/api/openid_connect/subject_identifier_creator.rb @@ -1,7 +1,7 @@ module Api module OpenidConnect - class SubjectIdentifierCreator - def self.createSub(auth) + module SubjectIdentifierCreator + def self.create(auth) if auth.o_auth_application.ppid? identifier = auth.o_auth_application.sector_identifier_uri || URI.parse(auth.o_auth_application.redirect_uris[0]).host -- GitLab