-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[PM-10607] Require userId for getKeyForCipherKeyDecryption #10509
Conversation
…acySupport and use the version that requires a user id
New Issues
|
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #10509 +/- ##
==========================================
- Coverage 32.83% 32.79% -0.05%
==========================================
Files 2670 2670
Lines 81596 81769 +173
Branches 15381 15414 +33
==========================================
+ Hits 26794 26816 +22
- Misses 52745 52877 +132
- Partials 2057 2076 +19 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Auth changes look good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non blocking informational: In case you didn't know, there is a nice helper function called mockAccountServiceWith(...)
which makes it easy to mock AccountService
context without having to stand up your own active account observable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AC changes look good!
4410ae9
05cbd23
* updated cipher service to stop using the deprecated getUserKeyWithLegacySupport and use the version that requires a user id * Added account service mock * fixed cipher test * Fixed test * removed async from encryptCipher * updated encryptSharedCipher to pass userId to the encrypt function * Pass userId to getUserKeyWithLegacySupport on encryptSharedCipher * pass in userid when setting masterKeyEncryptedUserKey * Added activer usedId to new web refresh function
🎟️ Tracking
PM-10607
📔 Objective
Due to recent reports from some users getting "Error: Must provide key", a potential resolution for this is to stop using the deprecated version of
getUserKeyWithLegacySupport
in thegetKeyForCipherKeyDecryption
function and instead requireuserId
to be passed down from components or services.📸 Screenshots
⏰ 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