Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions apps/webapp/app/assets/logos/GoogleLogo.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
export function GoogleLogo({ className }: { className?: string }) {
return (
<svg className={className} viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg">
<path
d="M19.9075 21.0983C22.7427 18.4521 24.0028 14.0417 23.2468 9.82031H11.9688V14.4827H18.3953C18.1433 15.9949 17.2612 17.255 16.0011 18.0741L19.9075 21.0983Z"
fill="#4285F4"
/>
<path
d="M1.25781 17.3802C2.08665 19.013 3.27532 20.4362 4.73421 21.5428C6.1931 22.6493 7.88415 23.4102 9.67988 23.7681C11.4756 24.1261 13.3292 24.0717 15.1008 23.6091C16.8725 23.1465 18.516 22.2877 19.9075 21.0976L16.0011 18.0733C12.6618 20.2785 7.11734 19.4594 5.22717 14.293L1.25781 17.3802Z"
fill="#34A853"
/>
<path
d="M5.22701 14.2922C4.72297 12.717 4.72297 11.2679 5.22701 9.69275L1.25765 6.60547C-0.191479 9.50373 -0.632519 13.5991 1.25765 17.3794L5.22701 14.2922Z"
fill="#FBBC02"
/>
<path
d="M5.22717 9.69209C6.6133 5.34469 12.5358 2.82446 16.5052 6.5418L19.9705 3.13949C15.0561 -1.58594 5.47919 -1.39692 1.25781 6.60481L5.22717 9.69209Z"
fill="#EA4335"
/>
</svg>
);
}
1 change: 1 addition & 0 deletions apps/webapp/app/components/UserProfilePhoto.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export function UserAvatar({
className={cn("aspect-square rounded-full p-[7%]")}
src={avatarUrl}
alt={name ?? "User"}
referrerPolicy="no-referrer"
/>
</div>
) : (
Expand Down
2 changes: 2 additions & 0 deletions apps/webapp/app/env.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ const EnvironmentSchema = z
TRIGGER_TELEMETRY_DISABLED: z.string().optional(),
AUTH_GITHUB_CLIENT_ID: z.string().optional(),
AUTH_GITHUB_CLIENT_SECRET: z.string().optional(),
AUTH_GOOGLE_CLIENT_ID: z.string().optional(),
AUTH_GOOGLE_CLIENT_SECRET: z.string().optional(),
EMAIL_TRANSPORT: z.enum(["resend", "smtp", "aws-ses"]).optional(),
FROM_EMAIL: z.string().optional(),
REPLY_TO_EMAIL: z.string().optional(),
Expand Down
118 changes: 117 additions & 1 deletion apps/webapp/app/models/user.server.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { Prisma, User } from "@trigger.dev/database";
import type { GitHubProfile } from "remix-auth-github";
import type { GoogleProfile } from "remix-auth-google";
import { prisma } from "~/db.server";
import { env } from "~/env.server";
import {
Expand All @@ -20,7 +21,14 @@ type FindOrCreateGithub = {
authenticationExtraParams: Record<string, unknown>;
};

type FindOrCreateUser = FindOrCreateMagicLink | FindOrCreateGithub;
type FindOrCreateGoogle = {
authenticationMethod: "GOOGLE";
email: User["email"];
authenticationProfile: GoogleProfile;
authenticationExtraParams: Record<string, unknown>;
};

type FindOrCreateUser = FindOrCreateMagicLink | FindOrCreateGithub | FindOrCreateGoogle;

type LoggedInUser = {
user: User;
Expand All @@ -35,6 +43,9 @@ export async function findOrCreateUser(input: FindOrCreateUser): Promise<LoggedI
case "MAGIC_LINK": {
return findOrCreateMagicLinkUser(input);
}
case "GOOGLE": {
return findOrCreateGoogleUser(input);
}
}
}

Expand Down Expand Up @@ -162,6 +173,111 @@ export async function findOrCreateGithubUser({
};
}

export async function findOrCreateGoogleUser({
email,
authenticationProfile,
authenticationExtraParams,
}: FindOrCreateGoogle): Promise<LoggedInUser> {
assertEmailAllowed(email);

const name = authenticationProfile._json.name;
let avatarUrl: string | undefined = undefined;
if (authenticationProfile.photos[0]) {
avatarUrl = authenticationProfile.photos[0].value;
}
const displayName = authenticationProfile.displayName;
const authProfile = authenticationProfile
? (authenticationProfile as unknown as Prisma.JsonObject)
: undefined;
const authExtraParams = authenticationExtraParams
? (authenticationExtraParams as unknown as Prisma.JsonObject)
: undefined;

const authIdentifier = `google:${authenticationProfile.id}`;

const existingUser = await prisma.user.findUnique({
where: {
authIdentifier,
},
});

const existingEmailUser = await prisma.user.findUnique({
where: {
email,
},
});

if (existingEmailUser && !existingUser) {
// Link existing email account to Google auth
const user = await prisma.user.update({
where: {
email,
},
data: {
authenticationMethod: "GOOGLE",
authenticationProfile: authProfile,
authenticationExtraParams: authExtraParams,
avatarUrl,
authIdentifier,
},
});

return {
user,
isNewUser: false,
};
}
Comment on lines +210 to +229
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Remove authenticationMethod update when linking existing users.

When linking an existing email-based user to Google authentication, line 217 overwrites the user's authenticationMethod to "GOOGLE". This is inconsistent with the GitHub authentication implementation (lines 120-137), which does not update authenticationMethod when linking accounts.

Overwriting the authentication method could:

  • Break existing authentication flows that rely on this field
  • Prevent users from using their original authentication method
  • Create confusion about the user's original signup method

Apply this diff to maintain consistency:

     const user = await prisma.user.update({
       where: {
         email,
       },
       data: {
-        authenticationMethod: "GOOGLE",
         authenticationProfile: authProfile,
         authenticationExtraParams: authExtraParams,
         avatarUrl,
         authIdentifier,
       },
     });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (existingEmailUser && !existingUser) {
// Link existing email account to Google auth
const user = await prisma.user.update({
where: {
email,
},
data: {
authenticationMethod: "GOOGLE",
authenticationProfile: authProfile,
authenticationExtraParams: authExtraParams,
avatarUrl,
authIdentifier,
},
});
return {
user,
isNewUser: false,
};
}
if (existingEmailUser && !existingUser) {
// Link existing email account to Google auth
const user = await prisma.user.update({
where: {
email,
},
data: {
authenticationProfile: authProfile,
authenticationExtraParams: authExtraParams,
avatarUrl,
authIdentifier,
},
});
return {
user,
isNewUser: false,
};
}
🤖 Prompt for AI Agents
In apps/webapp/app/models/user.server.ts around lines 210 to 229, the code that
links an existing email user to Google currently overwrites authenticationMethod
to "GOOGLE"; remove that field from the prisma.user.update data so the existing
authenticationMethod is preserved (keep updating authenticationProfile,
authenticationExtraParams, avatarUrl and authIdentifier as before) to match the
GitHub linking behavior and avoid changing the user's original signup method.


if (existingEmailUser && existingUser) {
// User already linked to Google, update profile info
const user = await prisma.user.update({
where: {
id: existingUser.id,
},
data: {
avatarUrl,
authenticationProfile: authProfile,
authenticationExtraParams: authExtraParams,
},
});

return {
user,
isNewUser: false,
};
}
Comment on lines +231 to +248
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Handle the edge case when different users have conflicting email and auth identifiers.

The condition on line 231 doesn't verify that existingEmailUser and existingUser reference the same user. This edge case can occur when a user changes their Google email to an address already taken by a different user in your system.

Current behavior:

  • Updates the Google-authenticated user's profile (lines 233-242)
  • Does not update the email to match Google's current email
  • Leaves the email mismatch unresolved
  • Does not log or alert about the conflict

Inconsistency: The update (lines 237-241) only modifies avatarUrl and auth profiles, while the upsert update path (lines 257-262) also updates email, displayName, and name. This creates inconsistent behavior.

Based on learnings, as per the past review comment, this edge case should be addressed.

Apply this diff to detect the conflict and update all relevant fields:

   if (existingEmailUser && existingUser) {
-    // User already linked to Google, update profile info
+    if (existingEmailUser.id !== existingUser.id) {
+      // Conflict: Google account and email belong to different users
+      // This can happen if user changes their Google email to one already in use
+      console.error(
+        `Google auth conflict: Google ID ${authenticationProfile.id} belongs to user ${existingUser.id}, ` +
+        `but email ${email} belongs to user ${existingEmailUser.id}`
+      );
+      // Return the existing Google-authenticated user without updating email
+      return {
+        user: existingUser,
+        isNewUser: false,
+      };
+    }
+
+    // User already linked to Google, update profile info
     const user = await prisma.user.update({
       where: {
         id: existingUser.id,
       },
       data: {
+        email,
+        displayName,
+        name,
         avatarUrl,
         authenticationProfile: authProfile,
         authenticationExtraParams: authExtraParams,
       },
     });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (existingEmailUser && existingUser) {
// User already linked to Google, update profile info
const user = await prisma.user.update({
where: {
id: existingUser.id,
},
data: {
avatarUrl,
authenticationProfile: authProfile,
authenticationExtraParams: authExtraParams,
},
});
return {
user,
isNewUser: false,
};
}
if (existingEmailUser && existingUser) {
if (existingEmailUser.id !== existingUser.id) {
// Conflict: Google account and email belong to different users
// This can happen if user changes their Google email to one already in use
console.error(
`Google auth conflict: Google ID ${authenticationProfile.id} belongs to user ${existingUser.id}, ` +
`but email ${email} belongs to user ${existingEmailUser.id}`
);
// Return the existing Google-authenticated user without updating email
return {
user: existingUser,
isNewUser: false,
};
}
// User already linked to Google, update profile info
const user = await prisma.user.update({
where: {
id: existingUser.id,
},
data: {
email,
displayName,
name,
avatarUrl,
authenticationProfile: authProfile,
authenticationExtraParams: authExtraParams,
},
});
return {
user,
isNewUser: false,
};
}
🤖 Prompt for AI Agents
In apps/webapp/app/models/user.server.ts around lines 231 to 248, detect if
existingEmailUser and existingUser refer to different users
(existingEmailUser.id !== existingUser.id): if they are the same user proceed to
update all profile fields (email, displayName, name, avatarUrl,
authenticationProfile, authenticationExtraParams) to match the upsert path; if
they differ, log a clear warning about the email/auth conflict and return a
conflict result (or throw a specific error) instead of silently updating, so the
caller can surface the conflict to the user for resolution.


// When the IDP user (Google) already exists, the "update" path will be taken and the email will be updated
// It's not possible that the email is already taken by a different user because that would have been handled
// by one of the if statements above.
const user = await prisma.user.upsert({
where: {
authIdentifier,
},
update: {
email,
displayName,
name,
avatarUrl,
},
create: {
authenticationProfile: authProfile,
authenticationExtraParams: authExtraParams,
name,
avatarUrl,
displayName,
authIdentifier,
email,
authenticationMethod: "GOOGLE",
},
});

return {
user,
isNewUser: !existingUser,
};
}

export type UserWithDashboardPreferences = User & {
dashboardPreferences: DashboardPreferences;
};
Expand Down
21 changes: 11 additions & 10 deletions apps/webapp/app/routes/auth.github.callback.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { redirect } from "@remix-run/node";
import { prisma } from "~/db.server";
import { getSession, redirectWithErrorMessage } from "~/models/message.server";
import { authenticator } from "~/services/auth.server";
import { setLastAuthMethodHeader } from "~/services/lastAuthMethod.server";
import { commitSession } from "~/services/sessionStorage.server";
import { redirectCookie } from "./auth.github";
import { sanitizeRedirectPath } from "~/utils";
Expand Down Expand Up @@ -41,19 +42,19 @@ export let loader: LoaderFunction = async ({ request }) => {
session.set("pending-mfa-user-id", userRecord.id);
session.set("pending-mfa-redirect-to", redirectTo);

return redirect("/login/mfa", {
headers: {
"Set-Cookie": await commitSession(session),
},
});
const headers = new Headers();
headers.append("Set-Cookie", await commitSession(session));
headers.append("Set-Cookie", await setLastAuthMethodHeader("github"));

return redirect("/login/mfa", { headers });
}

// and store the user data
session.set(authenticator.sessionKey, auth);

return redirect(redirectTo, {
headers: {
"Set-Cookie": await commitSession(session),
},
});
const headers = new Headers();
headers.append("Set-Cookie", await commitSession(session));
headers.append("Set-Cookie", await setLastAuthMethodHeader("github"));

return redirect(redirectTo, { headers });
};
11 changes: 8 additions & 3 deletions apps/webapp/app/routes/auth.github.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
import { type ActionFunction, type LoaderFunction, redirect, createCookie } from "@remix-run/node";
import { authenticator } from "~/services/auth.server";
import { env } from "~/env.server";
import { sanitizeRedirectPath } from "~/utils";

export let loader: LoaderFunction = () => redirect("/login");

export let action: ActionFunction = async ({ request }) => {
const url = new URL(request.url);
const redirectTo = url.searchParams.get("redirectTo");
const safeRedirect = sanitizeRedirectPath(redirectTo, "/");

try {
// call authenticate as usual, in successRedirect use returnTo or a fallback
return await authenticator.authenticate("github", request, {
successRedirect: redirectTo ?? "/",
successRedirect: safeRedirect,
failureRedirect: "/login",
});
} catch (error) {
Expand All @@ -19,8 +22,8 @@ export let action: ActionFunction = async ({ request }) => {
// if the error is a Response and is a redirect
if (error instanceof Response) {
// we need to append a Set-Cookie header with a cookie storing the
// returnTo value
error.headers.append("Set-Cookie", await redirectCookie.serialize(redirectTo));
// returnTo value (store the sanitized path)
error.headers.append("Set-Cookie", await redirectCookie.serialize(safeRedirect));
}
throw error;
}
Expand All @@ -29,4 +32,6 @@ export let action: ActionFunction = async ({ request }) => {
export const redirectCookie = createCookie("redirect-to", {
maxAge: 60 * 60, // 1 hour
httpOnly: true,
sameSite: "lax",
secure: env.NODE_ENV === "production",
});
61 changes: 61 additions & 0 deletions apps/webapp/app/routes/auth.google.callback.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import type { LoaderFunction } from "@remix-run/node";
import { redirect } from "@remix-run/node";
import { prisma } from "~/db.server";
import { getSession, redirectWithErrorMessage } from "~/models/message.server";
import { authenticator } from "~/services/auth.server";
import { setLastAuthMethodHeader } from "~/services/lastAuthMethod.server";
import { commitSession } from "~/services/sessionStorage.server";
import { redirectCookie } from "./auth.google";
import { sanitizeRedirectPath } from "~/utils";

export let loader: LoaderFunction = async ({ request }) => {
const cookie = request.headers.get("Cookie");
const redirectValue = await redirectCookie.parse(cookie);
const redirectTo = sanitizeRedirectPath(redirectValue);

const auth = await authenticator.authenticate("google", request, {
failureRedirect: "/login", // If auth fails, the failureRedirect will be thrown as a Response
});

// manually get the session
const session = await getSession(request.headers.get("cookie"));

const userRecord = await prisma.user.findFirst({
where: {
id: auth.userId,
},
select: {
id: true,
mfaEnabledAt: true,
},
});

if (!userRecord) {
return redirectWithErrorMessage(
"/login",
request,
"Could not find your account. Please contact support."
);
}

if (userRecord.mfaEnabledAt) {
session.set("pending-mfa-user-id", userRecord.id);
session.set("pending-mfa-redirect-to", redirectTo);

const headers = new Headers();
headers.append("Set-Cookie", await commitSession(session));
headers.append("Set-Cookie", await setLastAuthMethodHeader("google"));

return redirect("/login/mfa", { headers });
}

// and store the user data
session.set(authenticator.sessionKey, auth);

const headers = new Headers();
headers.append("Set-Cookie", await commitSession(session));
headers.append("Set-Cookie", await setLastAuthMethodHeader("google"));

return redirect(redirectTo, { headers });
};

38 changes: 38 additions & 0 deletions apps/webapp/app/routes/auth.google.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { type ActionFunction, type LoaderFunction, redirect, createCookie } from "@remix-run/node";
import { authenticator } from "~/services/auth.server";
import { env } from "~/env.server";
import { sanitizeRedirectPath } from "~/utils";

export let loader: LoaderFunction = () => redirect("/login");

export let action: ActionFunction = async ({ request }) => {
const url = new URL(request.url);
const redirectTo = url.searchParams.get("redirectTo");
const safeRedirect = sanitizeRedirectPath(redirectTo, "/");

try {
// call authenticate as usual, in successRedirect use returnTo or a fallback
return await authenticator.authenticate("google", request, {
successRedirect: safeRedirect,
failureRedirect: "/login",
});
} catch (error) {
// here we catch anything authenticator.authenticate throw, this will
// include redirects
// if the error is a Response and is a redirect
if (error instanceof Response) {
// we need to append a Set-Cookie header with a cookie storing the
// returnTo value (store the sanitized path)
error.headers.append("Set-Cookie", await redirectCookie.serialize(safeRedirect));
}
throw error;
}
};

export const redirectCookie = createCookie("google-redirect-to", {
maxAge: 60 * 60, // 1 hour
httpOnly: true,
sameSite: "lax",
secure: env.NODE_ENV === "production",
});

Loading