[PM-3797] Client changes to use new key rotation process #6881
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
82a5755 to
4417214
Compare
b3f4ff3 to
0664ced
Compare
|
Current dependencies on/for this PR:
This stack of pull requests is managed by Graphite. |
953e7d4 to
9620161
Compare
0664ced to
88a064c
Compare
9620161 to
2c4a5d6
Compare
88a064c to
b834845
Compare
Base automatically changed from
auth/pm-3797/admin-recovery-service
to
master
December 1, 2023 22:21
- wasn't caught as rename since diff is too different
- use old request model and endpoint for new key rotation - return empty arrays instead of null when building request - call new send service logic for getRotatedKeys
b834845 to
59f6a0a
Compare
|
New Issues
Fixed Issues
|
59f6a0a to
45985a8
Compare
45985a8 to
f7b2d82
Compare
ike-kottlowski
previously approved these changes
Dec 15, 2023
audreyality
reviewed
Dec 19, 2023
| }); | ||
|
|
||
| it("throws if the new user key is null", async () => { | ||
| await expect(sendService.getRotatedKeys(null)).rejects.toThrow(); |
There was a problem hiding this comment.
⛏️ The throw should check to ensure that the message is correct.
audreyality
reviewed
Dec 19, 2023
| throw new Error("New user key is required for rotation."); | ||
| } | ||
|
|
||
| return await Promise.all( |
There was a problem hiding this comment.
Sorry. Took a closer look and found one more nit:
⛏️ Could you break out the result of the promise.all(...) call into a separate variable? The combination of asynchronous code and the inlined map call will make this a little difficult to step through with a debugger.
There was a problem hiding this comment.
Good point, I've definitely had frustrating debugging sessions dealing with the same thing. Updated!
- add easier debuggin - check for specific error in tests
addisonbeck
approved these changes
Dec 20, 2023
ike-kottlowski
approved these changes
Dec 22, 2023
1 task
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.
Type of change
Objective
Final Client changes for Key Rotation Improvements.
KeyRotationServicethat is responsible for owning rotation process.Sendre-encryption to theSendService(KeyRotationServiceshouldn't have knowledge about how domains are encrypted).EmergencyAccessre-encryption to theEmergencyAccessService.AccountRecoveryServicetoOrganizationUserResetPasswordServiceafter feedback from Admin ConsoleCode changes
Auth
EmergencyAccessre-encryption to theEmergencyAccessService. Add deprecated method for legacy key rotations if feature flag is offAdmin Console
AccountRecoveryServicetoOrganizationUserResetPasswordServiceAccountRecoveryServicetoOrganizationUserResetPasswordServiceTools
Other
postAccountKeytoKeyRotationApiServiceScreenshots
Before you submit