Add an optional Auth0 sign-in gate alongside Clerk - #1853
Merged
Conversation
Hosted deployments can already gate the web app behind Clerk. This adds Auth0 as an alternative provider, configured independently and loaded instead of Clerk, for deployments that already run an Auth0 tenant. Auth0 has no embedded sign-in card, so the gate uses Universal Login: the visitor is redirected to the tenant's hosted page and returned to the app. The URL they arrived on travels in the transaction's appState, so a shared `?project=...` link still opens its project after signing in, and the single-use `code`/`state` parameters are stripped from the address bar on return. Provider selection lives in one place (lib/auth-gate.ts) and follows the precedence the individual settings already use: a provider named in the deployment env beats one baked into the build, and naming both at the same level keeps Clerk so an existing gated image never switches on its own. The Docker entrypoint refuses to boot when both are passed at runtime, and validates the Auth0 pair together rather than serving an ungated app from a half configuration. Each provider stays in its own dynamically imported chunk, both excluded from the service-worker precache, so an ungated deployment downloads neither SDK. The gate remains a property of the build, not the request: the Tauri, mobile, and embedded builds are compiled without it, and `?embed=1` cannot switch it off. Also apply the initial theme in main.tsx when a gate is configured. The signed-out screen paints before <App /> mounts, and App is where useThemeMode adds the `dark` class, so a dark-mode visitor was shown a white sign-in screen that only flipped after signing in. This affected the existing Clerk gate too. Verified against the running app with the tenant endpoints stubbed: the sign-in screen replaces the app, /authorize carries the configured client_id, the registered redirect_uri and PKCE, the app renders with a clean URL once signed in, the account menu offers sign-out, a reload stays signed in without another trip to the tenant, and a deep link's query survives. An ungated build still boots straight to the map and requests neither SDK chunk.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe web application now supports Auth0 as an alternative to Clerk. It validates provider settings, preserves return URLs, loads the selected gate, supports Auth0 sign-in and sign-out, updates container CSP settings, and documents deployment requirements. ChangesAuth0 authentication provider
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Deployment
participant AuthResolver
participant ApplicationStartup
participant Auth0Gate
participant Auth0
Deployment->>AuthResolver: Read deployment and build settings
AuthResolver->>ApplicationStartup: Return Auth0 configuration
ApplicationStartup->>Auth0Gate: Load and apply Auth0 gate
Auth0Gate->>Auth0: Start sign-in or restore session
Auth0-->>Auth0Gate: Return authentication state
Auth0Gate-->>ApplicationStartup: Render authenticated application
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
🔍 Cloudflare PR preview
|
Contributor
🔍 GitHub Pages PR preview
Note GitHub Pages built this preview successfully, but its serving edge returned HTTP 403 when checked. The links may still be propagating. |
Contributor
Code reviewI reviewed the full diff (Auth0 sign-in gate, provider-precedence logic, Docker entrypoint validation, CSP wiring, docs, and the two new test files) against the surrounding source ( Bugs
Security
Performance
Quality
CLAUDE.md
No inline comments posted — I didn't find any finding I'd consider actionable enough to anchor to a specific line. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/geolibre-desktop/src/lib/auth-gate.ts`:
- Around line 57-62: Update the provider-presence logic in the resolver around
resolveAuth0Config so either Auth0 runtime setting, including the client ID or
domain, marks Auth0 as runtime-named; preserve Auth0 precedence over Clerk when
that condition is met. Add a precedence test covering a runtime Auth0 client ID,
build-time Auth0 domain, and build-time Clerk key.
In `@apps/geolibre-desktop/src/lib/auth0-auth.ts`:
- Around line 82-95: Update the Auth0 configuration handling around the
domain/clientId normalization to detect whether either raw deployment value was
provided before normalization, then log incomplete configuration whenever raw
input exists but normalization yields a missing value, including when both
values are malformed. Preserve silent behavior only when both raw inputs are
unset, and add tests covering both malformed values and one malformed value.
In `@docs/getting-started.md`:
- Around line 352-357: Update the Auth0 settings guidance to keep the full
deployment URL, including any subpath and trailing slash, for Allowed Callback
URLs and Allowed Logout URLs, but instruct users to enter only the origin
without a trailing slash or path for Allowed Web Origins.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: da349e70-ea42-41e2-b5a2-2d130ea3ad75
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
apps/geolibre-desktop/package.jsonapps/geolibre-desktop/src/components/auth/Auth0Gate.tsxapps/geolibre-desktop/src/i18n/locales/en.jsonapps/geolibre-desktop/src/lib/auth-gate.tsapps/geolibre-desktop/src/lib/auth0-auth.tsapps/geolibre-desktop/src/main.tsxapps/geolibre-desktop/vite.config.tsdocker-compose.ymldocker/entrypoint.shdocker/nginx.confdocs/getting-started.mddocs/self-hosting.mdtests/auth-gate.test.tstests/auth0-auth.test.ts
- auth-gate.ts: count either half of the Auth0 pair as naming Auth0 in the deployment env. The pair can be split across the runtime and build tiers, so a deployment supplying only the client ID at runtime had still named Auth0 there, but checking the domain alone handed it back to Clerk. Covered by a new precedence test. - auth0-auth.ts: decide whether to log an incomplete configuration from the raw environment values rather than the normalized ones. Two malformed values both normalize to undefined, so the loudest misconfiguration — nothing usable at all — was the one case that disabled the gate silently. The half/malformed test now covers both-malformed and each single-malformed value. - docs/getting-started.md: Allowed Web Origins takes the origin only, with no trailing slash and no path, unlike Allowed Callback/Logout URLs. The previous text told operators to paste the same trailing-slash URL into all three, which for a subpath deployment would break silent authentication.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
|
Both findings posted as inline comments. Final summary below. Code reviewBugs
Security
Performance / Quality / CLAUDE.md
|
- Restore a deep link's query before the boot-time settings read it. Auth0 returns to the registered callback URL, so `?locale=`/`?theme=` were gone on the load that followed. `onRedirectCallback` put the address bar back, but it runs in a React effect — after `getInitialLanguage()` has already resolved the UI language at module scope during `import "./i18n"`. The query is now stashed before the redirect leaves and merged back in on the callback load, ahead of that read (lib/auth-return-url.ts, imported for its side effect above ./i18n). Confirmed with a negative control: with the restore disabled, a visit to `?theme=dark&locale=fr` comes back English. The theme self-healed already, since useThemeMode re-reads the URL when App mounts after the callback, but it now also avoids a light-to-dark flash. - Document the localStorage session cache for operators, and say in the code what bounds it: no API audience and no refresh token are requested, so the cached entry is a short-lived identity assertion that cannot be renewed. It does outlive the tab and is readable by same-origin plugin code, which the Auth0 section of the docs now states.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/geolibre-desktop/src/lib/auth-return-url.ts`:
- Around line 75-77: Update the callback handling around the query-rewrite guard
to accept Auth0 error callbacks containing error and state, while preserving the
existing code-and-state behavior. When stashing or restoring the retry query,
exclude Auth0 callback parameters such as error so a later authorization-code
callback does not retain rejected error fields.
In `@docs/getting-started.md`:
- Around line 384-388: Update the authentication explanation near the cached
identity assertion to state that, without useRefreshTokens, Auth0 can renew
tokens through silent authentication while the Auth0 session cookie remains
available, but renewal may fail when the browser blocks that cookie. Preserve
the clarification that omitting an API audience means the token grants no
upstream API access by itself.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9bfaf480-1767-4b36-b68f-d8d957cc40af
📒 Files selected for processing (6)
apps/geolibre-desktop/src/components/auth/Auth0Gate.tsxapps/geolibre-desktop/src/lib/auth-return-url-boot.tsapps/geolibre-desktop/src/lib/auth-return-url.tsapps/geolibre-desktop/src/main.tsxdocs/getting-started.mdtests/auth-return-url.test.ts
Contributor
Code reviewBugs
Security
Performance
Quality
CLAUDE.md
|
The guard required both `code` and `state`, so it matched only a successful return from Auth0. A refusal — an Action calling api.access.deny(), a declined consent — comes back as `error`/`error_description` with `state` and no `code`, which fell through and discarded the stash. The theme and language the visitor asked for were then missing from the one screen that has something to tell them. Extracted the check as `isSignInCallback`, which now requires `state` (Auth0's CSRF token, present on every return) plus either `code` or `error`, and unit tested both returns along with the loads that must not qualify. Verified against the running app with the tenant stubbed to refuse: the denial lands on the error screen showing Auth0's own reason, with the requested language and theme applied. Note the auth.* strings themselves are English in every non-English catalog today — pre-existing, and equally true of the Clerk gate — so this restores the language, not yet the translation.
Contributor
Code reviewBugs
Security
Performance
Quality
CLAUDE.md
|
- Keep one login attempt out of the next. auth0-react cleans the callback URL only after a callback it accepted, so a refusal leaves `error` in the address bar; "Try again" then stashed it, the restore merged it into the next, successful callback, and the SDK rejected a login that had actually worked — a permanent lockout, since every retry re-armed it. The stash now strips Auth0's own parameters (CALLBACK_PARAMS, shared with onRedirectCallback so the list has one home). Confirmed with a negative control: without the strip the retry never reaches the map. - Correct what the localStorage session cache is claimed to be. Without `useRefreshTokens` the SDK still renews through silent authentication while the Auth0 session cookie answers, so "cannot be renewed" was wrong. The docs and the code comment now say no refresh token is stored and that renewal depends on that flow, which a browser blocking the cookie will fail. The missing API audience still means no upstream access on its own. - Log the normalized Auth0 host at boot, not the raw variable, so the line matches the host actually written to the runtime config and the CSP frame-src. Uses two scheme-specific sed expressions rather than a case-insensitive match, which is a GNU extension.
Comment on lines
+86
to
+99
| if (!domain || !clientId) { | ||
| // Only complain when something was configured: an unset gate is the normal | ||
| // case for every public deployment and must stay silent. Tested on the raw | ||
| // values, not the normalized ones — two malformed values normalize to | ||
| // undefined, and reading those would turn the loudest misconfiguration | ||
| // (nothing usable at all) into the one that says nothing. | ||
| if (rawDomain || rawClientId) { | ||
| console.error( | ||
| `[GeoLibre] Ignoring an incomplete Auth0 configuration: ${AUTH0_DOMAIN_ENV} and ` + | ||
| `${AUTH0_CLIENT_ID_ENV} must both be set to valid values. The sign-in gate is OFF.`, | ||
| ); | ||
| } | ||
| return undefined; | ||
| } |
Contributor
There was a problem hiding this comment.
Fail-open on a malformed build-time config. When domain/clientId fail validation, resolveAuth0Config returns undefined, and resolveAuthGate then treats the deployment as ungated — the app boots publicly, with only a console.error (invisible unless someone opens devtools) as the signal.
For the Docker path this is safe: entrypoint.sh validates GEOLIBRE_AUTH0_DOMAIN/GEOLIBRE_AUTH0_CLIENT_ID at container boot and refuses to start on a half or malformed pair. But the build-time equivalents (VITE_GEOLIBRE_AUTH0_DOMAIN/VITE_GEOLIBRE_AUTH0_CLIENT_ID, baked in via npm run build outside Docker) never go through that check — a typo there silently ships an ungated deployment instead of a blocked one.
Contrast with Clerk: an malformed/garbage VITE_GEOLIBRE_CLERK_PUBLISHABLE_KEY isn't pre-validated in JS at all, so it reaches <ClerkProvider>, which fails to load and lands on ClerkFailed — i.e. Clerk fails closed (blocks everyone) in the equivalent scenario, while Auth0 fails open (blocks no one). Given this is auth-gating code, is that asymmetry intentional? If so, might be worth a louder signal than a console.error for the "both present but malformed" case specifically (as opposed to "unset", which should stay silent).
Confidence: medium — this is a real behavioral difference from Clerk, but it's scoped to the build-time (non-Docker-validated) config path, and the code comments suggest the author considered the "half config" case deliberately.
| // lib/auth-return-url.ts. | ||
| const signIn = useCallback(() => { | ||
| stashAuthReturnQuery(); | ||
| void loginWithRedirect({ appState: { returnTo: window.location.href } }); |
Contributor
There was a problem hiding this comment.
Minor: void loginWithRedirect(...) here and void logout(...) at line 108 have no .catch(). If either promise rejects before the navigation fires (e.g. PKCE/crypto setup failure, a blocked redirect), it becomes an unhandled promise rejection with no user-visible feedback — the button just appears to do nothing. Other fire-and-forget calls in this codebase (e.g. the dynamic imports in main.tsx) attach a .catch() that at least console.errors. Low severity since this mirrors an SDK call that almost always navigates away successfully, but worth a .catch((error) => console.error(...)) for consistency.
Confidence: low.
Contributor
Code reviewI reviewed the Auth0 sign-in gate PR: provider-selection logic ( Bugs
Security
Performance
Quality
CLAUDE.md
|
giswqs
added a commit
that referenced
this pull request
Aug 12, 2026
…1865) The Auth0/Clerk sign-in gate (#1853) added eight auth.* keys and the COG DEM terrain source (#1862) added thirteen terrainSettings.* keys, but only en.json carried them, so every other locale fell back to English at runtime. Nothing in CI catches that: the parity test forbids extra keys and placeholder drift, not missing ones. Fill auth.* in all seventeen non-English catalogs, and fill the terrainSettings.* source keys plus the map.directionsMode.waypointCount zero override in vi.json, which shipped one commit before that feature. Also translate six vi strings that were left as English prose. Terminology follows each catalog's existing wording: the noun for 'deployment' is taken from gallery.errorNotConfigured, the sign-in and account vocabulary from share.step1Description, huggingFace.signedInAs and share.openAccountSettings, and 'Try again' from about.tryAgain.
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
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.
What
Hosted deployments can already be gated behind Clerk (
GEOLIBRE_CLERK_PUBLISHABLE_KEY). This adds Auth0 as an alternative provider for deployments that already run an Auth0 tenant. The two are configured independently and only one is ever loaded — you pick a provider, you do not run both.Both values are public by design; an Auth0 client secret is neither needed nor accepted. Build-time equivalents are
VITE_GEOLIBRE_AUTH0_DOMAIN/VITE_GEOLIBRE_AUTH0_CLIENT_ID.How it differs from the Clerk gate
Auth0 has no embedded sign-in card, so this uses Universal Login: the visitor is redirected to the tenant's hosted page and returned to the app. Embedded cross-origin login depends on third-party cookies that browsers now block.
Two consequences worth calling out:
?project=…link. The arrival URL travels in the transaction'sappStateand is restored on return, along with stripping the single-usecode/stateparameters from the address bar.GEOLIBRE_CLERK_WAITLISThas no twin. The docs point operators at sign-up restrictions and a post-login Action (api.access.deny()) instead — a denial lands on the gate's own error screen with a way to try another account, rather than a blank page.Provider selection
One place decides,
apps/geolibre-desktop/src/lib/auth-gate.ts, following the precedence the individual settings already use (seedeployment-env.ts): a provider named in the deployment env beats one baked into the build, and naming both at the same level keeps Clerk — an image built with a Clerk key must not switch providers on its own. The Docker entrypoint refuses to boot when both are passed at runtime, so that tie only arises from build-time variables.The entrypoint validates the Auth0 pair together rather than serving an ungated app from a half configuration, normalizes a domain pasted as
https://tenant/to a bare hostname, and rejects a client ID outside Auth0's charset.Scope guarantees
?embed=1cannot switch a configured gate off.script-srcentry (its SDK is bundled) and its token endpoint is covered by the existing barehttps:inconnect-src; onlyframe-srcgains the tenant host, for the silent-authentication iframe. Byte-identical to before when unset./sidecar,/aiand friends must be protected separately. Documented in the same terms as the Clerk section.Drive-by fix
The signed-out screen paints before
<App />mounts, andAppis whereuseThemeModeadds thedarkclass — so a dark-mode visitor was shown a white sign-in screen that only flipped to dark after signing in.main.tsxnow applies the initial theme when a gate is configured (no-op for ungated builds, and idempotent withuseThemeMode). This affected the existing Clerk gate too.Testing
18 new unit tests (
tests/auth0-auth.test.ts,tests/auth-gate.test.ts) covering env resolution, domain/client-ID normalization and rejection, half configurations, and every provider-precedence combination.Verified against the running app in a browser with the tenant endpoints stubbed, so a full login round trip completes:
/authorizecarries the configuredclient_id, the registeredredirect_uri, and a PKCES256challengecode/stateare gone from the URL?project=…deep link is restored after loginThe entrypoint's validation was exercised directly: half config, bad domain, bad client ID, and Clerk + Auth0 together each fail the boot with a clear message.
npm run build, the full frontend suite (5802 tests), eslint,audit:ci(no new advisories), andpre-commiton the changed files all pass.Summary by CodeRabbit