diff --git a/app/controllers/api/openid_connect/authorizations_controller.rb b/app/controllers/api/openid_connect/authorizations_controller.rb index 7103ad95ef1..cc2e6a0a62e 100644 --- a/app/controllers/api/openid_connect/authorizations_controller.rb +++ b/app/controllers/api/openid_connect/authorizations_controller.rb @@ -125,6 +125,7 @@ def save_request_parameters session[:response_type] = @response_type session[:redirect_uri] = @redirect_uri session[:scopes] = scopes_as_space_seperated_values + session[:state] = params[:state] session[:nonce] = params[:nonce] end @@ -149,6 +150,7 @@ def delete_authorization_session_variables session.delete(:response_type) session.delete(:redirect_uri) session.delete(:scopes) + session.delete(:state) session.delete(:nonce) end @@ -162,6 +164,7 @@ def restore_request_parameters req.update_param("redirect_uri", session[:redirect_uri]) req.update_param("response_type", response_type_as_space_seperated_values) req.update_param("scope", session[:scopes]) + req.update_param("state", session[:state]) req.update_param("nonce", session[:nonce]) end diff --git a/spec/controllers/api/openid_connect/authorizations_controller_spec.rb b/spec/controllers/api/openid_connect/authorizations_controller_spec.rb index 41f49e53d29..f32ae09570e 100644 --- a/spec/controllers/api/openid_connect/authorizations_controller_spec.rb +++ b/spec/controllers/api/openid_connect/authorizations_controller_spec.rb @@ -1,4 +1,4 @@ -describe Api::OpenidConnect::AuthorizationsController, type: :controller do +describe Api::OpenidConnect::AuthorizationsController, type: :request do let!(:client) { FactoryGirl.create(:o_auth_application) } let!(:client_with_xss) { FactoryGirl.create(:o_auth_application_with_xss) } let!(:client_with_multiple_redirects) { FactoryGirl.create(:o_auth_application_with_multiple_redirects) } @@ -10,10 +10,10 @@ describe "#new" do context "when not yet authorized" do context "when valid parameters are passed" do - render_views context "as GET request" do it "should return a form page" do - get :new, client_id: client.client_id, redirect_uri: "http://localhost:3000/", response_type: "id_token", + get new_api_openid_connect_authorization_path, client_id: client.client_id, + redirect_uri: "http://localhost:3000/", response_type: "id_token", scope: "openid", nonce: SecureRandom.hex(16), state: SecureRandom.hex(16) expect(response.body).to match("Diaspora Test Client") end @@ -21,7 +21,8 @@ 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", + get new_api_openid_connect_authorization_path, 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) expect(response.body).to match("Diaspora Test Client") @@ -36,7 +37,8 @@ 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", + get new_api_openid_connect_authorization_path, client_id: client.client_id, + redirect_uri: "http://localhost:3000/", response_type: "id_token", scope: "openid", nonce: "hello", state: "hello", request: request_object expect(response.body).to match("Diaspora Test Client") end @@ -49,7 +51,8 @@ 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", + get new_api_openid_connect_authorization_path, client_id: client.client_id, + redirect_uri: "http://localhost:3000/", response_type: "id_token", scope: "openid", nonce: "hello", state: "hello", request: request_object expect(response.body).to match("Diaspora Test Client") end @@ -57,7 +60,8 @@ context "as POST request" do it "should return a form page" do - post :new, client_id: client.client_id, redirect_uri: "http://localhost:3000/", response_type: "id_token", + post api_openid_connect_authorizations_new_path, client_id: client.client_id, + redirect_uri: "http://localhost:3000/", response_type: "id_token", scope: "openid", nonce: SecureRandom.hex(16), state: SecureRandom.hex(16) expect(response.body).to match("Diaspora Test Client") end @@ -66,7 +70,8 @@ context "when client id is missing" do it "should return an bad request error" do - post :new, redirect_uri: "http://localhost:3000/", response_type: "id_token", + post api_openid_connect_authorizations_new_path, + redirect_uri: "http://localhost:3000/", response_type: "id_token", scope: "openid", nonce: SecureRandom.hex(16), state: SecureRandom.hex(16) expect(response.body).to include("The request was malformed") end @@ -79,7 +84,7 @@ # When client has only one redirect uri registered, only that redirect uri can be used. Hence, # we should implicitly assume the client wants to use that registered URI. # See https://github.com/nov/rack-oauth2/blob/master/lib/rack/oauth2/server/authorize.rb#L63 - post :new, client_id: client.client_id, response_type: "id_token", + post api_openid_connect_authorizations_new_path, client_id: client.client_id, response_type: "id_token", scope: "openid", nonce: SecureRandom.hex(16), state: SecureRandom.hex(16) expect(response.body).to match("Diaspora Test Client") end @@ -88,7 +93,8 @@ context "when multiple redirect URLs are pre-registered" do it "should return an invalid request error" do - post :new, client_id: client_with_multiple_redirects.client_id, response_type: "id_token", + post api_openid_connect_authorizations_new_path, client_id: client_with_multiple_redirects.client_id, + response_type: "id_token", scope: "openid", nonce: SecureRandom.hex(16), state: SecureRandom.hex(16) expect(response.body).to include("The request was malformed") end @@ -96,7 +102,8 @@ context "when redirect URI does not match pre-registered URIs" do it "should return an invalid request error" do - post :new, client_id: client.client_id, redirect_uri: "http://localhost:2000/", + post api_openid_connect_authorizations_new_path, client_id: client.client_id, + redirect_uri: "http://localhost:2000/", response_type: "id_token", scope: "openid", nonce: SecureRandom.hex(16) expect(response.body).to include("Invalid client id or redirect uri") end @@ -104,7 +111,8 @@ context "when an unsupported scope is passed in" do it "should return an invalid scope error" do - post :new, client_id: client.client_id, redirect_uri: "http://localhost:3000/", response_type: "id_token", + post api_openid_connect_authorizations_new_path, client_id: client.client_id, + redirect_uri: "http://localhost:3000/", response_type: "id_token", scope: "random", nonce: SecureRandom.hex(16), state: SecureRandom.hex(16) expect(response.body).to match("error=invalid_scope") end @@ -112,7 +120,8 @@ context "when nonce is missing" do it "should return an invalid request error" do - post :new, client_id: client.client_id, redirect_uri: "http://localhost:3000/", + post api_openid_connect_authorizations_new_path, client_id: client.client_id, + redirect_uri: "http://localhost:3000/", response_type: "id_token", scope: "openid", state: SecureRandom.hex(16) expect(response.location).to match("error=invalid_request") end @@ -120,7 +129,8 @@ context "when prompt is none" do it "should return an interaction required error" do - post :new, client_id: client.client_id, redirect_uri: "http://localhost:3000/", + post api_openid_connect_authorizations_new_path, client_id: client.client_id, + redirect_uri: "http://localhost:3000/", response_type: "id_token", scope: "openid", state: 1234, display: "page", prompt: "none" expect(response.body).to include("User must already be authorized when `prompt` is `none`") end @@ -132,7 +142,8 @@ end it "should return an interaction required error" do - post :new, client_id: client.client_id, redirect_uri: "http://localhost:3000/", + post api_openid_connect_authorizations_new_path, client_id: client.client_id, + redirect_uri: "http://localhost:3000/", response_type: "id_token", scope: "openid", state: 1234, display: "page", prompt: "none" expect(response.body).to include("User must already be logged in when `prompt` is `none`") end @@ -140,7 +151,8 @@ context "when prompt is none and consent" do it "should return an interaction required error" do - post :new, client_id: client.client_id, redirect_uri: "http://localhost:3000/", + post api_openid_connect_authorizations_new_path, client_id: client.client_id, + redirect_uri: "http://localhost:3000/", response_type: "id_token", scope: "openid", state: 1234, display: "page", prompt: "none consent" expect(response.location).to match("error=invalid_request") end @@ -148,7 +160,8 @@ context "when prompt is select_account" do it "should return an account_selection_required error" do - post :new, client_id: client.client_id, redirect_uri: "http://localhost:3000/", + post api_openid_connect_authorizations_new_path, client_id: client.client_id, + redirect_uri: "http://localhost:3000/", response_type: "id_token", scope: "openid", state: 1234, display: "page", prompt: "select_account" expect(response.location).to match("error=account_selection_required") expect(response.location).to match("state=1234") @@ -157,7 +170,7 @@ context "when prompt is none and client ID is invalid" do it "should return an account_selection_required error" do - post :new, client_id: "random", redirect_uri: "http://localhost:3000/", + post api_openid_connect_authorizations_new_path, client_id: "random", redirect_uri: "http://localhost:3000/", response_type: "id_token", scope: "openid", state: 1234, display: "page", prompt: "none" expect(response.body).to include("Invalid client id or redirect uri") end @@ -165,7 +178,8 @@ context "when prompt is none and redirect URI does not match pre-registered URIs" do it "should return an account_selection_required error" do - post :new, client_id: client.client_id, redirect_uri: "http://randomuri:3000/", + post api_openid_connect_authorizations_new_path, client_id: client.client_id, + redirect_uri: "http://randomuri:3000/", response_type: "id_token", scope: "openid", state: 1234, display: "page", prompt: "none" expect(response.body).to include("Invalid client id or redirect uri") end @@ -173,7 +187,8 @@ context "when XSS script is passed as name" do it "should escape html" do - post :new, client_id: client_with_xss.client_id, redirect_uri: "http://localhost:3000/", + post api_openid_connect_authorizations_new_path, client_id: client_with_xss.client_id, + redirect_uri: "http://localhost:3000/", response_type: "id_token", scope: "openid", nonce: SecureRandom.hex(16), state: SecureRandom.hex(16) expect(response.body).to_not include("") end @@ -188,12 +203,13 @@ context "when valid parameters are passed" do before do - get :new, client_id: client.client_id, redirect_uri: "http://localhost:3000/", response_type: "id_token", + get new_api_openid_connect_authorization_path, client_id: client.client_id, + redirect_uri: "http://localhost:3000/", response_type: "id_token", scope: "openid", nonce: 413_093_098_3, state: 413_093_098_3 end it "should return the id token in a fragment" do - expect(response.location).to have_content("id_token=") + expect(response.location).to include("id_token=") encoded_id_token = response.location[/(?<=id_token=)[^&]+/] decoded_token = OpenIDConnect::ResponseObject::IdToken.decode encoded_id_token, Api::OpenidConnect::IdTokenConfig::PUBLIC_KEY @@ -202,16 +218,17 @@ end it "should return the passed in state" do - expect(response.location).to have_content("state=4130930983") + expect(response.location).to include("state=4130930983") end end context "when prompt is none" do it "should return the id token in a fragment" do - post :new, client_id: client.client_id, redirect_uri: "http://localhost:3000/", + post api_openid_connect_authorizations_new_path, client_id: client.client_id, + redirect_uri: "http://localhost:3000/", response_type: "id_token", scope: "openid", nonce: 413_093_098_3, state: 413_093_098_3, display: "page", prompt: "none" - expect(response.location).to have_content("id_token=") + expect(response.location).to include("id_token=") encoded_id_token = response.location[/(?<=id_token=)[^&]+/] decoded_token = OpenIDConnect::ResponseObject::IdToken.decode encoded_id_token, Api::OpenidConnect::IdTokenConfig::PUBLIC_KEY @@ -222,7 +239,8 @@ context "when prompt contains consent" do it "should return a consent form page" do - get :new, client_id: client.client_id, redirect_uri: "http://localhost:3000/", + get new_api_openid_connect_authorization_path, client_id: client.client_id, + redirect_uri: "http://localhost:3000/", response_type: "id_token", scope: "openid", nonce: 413_093_098_3, state: 413_093_098_3, display: "page", prompt: "consent" expect(response.body).to match("Diaspora Test Client") @@ -231,7 +249,8 @@ context "when scopes are escalated" do before do - get :new, client_id: client.client_id, redirect_uri: "http://localhost:3000/", response_type: "id_token", + get new_api_openid_connect_authorization_path, client_id: client.client_id, + redirect_uri: "http://localhost:3000/", response_type: "id_token", scope: "openid read", nonce: 413_093_098_3, state: 413_093_098_3 end @@ -240,7 +259,7 @@ end it "should overwrite old authorization scope after approval" do - post :create, approve: "true" + post api_openid_connect_authorizations_path, approve: "true" authorization_with_old_scope = Api::OpenidConnect::Authorization.find_by_client_id_user_and_scopes(client.client_id, alice, ["openid"]) expect(authorization_with_old_scope).to be_nil @@ -252,13 +271,14 @@ describe "#create" do context "when id_token token" do before do - get :new, client_id: client.client_id, redirect_uri: "http://localhost:3000/", response_type: "id_token token", + get new_api_openid_connect_authorization_path, client_id: client.client_id, + redirect_uri: "http://localhost:3000/", response_type: "id_token token", scope: "openid", nonce: 418_093_098_3, state: 418_093_098_3 end context "when authorization is approved" do before do - post :create, approve: "true" + post api_openid_connect_authorizations_path, approve: "true" end it "should return the id token in a fragment" do @@ -282,17 +302,18 @@ context "when id_token" do before do - get :new, client_id: client.client_id, redirect_uri: "http://localhost:3000/", response_type: "id_token", + get new_api_openid_connect_authorization_path, client_id: client.client_id, + redirect_uri: "http://localhost:3000/", response_type: "id_token", scope: "openid", nonce: 418_093_098_3, state: 418_093_098_3 end context "when authorization is approved" do before do - post :create, approve: "true" + post api_openid_connect_authorizations_path, approve: "true" end it "should return the id token in a fragment" do - expect(response.location).to have_content("id_token=") + expect(response.location).to include("id_token=") encoded_id_token = response.location[/(?<=id_token=)[^&]+/] decoded_token = OpenIDConnect::ResponseObject::IdToken.decode encoded_id_token, Api::OpenidConnect::IdTokenConfig::PUBLIC_KEY @@ -301,56 +322,57 @@ end it "should return the passed in state" do - expect(response.location).to have_content("state=4180930983") + expect(response.location).to include("state=4180930983") end end context "when authorization is denied" do before do - post :create, approve: "false" + post api_openid_connect_authorizations_path, approve: "false" end it "should return an error in the fragment" do - expect(response.location).to have_content("error=") + expect(response.location).to include("error=") end it "should NOT contain a id token in the fragment" do - expect(response.location).to_not have_content("id_token=") + expect(response.location).to_not include("id_token=") end end end context "when code" do before do - get :new, client_id: client.client_id, redirect_uri: "http://localhost:3000/", response_type: "code", + get new_api_openid_connect_authorization_path, client_id: client.client_id, + redirect_uri: "http://localhost:3000/", response_type: "code", scope: "openid", nonce: 418_093_098_3, state: 418_093_098_3 end context "when authorization is approved" do before do - post :create, approve: "true" + post api_openid_connect_authorizations_path, approve: "true" end it "should return the code" do - expect(response.location).to have_content("code") + expect(response.location).to include("code") end it "should return the passed in state" do - expect(response.location).to have_content("state=4180930983") + expect(response.location).to include("state=4180930983") end end context "when authorization is denied" do before do - post :create, approve: "false" + post api_openid_connect_authorizations_path, approve: "false" end it "should return an error" do - expect(response.location).to have_content("error") + expect(response.location).to include("error") end it "should NOT contain code" do - expect(response.location).to_not have_content("code") + expect(response.location).to_not include("code") end end end @@ -361,7 +383,7 @@ context "with existent authorization" do before do - delete :destroy, id: auth_with_read.id + delete api_openid_connect_authorization_path(auth_with_read.id) end it "removes the authorization" do @@ -371,7 +393,7 @@ context "with non-existent authorization" do it "raises an error" do - delete :destroy, id: 123_456_789 + delete api_openid_connect_authorization_path(123_456_789) expect(response).to redirect_to(api_openid_connect_user_applications_url) expect(flash[:error]).to eq("The attempt to revoke the authorization with ID 123456789 failed") end