Skip to content

feat: add email login option alongside GitHub OAuth#299

Merged
khaliqgant merged 11 commits intomainfrom
claude/add-email-login-9nUjx
Jan 26, 2026
Merged

feat: add email login option alongside GitHub OAuth#299
khaliqgant merged 11 commits intomainfrom
claude/add-email-login-9nUjx

Conversation

@khaliqgant
Copy link
Copy Markdown
Member

@khaliqgant khaliqgant commented Jan 25, 2026

Add email/password authentication as an alternative to GitHub OAuth:

  • Database schema changes:

    • Add passwordHash, emailVerified, emailVerificationToken,
      emailVerificationExpires, displayName fields to users table
    • Make githubId and githubUsername nullable for email-only users
    • Add unique constraint and index on email
  • New API endpoints in /api/auth/email:

    • POST /signup - create account with email/password
    • POST /login - authenticate with email/password
    • POST /verify - verify email with token
    • POST /resend-verification - resend verification email
    • POST /set-email - set email for GitHub users without one
    • POST /set-password - add password to existing account
  • UI updates:

    • Login page: tabbed interface for GitHub/Email login
    • Signup page: tabbed interface for GitHub/Email signup
    • New complete-profile page for GitHub users without email

GitHub users without a public email are redirected to /complete-profile
to provide their email address before continuing.


Open with Devin

Add email/password authentication as an alternative to GitHub OAuth:

- Database schema changes:
  - Add passwordHash, emailVerified, emailVerificationToken,
    emailVerificationExpires, displayName fields to users table
  - Make githubId and githubUsername nullable for email-only users
  - Add unique constraint and index on email

- New API endpoints in /api/auth/email:
  - POST /signup - create account with email/password
  - POST /login - authenticate with email/password
  - POST /verify - verify email with token
  - POST /resend-verification - resend verification email
  - POST /set-email - set email for GitHub users without one
  - POST /set-password - add password to existing account

- UI updates:
  - Login page: tabbed interface for GitHub/Email login
  - Signup page: tabbed interface for GitHub/Email signup
  - New complete-profile page for GitHub users without email

GitHub users without a public email are redirected to /complete-profile
to provide their email address before continuing.
@my-senior-dev-pr-review
Copy link
Copy Markdown

my-senior-dev-pr-review Bot commented Jan 25, 2026

🤖 My Senior Dev — Analysis Complete

👤 For @khaliqgant

📁 Expert in src (662 edits) • ⚡ 152nd PR this month

View your contributor analytics →


📊 14 files reviewed • 5 high risk • 10 need attention

🚨 High Risk:

  • packages/cloud/src/api/email-auth.ts — This file contains the core authentication logic and multiple critical security vulnerabilities.
  • packages/cloud/src/db/drizzle.ts — Database operations responsiveness are critical for user data integrity.
  • packages/dashboard/ui/app/login/page.tsx — Modifications on login page need to accommodate both authentication methods securely.
  • +4 more

⚠️ Needs Attention:

  • packages/cloud/src/api/billing.ts — Changes affect user subscription handling and admin checks.
  • packages/dashboard/ui/app/signup/page.tsx — Sign-up alterations have implications for user data accuracy.
  • +1 more concerns...

🚀 Open Interactive Review →

The full interface unlocks features not available in GitHub:

  • 💬 AI Chat — Ask questions on any file, get context-aware answers
  • 🔍 Smart Hovers — See symbol definitions and usage without leaving the diff
  • 📚 Code Archeology — Understand how files evolved over time (/archeology)
  • 🎯 Learning Insights — See how this PR compares to similar changes

💬 Chat here: @my-senior-dev explain this change — or try @chaos-monkey @security-auditor @optimizer @skeptic @junior-dev

📖 View all 12 personas & slash commands

You can interact with me by mentioning @my-senior-dev in any comment:

In PR comments or on any line of code:

  • Ask questions about the code or PR
  • Request explanations of specific changes
  • Get suggestions for improvements

Slash commands:

  • /help — Show all available commands
  • /archeology — See the history and evolution of changed files
  • /profile — Performance analysis and suggestions
  • /expertise — Find who knows this code best
  • /personas — List all available AI personas

AI Personas (mention to get their perspective):

Persona Focus
@chaos-monkey 🐵 Edge cases & failure scenarios
@skeptic 🤨 Challenge assumptions
@optimizer Performance & efficiency
@security-auditor 🔒 Security vulnerabilities
@accessibility-advocate Inclusive design
@junior-dev 🌱 Simple explanations
@tech-debt-collector 💳 Code quality & shortcuts
@ux-champion 🎨 User experience
@devops-engineer 🚀 Deployment & scaling
@documentation-nazi 📚 Documentation gaps
@legacy-whisperer 🏛️ Working with existing code
@test-driven-purist Testing & TDD

For the best experience, view this PR on myseniordev.com — includes AI chat, file annotations, and interactive reviews.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View issue and 5 additional flags in Devin Review.

Open in Devin Review

Comment on lines +39 to +44
if (data.user?.email) {
window.location.href = '/app';
return;
}

setUser(data.user);
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.

🔴 Complete Profile page expects wrong response structure from /api/auth/me endpoint

The complete-profile page incorrectly accesses user data from the /api/auth/me API response, causing the page to malfunction.

Click to expand

Issue

The complete-profile/page.tsx fetches from /api/auth/me and expects the response to have user data nested under a user property:

// complete-profile/page.tsx:39
if (data.user?.email) {
  window.location.href = '/app';
  return;
}
setUser(data.user);  // line 44

However, the /api/auth/me endpoint (packages/cloud/src/api/auth.ts:73-83) returns user data at the top level:

res.json({
  id: user.id,
  githubUsername: user.githubUsername,
  email: user.email,
  avatarUrl: user.avatarUrl,
  // ... no 'user' wrapper
});

Impact

  1. data.user will always be undefined
  2. The email check data.user?.email will always be falsy, so users with email won't be redirected to /app
  3. setUser(data.user) sets user state to undefined, causing the user info section to not render properly
  4. GitHub users who need to provide an email will see a broken page with no user info displayed

Expected vs Actual

  • Expected: Page should access data.email and setUser(data) (or API should return { user: {...} })
  • Actual: Page accesses data.user?.email and setUser(data.user) which are always undefined

Recommendation: Change line 39 to if (data.email) and line 44 to setUser(data) to match the actual response structure from /api/auth/me. Alternatively, change the page to use /api/auth/session which does return { user: {...} } structure.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

*/
function isValidEmail(email: string): boolean {
const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
return emailRegex.test(email);

Check failure

Code scanning / CodeQL

Polynomial regular expression used on uncontrolled data High

This
regular expression
that depends on
a user-provided value
may run slow on strings starting with '!@!.' and with many repetitions of '!.'.
This
regular expression
that depends on
a user-provided value
may run slow on strings starting with '!@!.' and with many repetitions of '!.'.
Comment on lines +155 to +209
emailAuthRouter.post('/login', async (req: Request, res: Response) => {
try {
const { email, password } = req.body;

// Validate input
if (!email || typeof email !== 'string') {
return res.status(400).json({ error: 'Email is required' });
}

if (!password || typeof password !== 'string') {
return res.status(400).json({ error: 'Password is required' });
}

const normalizedEmail = email.toLowerCase().trim();

// Find user by email
const user = await db.users.findByEmail(normalizedEmail);
if (!user) {
// Use same message for security (don't reveal if email exists)
return res.status(401).json({ error: 'Invalid email or password' });
}

// Check if user has a password (might be GitHub-only user)
if (!user.passwordHash) {
return res.status(401).json({
error: 'This account uses GitHub login. Please sign in with GitHub.',
code: 'GITHUB_ACCOUNT',
});
}

// Verify password
const isValid = await verifyPassword(password, user.passwordHash);
if (!isValid) {
return res.status(401).json({ error: 'Invalid email or password' });
}

// Set session
req.session.userId = user.id;

res.json({
success: true,
user: {
id: user.id,
email: user.email,
displayName: user.displayName,
githubUsername: user.githubUsername,
avatarUrl: user.avatarUrl,
emailVerified: user.emailVerified,
},
});
} catch (error) {
console.error('Email login error:', error);
res.status(500).json({ error: 'Failed to login' });
}
});

Check failure

Code scanning / CodeQL

Missing rate limiting High

This route handler performs
authorization
, but is not rate-limited.

Copilot Autofix

AI 3 months ago

In general terms, the fix is to apply a rate-limiting middleware to the sensitive authentication routes so that repeated login attempts from the same client are limited over a time window. This prevents brute-force password attacks and reduces the impact of request floods on expensive operations like password hashing and database lookups.

The best way to fix this without changing existing functionality is to introduce a dedicated rate limiter (using a well-known library like express-rate-limit) and apply it to the /login route (and optionally other auth routes). We can define the limiter once in packages/cloud/src/api/email-auth.ts and then use it as a middleware before the async handler for the login endpoint. For example, limit each IP to a modest number of login attempts (e.g., 10) per 15 minutes, returning HTTP 429 when the limit is exceeded. Implementation steps in this file:

  • Add an import for express-rate-limit.
  • Define a loginRateLimiter configured for the login use case.
  • Change the /login route registration from emailAuthRouter.post('/login', async (...) => { ... }) to emailAuthRouter.post('/login', loginRateLimiter, async (...) => { ... }).
    No changes are needed to the logic inside the handler itself.
Suggested changeset 2
packages/cloud/src/api/email-auth.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/cloud/src/api/email-auth.ts b/packages/cloud/src/api/email-auth.ts
--- a/packages/cloud/src/api/email-auth.ts
+++ b/packages/cloud/src/api/email-auth.ts
@@ -9,6 +9,7 @@
  */
 
 import { Router, Request, Response } from 'express';
+import rateLimit from 'express-rate-limit';
 import { randomBytes, scrypt, timingSafeEqual } from 'node:crypto';
 import { promisify } from 'node:util';
 import { db } from '../db/index.js';
@@ -18,6 +19,15 @@
 
 export const emailAuthRouter = Router();
 
+// Rate limiting configuration for login endpoint
+const loginRateLimiter = rateLimit({
+  windowMs: 15 * 60 * 1000, // 15 minutes
+  max: 10, // limit each IP to 10 login requests per windowMs
+  standardHeaders: true,
+  legacyHeaders: false,
+  message: { error: 'Too many login attempts, please try again later.' },
+});
+
 // Password hashing configuration
 const SALT_LENGTH = 32;
 const KEY_LENGTH = 64;
@@ -164,7 +174,7 @@
  * Supports account reconciliation: users can log in with any email address
  * linked to their account via GitHub, not just their primary email.
  */
-emailAuthRouter.post('/login', async (req: Request, res: Response) => {
+emailAuthRouter.post('/login', loginRateLimiter, async (req: Request, res: Response) => {
   try {
     const { email, password } = req.body;
 
EOF
@@ -9,6 +9,7 @@
*/

import { Router, Request, Response } from 'express';
import rateLimit from 'express-rate-limit';
import { randomBytes, scrypt, timingSafeEqual } from 'node:crypto';
import { promisify } from 'node:util';
import { db } from '../db/index.js';
@@ -18,6 +19,15 @@

export const emailAuthRouter = Router();

// Rate limiting configuration for login endpoint
const loginRateLimiter = rateLimit({
windowMs: 15 * 60 * 1000, // 15 minutes
max: 10, // limit each IP to 10 login requests per windowMs
standardHeaders: true,
legacyHeaders: false,
message: { error: 'Too many login attempts, please try again later.' },
});

// Password hashing configuration
const SALT_LENGTH = 32;
const KEY_LENGTH = 64;
@@ -164,7 +174,7 @@
* Supports account reconciliation: users can log in with any email address
* linked to their account via GitHub, not just their primary email.
*/
emailAuthRouter.post('/login', async (req: Request, res: Response) => {
emailAuthRouter.post('/login', loginRateLimiter, async (req: Request, res: Response) => {
try {
const { email, password } = req.body;

packages/cloud/package.json
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/cloud/package.json b/packages/cloud/package.json
--- a/packages/cloud/package.json
+++ b/packages/cloud/package.json
@@ -42,7 +42,8 @@
     "@agent-relay/config": "2.0.16",
     "@agent-relay/resiliency": "2.0.16",
     "@agent-relay/storage": "2.0.16",
-    "@agent-relay/protocol": "2.0.16"
+    "@agent-relay/protocol": "2.0.16",
+    "express-rate-limit": "^8.2.1"
   },
   "devDependencies": {
     "@types/node": "^22.19.3",
EOF
@@ -42,7 +42,8 @@
"@agent-relay/config": "2.0.16",
"@agent-relay/resiliency": "2.0.16",
"@agent-relay/storage": "2.0.16",
"@agent-relay/protocol": "2.0.16"
"@agent-relay/protocol": "2.0.16",
"express-rate-limit": "^8.2.1"
},
"devDependencies": {
"@types/node": "^22.19.3",
This fix introduces these dependencies
Package Version Security advisories
express-rate-limit (npm) 8.2.1 None
Copilot is powered by AI and may make mistakes. Always verify output.
const { email, password } = req.body;

// Validate input
if (!email || typeof email !== 'string') {

Check failure

Code scanning / CodeQL

User-controlled bypass of security check High

This condition guards a sensitive
action
, but a
user-provided value
controls it.
return res.status(400).json({ error: 'Email is required' });
}

if (!password || typeof password !== 'string') {

Check failure

Code scanning / CodeQL

User-controlled bypass of security check High

This condition guards a sensitive
action
, but a
user-provided value
controls it.
Comment on lines +215 to +245
emailAuthRouter.post('/verify', async (req: Request, res: Response) => {
try {
const { token } = req.body;

if (!token || typeof token !== 'string') {
return res.status(400).json({ error: 'Verification token is required' });
}

// Find user by token
const user = await db.users.findByEmailVerificationToken(token);
if (!user) {
return res.status(400).json({ error: 'Invalid or expired verification token' });
}

// Check if token expired
if (user.emailVerificationExpires && user.emailVerificationExpires < new Date()) {
return res.status(400).json({ error: 'Verification token has expired' });
}

// Verify email
await db.users.verifyEmail(user.id);

res.json({
success: true,
message: 'Email verified successfully',
});
} catch (error) {
console.error('Email verification error:', error);
res.status(500).json({ error: 'Failed to verify email' });
}
});

Check failure

Code scanning / CodeQL

Missing rate limiting High

This route handler performs
authorization
, but is not rate-limited.

Copilot Autofix

AI 3 months ago

In general, the fix is to add a rate-limiting middleware to sensitive routes that perform authentication/authorization or other expensive operations. In this file, the key routes are POST /signup, POST /login, POST /verify, and potentially POST /resend-verification. The best approach without changing existing behavior is to introduce a rate limiter with conservative limits tailored for auth flows (e.g., a small number of attempts per IP per unit time) and apply it specifically to these routes. This keeps the rest of the router unchanged and adds only middleware wrappers.

Concretely for packages/cloud/src/api/email-auth.ts:

  • Import a well-known rate-limiting library such as express-rate-limit.
  • Define one or more limiters, for example:
    • An authRateLimiter for signup and login (e.g., 10–20 requests per 15 minutes).
    • A tokenRateLimiter for verification and resend-verification routes (e.g., 30 requests per 15 minutes).
  • Apply the limiters by adding them as middleware arguments to emailAuthRouter.post(...) for /signup, /login, /verify, and /resend-verification. This preserves all route logic while enforcing rate limits.
  • Keep the limits moderate to avoid breaking existing users but strong enough to mitigate brute force and DoS against the auth DB operations.

All changes will stay within this file: add the express-rate-limit import, define limiter constants near the top (after router creation), and update the route declarations to include the limiter middlewares.

Suggested changeset 2
packages/cloud/src/api/email-auth.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/cloud/src/api/email-auth.ts b/packages/cloud/src/api/email-auth.ts
--- a/packages/cloud/src/api/email-auth.ts
+++ b/packages/cloud/src/api/email-auth.ts
@@ -9,6 +9,7 @@
  */
 
 import { Router, Request, Response } from 'express';
+import rateLimit from 'express-rate-limit';
 import { randomBytes, scrypt, timingSafeEqual } from 'node:crypto';
 import { promisify } from 'node:util';
 import { db } from '../db/index.js';
@@ -18,6 +19,21 @@
 
 export const emailAuthRouter = Router();
 
+// Rate limiting configuration for email auth routes
+const authRateLimiter = rateLimit({
+  windowMs: 15 * 60 * 1000, // 15 minutes
+  max: 20, // limit each IP to 20 auth requests per window
+  standardHeaders: true,
+  legacyHeaders: false,
+});
+
+const tokenRateLimiter = rateLimit({
+  windowMs: 15 * 60 * 1000, // 15 minutes
+  max: 50, // limit each IP to 50 token-based requests per window
+  standardHeaders: true,
+  legacyHeaders: false,
+});
+
 // Password hashing configuration
 const SALT_LENGTH = 32;
 const KEY_LENGTH = 64;
@@ -81,7 +97,7 @@
  * POST /api/auth/email/signup
  * Create a new account with email/password
  */
-emailAuthRouter.post('/signup', async (req: Request, res: Response) => {
+emailAuthRouter.post('/signup', authRateLimiter, async (req: Request, res: Response) => {
   try {
     const { email, password, displayName } = req.body;
 
@@ -164,7 +180,7 @@
  * Supports account reconciliation: users can log in with any email address
  * linked to their account via GitHub, not just their primary email.
  */
-emailAuthRouter.post('/login', async (req: Request, res: Response) => {
+emailAuthRouter.post('/login', authRateLimiter, async (req: Request, res: Response) => {
   try {
     const { email, password } = req.body;
 
@@ -225,7 +241,7 @@
  * POST /api/auth/email/verify
  * Verify email with token
  */
-emailAuthRouter.post('/verify', async (req: Request, res: Response) => {
+emailAuthRouter.post('/verify', tokenRateLimiter, async (req: Request, res: Response) => {
   try {
     const { token } = req.body;
 
@@ -261,7 +277,7 @@
  * POST /api/auth/email/resend-verification
  * Resend verification email (requires auth)
  */
-emailAuthRouter.post('/resend-verification', requireAuth, async (req: Request, res: Response) => {
+emailAuthRouter.post('/resend-verification', requireAuth, tokenRateLimiter, async (req: Request, res: Response) => {
   try {
     const userId = req.session.userId!;
     const user = await db.users.findById(userId);
EOF
@@ -9,6 +9,7 @@
*/

import { Router, Request, Response } from 'express';
import rateLimit from 'express-rate-limit';
import { randomBytes, scrypt, timingSafeEqual } from 'node:crypto';
import { promisify } from 'node:util';
import { db } from '../db/index.js';
@@ -18,6 +19,21 @@

export const emailAuthRouter = Router();

// Rate limiting configuration for email auth routes
const authRateLimiter = rateLimit({
windowMs: 15 * 60 * 1000, // 15 minutes
max: 20, // limit each IP to 20 auth requests per window
standardHeaders: true,
legacyHeaders: false,
});

const tokenRateLimiter = rateLimit({
windowMs: 15 * 60 * 1000, // 15 minutes
max: 50, // limit each IP to 50 token-based requests per window
standardHeaders: true,
legacyHeaders: false,
});

// Password hashing configuration
const SALT_LENGTH = 32;
const KEY_LENGTH = 64;
@@ -81,7 +97,7 @@
* POST /api/auth/email/signup
* Create a new account with email/password
*/
emailAuthRouter.post('/signup', async (req: Request, res: Response) => {
emailAuthRouter.post('/signup', authRateLimiter, async (req: Request, res: Response) => {
try {
const { email, password, displayName } = req.body;

@@ -164,7 +180,7 @@
* Supports account reconciliation: users can log in with any email address
* linked to their account via GitHub, not just their primary email.
*/
emailAuthRouter.post('/login', async (req: Request, res: Response) => {
emailAuthRouter.post('/login', authRateLimiter, async (req: Request, res: Response) => {
try {
const { email, password } = req.body;

@@ -225,7 +241,7 @@
* POST /api/auth/email/verify
* Verify email with token
*/
emailAuthRouter.post('/verify', async (req: Request, res: Response) => {
emailAuthRouter.post('/verify', tokenRateLimiter, async (req: Request, res: Response) => {
try {
const { token } = req.body;

@@ -261,7 +277,7 @@
* POST /api/auth/email/resend-verification
* Resend verification email (requires auth)
*/
emailAuthRouter.post('/resend-verification', requireAuth, async (req: Request, res: Response) => {
emailAuthRouter.post('/resend-verification', requireAuth, tokenRateLimiter, async (req: Request, res: Response) => {
try {
const userId = req.session.userId!;
const user = await db.users.findById(userId);
packages/cloud/package.json
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/cloud/package.json b/packages/cloud/package.json
--- a/packages/cloud/package.json
+++ b/packages/cloud/package.json
@@ -42,7 +42,8 @@
     "@agent-relay/config": "2.0.16",
     "@agent-relay/resiliency": "2.0.16",
     "@agent-relay/storage": "2.0.16",
-    "@agent-relay/protocol": "2.0.16"
+    "@agent-relay/protocol": "2.0.16",
+    "express-rate-limit": "^8.2.1"
   },
   "devDependencies": {
     "@types/node": "^22.19.3",
EOF
@@ -42,7 +42,8 @@
"@agent-relay/config": "2.0.16",
"@agent-relay/resiliency": "2.0.16",
"@agent-relay/storage": "2.0.16",
"@agent-relay/protocol": "2.0.16"
"@agent-relay/protocol": "2.0.16",
"express-rate-limit": "^8.2.1"
},
"devDependencies": {
"@types/node": "^22.19.3",
This fix introduces these dependencies
Package Version Security advisories
express-rate-limit (npm) 8.2.1 None
Copilot is powered by AI and may make mistakes. Always verify output.
try {
const { token } = req.body;

if (!token || typeof token !== 'string') {

Check failure

Code scanning / CodeQL

User-controlled bypass of security check High

This condition guards a sensitive
action
, but a
user-provided value
controls it.
Comment on lines +155 to +209
emailAuthRouter.post('/login', async (req: Request, res: Response) => {
try {
const { email, password } = req.body;

// Validate input
if (!email || typeof email !== 'string') {
return res.status(400).json({ error: 'Email is required' });
}

if (!password || typeof password !== 'string') {
return res.status(400).json({ error: 'Password is required' });
}

const normalizedEmail = email.toLowerCase().trim();

// Find user by email
const user = await db.users.findByEmail(normalizedEmail);
if (!user) {
// Use same message for security (don't reveal if email exists)
return res.status(401).json({ error: 'Invalid email or password' });
}

// Check if user has a password (might be GitHub-only user)
if (!user.passwordHash) {
return res.status(401).json({
error: 'This account uses GitHub login. Please sign in with GitHub.',
code: 'GITHUB_ACCOUNT',
});
}

// Verify password
const isValid = await verifyPassword(password, user.passwordHash);
if (!isValid) {
return res.status(401).json({ error: 'Invalid email or password' });
}

// Set session
req.session.userId = user.id;

res.json({
success: true,
user: {
id: user.id,
email: user.email,
displayName: user.displayName,
githubUsername: user.githubUsername,
avatarUrl: user.avatarUrl,
emailVerified: user.emailVerified,
},
});
} catch (error) {
console.error('Email login error:', error);
res.status(500).json({ error: 'Failed to login' });
}
});

Check warning

Code scanning / CodeQL

Failure to abandon session Medium

Route handler does not invalidate session following login.
- Add user_emails table to store all GitHub-linked emails
- Fetch user emails from GitHub API (requires user:email scope in Nango)
- Sync all verified emails during GitHub login for account reconciliation
- Update email login to check user_emails table for account matching
- Users can now log in with any GitHub-linked email address

Note: Requires 'user:email' scope to be added to GitHub integration in Nango
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View issue and 8 additional flags in Devin Review.

Open in Devin Review

Comment on lines +360 to +382
async syncFromGitHub(userId: string, emails: Array<{ email: string; verified: boolean; primary: boolean }>): Promise<void> {
const db = getDb();

// Delete existing GitHub-sourced emails for this user
await db
.delete(schema.userEmails)
.where(and(
eq(schema.userEmails.userId, userId),
eq(schema.userEmails.source, 'github')
));

// Insert all new emails
if (emails.length > 0) {
await db.insert(schema.userEmails).values(
emails.map(e => ({
userId,
email: e.email.toLowerCase(),
verified: e.verified,
primary: e.primary,
source: 'github' as const,
}))
);
}
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.

🟡 Race condition in syncFromGitHub due to non-atomic delete-then-insert

The syncFromGitHub function performs a delete followed by an insert as two separate database operations without a transaction.

Click to expand

Issue

At packages/cloud/src/db/drizzle.ts:360-382, the function:

  1. Deletes all GitHub-sourced emails for the user
  2. Then inserts the new emails
// Delete existing GitHub-sourced emails for this user
await db.delete(schema.userEmails).where(...);

// Insert all new emails
if (emails.length > 0) {
  await db.insert(schema.userEmails).values(...);
}

Race Condition

If a concurrent login attempt via findUserByEmail (drizzle.ts:312-334) occurs between the DELETE and INSERT operations, the lookup will fail to find the user because their emails have been deleted but not yet re-inserted.

Impact

This could cause intermittent authentication failures for GitHub users logging in while their emails are being re-synced (e.g., during a concurrent login from another device or when the Nango webhook fires).

Expected Behavior

The delete and insert should be wrapped in a database transaction to ensure atomicity.

Recommendation: Wrap the delete and insert operations in a database transaction using Drizzle's transaction API: await db.transaction(async (tx) => { await tx.delete(...); await tx.insert(...); })

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

claude and others added 9 commits January 25, 2026 07:14
- Increase Docker provisioner health check timeout from 60s to 120s
  (allows time for GitHub token fetch retries + repo cloning)
- Fix Redis shutdown race condition by checking isOpen before quit()
- Prevent Codex OAuth double POST with completingRef guard
- Add inline GitHub OAuth for email users in RepoAccessPanel
  (uses Nango github integration, not github-app-oauth)
- Fix account linking when email user connects GitHub
  (check for existing user by email before creating new)
- Add displayName to both /api/auth/me and /api/auth/session endpoints
  with fallback chain: displayName -> githubUsername -> email prefix
- Wait for CLI ready before accepting messages in relay-pty orchestrator

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The isValidUsername regex was only allowing GitHub-style usernames
(alphanumeric with hyphens). Email users with display names like
"Khaliq Gant" were being silently rejected, causing messages to fail
with "Target not found" errors.

Changes:
- Update isValidUsername to allow spaces, underscores, and periods
- Increase max length from 39 to 50 chars for display names
- Add checks for no consecutive spaces
- Expand isValidAvatarUrl to allow Gravatar and UI Avatars
  (for email-based avatar services)

The UI already has proper fallback placeholders (showing initials)
when no avatar URL is provided.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add workspace scoping to user presence tracking
- Users only see other users from the same workspace
- Presence join/leave broadcasts scoped to workspace
- Typing indicators scoped to workspace
- Also fix username validation in cloud server (was missing from earlier fix)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@khaliqgant khaliqgant merged commit b0c70cc into main Jan 26, 2026
22 of 23 checks passed
@khaliqgant khaliqgant deleted the claude/add-email-login-9nUjx branch January 26, 2026 09:43
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.

3 participants