-
-
Notifications
You must be signed in to change notification settings - Fork 7
Fix oauth and credits integration 2704938752808797816 #440
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
3fae667
b02198c
a6c6e50
b5b2209
2a49291
22a330a
6f305ea
c8a1447
8aaf0d0
6c325e9
76d4da4
9108ced
f09b65b
88bb95e
f6d9246
7ad992b
acaa818
f018d85
f0765e6
db59402
c79b56c
caac000
8b53b6e
3ebae0c
7c96403
6604b7d
c8441fd
91488bb
c284e37
a40cd86
1f9de85
696c177
f30205b
584015e
7b9aeb2
8033da7
73bf12b
caeb97a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1 +1,15 @@ | ||||||||||||||||||||||||||||||||||||||||||||||
| DATABASE_URL="postgresql://user:password@host:port/db" | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # Server Actions Configuration | ||||||||||||||||||||||||||||||||||||||||||||||
| # Allow Server Actions in remote dev environments | ||||||||||||||||||||||||||||||||||||||||||||||
| SERVER_ACTIONS_ALLOWED_ORIGINS="*" | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # Authentication Configuration | ||||||||||||||||||||||||||||||||||||||||||||||
| # Disable Supabase auth and use mock user for development/preview | ||||||||||||||||||||||||||||||||||||||||||||||
| AUTH_DISABLED_FOR_DEV="false" | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # Standard Tier Configuration | ||||||||||||||||||||||||||||||||||||||||||||||
| STANDARD_TIER_PRICE_ID="price_standard_41_yearly" | ||||||||||||||||||||||||||||||||||||||||||||||
| STANDARD_TIER_CREDITS=8000 | ||||||||||||||||||||||||||||||||||||||||||||||
| STANDARD_TIER_MONTHLY_PRICE=41 | ||||||||||||||||||||||||||||||||||||||||||||||
| STANDARD_TIER_BILLING_CYCLE="yearly" | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+5
to
+15
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix dotenv-linter warnings (QuoteCharacter/UnorderedKey). The linter is flagging quoted values and ordering. If lint is enforced, this will keep CI green. 🔧 Suggested cleanup-SERVER_ACTIONS_ALLOWED_ORIGINS="*"
+SERVER_ACTIONS_ALLOWED_ORIGINS=*
-AUTH_DISABLED_FOR_DEV="false"
+AUTH_DISABLED_FOR_DEV=false
# Standard Tier Configuration
-STANDARD_TIER_PRICE_ID="price_standard_41_yearly"
-STANDARD_TIER_CREDITS=8000
-STANDARD_TIER_MONTHLY_PRICE=41
-STANDARD_TIER_BILLING_CYCLE="yearly"
+STANDARD_TIER_BILLING_CYCLE=yearly
+STANDARD_TIER_CREDITS=8000
+STANDARD_TIER_MONTHLY_PRICE=41
+STANDARD_TIER_PRICE_ID=price_standard_41_yearly📝 Committable suggestion
Suggested change
🧰 Tools🪛 dotenv-linter (4.0.0)[warning] 5-5: [QuoteCharacter] The value has quote characters (', ") (QuoteCharacter) [warning] 9-9: [QuoteCharacter] The value has quote characters (', ") (QuoteCharacter) [warning] 12-12: [QuoteCharacter] The value has quote characters (', ") (QuoteCharacter) [warning] 13-13: [UnorderedKey] The STANDARD_TIER_CREDITS key should go before the STANDARD_TIER_PRICE_ID key (UnorderedKey) [warning] 14-14: [UnorderedKey] The STANDARD_TIER_MONTHLY_PRICE key should go before the STANDARD_TIER_PRICE_ID key (UnorderedKey) [warning] 15-15: [QuoteCharacter] The value has quote characters (', ") (QuoteCharacter) [warning] 15-15: [UnorderedKey] The STANDARD_TIER_BILLING_CYCLE key should go before the STANDARD_TIER_CREDITS key (UnorderedKey) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,12 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Supabase Configuration | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| NEXT_PUBLIC_SUPABASE_URL=https://your-project.supabase.co | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| NEXT_PUBLIC_SUPABASE_ANON_KEY=your-anon-key-here | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Stripe Configuration | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| STANDARD_TIER_PRICE_ID=price_placeholder # must be real Stripe price ID in prod | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| STANDARD_TIER_CREDITS=8000 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| STANDARD_TIER_MONTHLY_PRICE=41 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| STANDARD_TIER_BILLING_CYCLE=yearly | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Other Environment Variables | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Add other existing env vars here with placeholder values | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+1
to
+12
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dotenv-linter warnings: quote and reorder keys. 🔧 Suggested reordering/quoting to satisfy dotenv-linter # Supabase Configuration
-NEXT_PUBLIC_SUPABASE_URL=https://your-project.supabase.co
-NEXT_PUBLIC_SUPABASE_ANON_KEY=your-anon-key-here
+NEXT_PUBLIC_SUPABASE_ANON_KEY=your-anon-key-here
+NEXT_PUBLIC_SUPABASE_URL=https://your-project.supabase.co
# Stripe Configuration
-STANDARD_TIER_PRICE_ID=price_placeholder # must be real Stripe price ID in prod
-STANDARD_TIER_CREDITS=8000
-STANDARD_TIER_MONTHLY_PRICE=41
-STANDARD_TIER_BILLING_CYCLE=yearly
+# must be real Stripe price ID in prod
+STANDARD_TIER_BILLING_CYCLE=yearly
+STANDARD_TIER_CREDITS=8000
+STANDARD_TIER_MONTHLY_PRICE=41
+STANDARD_TIER_PRICE_ID="price_placeholder"📝 Committable suggestion
Suggested change
🧰 Tools🪛 dotenv-linter (4.0.0)[warning] 3-3: [UnorderedKey] The NEXT_PUBLIC_SUPABASE_ANON_KEY key should go before the NEXT_PUBLIC_SUPABASE_URL key (UnorderedKey) [warning] 6-6: [ValueWithoutQuotes] This value needs to be surrounded in quotes (ValueWithoutQuotes) [warning] 7-7: [UnorderedKey] The STANDARD_TIER_CREDITS key should go before the STANDARD_TIER_PRICE_ID key (UnorderedKey) [warning] 8-8: [UnorderedKey] The STANDARD_TIER_MONTHLY_PRICE key should go before the STANDARD_TIER_PRICE_ID key (UnorderedKey) [warning] 9-9: [UnorderedKey] The STANDARD_TIER_BILLING_CYCLE key should go before the STANDARD_TIER_CREDITS key (UnorderedKey) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,59 +1,39 @@ | ||
| # See https://help.github.com/articles/ignoring-files/ for more about ignoring files. | ||
|
|
||
| # dependencies | ||
| /node_modules | ||
| /.pnp | ||
| .pnp.js | ||
| .yarn/install-state.gz | ||
|
|
||
| # testing | ||
| /coverage | ||
|
|
||
| # next.js | ||
| /.next/ | ||
| /out/ | ||
|
|
||
| # production | ||
| /build | ||
| # Dependency directories | ||
| node_modules/ | ||
| .bun/ | ||
|
|
||
| # Build outputs | ||
| .next/ | ||
| dist/ | ||
| build/ | ||
| out/ | ||
|
|
||
| # Environment variables | ||
| .env | ||
| .env.local | ||
| .env.development.local | ||
| .env.test.local | ||
| .env.production.local | ||
| .env.*.local | ||
|
|
||
| # misc | ||
| # IDE/Editor | ||
| .vscode/ | ||
| .idea/ | ||
| *.swp | ||
| *.swo | ||
| .DS_Store | ||
| *.pem | ||
|
|
||
| # debug | ||
| # Logs | ||
| npm-debug.log* | ||
| yarn-debug.log* | ||
| yarn-error.log* | ||
| bun.lockb | ||
|
Comment on lines
+26
to
+30
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Minor:
Suggested reorganization # Dependency directories
node_modules/
.bun/
+bun.lockb
# Build outputs
...
# Logs
npm-debug.log*
yarn-debug.log*
yarn-error.log*
-bun.lockb🤖 Prompt for AI Agents |
||
|
|
||
| # local env files | ||
| .env*.local | ||
|
|
||
| # log files | ||
| dev_server.log | ||
| server.log | ||
| # Testing | ||
| playwright-report/ | ||
| test-results/ | ||
| coverage/ | ||
|
|
||
| # vercel | ||
| .vercel | ||
|
|
||
| # typescript | ||
| # Misc | ||
| .vercel/ | ||
| *.tsbuildinfo | ||
|
Comment on lines
+1
to
39
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sensitive Supabase local metadata is being committed ( This is a clear repo hygiene/security issue. SuggestionAdd ignores and remove committed temp files:
Reply with "@CharlieHelps yes please" if you'd like me to add a commit that updates |
||
| next-env.d.ts | ||
|
|
||
| # Playwright | ||
| /playwright-report/ | ||
| /test-results/ | ||
| /dev.log | ||
| # AlphaEarth Embeddings - Sensitive Files | ||
| # Add these lines to your main .gitignore | ||
|
|
||
| # GCP Service Account Credentials (NEVER commit) | ||
| gcp_credentials.json | ||
| **/gcp_credentials.json | ||
|
|
||
| # AlphaEarth Index File (large, should be downloaded separately) | ||
| aef_index.csv | ||
|
|
||
| # Environment variables with GCP credentials | ||
| .env.local | ||
| .env.production.local | ||
| *.log | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| { | ||
| "editor.formatOnSave": true, | ||
| "editor.defaultFormatter": "esbenp.prettier-vscode" | ||
| "editor.defaultFormatter": "esbenp.prettier-vscode", | ||
| "IDX.corgiMode": true | ||
|
Comment on lines
+3
to
+4
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Confirm repo-wide intent for IDX.corgiMode. 🤖 Prompt for AI Agents |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| # Auth Backend Schema Fixes - PR #327 | ||
|
|
||
| ## Summary of Changes | ||
|
|
||
| This commit addresses critical security vulnerabilities and auth backend schema issues identified in the CodeRabbit review. | ||
|
|
||
| ## Critical Security Fixes | ||
|
|
||
| ### 1. ✅ Deleted RLS Disable Migration | ||
| **File:** `supabase/migrations/0002_disable_rls_for_testing.sql` (DELETED) | ||
| - **Issue:** This migration disabled Row Level Security on all tables, creating a critical security vulnerability | ||
| - **Risk:** Anyone could read, modify, or delete ANY user's chats, messages, and participants | ||
| - **Fix:** Completely removed this migration file to ensure RLS remains enabled in production | ||
|
|
||
| ### 2. ✅ Added pgcrypto Extension | ||
| **File:** `supabase/migrations/0000_init.sql` | ||
| - **Issue:** Used `gen_random_uuid()` without enabling the pgcrypto extension | ||
| - **Risk:** Migration would fail on typical Supabase setups | ||
| - **Fix:** Added `CREATE EXTENSION IF NOT EXISTS "pgcrypto";` at the start of the migration | ||
|
|
||
| ### 3. ✅ Fixed User Lookup in Collaboration | ||
| **File:** `lib/actions/collaboration.ts` | ||
| - **Issue:** Queried non-existent `public.users` table instead of `auth.users` | ||
| - **Risk:** User invitation flow always failed | ||
| - **Fix:** Updated `inviteUserToChat()` to use `auth.admin.listUsers()` via the service client to properly look up users by email | ||
|
|
||
| ### 4. ✅ Added Auth Check to RAG Function | ||
| **File:** `lib/actions/rag.ts` | ||
| - **Issue:** `retrieveContext()` had no authentication check | ||
| - **Risk:** Unauthorized users could access message embeddings | ||
| - **Fix:** Added authentication validation at the start of the function using `getCurrentUserIdOnServer()` | ||
|
|
||
| ### 5. ✅ Added Environment Validation | ||
| **File:** `lib/supabase/client.ts` | ||
| - **Issue:** Service client creation didn't validate required environment variables | ||
| - **Risk:** Service client could fail silently, bypassing RLS checks | ||
| - **Fix:** Added proper validation with descriptive error messages for missing `NEXT_PUBLIC_SUPABASE_URL` or `SUPABASE_SERVICE_ROLE_KEY` | ||
|
|
||
| ### 6. ✅ Improved INSERT Policy Security | ||
| **File:** `supabase/migrations/0002_add_insert_policy_for_chats.sql` | ||
| - **Issue:** Policy allowed any authenticated user to insert chats with any user_id | ||
| - **Risk:** Users could create chats impersonating other users | ||
| - **Fix:** Updated policy to enforce `auth.uid() = user_id`, ensuring users can only create chats where they are the owner | ||
|
|
||
| ## Files Modified | ||
|
|
||
| 1. `lib/actions/collaboration.ts` - Fixed user lookup to use auth.admin API | ||
| 2. `lib/actions/rag.ts` - Added authentication check | ||
| 3. `lib/supabase/client.ts` - Added environment variable validation | ||
| 4. `supabase/migrations/0000_init.sql` - Added pgcrypto extension | ||
| 5. `supabase/migrations/0002_add_insert_policy_for_chats.sql` - Improved security policy | ||
| 6. `supabase/migrations/0002_disable_rls_for_testing.sql` - DELETED (critical security issue) | ||
|
|
||
| ## Security Improvements | ||
|
|
||
| - ✅ RLS remains enabled on all tables | ||
| - ✅ All server actions now validate authentication | ||
| - ✅ User lookup uses proper Supabase auth APIs | ||
| - ✅ Environment variables are validated before use | ||
| - ✅ INSERT policies enforce proper ownership | ||
| - ✅ Database migrations will run successfully on standard Supabase setups | ||
|
|
||
| ## Testing Recommendations | ||
|
|
||
| 1. Verify RLS policies are active: Check Supabase dashboard | ||
| 2. Test user invitation flow: Ensure users can be invited by email | ||
| 3. Test RAG context retrieval: Verify auth check prevents unauthorized access | ||
| 4. Test chat creation: Ensure users can only create chats as themselves | ||
| 5. Run migrations on a test Supabase project to verify they execute without errors | ||
|
|
||
| ## Related Issues | ||
|
|
||
| Addresses CodeRabbit review comments: | ||
| - https://github.com/QueueLab/QCX/pull/327#issuecomment-3714336689 | ||
|
Comment on lines
+7
to
+74
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix markdownlint MD022/MD034 violations. 📝 Example fix (apply consistently)-## Critical Security Fixes
+## Critical Security Fixes
-### 1. ✅ Deleted RLS Disable Migration
+### 1. ✅ Deleted RLS Disable Migration
-## Related Issues
+## Related Issues
-Addresses CodeRabbit review comments:
-- https://github.com/QueueLab/QCX/pull/327#issuecomment-3714336689
+Addresses CodeRabbit review comments:
+- [Review comment](https://github.com/QueueLab/QCX/pull/327#issuecomment-3714336689)🧰 Tools🪛 markdownlint-cli2 (0.18.1)9-9: Headings should be surrounded by blank lines (MD022, blanks-around-headings) 15-15: Headings should be surrounded by blank lines (MD022, blanks-around-headings) 21-21: Headings should be surrounded by blank lines (MD022, blanks-around-headings) 27-27: Headings should be surrounded by blank lines (MD022, blanks-around-headings) 33-33: Headings should be surrounded by blank lines (MD022, blanks-around-headings) 39-39: Headings should be surrounded by blank lines (MD022, blanks-around-headings) 74-74: Bare URL used (MD034, no-bare-urls) 🤖 Prompt for AI Agents |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not ship wildcard Server Actions origins to production.
SERVER_ACTIONS_ALLOWED_ORIGINS="*"allows any origin to invoke Server Actions. Ensure production environments override this with explicit trusted origins or keep the wildcard only in a dev-only.env.local.🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 5-5: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
🤖 Prompt for AI Agents