IFC-2664 lock password changes for externally authenticated accounts#9391
Draft
gmazoyer wants to merge 4 commits into
Draft
IFC-2664 lock password changes for externally authenticated accounts#9391gmazoyer wants to merge 4 commits into
gmazoyer wants to merge 4 commits into
Conversation
031ac8d to
d04edee
Compare
Contributor
There was a problem hiding this comment.
2 issues found across 11 files
Confidence score: 3/5
- There is some merge risk:
backend/infrahub/graphql/resolvers/resolver.pymay return inconsistent external-identity data if GraphQL branch/timestamp context is not passed through, which can surface incorrect results across branch/time views. frontend/app/src/entities/user-profile/ui/tab-update-password.tsxcurrently renders the password form before profile state is resolved, so users can briefly see the wrong UI state instead of a proper loading/unknown guard (as noted in IFC-2664).- Pay close attention to
backend/infrahub/graphql/resolvers/resolver.pyandfrontend/app/src/entities/user-profile/ui/tab-update-password.tsx- context propagation and loading-state handling are the main user-facing risk areas.
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
ead4c35 to
da45250
Compare
da45250 to
d8185f7
Compare
d8185f7 to
3d98f30
Compare
Contributor
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Shadow auto-approve: would require human review. This security fix modifies core backend mutation logic, adds a dynamic GraphQL field, and alters frontend account behavior, which carries risk of unintended side effects in account management and requires human review.
Re-trigger cubic
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
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.
Why
Any account provisioned through an external identity provider (LDAP, OIDC, OAuth2) could overwrite its local password through the self-update mutation, then sign in via the local-auth endpoint with the chosen password. The bypass survived directory-side revocation. This PR closes that gap on both the security side (backend) and the UX side (the frontend never lets the user reach the form).
Found while exercising LDAP against a live Active Directory; the bug applies to every account linked to an external identity, not just LDAP ones.
What changed
The self-update mutation now refuses to write the password when the account is linked to an external identity, while continuing to accept description-only updates. The current account profile gains a boolean flag so the frontend can act before any submission. The synthesized field is added on the account interface.
Three alternative shapes for the signal can be considered (whitelisting the relationship in the schema-gen filter, moving the underlying type out of the Internal namespace, encoding the source in JWT claims).
The profile page reads the new flag, hides the Password tab for externally managed accounts, and shows an "externally managed" panel on the password page for direct navigation. The earlier reactive error-string workaround is gone.
Checklist
uv run towncrier create ...)Summary by cubic
Blocks local password changes for accounts linked to external identity providers and hides the password UI for those users. Fixes the bypass letting LDAP/OIDC/OAuth2 users set a local password and log in via local auth (IFC-2664).
InfrahubAccountSelfUpdaterejectspasswordupdates when anExternalIdentityexists; description-only updates still allowed.is_externally_managed(Boolean!) toCoreGenericAccount/CoreAccountandAccountProfile; resolver returns true if a linked external identity exists.is_externally_managed; hides the Password tab unless it’sfalse(also while loading); shows a loading state on the Password tab while the profile resolves; shows a “Password managed externally” panel; toasts mutation errors.Written for commit c57ff6f. Summary will update on new commits.