Make authentication a plug-in, and move our Clerk wiring out - #121
Merged
Conversation
The public repo carried the authentication setup of one hosted instance. Anyone
cloning it read that as part of the platform, when it described how we host it.
Multi-tenancy stays here: the groups table, /api/g/{slug}/ routing, and the
Identity attached to every request. What leaves is any knowledge of how a caller
proves who they are. A deployment supplies that as a distribution declaring one
entry point under glossogen.identity_provider.v1.
With no provider installed the server is single-tenant: every request resolves to
the synthetic local group. That is why there is no built-in implementation of the
contract, and why nothing in it is optional.
The middleware keeps the parts a provider must not get wrong. It extracts the
bearer credential, parses the URL's group slug, and resolves that slug to a groups
row before calling resolve_identity, so a provider never queries that table and
cannot get tenancy isolation wrong. It answers one question: does this credential
grant access to this group, and as whom.
Ambiguity refuses to boot. Two declared providers, or one declared under a
contract version this platform does not read, raises. The scenario and metric
loaders warn and carry on in the same situation, which is right for them: a
missing scenario is a missing feature. A missing auth provider is a server that
authenticates nothing while an operator believes it is protected.
Deferred OAuth consent stays platform code. Parking a request is about having more
than one group to choose from, not about any one vendor, and approve_pending_consent
already took a resolved group_id. Only the approval endpoint is pluggable.
/mcp/whoami stays too, since the CLI calls it in either mode.
create_app() exists so a test can build an app around a stub provider, and so the
export script can pin the provider off. That second one matters: export_openapi.py
imported the module-level app, so a developer with a provider installed would have
had `make gen-api-types` quietly add its routes to the committed schema and break
check-api-types for everyone else.
groups.clerk_org_id becomes external_org_id. The column holds the id a group
carries in whichever provider a deployment configures, so naming it after one made
the schema describe a deployment choice. Migration 0005 renames the column, its
index, and the auto-generated UNIQUE constraint, which RENAME COLUMN leaves behind.
Its revision id is under 32 characters because alembic_version.version_num is
varchar(32) and a longer id applies the change then fails the bookkeeping.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Next.js resolves pages by file path, so /sign-in, /sign-up, /select-org and /mcp-consent cannot live outside this repo. They stay as shells that render one adapter component each and own nothing else. src/proxy.ts stays for the same reason and keeps its matcher as a literal, because Next reads it statically and rejects a re-exported one. The contract is auth-adapter.ts; the implementation is adapter/, and the copy here answers "no provider configured" to every slot. Replacing that directory is the whole integration surface. Four modules rather than one object, because React's module graph forbids one. readSession needs a server-only import, AuthProvider is a client component, getSessionToken runs in the browser with no React so api-client.ts stays importable from either side, and the proxy delegate runs in the edge runtime. No single module can be imported from all four places. The no-op components declare the props they ignore. A component taking no props is assignable to ComponentType<P>, so the conformance assertion at the foot of each module accepts a zero-prop version while every call site fails to type-check. tsc caught that; the assertion did not. CLERK_PUBLISHABLE_KEY becomes AUTH_PUBLIC_PUBLISHABLE_KEY, and RuntimeConfig gains an auth map collected from every AUTH_PUBLIC_* variable rather than a named field, since the platform cannot know what values a provider needs. The request-time read is unchanged, so one compiled image still serves any environment. Those values reach the browser and are visible in page source; an adapter's secrets stay on the server and never enter that object. The MCP consent page splits along the same line as the backend. The endpoint, its payload and the approve panel are our API and our copy, so they stay. Only "who is this, and which group" goes to the adapter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The docs named one identity provider ninety times, in the env tables, the architecture overview, the deployment guide and the security notes. Someone self-hosting had to read past all of it to find out that authentication is optional and pluggable. The two modes are now single-tenant, meaning no provider installed, and multi-tenant, meaning one is. Both halves of the contract are documented in docs/web-ui.md under the existing Authentication heading, so every inbound anchor keeps working and mkdocs needs no nav change. CHANGELOG.md keeps its mentions. Those entries describe what a shipped release did, and rewriting them would make the log lie. Migrations 0001 and 0003 keep theirs for the same reason: they record SQL that already ran. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
the
release:patch
Contributor
Vulture reads only src/, scripts/ and linter/, so a function whose sole caller is an installed identity provider looks dead. Three are: soft_delete_group_by_external_org_id, resolve_frontend_url, and approve_pending_consent. All three are platform API a provider is expected to call, so they are whitelisted rather than deleted. Regenerated over the same paths the lint targets check, per the pre-commit checklist: doing it over a narrower set drops the entries covering the ones it left out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| depends_on # unused variable (src/glossogen/db/migrations/versions/0004_add_evaluation_content_hash.py:25) | ||
| upgrade # unused function (src/glossogen/db/migrations/versions/0004_add_evaluation_content_hash.py:28) | ||
| downgrade # unused function (src/glossogen/db/migrations/versions/0004_add_evaluation_content_hash.py:32) | ||
| revision # unused variable (src/glossogen/db/migrations/versions/0005_rename_external_org_id.py:20) |
| upgrade # unused function (src/glossogen/db/migrations/versions/0004_add_evaluation_content_hash.py:28) | ||
| downgrade # unused function (src/glossogen/db/migrations/versions/0004_add_evaluation_content_hash.py:32) | ||
| revision # unused variable (src/glossogen/db/migrations/versions/0005_rename_external_org_id.py:20) | ||
| down_revision # unused variable (src/glossogen/db/migrations/versions/0005_rename_external_org_id.py:21) |
| downgrade # unused function (src/glossogen/db/migrations/versions/0004_add_evaluation_content_hash.py:32) | ||
| revision # unused variable (src/glossogen/db/migrations/versions/0005_rename_external_org_id.py:20) | ||
| down_revision # unused variable (src/glossogen/db/migrations/versions/0005_rename_external_org_id.py:21) | ||
| branch_labels # unused variable (src/glossogen/db/migrations/versions/0005_rename_external_org_id.py:22) |
| revision # unused variable (src/glossogen/db/migrations/versions/0005_rename_external_org_id.py:20) | ||
| down_revision # unused variable (src/glossogen/db/migrations/versions/0005_rename_external_org_id.py:21) | ||
| branch_labels # unused variable (src/glossogen/db/migrations/versions/0005_rename_external_org_id.py:22) | ||
| depends_on # unused variable (src/glossogen/db/migrations/versions/0005_rename_external_org_id.py:23) |
| down_revision # unused variable (src/glossogen/db/migrations/versions/0005_rename_external_org_id.py:21) | ||
| branch_labels # unused variable (src/glossogen/db/migrations/versions/0005_rename_external_org_id.py:22) | ||
| depends_on # unused variable (src/glossogen/db/migrations/versions/0005_rename_external_org_id.py:23) | ||
| upgrade # unused function (src/glossogen/db/migrations/versions/0005_rename_external_org_id.py:39) |
| branch_labels # unused variable (src/glossogen/db/migrations/versions/0005_rename_external_org_id.py:22) | ||
| depends_on # unused variable (src/glossogen/db/migrations/versions/0005_rename_external_org_id.py:23) | ||
| upgrade # unused function (src/glossogen/db/migrations/versions/0005_rename_external_org_id.py:39) | ||
| downgrade # unused function (src/glossogen/db/migrations/versions/0005_rename_external_org_id.py:50) |
| depends_on # unused variable (src/glossogen/db/migrations/versions/0005_rename_external_org_id.py:23) | ||
| upgrade # unused function (src/glossogen/db/migrations/versions/0005_rename_external_org_id.py:39) | ||
| downgrade # unused function (src/glossogen/db/migrations/versions/0005_rename_external_org_id.py:50) | ||
| soft_delete_group_by_external_org_id # unused function (src/glossogen/db/queries.py:98) |
| health # unused function (src/glossogen/server/app.py:241) | ||
| server_config # unused function (src/glossogen/server/app.py:247) | ||
| resolve_frontend_url # unused function (src/glossogen/server/frontend_url.py:11) | ||
| health # unused function (src/glossogen/server/health_router.py:17) |
| server_config # unused function (src/glossogen/server/app.py:247) | ||
| resolve_frontend_url # unused function (src/glossogen/server/frontend_url.py:11) | ||
| health # unused function (src/glossogen/server/health_router.py:17) | ||
| server_config # unused function (src/glossogen/server/health_router.py:23) |
…state Two findings from review, both real. The provider-facing surface was undeclared. IdentityProvider says what a provider gives the platform; nothing said what the platform gives back, and that set was exactly what vulture had just flagged as dead. The sharpest case was approve_pending_consent, the one call a provider must make to finish a deferred MCP authorization: reachable only by reaching into request.app.state.oauth_provider, mentioned in no docstring, discoverable only by reading a file this branch deleted. provider_services.py is now that half of the seam. approve_parked_consent wraps the app.state reach-in so a provider never touches it, which also gives approve_pending_consent an in-tree caller and takes it out of the whitelist. frontend_base_url absorbs the orphaned frontend_url module, whose docstring claimed two in-tree readers that did not exist. The two groups query helpers keep their home in queries.py and are named in the module docstring, so the surface is one list rather than four discoveries. The three entries still in the whitelist are declared API with a stated reason, not residue. Whitelisting alone would have papered over that, which was the reviewer's point. The frontend had two disagreeing answers to "is auth configured". runtime-config counted AUTH_PUBLIC_* variables while the adapter knew what it actually exported, and the route gates read the env-var answer. An adapter installed but keeping its configuration server-side, or missing one variable, skipped the /g/<slug> sign-in redirect and pointed the landing CTA at the local workspace while the backend rejected every call. Not a hole, since the backend still enforces, but a symptom nowhere near its cause. AuthSession gains `configured`, so the adapter answers once and the platform stops inferring. isAuthConfigured is gone rather than left as an invitation to the same bug. Also: the MCP-token fallback no longer depends on which status a provider chose. Gating it on 401 meant a provider that answers 403 for an unrecognised credential would silently break glossogen push-to-prod against a hosted backend. The lookup is exact and runs only on an already-failing request, so admitting both costs nothing. Leftovers: DockerfileFrontend's trailing comment and CLAUDE.md's file inventory both still named deleted things. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| model_config # unused variable (src/glossogen/server/mcp/consent_router.py:34) | ||
| approve_consent # unused function (src/glossogen/server/mcp/consent_router.py:64) | ||
| whoami # unused function (src/glossogen/server/mcp/consent_router.py:135) | ||
| frontend_base_url # unused function (src/glossogen/server/identity/provider_services.py:34) |
NewtonSander
added
release:minor
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
1 participant
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.
Authentication becomes a plug-in. Multi-tenancy stays here; knowledge of how a
caller proves who they are does not.
With no provider installed the server is single-tenant: every request resolves to
the synthetic
localgroup. That is the configuration a clone gets, and it is whythe platform needs no built-in implementation of the contract.
Our Clerk wiring moves to
glossogen-deploy, which is where what production runsalready lives. See its companion pull request.
What a provider gets, and what it never sees
A provider is a separate distribution declaring one entry point under
glossogen.identity_provider.v1. The middleware keeps the parts a provider mustnot get wrong: it extracts the bearer credential, parses the URL's group slug, and
resolves that slug to a
groupsrow before callingresolve_identity. So aprovider never queries that table and cannot get tenancy isolation wrong. It
answers one question, and raises
IdentityRejectedwith 401 for a credential thatdoes not verify or 403 for one that does not cover the group.
Ambiguity refuses to boot. Two declared providers, or one declared under a
contract version this platform does not read, raises. The scenario and metric
loaders warn and carry on in the same situation, which is right for them: a missing
scenario is a missing feature. A missing auth provider is a server that
authenticates nothing while an operator believes it is protected.
Frontend
auth-adapter.tsis the contract;adapter/is the implementation, and the copyhere answers "not configured" to every slot. Four modules, not one object, because
React's module graph forbids one:
readSessionneeds a server-only import,AuthProvideris a client component,getSessionTokenruns in the browser with noReact so
api-client.tsstays importable from either side, and the proxy delegateruns in the edge runtime.
/sign-in,/sign-up,/select-organd/mcp-consentstay as shells because theApp Router resolves pages by file path.
CLERK_PUBLISHABLE_KEYbecomesAUTH_PUBLIC_PUBLISHABLE_KEY, andRuntimeConfiggains anauthmap rather than anamed field. The request-time read is unchanged, so one image still serves any
environment.
Verification
make lint-server,make test(657 pass, 30 new),make check-frontend,make docs-build --strictall cleanmake gen-api-typesproduces no diff against the committed schemaadapter overlay compiles, type-checks, and its proxy chunk carries both
organizationPatterns, so multi-group URL routing survivesthe entry point, mounts its routers, and answers 401 on a bad credential, 404 on
an unknown slug, 200 on
/api/healthTwo bugs this caught
Migration revision ids must stay under 32 characters.
alembic_version.version_numis
varchar(32), and0004_add_evaluation_content_hashis exactly 32. A longer idapplies the schema change and then fails the version bookkeeping, leaving the
database migrated but recorded one revision behind.
groups.clerk_org_id TEXT UNIQUEcarries an auto-generated constraint thatALTER TABLE ... RENAME COLUMNdoes not rename. Migration0005renames thecolumn, the index and the constraint, and round-trips on a scratch database.
Merging this cuts a release
The label decides what ships. Merging publishes Clerk-free backend and frontend
images, which is why the promoter in
glossogen-deployis currently disabled: itmust not pin an unauthenticated frontend into production. Re-enable it only after
that repo's pins point at the derived image.
AUTH_PUBLIC_PUBLISHABLE_KEYis already set on the production frontend servicealongside the old variable, so the rename is staged rather than a cutover.
🤖 Generated with Claude Code