Admin social login (Google/Microsoft → admin session) - #49
Merged
Merged
Conversation
…ogin seed_admin_from_env now keys the admin by normalized email, accepts a pinned AGAMI_ADMIN_PROVIDER + first/last name, and makes the password optional (an OIDC-only admin) — requiring at least one credential. The admin-gate username and the password-login lookup normalize the email so it's case-insensitive.
…pinned) Add admin_oidc_start (purpose-marked state) + a purpose branch in the shared oidc_callback that mints an admin SESSION via complete_admin_oidc_login when the verified identity resolves to the configured admin (reusing _resolve_oidc_user's provider+subject binding, so the pin closes IdP-confusion). Re-add the pinned provider button on the admin login; wire /admin/oidc/start into the routes + bearer public-skip. The connector flow still mints a bearer; the two can't cross.
README: the admin is identified by email and signs in with a pinned provider (AGAMI_ADMIN_PROVIDER) and/or password — matching the MCP login options; one OAuth redirect URI serves both flows. render_previews shows the admin login with the provider button.
- Admin login is strictly onboarded-only: pass allow_signup=False into _resolve_oidc_user for the admin_login purpose so an admin sign-in attempt can never self-provision a demo user (even with public signup on). - Keep the social button on the admin login when a password attempt fails. - Fix the README run block (an inline comment broke the line continuation). - Drop a bogus noqa on a used lazy import. + tests for the first two.
There was a problem hiding this comment.
Pull request overview
This PR adds admin social login (Google/Microsoft) for the /admin console by reusing the existing OIDC callback (/oauth/oidc/callback) and branching based on a signed purpose=admin_login marker in state, minting an admin session cookie instead of an /mcp bearer token.
Changes:
- Introduces an admin OIDC start route and callback branching to mint an admin session (
admin_oidc_start+oidc_callbackpurpose branch). - Adds admin-side handling/UI for a pinned-provider login button and an OIDC completion path (
complete_admin_oidc_login). - Updates admin seeding to support email-keyed identity, optional password, optional pinned provider, and display names; adds/updates tests and docs.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_user_store.py | Adds coverage for provider-only/hybrid admin seeding and validation behaviors. |
| tests/test_oidc.py | Adds end-to-end tests for admin OIDC session minting, refusal paths, and IdP-confusion pinning. |
| tests/test_admin.py | Adds a test ensuring admin email login is case-insensitive. |
| render_previews.py | Updates preview rendering to show the pinned-provider button on the admin login page. |
| packages/agami-core/src/user_store.py | Expands admin seeding to support email identity + optional pinned provider/password. |
| packages/agami-core/src/oauth_server.py | Adds admin OIDC start and purpose-based branching in the shared callback. |
| packages/agami-core/src/admin.py | Adds pinned-provider login UI and session-minting completion for admin OIDC. |
| packages/agami-core/README.md | Documents admin identity, pinned provider behavior, and the single redirect URI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+176
to
+180
| email = _normalize_email(os.environ.get("AGAMI_ADMIN_USERNAME", "")) | ||
| if email is None: | ||
| return None | ||
| if get_user(store, username) is not None: | ||
| password = os.environ.get("AGAMI_ADMIN_PASSWORD", "") or None | ||
| provider = os.environ.get("AGAMI_ADMIN_PROVIDER", "").strip().lower() or None |
Comment on lines
+250
to
+258
| def _admin_provider() -> str | None: | ||
| """The admin's pinned OIDC provider (`AGAMI_ADMIN_PROVIDER`), or None — only returned when it's a | ||
| provider that's actually configured, so the login page never shows a button that can't work.""" | ||
| key = os.environ.get("AGAMI_ADMIN_PROVIDER", "").strip().lower() | ||
| if not key: | ||
| return None | ||
| import oidc # lazy: the egress module, server-only | ||
|
|
||
| return key if key in oidc.available_providers() else None |
- seed_admin_from_env: treat a whitespace-only password as unset (don't crash startup), and make the idempotency check also match a pre-existing (mixed-case) raw username so an upgrade can't create a duplicate admin row. - Show the admin-login provider button only when the admin's stored row is actually bound to that provider (not merely when the provider is configured), so config drift can't render a dead button. + regression tests.
Contributor
Author
|
Thanks @copilot — both addressed in 8a8b73c:
Both have regression tests (whitespace-password / upgrade-idempotency in |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Summary
Adds admin social login — signing in to the
/adminconsole with Google/Microsoft, so the adminlogin offers the same options as the MCP login (most users authenticate with a social provider).
The MCP buttons run the OIDC flow and mint a bearer JWT for
/mcp; the admin needs the same IdPflow but ending in an admin session cookie. The key reuse: the admin flow goes through the one
registered OIDC callback (
/oauth/oidc/callback) and branches on apurposemarker in the signedstate — so an admin-login state can only mint a session and a connector state can only mint a bearer,
and the deployer registers a single redirect URI.
Provider-pinned admin (owner decision): the admin is identified by email
(
AGAMI_ADMIN_USERNAME) and pinned to one provider (AGAMI_ADMIN_PROVIDER). Admin login reusesACE-006's hardened
_resolve_oidc_user(provider + subject binding), then requires the resolvedidentity == the configured admin before minting a session — so IdP-confusion (the admin email
controlled at a different IdP) is closed for free.
Changes
oauth_server.py—admin_oidc_start(purpose-marked state, CSRF cookie, redirect to the IdP viathe shared callback) + a
purposebranch inoidc_callbackthat mints an admin session viaadmin.complete_admin_oidc_login. Admin login is strictly onboarded-only (allow_signup=False), soan admin sign-in attempt never self-provisions a user.
admin.py—complete_admin_oidc_login(mints a session only for the configured admin, else a403 page, no session),
_admin_provider(), the pinned-provider button on the admin login, andemail-normalized admin identity (case-insensitive login + gate).
user_store.py—seed_admin_from_envkeys the admin by email, acceptsAGAMI_ADMIN_PROVIDERAGAMI_ADMIN_FIRST_NAME/LAST_NAME, and makes the password optional (a provider-only admin) —requiring at least one credential.
redirect URI;
render_previews.pyshows the admin login with the provider button.Test plan
tests/test_oidc.py— admin login mints a session (302/admin+ hardened cookie); a non-admin and anunknown identity are refused (403, no session); IdP-confusion via a different provider is
refused; the login page shows only the pinned provider; an unconfigured provider is a clean 400; admin
login never self-provisions even with public signup on; the social button survives a bad password.
tests/test_user_store.py— seed: provider-only (passwordless), hybrid, unknown-provider ignored,requires-a-credential, email normalized.
tests/test_admin.py— case-insensitive admin email login./code-review+/security-review(new credential-minting path); findings addressed.Checklist
oidc.py, the existing single egress module)Spec: ACE-012