-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
Feature/sign in issue #1087
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?
Feature/sign in issue #1087
Conversation
|
@DominikLudwiczak is attempting to deploy a commit to the Listinai Team on Vercel. A member of the Team first needs to authorize it. |
| cors: { | ||
| ...(!process.env.NOT_SECURED ? { credentials: true } : {}), | ||
| allowedHeaders: ['Content-Type', 'Authorization', 'x-copilotkit-runtime-client-gql-version'], | ||
| credentials: true, |
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.
Bug: Backend CORS unconditionally requires credentials, but frontend does not send them in dev mode (NOT_SECURED=true), causing CORS errors.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
The PR's change to CORS configuration unconditionally sets credentials: true on the backend. However, in development mode (process.env.NOT_SECURED=true), the frontend explicitly does not send credentials: 'include' in its fetch requests. This mismatch causes browsers to block responses with CORS errors, preventing authentication from working in dev mode. This contradicts the PR's goal of fixing sign-in issues in development.
💡 Suggested Fix
Revert the CORS configuration change to preserve conditional logic: ...(process.env.NOT_SECURED ? {} : { credentials: true }) instead of unconditionally setting credentials: true.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: apps/backend/src/main.ts#L23
Potential issue: The PR's change to CORS configuration unconditionally sets
`credentials: true` on the backend. However, in development mode
(`process.env.NOT_SECURED=true`), the frontend explicitly does *not* send `credentials:
'include'` in its fetch requests. This mismatch causes browsers to block responses with
CORS errors, preventing authentication from working in dev mode. This contradicts the
PR's goal of fixing sign-in issues in development.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 196827
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.
it worked for me when I set NOT_SECURED to TRUE. If you want I can change it back, because most important change here is with headers
Problem
When signing in on the dev mode with NOT_SECURED flag set to TRUE the CORS error occurs and no action is done.
This PR fixes issue #1053
Solution
Change CORS settings by allowing new headers when the flag NOT_SECURED is set to TRUE. Also added prima packages ti the onlyBuiltDependencies in package.json because it was required during setting the dev mode up
Checklist:
Put a "X" in the boxes below to indicate you have followed the checklist;