Skip to content

Fix csrf_detected for concurrent requests#187

Open
nickstanish wants to merge 2 commits intoomniauth:masterfrom
nickstanish:master
Open

Fix csrf_detected for concurrent requests#187
nickstanish wants to merge 2 commits intoomniauth:masterfrom
nickstanish:master

Conversation

@nickstanish
Copy link
Copy Markdown

Relates to #103

Went with a hash approach instead of array to better work alongside the pkce verifier

Added tests and verified in my application. Without these changes this test fails:

       Expected "/auth/failure?message=csrf_detected&strategy=auth0"
          to eq "http://www.example.com/dashboard"
     # ./spec/requests/oauth_spec.rb:194:in 'block (5 levels) in <main>'
        it 'fails with csrf_detected when multiple tabs initiate auth0 flows simultaneously' do
          # Tab 1 initiates
          post '/auth/auth0'
          expect(response).to have_http_status(:found)
          auth0_redirect_uri_1 = Addressable::URI.parse(response.headers['Location'])
          query_params_1 = CGI.parse(auth0_redirect_uri_1.query).transform_values(&:first)
          state_1 = query_params_1['state']

          # Then Tab 2 follows
          post '/auth/auth0'
          expect(response).to have_http_status(:found)
          auth0_redirect_uri_2 = Addressable::URI.parse(response.headers['Location'])
          query_params_2 = CGI.parse(auth0_redirect_uri_2.query).transform_values(&:first)
          state_2 = query_params_2['state']

          allow_any_instance_of(OmniAuth::Strategies::Auth0).to receive(:raw_info).and_return({
            sub: SecureRandom.uuid,
            name: Faker::Name.name,
            email: Faker::Internet.email,
          })

          stub_request(:post, 'https://auth0.local/oauth/token')
            .to_return(
              status: 200,
              body: {
                access_token: 'mock_access_token',
                token_type: 'Bearer',
                expires_in: 86_400,
                id_token: 'mock_id_token',
              }.to_json,
              headers: { 'Content-Type' => 'application/json' },
            )

          # Tab 1 callback completes
          get '/auth/auth0/callback', params: { state: state_1, code: 'mock_code_1' }
          expect(response).to have_http_status(:found)
          expect(response.headers['Location']).to eq('http://www.example.com/dashboard')

          # Tab 2 callback completes
          get '/auth/auth0/callback', params: { state: state_2, code: 'mock_code_1' }
          expect(response).to have_http_status(:found)
          expect(response.headers['Location']).to eq('http://www.example.com/dashboard')
        end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant