Fix authentication failures after going halfway through a sign-in attempt #16607
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
My first (unpublished) attempt at fixing the issues introduced (extremely hard-to-exploit) security vulnerabilities, addressing them in a test.
ClearlyClaire
changed the title
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
app/controllers/concerns/sign_in_token_authentication_concern.rb
Outdated
Show resolved
Hide resolved
ClearlyClaire
force-pushed
the
fixes/two-step-authentication-mixup
branch
from
913e845
to
fc80220
Compare
…_factor` to make the two authentication steps more obvious
ClearlyClaire
force-pushed
the
fixes/two-step-authentication-mixup
branch
from
a0df0b6
to
8d838f7
Compare
Gargron
approved these changes
Aug 25, 2021
Gargron
pushed a commit
that referenced
this pull request
Nov 5, 2021
…empt (#16607) * Add tests * Add security-related tests My first (unpublished) attempt at fixing the issues introduced (extremely hard-to-exploit) security vulnerabilities, addressing them in a test. * Fix authentication failures after going halfway through a sign-in attempt * Refactor `authenticate_with_sign_in_token` and `authenticate_with_two_factor` to make the two authentication steps more obvious
kadoshita
pushed a commit
to kadoshita/mastodon
that referenced
this pull request
Nov 6, 2021
…empt (mastodon#16607) * Add tests * Add security-related tests My first (unpublished) attempt at fixing the issues introduced (extremely hard-to-exploit) security vulnerabilities, addressing them in a test. * Fix authentication failures after going halfway through a sign-in attempt * Refactor `authenticate_with_sign_in_token` and `authenticate_with_two_factor` to make the two authentication steps more obvious
kadoshita
pushed a commit
to kadoshita/mastodon
that referenced
this pull request
Nov 7, 2021
…empt (mastodon#16607) * Add tests * Add security-related tests My first (unpublished) attempt at fixing the issues introduced (extremely hard-to-exploit) security vulnerabilities, addressing them in a test. * Fix authentication failures after going halfway through a sign-in attempt * Refactor `authenticate_with_sign_in_token` and `authenticate_with_two_factor` to make the two authentication steps more obvious
ClearlyClaire
added a commit
to ClearlyClaire/mastodon
that referenced
this pull request
Jan 28, 2022
…empt (mastodon#16607) * Add tests * Add security-related tests My first (unpublished) attempt at fixing the issues introduced (extremely hard-to-exploit) security vulnerabilities, addressing them in a test. * Fix authentication failures after going halfway through a sign-in attempt * Refactor `authenticate_with_sign_in_token` and `authenticate_with_two_factor` to make the two authentication steps more obvious
dependabot bot
added a commit
to akyoz/Plustodon
that referenced
this pull request
Nov 12, 2022
…empt (mastodon#16607) * Add tests * Add security-related tests My first (unpublished) attempt at fixing the issues introduced (extremely hard-to-exploit) security vulnerabilities, addressing them in a test. * Fix authentication failures after going halfway through a sign-in attempt * Refactor `authenticate_with_sign_in_token` and `authenticate_with_two_factor` to make the two authentication steps more obvious
dependabot bot
added a commit
to akyoz/Plustodon
that referenced
this pull request
Nov 12, 2022
…empt (mastodon#16607) * Add tests * Add security-related tests My first (unpublished) attempt at fixing the issues introduced (extremely hard-to-exploit) security vulnerabilities, addressing them in a test. * Fix authentication failures after going halfway through a sign-in attempt * Refactor `authenticate_with_sign_in_token` and `authenticate_with_two_factor` to make the two authentication steps more obvious
dependabot bot
added a commit
to akyoz/Plustodon
that referenced
this pull request
Nov 12, 2022
…empt (mastodon#16607) * Add tests * Add security-related tests My first (unpublished) attempt at fixing the issues introduced (extremely hard-to-exploit) security vulnerabilities, addressing them in a test. * Fix authentication failures after going halfway through a sign-in attempt * Refactor `authenticate_with_sign_in_token` and `authenticate_with_two_factor` to make the two authentication steps more obvious
dependabot bot
added a commit
to akyoz/Plustodon
that referenced
this pull request
Nov 12, 2022
…empt (mastodon#16607) * Add tests * Add security-related tests My first (unpublished) attempt at fixing the issues introduced (extremely hard-to-exploit) security vulnerabilities, addressing them in a test. * Fix authentication failures after going halfway through a sign-in attempt * Refactor `authenticate_with_sign_in_token` and `authenticate_with_two_factor` to make the two authentication steps more obvious
dependabot bot
added a commit
to akyoz/Plustodon
that referenced
this pull request
Nov 12, 2022
…empt (mastodon#16607) * Add tests * Add security-related tests My first (unpublished) attempt at fixing the issues introduced (extremely hard-to-exploit) security vulnerabilities, addressing them in a test. * Fix authentication failures after going halfway through a sign-in attempt * Refactor `authenticate_with_sign_in_token` and `authenticate_with_two_factor` to make the two authentication steps more obvious
dependabot bot
added a commit
to akyoz/Plustodon
that referenced
this pull request
Nov 12, 2022
…empt (mastodon#16607) * Add tests * Add security-related tests My first (unpublished) attempt at fixing the issues introduced (extremely hard-to-exploit) security vulnerabilities, addressing them in a test. * Fix authentication failures after going halfway through a sign-in attempt * Refactor `authenticate_with_sign_in_token` and `authenticate_with_two_factor` to make the two authentication steps more obvious
dependabot bot
added a commit
to akyoz/Plustodon
that referenced
this pull request
Nov 12, 2022
…empt (mastodon#16607) * Add tests * Add security-related tests My first (unpublished) attempt at fixing the issues introduced (extremely hard-to-exploit) security vulnerabilities, addressing them in a test. * Fix authentication failures after going halfway through a sign-in attempt * Refactor `authenticate_with_sign_in_token` and `authenticate_with_two_factor` to make the two authentication steps more obvious
dependabot bot
added a commit
to akyoz/Plustodon
that referenced
this pull request
Nov 12, 2022
…empt (mastodon#16607) * Add tests * Add security-related tests My first (unpublished) attempt at fixing the issues introduced (extremely hard-to-exploit) security vulnerabilities, addressing them in a test. * Fix authentication failures after going halfway through a sign-in attempt * Refactor `authenticate_with_sign_in_token` and `authenticate_with_two_factor` to make the two authentication steps more obvious
dependabot bot
added a commit
to akyoz/Plustodon
that referenced
this pull request
Nov 12, 2022
…empt (mastodon#16607) * Add tests * Add security-related tests My first (unpublished) attempt at fixing the issues introduced (extremely hard-to-exploit) security vulnerabilities, addressing them in a test. * Fix authentication failures after going halfway through a sign-in attempt * Refactor `authenticate_with_sign_in_token` and `authenticate_with_two_factor` to make the two authentication steps more obvious
dependabot bot
added a commit
to akyoz/Plustodon
that referenced
this pull request
Nov 12, 2022
…empt (mastodon#16607) * Add tests * Add security-related tests My first (unpublished) attempt at fixing the issues introduced (extremely hard-to-exploit) security vulnerabilities, addressing them in a test. * Fix authentication failures after going halfway through a sign-in attempt * Refactor `authenticate_with_sign_in_token` and `authenticate_with_two_factor` to make the two authentication steps more obvious
dependabot bot
added a commit
to akyoz/Plustodon
that referenced
this pull request
Nov 12, 2022
…empt (mastodon#16607) * Add tests * Add security-related tests My first (unpublished) attempt at fixing the issues introduced (extremely hard-to-exploit) security vulnerabilities, addressing them in a test. * Fix authentication failures after going halfway through a sign-in attempt * Refactor `authenticate_with_sign_in_token` and `authenticate_with_two_factor` to make the two authentication steps more obvious
dependabot bot
added a commit
to akyoz/Plustodon
that referenced
this pull request
Nov 12, 2022
…empt (mastodon#16607) * Add tests * Add security-related tests My first (unpublished) attempt at fixing the issues introduced (extremely hard-to-exploit) security vulnerabilities, addressing them in a test. * Fix authentication failures after going halfway through a sign-in attempt * Refactor `authenticate_with_sign_in_token` and `authenticate_with_two_factor` to make the two authentication steps more obvious
dependabot bot
added a commit
to akyoz/Plustodon
that referenced
this pull request
Nov 12, 2022
…empt (mastodon#16607) * Add tests * Add security-related tests My first (unpublished) attempt at fixing the issues introduced (extremely hard-to-exploit) security vulnerabilities, addressing them in a test. * Fix authentication failures after going halfway through a sign-in attempt * Refactor `authenticate_with_sign_in_token` and `authenticate_with_two_factor` to make the two authentication steps more obvious
dependabot bot
added a commit
to akyoz/Plustodon
that referenced
this pull request
Nov 12, 2022
…empt (mastodon#16607) * Add tests * Add security-related tests My first (unpublished) attempt at fixing the issues introduced (extremely hard-to-exploit) security vulnerabilities, addressing them in a test. * Fix authentication failures after going halfway through a sign-in attempt * Refactor `authenticate_with_sign_in_token` and `authenticate_with_two_factor` to make the two authentication steps more obvious
dependabot bot
added a commit
to akyoz/Plustodon
that referenced
this pull request
Nov 12, 2022
…empt (mastodon#16607) * Add tests * Add security-related tests My first (unpublished) attempt at fixing the issues introduced (extremely hard-to-exploit) security vulnerabilities, addressing them in a test. * Fix authentication failures after going halfway through a sign-in attempt * Refactor `authenticate_with_sign_in_token` and `authenticate_with_two_factor` to make the two authentication steps more obvious
dependabot bot
added a commit
to akyoz/Plustodon
that referenced
this pull request
Nov 12, 2022
…empt (mastodon#16607) * Add tests * Add security-related tests My first (unpublished) attempt at fixing the issues introduced (extremely hard-to-exploit) security vulnerabilities, addressing them in a test. * Fix authentication failures after going halfway through a sign-in attempt * Refactor `authenticate_with_sign_in_token` and `authenticate_with_two_factor` to make the two authentication steps more obvious
dependabot bot
added a commit
to akyoz/Plustodon
that referenced
this pull request
Nov 12, 2022
…empt (mastodon#16607) * Add tests * Add security-related tests My first (unpublished) attempt at fixing the issues introduced (extremely hard-to-exploit) security vulnerabilities, addressing them in a test. * Fix authentication failures after going halfway through a sign-in attempt * Refactor `authenticate_with_sign_in_token` and `authenticate_with_two_factor` to make the two authentication steps more obvious
dependabot bot
added a commit
to akyoz/Plustodon
that referenced
this pull request
Nov 12, 2022
…empt (mastodon#16607) * Add tests * Add security-related tests My first (unpublished) attempt at fixing the issues introduced (extremely hard-to-exploit) security vulnerabilities, addressing them in a test. * Fix authentication failures after going halfway through a sign-in attempt * Refactor `authenticate_with_sign_in_token` and `authenticate_with_two_factor` to make the two authentication steps more obvious
ClearlyClaire
added a commit
to ClearlyClaire/mastodon
that referenced
this pull request
Jul 6, 2023
…empt (mastodon#16607) * Add tests * Add security-related tests My first (unpublished) attempt at fixing the issues introduced (extremely hard-to-exploit) security vulnerabilities, addressing them in a test. * Fix authentication failures after going halfway through a sign-in attempt * Refactor `authenticate_with_sign_in_token` and `authenticate_with_two_factor` to make the two authentication steps more obvious
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In some cases, attempting to sign in may result in an incorrect “Invalid E-Mail or password.” error.
This can occur when first entering the correct password for an account, but not resolving the 2FA or sign-in token challenge, then trying to log into another user that would require a 2FA or sign-in token challenge.
Indeed, upon the first successful password login, the
attempt_user_idfield of the (securely encrypted) session is set to the account being logged into. However, while this field is set, subsequent log-in attempts will ignore the provided email and look up theattempt_user_idinstead, resulting in password comparison with the wrong account (which also means someone managing two accounts with the same password can mistakenly log into the wrong account, should they solve the correct challenge).This PR fixes it by using the provided
emailfirst to find the appropriate account, then check whether the found account andattempt_user_idmatch. Thorough review is needed as this may have security implications (I'm fairly confident it does not, but my first attempt did not check thatattempt_user_id, making it possible, although very improbable, to bypass password authentication).