oidc: multiple redirect URIs per client + goose migrations#749
Merged
Conversation
Apps such as audiobookshelf need more than one redirect URI per OIDC client (a web callback plus a mobile-app callback). The client registration stored and emitted exactly one. - oidc_client gains a nullable redirect_uris column (idempotent ALTER, default ''); the legacy redirect_uri column is kept and still populated with the first URI, so existing rows and a downgrade keep working. - Clients() returns RedirectURIs, falling back to [redirect_uri] when the new column is empty; the Authelia template ranges over the list. - /oidc/register accepts a repeated redirect_uri form field, so the existing single-value callers are unchanged and new callers can pass several. Backward compatible: no manual migration; existing single-redirect clients render identically and re-populate redirect_uris on their next registration.
A comma is a legal unencoded sub-delimiter in a URL path/query (RFC 3986), so a redirect URI may contain one; splitting on comma could corrupt it. A literal space can never appear in a URL, so store the list space-separated and split on whitespace. Added a test with a comma-bearing redirect URI.
Replace the hand-rolled idempotent migrator with pressly/goose (pure Go, works with modernc.org/sqlite, CGO_ENABLED=0) so migrations are versioned and the boot path can run to the latest version. The existing steps (config, oidc_client, custom_proxy + its https/authelia columns) become ordered Go migrations; they stay idempotent so a pre-goose device with no goose_db_version table upgrades cleanly. Multiple redirect URIs per OIDC client now live in their own oidc_redirect_uri table (client_id, redirect_uri) — no delimiter, so a redirect URI may contain any character (commas are legal in URLs). Migration v6 creates the table, backfills from the legacy oidc_client.redirect_uri, and drops that column. Tests use the real migration code via MigrateTo(version): bring the DB to the pre-change schema, insert a legacy client row, migrate to latest, and assert the row moved to oidc_redirect_uri and the column is gone. Existing custom_proxy legacy-upgrade tests still pass (goose runs the idempotent steps over partial legacy schemas).
cyberb
changed the title
The appcenter 'files' search now also surfaces Nextcloud because its store description changed to 'Access & share your files from any device.' and the filter intentionally matches description (unit-tested via 'photos' -> PhotoPrism). That is store-metadata drift, not a UI regression, so the stale stable #2884 baseline no longer matches. Point the skip-build at the current latest stable build so the visual-diff step skips the outdated baseline until stable rebuilds.
- authelia: read the redirect URI value in range instead of indexing twice - drop TestOIDC_RedirectURIContainingCommaIsPreserved (moot now that URIs live in a row-per-URI table with no delimiter) - add TestMigrator_UpgradesPreGooseDbWithoutVersionTable: simulate a pre-goose device (legacy oidc_client with redirect_uri, no goose_db_version), run Migrate(), assert the row is moved to oidc_redirect_uri and the column dropped
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.
Why
Apps like audiobookshelf need more than one redirect URI per OIDC client — a web callback (
/auth/openid/callback) and a mobile-app callback (/auth/openid/mobile-redirect). The client model stored a singleredirect_uri, so web SSO and the official mobile app couldn't both work against one client.What changed
1. Multiple redirect URIs, normalized. They now live in their own table
oidc_redirect_uri (client_id, redirect_uri)— one row per URI, no delimiter, so a redirect URI may contain any character (commas/sub-delims are legal in URLs per RFC 3986).Clients()returnsRedirectURIs []string; the Authelia template ranges over them./oidc/registeraccepts a repeatedredirect_uriform field, so single-value callers are unchanged and new callers can pass several.2. Migrations moved to goose. The hand-rolled idempotent migrator is replaced by goose (pure-Go, works with
modernc.org/sqlite,CGO_ENABLED=0). Existing steps become ordered Go migrations (v1–v5); they stay idempotent so a pre-goose device — which has the tables but nogoose_db_versionledger — upgrades cleanly. v6 createsoidc_redirect_uri, backfills from the legacyoidc_client.redirect_uri, and drops that column.Is it breaking for existing devices? No.
goose_db_versionon a legacy DB, runs v1–v6; the create/alter steps are idempotent (guardedif not exists/ column-existence checks), so existing tables/rows are untouched.redirect_uriinto the new table, then drops the column — data is preserved.redirect_uriform field still works, so the currentgolibclient and every shipped app register unchanged.$SNAP_DATA, so a failed platform refresh rolls the DB back with the revision snapshot — the dropped column returns together with the older binary.Tests (use the real migration code)
TestMigrator_MigratesLegacyRedirectUriIntoTableAndDropsColumn:MigrateTo(5)builds the pre-change schema via the actual migrations → insert a legacy client row →Migrate()to latest → assert the row moved tooidc_redirect_uriandoidc_client.redirect_uriis gone.custom_proxylegacy-upgrade + prod-row-preservation tests still pass (goose runs the idempotent steps over partial legacy schemas).config/auth/restgreen locally. (Unrelatedidentification/network/backupfailures are Android/Termux dev-sandbox only: hardware-ID, netlink routes,useradd/os/user.)Follow-up (separate PRs)
golib: slice-basedRegisterOIDCClienthelper (platform already accepts repeated values).audiobookshelf: register web + mobile callbacks so the official Android app gets SSO.