Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New #mock_call implementation improperly runs authenticity token verification on all requests #1032

Closed
jsdalton opened this issue Feb 2, 2021 · 2 comments

Comments

@jsdalton
Copy link
Contributor

jsdalton commented Feb 2, 2021

A recent change altered the behavior of omniauth when run in test mode. See below for details.

The change appears to have been introduced here 1b784ff .

Configuration

  • Provider Gem: omniauth-*
  • Ruby Version: 2.7.2
  • Framework: Rails
  • Platform: Mac

Expected Behavior

When calling an Omniauth strategy while in test mode, the mock checks and tests should be executed in a way that mirrors normal mode.

In regular mode it would look like this:

  1. Execute request_call phase if on_request_path? is true: https://github.com/omniauth/omniauth/blob/v2.0.1/lib/omniauth/strategy.rb#L192
  2. Within request_call, execute authenticity token verification: https://github.com/omniauth/omniauth/blob/v2.0.1/lib/omniauth/strategy.rb#L236

In other words, token verification only occurs if we are on the request path.

Again the expectation is that test mode would behave the same way: only executing verification if we are on the request path.

Actual Behavior

When test mode is true, mock_call is used early in the execution of the middleware. Unfortunately, the authenticity token verification happens differently than it does in normal mode.

See https://github.com/omniauth/omniauth/blob/v2.0.1/lib/omniauth/strategy.rb#L302 .

The verification happens before the check to on_request_path? is made. As a result, the verification happens on very request, not just those that are on the request path provided by omniauth.

The problem is twofold:

  1. Any request executed while in test mode that is not part of Omniauth and that does not require a CSRF token will fail (an example would be API calls that are part of the same application).
  2. The behavior is different than normal/production mode, meaning developers might inadvertently rely on this addtional verification check which would not be executed in outside of test mode.

Steps to Reproduce

  1. Set OmniAuth.config.test_mode = true
  2. Execute any non-GET request to any endpoint on the same application that is not on the Omniauth request path.
  3. The on_failure block is triggered.

Fix

For what it's worth the fix seems relatively straightforward: Move the request_validation_phase check...

Happy to add a pull request but wanted to submit the issue first in case I was missing something

@jsdalton
Copy link
Contributor Author

jsdalton commented Feb 2, 2021

Opened a PR reproducing this issue, with a fix: #1033

@BobbyMcWho
Copy link
Member

Closed via #1033

Released in OmniAuth v2.0.2

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

No branches or pull requests

2 participants