Skip to content

feat(auth): opt-in flag to skip OIDC email_verified check for enterprise IdPs - #1859

Merged
dhruv0811 merged 1 commit into
omnigent-ai:mainfrom
royreznik:feat/oidc-skip-email-verification
Jul 7, 2026
Merged

feat(auth): opt-in flag to skip OIDC email_verified check for enterprise IdPs#1859
dhruv0811 merged 1 commit into
omnigent-ai:mainfrom
royreznik:feat/oidc-skip-email-verification

Conversation

@royreznik

Copy link
Copy Markdown
Contributor

Related issue

N/A

Summary

  • Standard Okta tiers (without custom API Access Management) omit the email_verified claim from id_tokens for directory-provisioned users. Omnigent's OIDC callback hard-rejects any token where the claim isn't affirmatively true, so SSO fails with a 400 ("Could not determine user email from IdP") for these deployments.
  • Adds OMNIGENT_OIDC_SKIP_EMAIL_VERIFICATION (default off): when set, the callback accepts the signed id_token email claim without requiring email_verified. The default path is unchanged — absent/false claims are still a hard reject, preserving the account-takeover protection for IdPs that allow user-asserted emails.
  • Enabling the flag logs a warning at startup and an info line per bypassed login, so the relaxed posture is visible in logs. GitHub OAuth is unaffected (it keeps requiring a verified primary email).

ELI5: the server normally only trusts an email if the identity provider stamps it "verified". Okta's standard tier doesn't include that stamp for company-directory users even though the directory owns the address, so login breaks. This adds an opt-in switch for operators who trust their directory to say "accept the email without the stamp".

id_token → signature/iss/aud valid? ──no──▶ reject
              │yes
   email_verified is true? ──yes──▶ accept
              │no
   OMNIGENT_OIDC_SKIP_EMAIL_VERIFICATION? ──yes──▶ accept (logged)
              │no
            reject (unchanged default)

Test Plan

  • pytest tests/server/test_oidc_callback.py — 12 passed. New parametrized test drives the real callback route with a genuinely RS256-signed id_token that omits email_verified (the Okta shape) and one with email_verified: false; with the flag on, both mint a session for the correct email. All pre-existing rejection tests still pass, proving the default is unchanged.
  • pre-commit run clean on all touched files.

Demo

N/A (non-visual backend change)

Type of change

  • Bug fix
  • Feature
  • UI / frontend change
  • Refactor / chore
  • Docs
  • Test / CI
  • Breaking change

Test coverage

  • Unit tests added / updated
  • Integration tests added / updated
  • E2E tests added / updated
  • Manual verification completed
  • Existing tests cover this change
  • Not applicable

Coverage notes

New tests cover the flag-on path (absent and false claims); the existing test_callback_unverified_email_rejected matrix locks the flag-off default.

Changelog

OMNIGENT_OIDC_SKIP_EMAIL_VERIFICATION=1 lets OIDC logins through when the IdP omits the email_verified claim (e.g. standard-tier Okta with directory-provisioned users)

🤖 Generated with Claude Code

@github-actions github-actions Bot added the size/M Pull request size: M label Jul 2, 2026
@github-actions
github-actions Bot requested a review from dhruv0811 July 2, 2026 13:09
@royreznik

Copy link
Copy Markdown
Contributor Author

The two E2E failures look pre-existing on main rather than caused by this PR:

  • Both failures are pexpect.exceptions.TIMEOUT in tests/e2e/test_repl_approval_e2e.py (test_repl_tool_call_refusal_blocks_tool on shard 0, test_repl_label_driven_ask_approves on shard 1) — REPL approval flow, untouched by this diff (server-side OIDC only).
  • main's latest E2E run (28586422203, ~2h before this PR) fails with the identical timeout in the same file; the last green main E2E run was July 1.

I don't have permission to rerun the failed jobs — happy to rebase/retrigger if that helps once main is green.

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

@royreznik This PR is a Bug fix, Feature, or UI / frontend change but the Demo section is missing or only contains a placeholder.

These change types require a screenshot or screen recording so reviewers can see the new behaviour without checking out the branch. Please update the Demo section with:

  • A screenshot or screen recording of the change, or
  • A link to a hosted video or GIF showing the new behaviour.

Use N/A only when the change has no user-visible effect whatsoever (e.g. a pure refactor or test-only change). If that's the case, uncheck the relevant type box and check Refactor / chore or Test / CI instead.

@github-actions github-actions Bot added the needs-demo PR needs a demo screenshot or recording label Jul 2, 2026
@royreznik

Copy link
Copy Markdown
Contributor Author

@dhruv0811 Any comments?

@dhruv0811

Copy link
Copy Markdown
Member

/review

@omnigent-ci

omnigent-ci Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Polly AI Review

Blocking issues

None. The change is correctly scoped: the opt-out is checked only inside _resolve_oidc_email after signature/iss/aud/email-presence validation still pass, so an unsigned or wrong-issuer token is still rejected. The default (False) path is byte-for-byte the prior behavior, and the new test (test_callback_skip_verification_flag_admits_unverified) plus the retained test_callback_unverified_email_rejected lock both sides. The dataclass addition (skip_email_verification: bool = False as the last field) is ordering-safe.

Security vulnerabilities

None introduced. The flag intentionally relaxes a security boundary, but it is off by default, gated behind an explicit env var, and the residual defenses remain in force downstream:

  • The signed-JWT chain (PyJWKClient + iss/aud verification) is unchanged, so only tokens genuinely issued by the configured IdP reach the bypass.
  • Admission control (admission.is_admitted domain allowlist, reserved-name rejection, admin-list promotion) runs after email resolution regardless of the flag, so the blast radius is bounded to the trusted issuer's own directory rather than arbitrary emails.
  • The relaxed posture is observable: a startup warning and a per-login info line. Good operational hygiene.

The account-takeover risk (an IdP that lets users self-assert arbitrary unverified emails) is accurately described in the docstrings/comments and correctly framed as the operator's explicit decision. GitHub OAuth is unaffected — it goes through _resolve_github_email, which never consults this flag.

Non-blocking notes

  • The flag is global, not per-issuer. Since the config models a single issuer per deployment this is fine today, but if multi-issuer support is ever added the bypass would apply to all of them — worth a note in that future work.
  • Consider surfacing the bypassed logins in an audit/metrics channel rather than only an info log, since info is often filtered in production; the current logging is adequate but easy to lose.

Summary

A tight, well-tested, opt-in feature that relaxes the email_verified requirement only when an operator explicitly sets OMNIGENT_OIDC_SKIP_EMAIL_VERIFICATION. Signature/issuer/audience validation and domain admission control remain intact, the default is provably unchanged, the relaxed path is logged, and the new parametrized tests cover both the absent- and false-claim cases. Backend-only change with no user-visible surface, so no visual demonstration is needed. Approve.


Automated review by Polly · workflow run

@dhruv0811 dhruv0811 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good! Sorry for the late reply :) Thanks for adding this!!

Standard Okta tiers (without custom API Access Management) omit the
email_verified claim from id_tokens for directory-provisioned users,
so the OIDC callback's hard reject breaks SSO for those deployments.

Add OMNIGENT_OIDC_SKIP_EMAIL_VERIFICATION (default off): when set,
accept the signed id_token email claim without requiring
email_verified. Default path unchanged — absent/false claims still
hard-reject. Enabling logs a startup warning plus an info line per
bypassed login. GitHub OAuth unaffected.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@dhruv0811
dhruv0811 force-pushed the feat/oidc-skip-email-verification branch from f8ca059 to 954ff5d Compare July 7, 2026 22:48
@dhruv0811
dhruv0811 enabled auto-merge (squash) July 7, 2026 22:49
@omnigent-ci

omnigent-ci Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Polly AI Review

Blocking issues

None. The change is correct and well-scoped:

  • The new skip_email_verification dataclass field is added as the last field of OIDCConfig with a = False default, so it does not break the frozen dataclass's positional field ordering (all preceding fields are non-default).
  • Both construction sites (github and generic oidc) pass the flag through, and the callback (_resolve_oidc_email) only consults it after JWT signature / iss / aud validation has already succeeded — so the signed-token provenance guarantee is preserved; only the mailbox-ownership assertion is waived.
  • Downstream admission control (domain allowlist, reserved-name rejection, admin promotion) in _do_callback still runs on the returned email, so the flag does not bypass the domain gate.

Security vulnerabilities

None introduced beyond the deliberate, documented trade-off. Worth stating the posture explicitly so it's an informed decision:

  • The flag intentionally weakens a security boundary: when enabled, any email claim in a validly-signed id_token is trusted as identity, even if the IdP did not verify mailbox ownership. For an IdP that lets users set an arbitrary unverified email, this permits impersonation of any address within an admitted domain. This is the documented risk and is correctly gated behind (a) default-off, (b) a startup warning log, and (c) a per-login info log. The GitHub path is untouched. The mitigation posture (opt-in + logging + docs emphasizing "trusted enterprise directory only") is appropriate — not a blocking issue.

Non-blocking notes

  • _resolve_oidc_email's docstring :returns: line still reads "marked verified … or email_verified is not truthy" as the rejection condition. The diff amended the inline comment and the summary but the enumerated return contract wasn't updated to mention the skip path — minor doc drift.
  • Consider scoping the bypass more tightly than "email present but not verified": the flag currently also admits an explicit email_verified: false, not just an absent claim. The Okta case being solved is the absent-claim shape; accepting an affirmative false is a slightly broader relaxation than the motivation requires. This is a design judgment, not a defect — the test suite deliberately covers both, so it's intentional.
  • Test coverage is good: the parametrized test exercises both absent and false claims through the real RS256-signed callback route, and the existing rejection matrix locks the default-off behavior.

Summary

A tightly-scoped, opt-in feature that fixes a real login failure for standard-tier Okta / directory-provisioned users while leaving the secure default fully intact. Signature/issuer/audience validation is unchanged, the relaxation is default-off with startup and per-login logging, and downstream domain admission still applies. The deliberate security trade-off is well-documented and clearly surfaced in logs. No blocking issues; only minor docstring drift and an optional suggestion to narrow the bypass to the absent-claim case. Approve.


Automated review by Polly · workflow run

@dhruv0811
dhruv0811 merged commit 42177d0 into omnigent-ai:main Jul 7, 2026
46 checks passed
@github-actions github-actions Bot added the needs-doc-update Merged PR needs a user-facing docs update label Jul 7, 2026
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🏷️ Doc impact: needs-doc-update

Adds a new user-configurable OIDC deploy setting (OMNIGENT_OIDC_SKIP_EMAIL_VERIFICATION) that changes login/email-verification behavior, affecting the deploy/OIDC setup documentation.

Drafting a docs PR to omnigent-ai/omnigent-site (staged on 0.5-docs until release)…

Auto-classified on merge. Set the label manually before merging to override. · run

dhruv0811 pushed a commit to omnigent-ai/omnigent-site that referenced this pull request Jul 10, 2026
Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>
dhruv0811 added a commit to omnigent-ai/omnigent-site that referenced this pull request Jul 10, 2026
* docs: document omnigent-ai/omnigent#1722 (#261)

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>

* docs: document omnigent-ai/omnigent#2018 (#265)

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>

* docs: document omnigent-ai/omnigent#1386 (#272)

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>

* docs: document omnigent-ai/omnigent#2156 (#278)

* docs: document omnigent-ai/omnigent#2156

* Apply suggestion from @serena-ruan

* Apply suggestions from code review

Co-authored-by: Serena Ruan <82044803+serena-ruan@users.noreply.github.com>

---------

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>
Co-authored-by: Serena Ruan <82044803+serena-ruan@users.noreply.github.com>

* docs: document omnigent-ai/omnigent#2022 (#268)

* docs: document omnigent-ai/omnigent#2022

* docs: add steering gif and simplify message queue section

Co-authored-by: Isaac

---------

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>
Co-authored-by: Serena Ruan <serena.rxy@gmail.com>

* chore(api): sync openapi.json from omnigent@3c7a558 (#274)

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>

* docs: document omnigent-ai/omnigent#526 (#279)

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>

* docs: rename hindsight extra to memory (omnigent[memory]) (#282)

The memory tools ship under the `memory` extra (omnigent[memory]), not
`hindsight`. Update the install instruction and extra name to match.
The Hindsight product name and the hindsight_* tool names are unchanged.

* docs: document default base branch for new worktrees (#284)

* docs: document default base branch for new worktrees

* docs: condense worktree branches section and add setting demo gif

Co-authored-by: Isaac

---------

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>
Co-authored-by: Serena Ruan <serena.rxy@gmail.com>

* chore(api): sync openapi.json from omnigent@7fb779f (#281)

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>

* chore(api): sync openapi.json from omnigent@60e775a (#288)

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>

* docs: document omnigent-ai/omnigent#2152 (#280)

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>

* docs: document omnigent-ai/omnigent#1859 (#277)

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>

* docs: document omnigent-ai/omnigent#2135 (#276)

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>

---------

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>
Co-authored-by: Serena Ruan <82044803+serena-ruan@users.noreply.github.com>
Co-authored-by: Serena Ruan <serena.rxy@gmail.com>
Co-authored-by: Pat Sukprasert <pattara.sk127@gmail.com>
Co-authored-by: Dhruv Gupta <dhruv.gupta@databricks.com>
yours-aditya pushed a commit to yours-aditya/omnigent that referenced this pull request Jul 16, 2026
…i#1859)

Standard Okta tiers (without custom API Access Management) omit the
email_verified claim from id_tokens for directory-provisioned users,
so the OIDC callback's hard reject breaks SSO for those deployments.

Add OMNIGENT_OIDC_SKIP_EMAIL_VERIFICATION (default off): when set,
accept the signed id_token email claim without requiring
email_verified. Default path unchanged — absent/false claims still
hard-reject. Enabling logs a startup warning plus an info line per
bypassed login. GitHub OAuth unaffected.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Aditya Devarapalli <adityareddyd2@gmail.com>
daniellok-db added a commit to omnigent-ai/omnigent-site that referenced this pull request Jul 17, 2026
* docs: document omnigent-ai/omnigent#1722 (#261)

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>

* docs: document omnigent-ai/omnigent#2018 (#265)

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>

* docs: document omnigent-ai/omnigent#1386 (#272)

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>

* docs: document omnigent-ai/omnigent#2156 (#278)

* docs: document omnigent-ai/omnigent#2156

* Apply suggestion from @serena-ruan

* Apply suggestions from code review

Co-authored-by: Serena Ruan <82044803+serena-ruan@users.noreply.github.com>

---------

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>
Co-authored-by: Serena Ruan <82044803+serena-ruan@users.noreply.github.com>

* docs: document omnigent-ai/omnigent#2022 (#268)

* docs: document omnigent-ai/omnigent#2022

* docs: add steering gif and simplify message queue section

Co-authored-by: Isaac

---------

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>
Co-authored-by: Serena Ruan <serena.rxy@gmail.com>

* chore(api): sync openapi.json from omnigent@3c7a558 (#274)

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>

* docs: document omnigent-ai/omnigent#526 (#279)

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>

* docs: rename hindsight extra to memory (omnigent[memory]) (#282)

The memory tools ship under the `memory` extra (omnigent[memory]), not
`hindsight`. Update the install instruction and extra name to match.
The Hindsight product name and the hindsight_* tool names are unchanged.

* docs: document default base branch for new worktrees (#284)

* docs: document default base branch for new worktrees

* docs: condense worktree branches section and add setting demo gif

Co-authored-by: Isaac

---------

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>
Co-authored-by: Serena Ruan <serena.rxy@gmail.com>

* chore(api): sync openapi.json from omnigent@7fb779f (#281)

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>

* chore(api): sync openapi.json from omnigent@60e775a (#288)

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>

* docs: document omnigent-ai/omnigent#2152 (#280)

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>

* docs: document omnigent-ai/omnigent#1859 (#277)

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>

* docs: document omnigent-ai/omnigent#2135 (#276)

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>

* docs: document official kubernetes server image variant (#285)

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>

---------

Co-authored-by: omnigent-ci[bot] <294685417+omnigent-ci[bot]@users.noreply.github.com>
Co-authored-by: Serena Ruan <82044803+serena-ruan@users.noreply.github.com>
Co-authored-by: Serena Ruan <serena.rxy@gmail.com>
Co-authored-by: Pat Sukprasert <pattara.sk127@gmail.com>
Co-authored-by: Dhruv Gupta <dhruv.gupta@databricks.com>
Co-authored-by: Daniel Lok <daniel.lok@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-demo PR needs a demo screenshot or recording needs-doc-update Merged PR needs a user-facing docs update size/M Pull request size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants