Keep original state on errors

app/controllers/carto/oauth_provider_controller.rb

@@ -32,14 +32,14 @@ module Carto
     skip_before_action :verify_authenticity_token, only: [:token]
 
     before_action :x_frame_options_allow, only: :consent, if: :silent_flow?
-    before_action :set_redirection_error_handling, only: [:consent, :authorize]
+    before_action :set_redirection_error_handling, :set_state, only: [:consent, :authorize]
     before_action :ensure_required_token_params, only: [:token]
     before_action :load_oauth_app, :verify_redirect_uri
     before_action :login_required_any_user, only: [:consent, :authorize]
     before_action :validate_prompt_request, only: [:consent]
     before_action :reject_client_secret, only: [:consent, :authorize]
     before_action :ensure_required_authorize_params, only: [:consent, :authorize]
-    before_action :validate_response_type, :validate_scopes, :set_state, only: [:consent, :authorize]
+    before_action :validate_response_type, :validate_scopes, only: [:consent, :authorize]
     before_action :load_oauth_app_user, only: [:consent, :authorize]
     before_action :validate_grant_type, :verify_client_secret, only: [:token]
 

spec/requests/carto/oauth_provider_controller_spec.rb

@@ -87,9 +87,10 @@ describe Carto::OauthProviderController do
 
       it 'redirects with an error if requesting unknown scopes' do
         request_endpoint(valid_payload.merge(scope: 'invalid wadus'))
-
+                
         expect(response.status).to(eq(302))
         expect(response.location).to(start_with(@oauth_app.redirect_uris.first))
+        expect(response.location).to(include(valid_payload[:state]))
         qs = parse_uri_parameters(response.location)
         expect(qs['error']).to(eq('invalid_scope'))
       end
@@ -99,6 +100,7 @@ describe Carto::OauthProviderController do
 
         expect(response.status).to(eq(302))
         expect(response.location).to(start_with(@oauth_app.redirect_uris.first))
+        expect(response.location).to(include(valid_payload[:state]))
         qs = parse_uri_parameters(response.location)
         expect(qs['error']).to(eq('invalid_scope'))
       end
@@ -108,6 +110,7 @@ describe Carto::OauthProviderController do
 
         expect(response.status).to(eq(302))
         expect(response.location).to(start_with(@oauth_app.redirect_uris.first))
+        expect(response.location).to(include(valid_payload[:state]))
         qs = parse_uri_parameters(response.location)
         expect(qs['error']).to(eq('invalid_scope'))
       end
@@ -118,6 +121,7 @@ describe Carto::OauthProviderController do
 
         expect(response.status).to(eq(302))
         expect(response.location).to(start_with(@oauth_app.redirect_uris.first))
+        expect(response.location).to(include(valid_payload[:state]))
         qs = parse_uri_parameters(response.location)
         expect(qs['error']).to(eq('invalid_scope'))
       end
@@ -127,6 +131,7 @@ describe Carto::OauthProviderController do
 
         expect(response.status).to(eq(302))
         expect(response.location).to(start_with(@oauth_app.redirect_uris.first))
+        expect(response.location).to(include(valid_payload[:state]))
         qs = parse_uri_parameters(response.location)
         expect(qs['error']).to(eq('invalid_scope'))
       end
@@ -136,6 +141,7 @@ describe Carto::OauthProviderController do
 
         expect(response.status).to(eq(302))
         expect(response.location).to(start_with(@oauth_app.redirect_uris.first))
+        expect(response.location).to(include(valid_payload[:state]))
         qs = parse_uri_parameters(response.location)
         expect(qs['error']).to(eq('invalid_request'))
         expect(qs['error_description']).to(eq('The redirect_uri must match the redirect_uri param used in the authorization request'))