-
Notifications
You must be signed in to change notification settings - Fork 11.5k
chore: Add logging in next-auth #26404
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
Conversation
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
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.
1 issue found across 1 file
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/features/auth/lib/next-auth-options.ts">
<violation number="1" location="packages/features/auth/lib/next-auth-options.ts:1067">
P1: Rule violated: **Avoid Logging Sensitive Information**
This log statement exposes PII (email addresses) in plain text. The `user.id` and `providerAccountId` can contain email addresses as shown in the PR description example. Use `hashEmail()` (already imported in this file) to anonymize these values before logging.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
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.
2 issues found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/features/auth/lib/next-auth-options.ts">
<violation number="1" location="packages/features/auth/lib/next-auth-options.ts:1041">
P2: This `log.error` is in unreachable code. The preceding `if` block already handles all cases where `identityProvider === IdentityProvider.CAL` because `idP` (from `mapIdentityProvider`) is always either `GOOGLE` or `SAML`. This else-if branch will never execute.
This appears to be a pre-existing dead code issue. If you want to log when a CAL user tries to login with a different provider but conversion fails, the log should be added inside the preceding `if` block after validation failures.</violation>
<violation number="2" location="packages/features/auth/lib/next-auth-options.ts:1041">
P1: Rule violated: **Avoid Logging Sensitive Information**
Logging `user.id` exposes user email addresses (PII) in logs. Consider using a hashed version of the email or a non-PII identifier. The codebase already has `hashEmail` from `@calcom/lib/server/PiiHasher` available for this purpose.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
* Add logging in next-auth * Add logging at other return
What does this PR do?
Example of log
Visual Demo (For contributors especially)
A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).
Video Demo (if applicable):
Image Demo (if applicable):
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist
Summary by cubic
Added error logging in next-auth when a user signs in with the wrong identity provider, including a specific log for accounts that already exist with CAL. Logs userId and account details to help diagnose mismatched provider issues.
Written for commit 1ceddfc. Summary will update on new commits.