Skip to content
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-15061] extract encryptors from generator service #12068

Merged

Conversation

audreyality
Copy link
Contributor

@audreyality audreyality commented Nov 20, 2024

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-15061

📔 Objective

Extract encryptors from generator service so they can be reused across the tools codebase.

🦖 System Evolution

A new set of encryptors that distinguishes key slots from key instances is in the works. These encryptors also have specialized key rotation interfaces to enable integration with key services.

This PR introduces a parallel set of interfaces (LegacyEncryptorProvider et. al.) as a step towards that new system. Like the legacy generator services before it, the legacy encryptor interfaces arrive deprecated so that the impact of importing them is clear--porting to the new interfaces will be required once they become available.

🦮 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

@audreyality audreyality requested a review from a team as a code owner November 20, 2024 17:12
@audreyality audreyality added the needs-qa Marks a PR as requiring QA approval label Nov 20, 2024
@djsmith85 djsmith85 self-requested a review November 20, 2024 17:19
Copy link

codecov bot commented Nov 20, 2024

Codecov Report

Attention: Patch coverage is 79.56204% with 28 lines in your changes missing coverage. Please review.

Project coverage is 33.53%. Comparing base (7d6da0a) to head (332df67).
Report is 4 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
libs/common/src/tools/state/user-state-subject.ts 56.09% 16 Missing and 2 partials ⚠️
.../core/src/services/credential-generator.service.ts 61.53% 5 Missing ⚠️
libs/common/src/tools/state/object-key.ts 25.00% 3 Missing ⚠️
...rc/tools/cryptography/legacy-encryptor-provider.ts 0.00% 1 Missing ⚠️
...ols/send/send-ui/src/send-form/send-form.module.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #12068      +/-   ##
==========================================
+ Coverage   33.47%   33.53%   +0.05%     
==========================================
  Files        2878     2882       +4     
  Lines       89941    90022      +81     
  Branches    17116    17125       +9     
==========================================
+ Hits        30106    30186      +80     
- Misses      57455    57456       +1     
  Partials     2380     2380              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@audreyality audreyality removed the needs-qa Marks a PR as requiring QA approval label Nov 22, 2024
@djsmith85 djsmith85 linked an issue Nov 25, 2024 that may be closed by this pull request
1 task
Copy link
Contributor

@djsmith85 djsmith85 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@audreyality Just some documentation updates and thank you for answering any questions I had offline.

libs/common/src/tools/rx.ts Show resolved Hide resolved
libs/common/src/tools/rx.ts Show resolved Hide resolved
djsmith85
djsmith85 previously approved these changes Nov 25, 2024
quexten
quexten previously approved these changes Nov 25, 2024
Copy link
Contributor

@quexten quexten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good from my side.

@audreyality audreyality dismissed stale reviews from quexten and djsmith85 via 312ebee November 27, 2024 17:30
@audreyality audreyality enabled auto-merge (squash) November 27, 2024 20:05
@audreyality audreyality merged commit ab21b78 into main Nov 28, 2024
69 of 71 checks passed
@audreyality audreyality deleted the tools/pm-15061/extract-encryptors-from-generator-service branch November 28, 2024 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Beta: Unable to Generate Aliases With Addy.io
3 participants