Skip to content

fix(admin): GCP Secret Manager + auth fix#6254

Merged
beastoin merged 4 commits into
mainfrom
feat/opensource-omi-admin-6227
Apr 1, 2026
Merged

fix(admin): GCP Secret Manager + auth fix#6254
beastoin merged 4 commits into
mainfrom
feat/opensource-omi-admin-6227

Conversation

@beastoin
Copy link
Copy Markdown
Collaborator

@beastoin beastoin commented Apr 1, 2026

Summary

  • Switches to GCP Secret Manager for all 29 deploy secrets (no more GitHub environment secrets needed)
  • NEXT_PUBLIC_ vars (9): fetched from Secret Manager at build time via gcloud secrets versions access, passed as Docker build-args (they get baked into the JS bundle)
  • Server-side vars (20): injected at Cloud Run runtime via --set-secrets — no longer baked into the Docker image (more secure, rotatable without rebuild)
  • Dockerfile cleaned up: removed 40+ lines of server-side ARG/ENV declarations
  • Auth fix: removed extra server-side adminData Firestore check (matching original omi-admin behavior), added try-catch error handling

Secret Manager naming

All secrets use WEB_ADMIN_ prefix in project based-hardware:

  • WEB_ADMIN_FIREBASE_PROJECT_ID, WEB_ADMIN_STRIPE_SECRET_KEY, etc.

Prerequisites

  • Cloud Run service account needs roles/secretmanager.secretAccessor on the 20 server-side secrets
  • The GCP credentials used for deployment need roles/secretmanager.secretAccessor to fetch build-time secrets

Test plan

  • Deploy workflow fetches NEXT_PUBLIC_ vars from Secret Manager
  • Docker build succeeds with build-args
  • Cloud Run mounts server-side secrets at runtime
  • Admin login works
  • API routes return data (not 500)

🤖 Generated with Claude Code

… verifyAdmin

The original omi-admin routes only verified the Firebase token server-side.
The adminData collection check was client-side only (auth-provider.tsx).
Adding the server-side Firestore check caused 500s when the Admin SDK
couldn't initialize. Revert to original behavior: token-only server auth.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 1, 2026

Greptile Summary

This PR fixes unhandled 500 errors in the admin auth layer by wrapping verifyFirebaseToken in a try-catch, and removes the server-side adminData Firestore membership check from verifyAdmin(). The error-handling improvement is solid and necessary. However, dropping the Firestore check creates a meaningful security regression: any valid Firebase user (i.e., any Omi account holder) can now call the admin API routes directly — bypassing the client-side auth-provider gate — simply by supplying their own Firebase ID token in the Authorization header.

Key changes:

  • ✅ Added try-catch around verifyFirebaseToken to return a structured 500 JSON response instead of an unhandled throw when Firebase env vars are missing
  • ❌ Removed server-side adminData collection check, leaving admin endpoint authorization enforced only client-side
  • The root cause (missing FIREBASE_* secrets in the prod GitHub environment) is correctly identified and documented in the PR description

Confidence Score: 2/5

Not safe to merge as-is — removes server-side admin authorization, making all admin API routes accessible to any authenticated Firebase user

The error-handling fix is correct, but the removal of the Firestore admin membership check is a P1 security regression. Any valid Firebase token can now authorize calls to sensitive admin endpoints (app review, payouts, user data). The two changes are independent and the Firestore check should be restored before merging.

web/admin/lib/auth.ts — server-side authorization logic

Important Files Changed

Filename Overview
web/admin/lib/auth.ts Adds try-catch to prevent unhandled 500s (good), but removes the server-side Firestore adminData check, leaving admin API routes accessible to any valid Firebase user

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Admin API Request] --> B{Authorization header present?}
    B -- No --> C[401 Unauthorized]
    B -- Yes --> D[verifyFirebaseToken]
    D -- throws --> E[500 Internal Server Error - new try-catch]
    D -- null --> F[401 Unauthorized]
    D -- decoded token --> G{adminData check removed by this PR}
    G --> H[Return uid - any Firebase user passes]

    style G fill:#ff9999,stroke:#cc0000
    style H fill:#ff9999,stroke:#cc0000
    style E fill:#90ee90,stroke:#008000
Loading

Reviews (1): Last reviewed commit: "fix(admin): remove server-side adminData..." | Re-trigger Greptile

Comment thread web/admin/lib/auth.ts
Comment on lines +19 to +24
try {
const decodedToken = await verifyFirebaseToken(token);
if (!decodedToken) {
return NextResponse.json({ error: 'Unauthorized: Invalid token' }, { status: 401 });
}
return { uid: decodedToken.uid };
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Server-side admin authorization removed — any Firebase user can call admin APIs

The adminData Firestore check has been moved entirely to the client-side auth-provider.tsx. After this change, verifyAdmin only confirms the bearer token belongs to any valid Firebase user, not specifically an admin.

Because the Next.js API routes (e.g. GET /api/omi/apps, POST /api/omi/apps/[app_id]/review) are publicly reachable HTTP endpoints, an attacker only needs a valid Firebase ID token — obtainable simply by signing into any Omi account — to call them directly, bypassing the client-side guard entirely.

The previous two-step check (valid token + document in adminData collection) is the correct pattern for protecting server-side routes. The client-side check in auth-provider.tsx is useful for UI gating, but it cannot substitute for server-side authorization.

Consider restoring the Firestore check inside verifyAdmin, independently of the try-catch fix (which is correct and valuable on its own):

try {
  const decodedToken = await verifyFirebaseToken(token);
  if (!decodedToken) {
    return NextResponse.json({ error: 'Unauthorized: Invalid token' }, { status: 401 });
  }

  const db = getDb();
  const adminDoc = await db.collection('adminData').doc(decodedToken.uid).get();
  if (!adminDoc.exists) {
    return NextResponse.json({ error: 'Forbidden: Not an admin' }, { status: 403 });
  }

  return { uid: decodedToken.uid };
} catch (error) {
  console.error('Error verifying admin token:', error);
  return NextResponse.json({ error: 'Internal server error during auth' }, { status: 500 });
}

Once the three Firebase secrets are set in the prod environment, getDb() will work correctly and this check can be safely restored.

beastoin and others added 2 commits April 1, 2026 12:15
- NEXT_PUBLIC_ vars: fetched from Secret Manager at build time via
  gcloud secrets versions access, passed as Docker build-args
- Server-side vars: injected at Cloud Run runtime via --set-secrets,
  no longer baked into Docker image (more secure)
- All secrets use WEB_ADMIN_ prefix in Secret Manager

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Server-side env vars are now injected at runtime via Cloud Run
Secret Manager instead of being baked into the Docker image.
Only NEXT_PUBLIC_ build-time vars remain as ARGs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@beastoin beastoin changed the title fix(admin): fix auth error handling and remove extra Firestore check fix(admin): GCP Secret Manager + auth fix Apr 1, 2026
…dling

The adminData Firestore check is good security — it was only causing
500s due to missing env vars. Now wrapped in try-catch so it returns
proper error responses instead of unhandled throws.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@beastoin beastoin merged commit c96cf87 into main Apr 1, 2026
2 checks passed
@beastoin beastoin deleted the feat/opensource-omi-admin-6227 branch April 1, 2026 12:21
@beastoin
Copy link
Copy Markdown
Collaborator Author

beastoin commented Apr 1, 2026

lgtm

Glucksberg pushed a commit to Glucksberg/omi-local that referenced this pull request Apr 28, 2026
## Summary
- **Switches to GCP Secret Manager** for all 29 deploy secrets (no more
GitHub environment secrets needed)
- **NEXT_PUBLIC_ vars** (9): fetched from Secret Manager at build time
via `gcloud secrets versions access`, passed as Docker build-args (they
get baked into the JS bundle)
- **Server-side vars** (20): injected at Cloud Run runtime via
`--set-secrets` — no longer baked into the Docker image (more secure,
rotatable without rebuild)
- **Dockerfile cleaned up**: removed 40+ lines of server-side ARG/ENV
declarations
- **Auth fix**: removed extra server-side adminData Firestore check
(matching original omi-admin behavior), added try-catch error handling

## Secret Manager naming
All secrets use `WEB_ADMIN_` prefix in project `based-hardware`:
- `WEB_ADMIN_FIREBASE_PROJECT_ID`, `WEB_ADMIN_STRIPE_SECRET_KEY`, etc.

## Prerequisites
- Cloud Run service account needs `roles/secretmanager.secretAccessor`
on the 20 server-side secrets
- The GCP credentials used for deployment need
`roles/secretmanager.secretAccessor` to fetch build-time secrets

## Test plan
- [ ] Deploy workflow fetches NEXT_PUBLIC_ vars from Secret Manager
- [ ] Docker build succeeds with build-args
- [ ] Cloud Run mounts server-side secrets at runtime
- [ ] Admin login works
- [ ] API routes return data (not 500)

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant