[claude-proposal] Add default dev OAuth credentials for Google#2544
Open
[claude-proposal] Add default dev OAuth credentials for Google#2544
Conversation
Contributor
📝 WalkthroughWalkthroughAdds a "use default credentials" flow: frontend chooser and submit flag; API accepts Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as Frontend UI
participant API as API Handler
participant Config as Config Manager
participant Model as OAuth Client Model
User->>UI: Choose "Use dev credentials"
UI->>API: POST /dash/apps/{appId}/oauth_clients { use_default_credentials: true, meta: {appType: "web"} }
API->>Config: get-default-app-oauth-client(provider)
Config-->>API: return default client-id & encrypted secret
API->>Model: create app-oauth-client with meta.useDefaultCredentials=true, defaultProvider=provider
Model->>Config: get-default-app-oauth-client(defaultProvider)
Config-->>Model: return default credentials
Model-->>API: resolved OAuth client (client-id from config, decrypted secret)
API-->>UI: respond created with meta indicating default credentials
UI-->>User: show "Dev credentials" badge and usage info
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![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 the current code and only fix it if needed.
Inline comments:
In `@server/src/instant/dash/routes.clj`:
- Around line 626-646: The code currently reads provider-name from
client-supplied meta and uses it in the use-default-credentials branch; instead,
resolve the provider name server-side from the authoritative provider_id and
ignore/override meta.providerName for credential/discovery selection. Replace
uses of the local provider-name in the block that calls
config/get-default-app-oauth-client, default-discovery-endpoints, and the github
check with a server-resolved value (e.g., resolved-provider-name obtained via
the provider_id lookup function you already have), merge that resolved name into
meta when setting "defaultProvider", and ensure discovery-endpoint and the
github conditional use resolved-provider-name rather than the client-supplied
provider-name.
- Around line 629-641: The use-default-credentials branch returns a potentially
nil discovery endpoint (from default-discovery-endpoints) causing delayed
failures; update the branch that binds default-creds in the function handling
use-default-credentials to check that the discovery endpoint (lookup of
default-discovery-endpoints with (or provider-name "google")) is present and if
missing call ex/throw-validation-err! (similar to the existing default-creds
check) with an appropriate :use_default_credentials / provider-name context and
message, so client creation is rejected immediately instead of returning nil
discovery info.
In `@server/src/instant/model/app_oauth_client.clj`:
- Around line 123-130: The function default-credentials currently returns nil
when useDefaultCredentials is set but config/get-default-app-oauth-client
returns no defaults; change default-credentials to fail fast: after detecting
(get (:meta oauth-client) "useDefaultCredentials") call
config/get-default-app-oauth-client with the resolved provider and if that call
returns nil throw an explicit exception (or use ex-info) with a clear message
including the provider and client id/context, instead of silently falling back
to stored credentials; update callers accordingly if they expect nil.
🪄 Autofix (Beta)
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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7f263dac-9a9f-4749-850a-b1a3d835437e
📒 Files selected for processing (8)
client/www/components/dash/auth/Google.tsxclient/www/components/dash/auth/shared.tsxserver/resources/config/dev.ednserver/src/instant/config.cljserver/src/instant/config_edn.cljserver/src/instant/dash/routes.cljserver/src/instant/model/app_oauth_client.cljserver/test/instant/runtime/routes_test.clj
Contributor
|
View Vercel preview at instant-www-js-stopa-default-oauth-credentials-jsv.vercel.app. |
Apps can now enable Google OAuth with zero configuration by using Instant's shared development credentials. This mirrors what Clerk does: developers toggle on Google in the dashboard and it works immediately without needing to set up a Google Cloud project. The flow: 1. Developer goes to Dashboard > Auth > Add client > Google 2. Clicks "Quick setup (recommended for development)" 3. Done. OAuth works with Instant's shared Google credentials. Backend changes: - New config key `default-app-oauth-clients` for shared credentials - `app_oauth_client/->OAuthClient` checks meta.useDefaultCredentials and pulls credentials from config instead of the DB - `dash/routes/oauth-clients-post` accepts `use_default_credentials` flag and auto-populates client_id/discovery from config Frontend changes: - Google OAuth setup now shows a choice: Quick setup vs Custom - Quick setup creates a default-credentials client with one click - Default clients display a "Dev credentials" badge - Setup instructions are simplified (no Google Console steps) The consent screen shows "InstantDB" branding in dev. For production, developers delete the default client and create one with their own Google credentials.
stopachka
force-pushed
the
stopa/default-oauth-credentials
branch
from
bdc38ce to
3684924
Compare
- Resolve provider name server-side from provider_id, not client meta - Fail fast when default discovery endpoint is missing - Fail fast in ->OAuthClient when useDefaultCredentials is set but config credentials are missing - Rewrite e2e test: verify ->OAuthClient builds correct auth URL with default credentials, test full sign-in flow, verify custom clients still use stored credentials
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
server/src/instant/dash/routes.clj (1)
626-655:⚠️ Potential issue | 🟠 MajorThe custom-client branch still trusts
meta.providerName.Lines 653-654 still use request-supplied
meta.providerNameto decide whetherdiscovery_endpointis required. A caller can send"providerName": "github"for anyprovider_idand skip discovery validation/create the wrong client shape later. Resolve the provider name once fromprovider_idand use that authoritative value in both branches, then merge it back intometaif downstream code still needs it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/instant/dash/routes.clj` around lines 626 - 655, The code still trusts request-supplied meta.providerName when deciding whether discovery_endpoint is required; resolve the authoritative provider name from the provider record (like how resolved-provider is computed when use-default-credentials is true) before the branch and use that resolved value instead of provider-name in both branches (including the coerce-optional-param! / ex/get-param! call that checks for "github"), then merge that resolved provider name back into meta so downstream code sees the authoritative provider (update references to provider-name, the discovery_endpoint validation, and the meta merge accordingly).
🧹 Nitpick comments (1)
server/test/instant/runtime/routes_test.clj (1)
885-895: Assert the image-write behavior this test name promises.This block says the sign-in flow "creates user with image", but it only checks
:createdand🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/test/instant/runtime/routes_test.clj` around lines 885 - 895, The test name promises the created user has an image but the assertions only check :created and :email; update the assertions after calling route/upsert-oauth-link! and app-user-model/get-by-id to verify the image was persisted (e.g., assert the stored image field on user matches "https://example.com/photo.jpg" — check the actual field name used in your model, such as :image, :image_url, or :avatar_url) or, if you don't want to assert image behavior, rename the test to reflect that it only verifies creation/email rather than image storage; locate this change around route/upsert-oauth-link! and app-user-model/get-by-id in the test block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/src/instant/dash/routes.clj`:
- Around line 631-633: The code currently defaults resolved-provider to "google"
when app-oauth-service-provider-model/get-by-id returns nil; instead, check the
retrieved provider (the provider binding from
app-oauth-service-provider-model/get-by-id using provider-id) and if it's nil
immediately return/reject with an appropriate client error (e.g., 404/400)
referencing the missing provider_id, before computing resolved-provider, so you
do not synthesize a valid-looking client for a nonexistent provider.
In `@server/src/instant/model/app_oauth_client.clj`:
- Around line 123-142: The code treats "useDefaultCredentials"/"defaultProvider"
as client-controlled metadata which allows later updates to flip a client into
default-credentials mode; change default-credentials and ->OAuthClient to read
these flags only from a server-controlled source (not (:meta oauth-client))—for
example consult a server-only field on the oauth-client record or a
config/db-backed map (i.e., replace reading (get (:meta oauth-client)
"useDefaultCredentials"/"defaultProvider") in default-credentials with a call to
the server-owned accessor), and make update! stop deep-merging those keys from
incoming :meta by filtering out "useDefaultCredentials" and "defaultProvider"
(or rejecting them) when merging client metadata so they remain immutable and
only set via the validated create path.
In `@server/test/instant/runtime/routes_test.clj`:
- Around line 872-883: The test references clojure.string/includes? but the
namespace does not require clojure.string; add a require entry like
[clojure.string :as string] to the ns require list and update the two calls in
the test to use string/includes? instead of clojure.string/includes? (affects
the testing block that constructs oauth-client via
app-oauth-client-model/->OAuthClient and calls oauth/create-authorization-url).
---
Duplicate comments:
In `@server/src/instant/dash/routes.clj`:
- Around line 626-655: The code still trusts request-supplied meta.providerName
when deciding whether discovery_endpoint is required; resolve the authoritative
provider name from the provider record (like how resolved-provider is computed
when use-default-credentials is true) before the branch and use that resolved
value instead of provider-name in both branches (including the
coerce-optional-param! / ex/get-param! call that checks for "github"), then
merge that resolved provider name back into meta so downstream code sees the
authoritative provider (update references to provider-name, the
discovery_endpoint validation, and the meta merge accordingly).
---
Nitpick comments:
In `@server/test/instant/runtime/routes_test.clj`:
- Around line 885-895: The test name promises the created user has an image but
the assertions only check :created and :email; update the assertions after
calling route/upsert-oauth-link! and app-user-model/get-by-id to verify the
image was persisted (e.g., assert the stored image field on user matches
"https://example.com/photo.jpg" — check the actual field name used in your
model, such as :image, :image_url, or :avatar_url) or, if you don't want to
assert image behavior, rename the test to reflect that it only verifies
creation/email rather than image storage; locate this change around
route/upsert-oauth-link! and app-user-model/get-by-id in the test block.
🪄 Autofix (Beta)
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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2e8e88c7-9d81-4b2b-b23f-48dcf40295cb
📒 Files selected for processing (3)
server/src/instant/dash/routes.cljserver/src/instant/model/app_oauth_client.cljserver/test/instant/runtime/routes_test.clj
Comment on lines
+631
to
+633
| (let [provider (app-oauth-service-provider-model/get-by-id | ||
| {:app-id app-id :id provider-id}) | ||
| resolved-provider (or (:provider_name provider) "google") |
Contributor
There was a problem hiding this comment.
Reject unknown provider_id instead of defaulting it to Google.
If app-oauth-service-provider-model/get-by-id returns nil, resolved-provider becomes "google" and the request can create a valid-looking default client tied to a nonexistent provider record. Please assert that the provider exists before deriving resolved-provider.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/instant/dash/routes.clj` around lines 631 - 633, The code
currently defaults resolved-provider to "google" when
app-oauth-service-provider-model/get-by-id returns nil; instead, check the
retrieved provider (the provider binding from
app-oauth-service-provider-model/get-by-id using provider-id) and if it's nil
immediately return/reject with an appropriate client error (e.g., 404/400)
referencing the missing provider_id, before computing resolved-provider, so you
do not synthesize a valid-looking client for a nonexistent provider.
Comment on lines
+123
to
+142
| (defn- default-credentials | ||
| "Returns {:client-id ... :client-secret ...} from config for a default | ||
| credentials client, or nil if the client doesn't use defaults." | ||
| [oauth-client] | ||
| (when (get (:meta oauth-client) "useDefaultCredentials") | ||
| (let [provider (get (:meta oauth-client) "defaultProvider" "google")] | ||
| (or (config/get-default-app-oauth-client provider) | ||
| (throw (ex-info "Missing default OAuth credentials in config" | ||
| {:provider provider | ||
| :oauth-client-id (:id oauth-client)})))))) | ||
|
|
||
| (defn ->OAuthClient [oauth-client] | ||
| (cond | ||
| (:discovery_endpoint oauth-client) | ||
| (oauth/generic-oauth-client-from-discovery-url | ||
| {:app-id (:app_id oauth-client) | ||
| :provider-id (:provider_id oauth-client) | ||
| :client-id (:client_id oauth-client) | ||
| :client-secret (when (:client_secret oauth-client) | ||
| (decrypted-client-secret oauth-client)) | ||
| :discovery-endpoint (:discovery_endpoint oauth-client) | ||
| :meta (:meta oauth-client)}) | ||
| (let [defaults (default-credentials oauth-client) | ||
| client-id (if defaults | ||
| (:client-id defaults) | ||
| (:client_id oauth-client)) | ||
| client-secret (if defaults | ||
| (:client-secret defaults) | ||
| (when (:client_secret oauth-client) | ||
| (decrypted-client-secret oauth-client)))] |
Contributor
There was a problem hiding this comment.
Treat useDefaultCredentials as server-owned, not mutable client metadata.
->OAuthClient now switches to shared config credentials purely from (:meta oauth-client), but update! in this namespace still deep-merges arbitrary :meta. That means a later metadata update can flip an existing custom client into default-credentials mode, or change defaultProvider, without going through the validated create path. Please make these flags immutable/server-controlled instead of trusting mutable metadata here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/instant/model/app_oauth_client.clj` around lines 123 - 142, The
code treats "useDefaultCredentials"/"defaultProvider" as client-controlled
metadata which allows later updates to flip a client into default-credentials
mode; change default-credentials and ->OAuthClient to read these flags only from
a server-controlled source (not (:meta oauth-client))—for example consult a
server-only field on the oauth-client record or a config/db-backed map (i.e.,
replace reading (get (:meta oauth-client)
"useDefaultCredentials"/"defaultProvider") in default-credentials with a call to
the server-owned accessor), and make update! stop deep-merging those keys from
incoming :meta by filtering out "useDefaultCredentials" and "defaultProvider"
(or rejecting them) when merging client metadata so they remain immutable and
only set via the validated create path.
Comment on lines
+872
to
+883
| (testing "->OAuthClient resolves default credentials and builds valid auth URL" | ||
| (let [oauth-client (app-oauth-client-model/->OAuthClient client) | ||
| auth-url (oauth/create-authorization-url | ||
| oauth-client | ||
| "test-state" | ||
| "http://localhost:8888/runtime/oauth/callback" | ||
| {})] | ||
| (is (= "test-default-client-id" (:client-id oauth-client))) | ||
| (is (= "test-default-secret" | ||
| (crypt-util/secret-value (:client-secret oauth-client)))) | ||
| (is (clojure.string/includes? auth-url "accounts.google.com")) | ||
| (is (clojure.string/includes? auth-url "test-default-client-id")))) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,20p' server/test/instant/runtime/routes_test.clj
echo
rg -n 'clojure\.string/includes\?' server/test/instant/runtime/routes_test.cljRepository: instantdb/instant
Length of output: 1137
🏁 Script executed:
sed -n '1,35p' server/test/instant/runtime/routes_test.cljRepository: instantdb/instant
Length of output: 1381
Add the missing clojure.string require.
The test uses clojure.string/includes? on lines 882 and 883 without requiring clojure.string in the ns form. Add [clojure.string :as string] to the require block and update the calls to use string/includes? instead so the namespace has explicit dependencies.
🧰 Tools
🪛 GitHub Check: lint
[warning] 882-882:
Unresolved namespace clojure.string. Are you missing a require?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/test/instant/runtime/routes_test.clj` around lines 872 - 883, The test
references clojure.string/includes? but the namespace does not require
clojure.string; add a require entry like [clojure.string :as string] to the ns
require list and update the two calls in the test to use string/includes?
instead of clojure.string/includes? (affects the testing block that constructs
oauth-client via app-oauth-client-model/->OAuthClient and calls
oauth/create-authorization-url).
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.
Summary
This PR adds support for default development OAuth credentials for Google, similar to how Clerk provides shared OAuth credentials for development. Developers can now enable Google OAuth with a single click, no Google Cloud project setup needed.
Motivation
Currently, adding Google OAuth to an Instant app requires:
This is a significant DX barrier, especially for developers who just want to try OAuth during development. Clerk solves this by providing shared development credentials that work immediately.
How it works
The flow
Opt-in, not automatic
Default credentials are not enabled by default. The developer must explicitly click "Quick setup" in the dashboard. This prevents unauthorized OAuth flows against apps that never intended to support Google sign-in.
Architecture
Config layer (
config_edn.clj,config.clj,dev.edn):default-app-oauth-clientsmaps provider names to{:client-id, :client-secret}pairsoverlay.edn(gitignored) to keep secrets out of the repoModel layer (
app_oauth_client.clj):->OAuthClientchecks formeta.useDefaultCredentialson the client recordclient-idandclient-secretfrom config instead of decrypting stored credentialsDashboard routes (
dash/routes.clj):oauth-clients-postaccepts a newuse_default_credentialsflaguseDefaultCredentials: trueDashboard UI (
Google.tsx,shared.tsx):Security considerations
id_tokenflow (for Google One Tap) works: the client ID is stored in the DB record so it can be used in code examples, and->OAuthClientprovides the secret from config for audience validation.id_tokenpath (sinceclient_secretis not in the DB record). The id_token itself is still fully validated (signature, issuer, audience, nonce).Files changed
server/src/instant/config_edn.clj::default-app-oauth-clientsspecserver/src/instant/config.cljget-default-app-oauth-clientaccessorserver/resources/config/dev.ednserver/src/instant/model/app_oauth_client.cljdefault-credentialshelper, updated->OAuthClientserver/src/instant/dash/routes.cljoauth-clients-postto handleuse_default_credentialsclient/www/components/dash/auth/Google.tsxclient/www/components/dash/auth/shared.tsxuseDefaultCredentialsparam toaddClientserver/test/instant/runtime/routes_test.cljdefault-credentials-oauth-client-testTest plan
default-credentials-oauth-client-testpasses (13 assertions)->OAuthClientuses config credentials (not DB)upsert-oauth-link!creates users correctlyFuture work
🤖 Generated with Claude Code