diff --git a/app/controllers/api/openid_connect/authorizations_controller.rb b/app/controllers/api/openid_connect/authorizations_controller.rb index 0a8459f2b..2f1b60565 100644 --- a/app/controllers/api/openid_connect/authorizations_controller.rb +++ b/app/controllers/api/openid_connect/authorizations_controller.rb @@ -197,20 +197,24 @@ module Api def handle_params_error(error, error_description) if params[:client_id] && params[:redirect_uri] - app = Api::OpenidConnect::OAuthApplication.find_by(client_id: params[:client_id]) - if app && app.redirect_uris.include?(params[:redirect_uri]) - redirect_prompt_error_display(error, error_description) - else - flash[:error] = I18n.t("api.openid_connect.authorizations.new.client_id_not_found", - client_id: params[:client_id], redirect_uri: params[:redirect_uri]) - redirect_to root_path - end + handle_params_error_when_client_id_and_redirect_uri_exists(error, error_description) else flash[:error] = I18n.t("api.openid_connect.authorizations.new.bad_request") redirect_to root_path end end + def handle_params_error_when_client_id_and_redirect_uri_exists(error, error_description) + app = Api::OpenidConnect::OAuthApplication.find_by(client_id: params[:client_id]) + if app && app.redirect_uris.include?(params[:redirect_uri]) + redirect_prompt_error_display(error, error_description) + else + flash[:error] = I18n.t("api.openid_connect.authorizations.new.client_id_not_found", + client_id: params[:client_id], redirect_uri: params[:redirect_uri]) + redirect_to root_path + end + end + def redirect_prompt_error_display(error, error_description) redirect_params_hash = {error: error, error_description: error_description, state: params[:state]} redirect_fragment = redirect_params_hash.compact.map {|key, value| key.to_s + "=" + value }.join("&") @@ -219,7 +223,7 @@ module Api def auth_user_unless_prompt_none! if params[:prompt] == "none" && !user_signed_in? - render json: {error: "login_required", + render json: {error: "login_required", description: "User must be first logged in when `prompt` is `none`"} else authenticate_user! diff --git a/app/controllers/api/openid_connect/user_info_controller.rb b/app/controllers/api/openid_connect/user_info_controller.rb index bcd639401..666c2a48e 100644 --- a/app/controllers/api/openid_connect/user_info_controller.rb +++ b/app/controllers/api/openid_connect/user_info_controller.rb @@ -10,8 +10,9 @@ module Api def show serializer = UserInfoSerializer.new(current_user) auth = current_token.authorization - serializer.serialization_options = { authorization: auth } - attributes_without_essential = serializer.attributes.with_indifferent_access.select{|scope| auth.scopes.include? scope } + serializer.serialization_options = {authorization: auth} + attributes_without_essential = + serializer.attributes.with_indifferent_access.select {|scope| auth.scopes.include? scope } attributes = attributes_without_essential.merge( sub: serializer.sub) render json: attributes.to_json diff --git a/features/desktop/oidc_auth_code_flow.feature b/features/desktop/oidc_auth_code_flow.feature index 7a25bca72..c322ac484 100644 --- a/features/desktop/oidc_auth_code_flow.feature +++ b/features/desktop/oidc_auth_code_flow.feature @@ -7,7 +7,7 @@ Feature: Access protected resources using auth code flow When I register a new client And I send a post request from that client to the code flow authorization endpoint using a invalid client id And I sign in as "kent@kent.kent" - Then I should see an "bad_request" error + Then I should see a flash message containing "No client with" Scenario: Application is denied authorization When I register a new client diff --git a/features/desktop/oidc_implicit_flow.feature b/features/desktop/oidc_implicit_flow.feature index 15565ac20..abb2a2b91 100644 --- a/features/desktop/oidc_implicit_flow.feature +++ b/features/desktop/oidc_implicit_flow.feature @@ -7,7 +7,7 @@ Feature: Access protected resources using implicit flow When I register a new client And I send a post request from that client to the authorization endpoint using a invalid client id And I sign in as "kent@kent.kent" - Then I should see an "bad_request" error + Then I should see a flash message containing "No client with" Scenario: Application is denied authorization When I register a new client diff --git a/features/step_definitions/auth_code_steps.rb b/features/step_definitions/auth_code_steps.rb index bec5caa34..8b19f7f52 100644 --- a/features/step_definitions/auth_code_steps.rb +++ b/features/step_definitions/auth_code_steps.rb @@ -1,7 +1,7 @@ O_AUTH_QUERY_PARAMS_WITH_CODE = { redirect_uri: "http://localhost:3000", response_type: "code", - scope: "openid read", + scope: "openid profile read", nonce: "hello", state: "hi" } diff --git a/features/step_definitions/implicit_flow_steps.rb b/features/step_definitions/implicit_flow_steps.rb index bbbc0462a..f1603ec1a 100644 --- a/features/step_definitions/implicit_flow_steps.rb +++ b/features/step_definitions/implicit_flow_steps.rb @@ -1,7 +1,7 @@ O_AUTH_QUERY_PARAMS = { redirect_uri: "http://localhost:3000", response_type: "id_token token", - scope: "openid read", + scope: "openid profile read", nonce: "hello", state: "hi", prompt: "login" @@ -10,7 +10,7 @@ O_AUTH_QUERY_PARAMS = { O_AUTH_QUERY_PARAMS_WITH_MAX_AGE = { redirect_uri: "http://localhost:3000", response_type: "id_token token", - scope: "openid read", + scope: "openid profile read", nonce: "hello", state: "hi", prompt: "login", diff --git a/lib/api/openid_connect/authorization_point/endpoint_confirmation_point.rb b/lib/api/openid_connect/authorization_point/endpoint_confirmation_point.rb index c411b293b..08e99b25d 100644 --- a/lib/api/openid_connect/authorization_point/endpoint_confirmation_point.rb +++ b/lib/api/openid_connect/authorization_point/endpoint_confirmation_point.rb @@ -19,11 +19,11 @@ module Api end end - def replace_profile_scope_with_specific_claims(req) + def replace_profile_scope_with_specific_claims(_req) # Empty end - def build_from_request_object(req) + def build_from_request_object(_req) # Empty end 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 3ce63cacf..87fd005fc 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.map {|scope| scope == "profile" ? profile_claims : [scope] }.flatten!.uniq req.update_param("scope", scopes_as_claims) end @@ -27,8 +27,6 @@ module Api OpenIDConnect::RequestObject.fetch req.request_uri elsif req.request.present? OpenIDConnect::RequestObject.decode req.request - else - nil end end end diff --git a/spec/controllers/api/openid_connect/authorizations_controller_spec.rb b/spec/controllers/api/openid_connect/authorizations_controller_spec.rb index 0e48a4404..3014b9309 100644 --- a/spec/controllers/api/openid_connect/authorizations_controller_spec.rb +++ b/spec/controllers/api/openid_connect/authorizations_controller_spec.rb @@ -25,8 +25,8 @@ describe Api::OpenidConnect::AuthorizationsController, type: :controller do context "using claims" do it "should return a form page" do get :new, client_id: client.client_id, redirect_uri: "http://localhost:3000/", response_type: "id_token", - scope: "openid", claims: "{\"userinfo\": {\"name\": {\"essential\": true}}}", nonce: SecureRandom.hex(16), - state: SecureRandom.hex(16) + scope: "openid", claims: "{\"userinfo\": {\"name\": {\"essential\": true}}}", + nonce: SecureRandom.hex(16), state: SecureRandom.hex(16) expect(response.body).to match("Diaspora Test Client") end end @@ -34,8 +34,9 @@ describe Api::OpenidConnect::AuthorizationsController, type: :controller do context "as a request object" do it "should return a form page" do header = JWT.encoded_header("none") - payload_hash = { client_id: client.client_id, redirect_uri: "http://localhost:3000/", response_type: "id_token", - scope: "openid", nonce: "hello", state: "hello", claims: { userinfo: { name: { essential: true } } } } + payload_hash = {client_id: client.client_id, redirect_uri: "http://localhost:3000/", + response_type: "id_token", scope: "openid", nonce: "hello", state: "hello", + claims: {userinfo: {name: {essential: true}}}} payload = JWT.encoded_payload(JSON.parse(payload_hash.to_json)) request_object = header + "." + payload + "." get :new, client_id: client.client_id, redirect_uri: "http://localhost:3000/", response_type: "id_token", @@ -47,8 +48,8 @@ describe Api::OpenidConnect::AuthorizationsController, type: :controller do context "as a request object with no claims" do it "should return a form page" do header = JWT.encoded_header("none") - payload_hash = { client_id: client.client_id, redirect_uri: "http://localhost:3000/", - response_type: "id_token", scope: "openid", nonce: "hello", state: "hello" } + payload_hash = {client_id: client.client_id, redirect_uri: "http://localhost:3000/", + response_type: "id_token", scope: "openid", nonce: "hello", state: "hello"} payload = JWT.encoded_payload(JSON.parse(payload_hash.to_json)) request_object = header + "." + payload + "." get :new, client_id: client.client_id, redirect_uri: "http://localhost:3000/", response_type: "id_token", diff --git a/spec/controllers/api/openid_connect/clients_controller_spec.rb b/spec/controllers/api/openid_connect/clients_controller_spec.rb index d124ff042..a67f7f61f 100644 --- a/spec/controllers/api/openid_connect/clients_controller_spec.rb +++ b/spec/controllers/api/openid_connect/clients_controller_spec.rb @@ -5,8 +5,8 @@ describe Api::OpenidConnect::ClientsController, type: :controller do context "when valid parameters are passed" do it "should return a client id" do stub_request(:get, "http://example.com/uris") - .with(headers: {"Accept" => "*/*", "Accept-Encoding" => "gzip;q=1.0,deflate;q=0.6,identity;q=0.3", - "Host" => "example.com", "User-Agent" => "Ruby"}) + .with(headers: {:Accept => "*/*", :"Accept-Encoding" => "gzip;q=1.0,deflate;q=0.6,identity;q=0.3", + :Host => "example.com", :"User-Agent" => "Ruby"}) .to_return(status: 200, body: "[\"http://localhost\"]", headers: {}) post :create, redirect_uris: ["http://localhost"], client_name: "diaspora client", response_types: [], grant_types: [], application_type: "web", contacts: [], @@ -22,8 +22,8 @@ describe Api::OpenidConnect::ClientsController, type: :controller do context "when valid parameters with jwks is passed" do it "should return a client id" do stub_request(:get, "http://example.com/uris") - .with(headers: {"Accept" => "*/*", "Accept-Encoding" => "gzip;q=1.0,deflate;q=0.6,identity;q=0.3", - "Host" => "example.com", "User-Agent" => "Ruby"}) + .with(headers: {:Accept => "*/*", :"Accept-Encoding" => "gzip;q=1.0,deflate;q=0.6,identity;q=0.3", + :Host => "example.com", :"User-Agent" => "Ruby"}) .to_return(status: 200, body: "[\"http://localhost\"]", headers: {}) post :create, redirect_uris: ["http://localhost"], client_name: "diaspora client", response_types: [], grant_types: [], application_type: "web", contacts: [], @@ -31,42 +31,43 @@ describe Api::OpenidConnect::ClientsController, type: :controller do policy_uri: "http://example.com/policy", tos_uri: "http://example.com/tos", sector_identifier_uri: "http://example.com/uris", subject_type: "pairwise", token_endpoint_auth_method: "private_key_jwt", - "jwks": { - "keys": + jwks: { + keys: [ { - "use": "enc", - "e": "AQAB", - "d": "-lTBWkI-----lvCO6tuiDsR4qgJnUwnndQFwEI_4mLmD3iNWXrc8N--5Cjq55eLtuJjtvuQ", - "n": "--zYRQNDvIVsBDLQQIgrbctuGqj6lrXb31Jj3JIEYqH_4h5X9d0Q", - "q": "1q-r----pFtyTz_JksYYaotc_Z3Zy-Szw6a39IDbuYGy1qL-15oQuc", - "p": "-BfRjdgYouy4c6xAnGDgSMTip1YnPRyvbMaoYT9E_tEcBW5wOeoc", - "kid": "a0", - "kty": "RSA" - }, - {"use": "sig", - "e": "AQAB", - "d": "--x-gW---LRPowKrdvTuTo2p--HMI0pIEeFs7H_u5OW3jihjvoFClGPynHQhgWmQzlQRvWRXh6FhDVqFeGQ", - "n": "---TyeadDqQPWgbqX69UzcGq5irhzN8cpZ_JaTk3Y_uV6owanTZLVvCgdjaAnMYeZhb0KFw", - "q": "5E5XKK5njT--Hx3nF5sne5fleVfU-sZy6Za4B2U75PcE62oZgCPauOTAEm9Xuvrt5aMMovyzR8ecJZhm9bw7naU", - "p": "-BUGA-", - "kid": "a1", - "kty": "RSA"}, - { - "use": "sig", - "crv": "P-256", - "kty": "EC", - "y": "Yg4IRzHBMIsuQK2Oz0Uukp1aNDnpdoyk6QBMtmfGHQQ", - "x": "L0WUeVlc9r6YJd6ie9duvOU1RHwxSkJKA37IK9B4Bpc", - "kid": "a2" + use: "enc", + e: "AQAB", + d: "-lTBWkI-----lvCO6tuiDsR4qgJnUwnndQFwEI_4mLmD3iNWXrc8N--5Cjq55eLtuJjtvuQ", + n: "--zYRQNDvIVsBDLQQIgrbctuGqj6lrXb31Jj3JIEYqH_4h5X9d0Q", + q: "1q-r----pFtyTz_JksYYaotc_Z3Zy-Szw6a39IDbuYGy1qL-15oQuc", + p: "-BfRjdgYouy4c6xAnGDgSMTip1YnPRyvbMaoYT9E_tEcBW5wOeoc", + kid: "a0", + kty: "RSA" }, { - "use": "enc", - "crv": "P-256", - "kty": "EC", - "y": "E6E6g5_ziIZvfdAoACctnwOhuQYMvQzA259aftPn59M", - "x": "Yu8_BQE2L0f1MqnK0GumZOaj_77Tx70-LoudyRUnLM4", - "kid": "a3" + use: "sig", + e: "AQAB", + d: "--x-gW---LRPowKrdvTuTo2p--HMI0pIEeFs7H_u5OW3jihjvoFClGPynHQhgWmQzlQRvWRXh6FhDVqFeGQ", + n: "---TyeadDqQPWgbqX69UzcGq5irhzN8cpZ_JaTk3Y_uV6owanTZLVvCgdjaAnMYeZhb0KFw", + q: "5E5XKK5njT--Hx3nF5sne5fleVfU-sZy6Za4B2U75PcE62oZgCPauOTAEm9Xuvrt5aMMovyzR8ecJZhm9bw7naU", + p: "-BUGA-", + kid: "a1", + kty: "RSA"}, + { + use: "sig", + crv: "P-256", + kty: "EC", + y: "Yg4IRzHBMIsuQK2Oz0Uukp1aNDnpdoyk6QBMtmfGHQQ", + x: "L0WUeVlc9r6YJd6ie9duvOU1RHwxSkJKA37IK9B4Bpc", + kid: "a2" + }, + { + use: "enc", + crv: "P-256", + kty: "EC", + y: "E6E6g5_ziIZvfdAoACctnwOhuQYMvQzA259aftPn59M", + x: "Yu8_BQE2L0f1MqnK0GumZOaj_77Tx70-LoudyRUnLM4", + kid: "a3" } ] } @@ -80,11 +81,11 @@ describe Api::OpenidConnect::ClientsController, type: :controller do it "should return a client id" do stub_request(:get, "http://example.com/uris") .with(headers: {:Accept => "*/*", :"Accept-Encoding" => "gzip;q=1.0,deflate;q=0.6,identity;q=0.3", - "Host" => "example.com", :"User-Agent" => "Ruby"}) + :Host => "example.com", :"User-Agent" => "Ruby"}) .to_return(status: 200, body: "[\"http://localhost\"]", headers: {}) stub_request(:get, "https://kentshikama.com/api/openid_connect/jwks.json") - .with(headers: {"Accept": "*/*", "Accept-Encoding": "gzip;q=1.0,deflate;q=0.6,identity;q=0.3", - "Host": "kentshikama.com", "User-Agent": "Ruby"}) + .with(headers: {:Accept => "*/*", :"Accept-Encoding" => "gzip;q=1.0,deflate;q=0.6,identity;q=0.3", + :Host => "kentshikama.com", :"User-Agent" => "Ruby"}) .to_return(status: 200, body: "{\"keys\":[{\"kty\":\"RSA\",\"e\":\"AQAB\",\"n\":\"qpW\",\"use\":\"sig\"}]}", headers: {}) post :create, redirect_uris: ["http://localhost"], client_name: "diaspora client",