Skip to content

Add feature flag to gate SSO display-name account fallback (IFC-2662)#9396

Merged
polmichel merged 8 commits into
stablefrom
pmi-20260529-feature-flag-security
May 31, 2026
Merged

Add feature flag to gate SSO display-name account fallback (IFC-2662)#9396
polmichel merged 8 commits into
stablefrom
pmi-20260529-feature-flag-security

Conversation

@polmichel
Copy link
Copy Markdown
Contributor

@polmichel polmichel commented May 29, 2026

Summary

Adds the security.sso_account_name_fallback setting (env INFRAHUB_SECURITY_SSO_ACCOUNT_NAME_FALLBACK, default enabled) to gate whether an SSO login with no linked identity may claim a pre-existing account that matches by display name.

  • Enabled (default): a first SSO login adopts a same-named account, as long as it has not already been linked to another identity — preserving the upgrade/migration path.
  • Disabled: such a login always provisions a separate account (named by email, or a hard error if the email is also taken) instead of reusing an existing one.

This lets operators turn off the transitional name-based adoption once all SSO users have completed their first login. Full removal of the fallback is tracked separately for 1.11.

Part of IFC-2662

🤖 Generated with Claude Code


Summary by cubic

Adds security.sso_account_name_fallback (INFRAHUB_SECURITY_SSO_ACCOUNT_NAME_FALLBACK, default true) to gate display‑name account adoption on first SSO login, registers it in docker-compose.yml and docs, and tightens the identity lookup to use account__id. When disabled, we create a separate account using the email; if both display name and email are taken the login fails, reducing spoofing risk per IFC-2662 ahead of removal in 1.11.

  • Refactors

    • Use scalar account__id filter in SSO signin and tests; add test to ensure unrelated identities don’t block adoption.
  • Migration

    • Keep enabled until all SSO users complete first login.
    • Then set INFRAHUB_SECURITY_SSO_ACCOUNT_NAME_FALLBACK=false to harden; with the flag off we create by email, and fail if both names are taken.

Written for commit 366fbeb. Summary will update on new commits.

Review in cubic

@github-actions github-actions Bot added type/documentation Improvements or additions to documentation group/backend Issue related to the backend (API Server, Git Agent) labels May 29, 2026
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

0 issues found across 1 file (changes from recent commits).

Shadow auto-approve: would require human review. This change modifies core SSO authentication logic by introducing a feature flag that controls whether a display name match can adopt a pre-existing account, which directly impacts account security and data integrity, making it too high-risk for auto-approval.

Re-trigger cubic

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 5 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Shadow auto-approve: would require human review. This PR modifies the core SSO authentication logic in auth.py by introducing a feature flag that controls whether an unlinked account may be adopted by display name—a security-sensitive change that could affect account ownership and spoofing protections, requiring human review to ensure correctness.

Re-trigger cubic

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

0 issues found across 1 file (changes from recent commits).

Shadow auto-approve: would require human review. This PR modifies core SSO authentication logic and adds a feature flag controlling account adoption, which is a security-sensitive change with potential impact on user provisioning and account linking, requiring human review to ensure correctness.

Re-trigger cubic

@polmichel polmichel marked this pull request as ready for review May 29, 2026 19:24
@polmichel polmichel requested review from a team as code owners May 29, 2026 19:25
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 29, 2026

Merging this PR will not alter performance

✅ 12 untouched benchmarks


Comparing pmi-20260529-feature-flag-security (366fbeb) with stable (692f318)

Open in CodSpeed

Comment thread backend/infrahub/auth.py Outdated
existing_identities = await NodeManager.query(
db=db,
schema=InfrahubKind.EXTERNALIDENTITY,
filters={"account__ids": [account_by_name.id]},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should this be "account_id": account_by_name.id?

Copy link
Copy Markdown
Contributor Author

@polmichel polmichel May 31, 2026

Choose a reason for hiding this comment

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

Yes it should, good point.
I think it still works as this is covered by the tests: the tests fail when account_ids is replaced by account__id.
But better to explicitly use that since this is a one-to-one relationship

register_core_models_schema: SchemaBranch,
sso_account_name_fallback_disabled: None,
) -> None:
"""With the fallback disabled a display-name match is not adopted, so a separate account is.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is just half a sentence with a period in the middle

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you, fixed.

@ogenstad
Copy link
Copy Markdown
Contributor

Small comment here is that we'd also want to backport this to 1.8.x and release a 1.8.7. It could be fine to have this PR target stable and then create a new branch from the 1.8.6 tag and cherry pick.

polmichel and others added 3 commits May 31, 2026 10:24
…signin

The ExternalIdentity.account relationship is single-cardinality, so a single-value
account__id filter expresses the intent more directly than passing a one-element
list to account__ids. Functionally equivalent — both forms scope correctly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous form split a single sentence across the summary line with a stray
period to satisfy D205, leaving the docstring unreadable.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pre-existing identity on an unrelated account must not block adoption of a
same-named, never-linked account. Exercises the scoping of the account filter
on the existing-identity lookup; catches a misspelled relationship filter that
would silently match every row.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

0 issues found across 2 files (changes from recent commits).

Shadow auto-approve: would auto-approve. The change adds a feature flag that defaults to the existing behavior, includes thorough tests for both the new and unchanged paths, and corrects an internal filter for clarity with no breaking effect on current deployments.

Re-trigger cubic

@polmichel polmichel merged commit 9dc56f1 into stable May 31, 2026
99 of 100 checks passed
@polmichel polmichel deleted the pmi-20260529-feature-flag-security branch May 31, 2026 13:39
polmichel added a commit that referenced this pull request Jun 1, 2026
… (#9399)

* Add feature flag to gate SSO display-name account fallback (IFC-2662) (#9396)

* new security settings that enable/disable the account creation fallback

* plug the new security settings about account assignation fallback into authentication layer

* chore: add changelog fragment for SSO account name fallback setting

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test: clarify docstring for disabled-fallback both-names-taken case

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* chore: register sso_account_name_fallback env var in docker-compose

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor: use scalar account__id filter for single-id lookups in SSO signin

The ExternalIdentity.account relationship is single-cardinality, so a single-value
account__id filter expresses the intent more directly than passing a one-element
list to account__ids. Functionally equivalent — both forms scope correctly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test: rewrite both-names-taken docstring as proper summary + description

The previous form split a single sentence across the summary line with a stray
period to satisfy D205, leaving the docstring unreadable.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test: assert SSO transition fallback ignores unrelated identities

Pre-existing identity on an unrelated account must not block adoption of a
same-named, never-linked account. Exercises the scoping of the account filter
on the existing-identity lookup; catches a misspelled relationship filter that
would silently match every row.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test: adapt SSO signin test to 1.8.x UserToken return shape

On 1.8.x signin_sso_account returns UserToken directly; the cherry-picked test
was written against the stable AuthResult shape (.token.access_token). Match
the surrounding tests in this file by using the returned UserToken inline.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

group/backend Issue related to the backend (API Server, Git Agent) type/documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants