feat: add optional Clerk access gate - #1841
Merged
Merged
Conversation
Allow hosted GeoLibre deployments to require individual Clerk sign-in while keeping public, native, and embedded builds unchanged. Load Clerk on demand and keep its chunk out of the default PWA precache.
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:
📝 WalkthroughWalkthroughThe change adds optional Clerk sign-in for web deployments. It resolves runtime or build-time configuration, gates the desktop app when enabled, injects Docker CSP origins, preserves native and embedded behavior, and documents the deployment settings. ChangesClerk authentication
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Docker
participant main_tsx
participant ClerkGate
participant Clerk
participant App
Docker->>main_tsx: Expose Clerk key and waitlist settings
main_tsx->>main_tsx: Resolve web-only authentication configuration
main_tsx->>ClerkGate: Load gate when a key is configured
ClerkGate->>Clerk: Initialize ClerkProvider
Clerk-->>ClerkGate: Return authentication state
ClerkGate->>App: Render sign-in, waitlist, or authenticated content
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
Code reviewBugs / Security
Quality
No performance issues or CLAUDE.md violations found in the changed files beyond the above. |
![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: 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/components/auth/ClerkGate.tsx`:
- Around line 32-37: Extend protection beyond the ClerkGate Show rendering check
by enforcing Clerk session validation at the reverse proxy or in every protected
server endpoint, including /sidecar and /ai. Ensure unauthenticated requests are
rejected before deployment resources or API handlers execute, while preserving
the existing signed-in UI behavior.
In `@docker/entrypoint.sh`:
- Around line 147-157: Update the Clerk key parsing block for clerk_key to
require the pk_test_ or pk_live_ prefix, validate the complete encoded payload
with strict Base64 decoding, and reject missing delimiters, ignored characters,
invalid padding, or payloads without exactly a trailing $. Parse the decoded
payload once, extract and validate the hostname, and reuse that hostname in the
CSP block instead of reparsing the key. Add coverage for invalid prefixes,
malformed delimiters, ignored characters, invalid padding, and valid test and
production keys.
🪄 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: ec6184a2-0944-49d9-88ba-7278b84c0dc9
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
apps/geolibre-desktop/package.jsonapps/geolibre-desktop/src/components/auth/ClerkGate.tsxapps/geolibre-desktop/src/lib/clerk-auth.tsapps/geolibre-desktop/src/main.tsxapps/geolibre-desktop/vite.config.tsdocker-compose.ymldocker/entrypoint.shdocker/nginx.confdocs/getting-started.mddocs/self-hosting.mdtests/clerk-auth.test.ts
- Decide the Clerk gate from the build target, not the request: main.tsx no longer consults isEmbedded(), whose `?embed=1` query parameter is client-controlled and let any visitor render a Clerk-gated deployment with no sign-in wall. A new __GEOLIBRE_EMBED_BUILD__ define (GEOLIBRE_EMBED=1) keeps the Jupyter embed wheel ungated as intended, and documents the constraint on resolveClerkPublishableKey and in docs/getting-started.md. - Give the CSP block in docker/entrypoint.sh its own try/except around the publishable-key base64 decode instead of depending on `set -e` and the earlier block having already validated the key, so a reorder or extraction cannot turn an invalid key into a raw traceback.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@docs/getting-started.md`:
- Around line 275-276: Update the Clerk configuration documentation around the
disabled-by-default explanation to state that Clerk is enabled when either the
deployment-time GEOLIBRE_CLERK_PUBLISHABLE_KEY or build-time
VITE_GEOLIBRE_CLERK_PUBLISHABLE_KEY is set. Clarify that both values must be
unset for Clerk to remain disabled and GeoLibre to behave as before.
🪄 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: 75c778eb-3e4a-44c8-bb9d-cfe1d1f4adfb
📒 Files selected for processing (6)
apps/geolibre-desktop/src/lib/clerk-auth.tsapps/geolibre-desktop/src/main.tsxapps/geolibre-desktop/src/vite-env.d.tsapps/geolibre-desktop/vite.config.tsdocker/entrypoint.shdocs/getting-started.md
Contributor
|
Both inline comments posted successfully. Code reviewBugs
Security
Performance
Quality
CLAUDE.md
|
- Validate the full Clerk publishable-key shape in docker/entrypoint.sh: require a pk_test_/pk_live_ prefix (so a secret key pasted into GEOLIBRE_CLERK_PUBLISHABLE_KEY can never be published in the runtime config instead of failing the boot), decode with validate=True so stray characters are an error rather than silently dropped, and require the decoded payload's trailing "$" delimiter. Mirrored in the CSP block for the same separate-process reason already documented there. - Note in ClerkGate's doc comment that it gates rendering only and is not a server authorization boundary, pointing at the deployment guidance in docs/getting-started.md. - Say in docs/getting-started.md that Clerk stays disabled only when neither the runtime nor the build-time key is set, and that the runtime one wins.
Contributor
Code reviewNo correctness bugs, security holes, or performance regressions found in the core logic — this is a well-reasoned, defense-in-depth implementation (build-time-only gating via Bugs
Security
Performance
Quality
CLAUDE.md
|
- Use the same "GEOLIBRE_CLERK_PUBLISHABLE_KEY contains an invalid Frontend API host." message in both entrypoint.sh Clerk blocks, so the duplicated validation cannot report the same failure two different ways. - Note in docker/nginx.conf why the Clerk CSP hosts are deliberately not mirrored into the Tauri CSP (the gate is compiled out of the desktop and embed builds), so the file's own mirror-to-Tauri rule stays accurate.
Contributor
|
All three inline comments posted successfully. Here's the final summary. Code reviewBugs
Security
Performance
Quality
CLAUDE.md
|
Restricted (invite-only) sign-up needs no configuration here, but waitlist mode does: Clerk renders a "Join the waitlist" link inside the sign-in card that has nowhere to go until the app mounts a <Waitlist /> and points waitlistUrl at it. Serve it at the #/waitlist fragment of the gate's own page, behind a new GEOLIBRE_CLERK_WAITLIST opt-in. Hash routing keeps the move between the two screens a same-document navigation, and gives <SignIn routing="hash"> a prefix its own sub-steps (#/factor-one, #/sso-callback) cannot collide with. Off by default: on a restricted instance the form would take submissions nobody can approve. Setting it without GEOLIBRE_CLERK_PUBLISHABLE_KEY fails at container startup rather than silently serving a public app, and an unrecognized value is rejected instead of read as false. No CSP change — the waitlist form talks to the same Frontend API host, and its bot check uses the challenges.cloudflare.com origin already allowed.
- Keep the beforeunload unsaved-work guard alive while signed out. <App /> mounts it, so ending a session unmounted it: the project survived (the Zustand store is module-scope, so signing back in re-renders it), but the tab could then be closed or reloaded with unsaved changes and no "Leave site?" prompt. Mount it in ClerkGate too, which covers exactly the window <App /> cannot.
- Render Clerk's error status instead of a blank page. ClerkLoading and ClerkLoaded both return null when Clerk's status is "error" (a key that no longer resolves, an unreachable Frontend API, an outage), so the gate left nothing on screen and no way to tell a stuck deployment from a slow one. Add a <ClerkFailed> branch with a message and a reload action. - Lower-case the boot-log check for GEOLIBRE_CLERK_WAITLIST. The Python validator compares after .lower(), so a spelling like `TRue` enabled the waitlist but logged the plain sign-in gate line.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docker/entrypoint.sh (1)
350-389: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftAlign Docker build-time Clerk configuration with runtime CSP.
resolveClerkPublishableKey()supportsVITE_GEOLIBRE_CLERK_PUBLISHABLE_KEY, butDockerfiledoes not preserve it anddocker/entrypoint.shgenerates CSP only fromGEOLIBRE_CLERK_PUBLISHABLE_KEY. A build-time-only image can therefore enable Clerk without the required CSP sources, whiledocs/self-hosting.mdincorrectly says that an unset runtime key keeps the app public. Add DockerARG/ENVsupport and derive CSP from the effective key, or explicitly reject and document build-time-only Docker configurations. State that Clerk is disabled only when both keys are unset.🤖 Prompt for 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. In `@docker/entrypoint.sh` around lines 350 - 389, Align Docker build-time and runtime Clerk configuration: update Dockerfile’s Clerk ARG/ENV declarations so VITE_GEOLIBRE_CLERK_PUBLISHABLE_KEY is preserved, and update docker/entrypoint.sh lines 350-389 to derive CSP from the effective build-time or runtime key. Update docs/self-hosting.md lines 144-145 to state Clerk is disabled only when both keys are unset, and document the precedence used when both are provided. Ensure resolveClerkPublishableKey and entrypoint validation use the same effective-key behavior.
🤖 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 `@docker/entrypoint.sh`:
- Around line 313-316: Update the GEOLIBRE_CLERK_WAITLIST case in the entrypoint
logging block to normalize the trimmed value consistently with validation, so
mixed-case true values such as tRuE select the waitlist message. Preserve the
existing generic message for all other values.
---
Outside diff comments:
In `@docker/entrypoint.sh`:
- Around line 350-389: Align Docker build-time and runtime Clerk configuration:
update Dockerfile’s Clerk ARG/ENV declarations so
VITE_GEOLIBRE_CLERK_PUBLISHABLE_KEY is preserved, and update
docker/entrypoint.sh lines 350-389 to derive CSP from the effective build-time
or runtime key. Update docs/self-hosting.md lines 144-145 to state Clerk is
disabled only when both keys are unset, and document the precedence used when
both are provided. Ensure resolveClerkPublishableKey and entrypoint validation
use the same effective-key behavior.
🪄 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: 51030ae3-edf1-4413-88e4-9285c77913db
📒 Files selected for processing (8)
apps/geolibre-desktop/src/components/auth/ClerkGate.tsxapps/geolibre-desktop/src/lib/clerk-auth.tsapps/geolibre-desktop/src/main.tsxdocker-compose.ymldocker/entrypoint.shdocs/getting-started.mddocs/self-hosting.mdtests/clerk-auth.test.ts
| // Optional hosted-web authentication. This chunk is requested only when a | ||
| // Clerk publishable key is configured, so public deployments should not | ||
| // download it during service-worker installation. | ||
| "**/ClerkGate-*.js", |
Contributor
There was a problem hiding this comment.
Confidence: medium. This single glob assumes the whole import("./components/auth/ClerkGate") subtree (ClerkGate.tsx + @clerk/react + @clerk/shared + @tanstack/query-core + js-cookie + glob-to-regexp, none of which match a manualChunks rule) collapses into one ClerkGate-*.js chunk.
That's not guaranteed — this very file documents a case where it wasn't (a few lines up): the cesium dynamic-import boundary needed two ignore patterns, **/cesium-* and **/Cesium-*, because Rollup emitted a differently-cased facade chunk that the first glob missed. If Rollup splits any of the Clerk-only deps into a separately-named chunk (or emits a facade), it would silently slip back into the PWA precache, defeating the point of this change for public (non-Clerk) deployments.
Worth confirming against the actual dist/assets output of a production build (or better, adding an automated check, since the PR's verification of this was a manual step) rather than relying on the single glob.
Comment on lines
+19
to
+28
| /** | ||
| * Whether to serve Clerk's waitlist form at {@link WAITLIST_HASH}. Off unless | ||
| * the deployment opts in, because it only makes sense for a Clerk instance in | ||
| * waitlist sign-up mode. | ||
| */ | ||
| waitlist?: boolean; | ||
| children: ReactNode; | ||
| } | ||
|
|
||
| // The gate lives on a single page with no router, so the two signed-out screens |
Contributor
There was a problem hiding this comment.
Confidence: low. ClerkFailed, Show (with when="signed-in"/"signed-out" string props), and Waitlist/waitlistUrl are a less commonly documented part of Clerk's React surface — most existing Clerk integrations use <SignedIn>/<SignedOut> wrapper components instead of a generic <Show when=...> primitive. Since @clerk/react is a brand-new dependency for this repo (this PR adds it), it's worth double-checking these exports/props against the actual installed @clerk/react@6.14.1 type declarations to make sure tsc -b is actually type-checking real APIs here rather than something that happens to compile loosely. If this was confirmed via the "production build" step in the test plan, feel free to disregard.
Contributor
Code reviewBugs
Security
Performance
Quality
CLAUDE.md
|
This was referenced Aug 11, 2026
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.
Summary
Test plan
Summary by CodeRabbit
New Features
GEOLIBRE_CLERK_PUBLISHABLE_KEYsetting.Documentation
Tests