Removing Master password for Oauth/SSO/LDAP users.#1944
Merged
maidul98 merged 8 commits intoInfisical:mainfrom Jun 19, 2024
Merged
Removing Master password for Oauth/SSO/LDAP users.#1944maidul98 merged 8 commits intoInfisical:mainfrom
maidul98 merged 8 commits intoInfisical:mainfrom
Conversation
akhilmhdh
force-pushed
the
feat/srp-handover
branch
from
a975c9c to
19049e5
Compare
maidul98
reviewed
Jun 10, 2024
maidul98
reviewed
Jun 10, 2024
maidul98
reviewed
Jun 11, 2024
frontend/src/views/Signup/components/UserInfoSSOStep/UserInfoSSOStep.tsx
Show resolved
Hide resolved
maidul98
reviewed
Jun 11, 2024
Collaborator
maidul98
left a comment
maidul98
left a comment
There was a problem hiding this comment.
Backend logic looks good to me. Also reviewed the frontend and most of it makes sense to me. Left comments where more context would be helpful. In terms of testing here is what I did:
- Successfully created a new account using Google SSO (this process works well). I understand that we generate a password specifically for SSO-only signups. How does this interact with the password reset process? I followed the entire flow for the new account created via Google SSO and set a new password. However, it appears that the auto-generated password wasn't replaced since I couldn't log in with the email and new password.
- Account created with username and password and then linked with both Google and Github SSO. Works well, no password step needed.
- @dangtony98 Can you please try SCIM/LDAP for testing?
- Haven't yet tested password reset for account created with username/password yet
akhilmhdh
force-pushed
the
feat/srp-handover
branch
from
1d690d0 to
93d5180
Compare
dangtony98
previously approved these changes
Jun 12, 2024
backend/src/db/migrations/20240609133400_private-key-handoff.ts
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| return { authMethod: decodedToken.authMethod, orgId: null }; | ||
| return { authMethod: decodedToken.authMethod, orgId: null, userName: decodedToken.username }; |
Collaborator
There was a problem hiding this comment.
Why do we return userName: decodedToken.username here? It doesn't seem to be used anywhere else.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Description 📣
First step in moving away from our SRP system to a more proficient automation platform. With SRP we will send password also and we save the password with bcrypt hashing in db.
To avoid asking for master password, we just fetch the private key on login. For oauth user we do a handoff that is the provider token act as password to get the access token for infisical.
This also means no more book keeping master password for third part authorized users like LDAP, Google Oauth etc.
Type ✨
Tests 🛠️
# Here's some code block to paste some code snippets