Auth/PM-14922 - LoginClient - Login with password MVP #549
+4,595
−66
Conversation
This file contains hidden or 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
JaredSnider-Bitwarden
changed the title
Contributor
|
Great job! No new security vulnerabilities introduced in this pull request |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #549 +/- ##
==========================================
+ Coverage 78.85% 79.78% +0.93%
==========================================
Files 283 298 +15
Lines 29680 31171 +1491
==========================================
+ Hits 23404 24871 +1467
- Misses 6276 6300 +24 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…rd_prelogin file to prevent identity client file from growing substantially as we add items (per feedback from platform)
…d suffixed (2) Add required mod declarations for usages to show up and fix imports.
…ationData usage in bitwarden-auth crate
…d required scopes for standard BW user and mechanism for converting array of scopes to space separated string (3) UserTokenApiRequest - build constructor with default scopes already included
…req + MP authN data to api payload
Contributor
🔍 SDK Breaking Change Detection ResultsSDK Version:
Breaking change detection completed. View SDK workflow |
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-14922
📔 Objective
To deliver an MVP of login with password in the SDK. This will not include 2FA or new device verification support at first.
Outstanding Notes / Questions for Reviewers
Several of the request / response models that I copied over from
bitwarden-coreused theschemars::JsonSchemamacro. However, per Claude: "JsonSchema is applied inconsistently and there's no clear, documented rule for when to use it." in the codebase. I've removed them from the auth crate for now until I know when we should apply them + it avoids adding an extra dependency ofschemars.a. Do we need it for API request / response models (SDK to API)?
b. Do we need it for WASM / UNIFFI SDK request / response models (SDK to consuming clients)?
The test coverage is running into a
rustc-LLVM ERROR: IO failure on output stream: No space left on deviceerror. Not sure what to do about that.cargo testrun locally passes all tests.I followed the steps to build the SDK for WASM here and npm linked it using the instructions here in order to try and wire up a demo in Auth/PM-14922 - WIP SDK password login demo (do not merge) clients#18103, but I'm getting a wall of errors about missing exported data from
@bitwarden/sdk-internal. Example:ERROR in ../../libs/admin-console/src/common/organization-user/models/requests/organization-user-bulk-confirm.request.ts:1:10 - error TS2305: Module '"@bitwarden/sdk-internal"' has no exported member 'EncString'.. Not sure what is going on there.⏰ Reminders before review
team
🦮 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 confirmedissue 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