Allow bypass of captcha token if the device is known #1626
Merged
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
cscharf
approved these changes
Oct 8, 2021
kspearrin
reviewed
Oct 11, 2021
Comment on lines
+62
to
+63
| var unknownDevice = !await KnownDeviceAsync(user, context.Request); | ||
| if (!unknownDevice && _captchaValidationService.RequireCaptchaValidation(_currentContext)) |
There was a problem hiding this comment.
Not sure if this logic is correct. Also, you are negating twice. Why not just drop the two !?
There was a problem hiding this comment.
🤦🏻 , I missed the double negative here, this may need some revision re-reading it. Thanks Kyle
Merged
MGibson1
added a commit
that referenced
this pull request
Oct 19, 2021
(cherry picked from commit 8a5a371)
dlundgren
pushed a commit
to dlundgren/server
that referenced
this pull request
Dec 14, 2021
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.
Overview
Captcha activation on our cloud instance has caused all CLI authentications using username + password to provide a captcha key.
The thought was that this would be a rare authentication flow rather than a change to the intended path. This is recorded in this CLI issue: bitwarden/cli#383
As a partial remediation, this work bypasses any captcha if the device ID of the client is already associated to the user. This way, any device can only require captcha on the first login. There is an increase in risk for compromised known hardware, but largely that would be an already lost situation.
Files Changed