[PM-9723] Refresh: LoginViaAuthRequestComponent #11545
Open
+1,025
−69
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
…(), and buildAuthRequestLoginCredentials()
…eqDeletedOrDenied()
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #11545 +/- ##
==========================================
- Coverage 33.48% 33.42% -0.07%
==========================================
Files 2846 2850 +4
Lines 89196 89399 +203
Branches 17002 17038 +36
==========================================
+ Hits 29867 29878 +11
- Misses 56976 57168 +192
Partials 2353 2353 ☔ View full report in Codecov by Sentry. |
|
New Issues
Fixed Issues
|
…already authed via SSO
JaredSnider-Bitwarden
requested changes
Nov 5, 2024
libs/auth/src/angular/login-via-auth-request/login-via-auth-request.component.ts
Show resolved
Hide resolved
…ing legacy LoginViaAuthRequestV1Components
rr-bw
requested review from
JaredSnider-Bitwarden
and removed request for
ike-kottlowski
rr-bw
commented
Nov 15, 2024
Comment on lines
+348
to
+362
| * *********************************** | ||
| * Admin Auth Request Flows | ||
| * *********************************** | ||
| * | ||
| * Flow 1: Authed SSO TD user who has a masterKey in memory requests admin approval. | ||
| * | ||
| * SSO TD user who has a masterKey in memory authenticates via SSO > navigates to /login-initiated > clicks "Request admin approval" | ||
| * > navigates to /admin-approval-requested which creates an AdminAuthRequest > receives approval from device with authRequestPublicKey(masterKey) | ||
| * > decrypts masterKey > decrypts userKey > establishes trust (if required) > proceeds to vault | ||
| * | ||
| * Flow 2: Authed SSO TD user who does NOT have a masterKey in memory requests admin approval. | ||
| * | ||
| * SSO TD user who does NOT have a masterKey in memory authenticates via SSO > navigates to /login-initiated > clicks "Request admin approval" | ||
| * > navigates to /admin-approval-requested which creates an AdminAuthRequest > receives approval from device with authRequestPublicKey(userKey) | ||
| * > decrypts userKey > establishes trust (if required) > proceeds to vault |
There was a problem hiding this comment.
Am I correct in understanding that "Flow 1" here is not actually a flow? That is, when dealing with an Admin Auth Request, we are always receiving an encrypted UserKey, not a MasterKey, meaning Flow 1 is not truly a flow?
If I am correct, then I would re-write this section as one flow in total, making no mention of having a MasterKey in memory or not. Which would look like:
***********************************
Admin Auth Request Flows
***********************************
Flow 1: Authed SSO TD user requests admin approval.
SSO TD user authenticates via SSO > navigates to /login-initiated > clicks "Request admin approval"
> navigates to /admin-approval-requested which creates an AdminAuthRequest
> receives approval from device with authRequestPublicKey(userKey) > decrypts userKey
> establishes trust (if required) > proceeds to vault
rr-bw
commented
Nov 15, 2024
Comment on lines
+382
to
+405
| const userHasAuthenticatedViaSSO = this.authStatus === AuthenticationStatus.Locked; | ||
|
|
||
| if (userHasAuthenticatedViaSSO) { | ||
| // Get the auth request from the server | ||
| // User is authenticated, therefore the endpoint does not require an access code. | ||
| const authRequestResponse = await this.authRequestApiService.getAuthRequest(requestId); | ||
|
|
||
| if (authRequestResponse.requestApproved) { | ||
| // Handles Standard Flows 3-4 and Admin Flows 1-2 | ||
| await this.handleAuthenticatedFlows(authRequestResponse); | ||
| } | ||
| } else { | ||
| // Get the auth request from the server | ||
| // User is unauthenticated, therefore the endpoint requires an access code for user verification. | ||
| const authRequestResponse = await this.authRequestApiService.getAuthResponse( | ||
| requestId, | ||
| this.authRequest.accessCode, | ||
| ); | ||
|
|
||
| if (authRequestResponse.requestApproved) { | ||
| // Handles Standard Flows 1-2 | ||
| await this.handleUnauthenticatedFlows(authRequestResponse, requestId); | ||
| } | ||
| } |
There was a problem hiding this comment.
Instead of checking the State to determine the codepath, as we did in the legacy component, I'm using the AuthStatus as this allows me to consolidate two things:
- Getting the auth request from the server using the correct authenticated/unauthenticated endpoint.
- Note that this now means Standard Flows 3-4, which are both authed, will now grab the auth request from
getAuthRequest()(authed endpoint) instead of fromgetAuthResponse()(unauthed endpoint). Those flows were previously using the unauthed endpoint because even though they are authed flows, the caseState.StandardAuthRequestsent them to the unauthed endpoint.
- Note that this now means Standard Flows 3-4, which are both authed, will now grab the auth request from
- Handling the actual flow with a method name that distinguishes authenticated vs unauthenticated flows.
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.
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-9723
📔 Objective
Creates a refreshed and consolidated
LoginViaAuthRequestComponentfor use on all visual clients, which will be used when theUnauthenticatedExtensionUIRefreshis on.Standard Auth Request Flows
Flow 1: Unauthed user requests approval from device; Approving device has a masterKey in memory.
/login-with-devicewhich creates a StandardAuthRequest >receives approval from a device with authRequestPublicKey(masterKey) > decrypts masterKey > decrypts userKey >
proceed to vault
Flow 2: Unauthed user requests approval from device; Approving device does NOT have a masterKey in memory.
Unauthed user clicks "Login with device" > navigates to
/login-with-devicewhich creates a StandardAuthRequest > receives approval from a device with authRequestPublicKey(userKey) > decrypts userKey > proceeds to vaultNote: this flow is an uncommon scenario and relates to TDE off-boarding. The following describes how a user could get into this flow:
Flow 3: Authed SSO TD user requests approval from device; Approving device has a masterKey in memory.
/login-initiated> clicks "Approve from your other device" > navigates to/login-with-devicewhich creates a StandardAuthRequest > receives approval from device with authRequestPublicKey(masterKey) > decrypts masterKey > decrypts userKey > establishes trust (if required) > proceeds to vaultFlow 4: Authed SSO TD user requests approval from device; Approving device does NOT have a masterKey in memory.
/login-initiated> clicks "Approve from your other device" > navigates to/login-with-devicewhich creates a StandardAuthRequest > receives approval from device with authRequestPublicKey(userKey) > decrypts userKey > establishes trust (if required) > proceeds to vaultAdmin Auth Request Flows
Flow 1: Authed SSO TD user who has a masterKey in memory requests admin approval.
/login-initiated> clicks "Request admin approval" > navigates to/admin-approval-requestedwhich creates an AdminAuthRequest > receives approval from device with authRequestPublicKey(masterKey) > decrypts masterKey > decrypts userKey > establishes trust (if required) > proceeds to vaultFlow 2: Authed SSO TD user who does NOT have a masterKey in memory requests admin approval.
/login-initiated> clicks "Request admin approval" > navigates to/admin-approval-requestedwhich creates an AdminAuthRequest > receives approval from device with authRequestPublicKey(userKey) > decrypts userKey > establishes trust (if required) > proceeds to vaultSummary Table
[active route][/login]/login-with-device[/login]/login-with-device[/login-initiated]/login-with-device[/login-initiated]/login-with-device[/login-initiated]/admin-approval-requested[/login-initiated]/admin-approval-requestedNote 1: The phrase "in memory" here is important. It is possible for a user to have a master password for their account, but not have a masterKey IN MEMORY for a specific device. For example, if a user registers an account with a master password, then joins an SSO TD org, then logs in to a device via SSO and admin auth request, they are now logged into that device but that device does not have masterKey IN MEMORY.
📸 Screenshots
Standard Auth Request Flow 1
Unauthed user requests approval from device; Approving device has a
masterKeyin memory.standard-flow-1.mov
Standard Auth Request Flow 2
Unauthed user requests approval from device; Approving device does NOT have a
masterKeyin memory.Note: this flow is an uncommon scenario and relates to TDE off-boarding. The following describes how a user could get into this flow:
masterKeyin memory.2a. Changes the member decryption options from "Trusted devices" to "Master password" AND
2b. Turns off the "Require single sign-on authentication" policy
masterKeyin memory (see step 1 above).standard-flow-2.mov
Standard Auth Request Flow 3
Authed SSO TD user requests approval from device; Approving device has a
masterKeyin memory.standard-flow-3.mov
Standard Auth Request Flow 4
Authed SSO TD user requests approval from device; Approving device does NOT have a
masterKeyin memory.standard-flow-4.mov
Admin Auth Request Flow 2
Authed SSO TD user requests admin approval.
admin-flow-2.mov
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes