Browse Source

Sign-in page redirects a signed-in user instead of showing the form (#393)

* Sign-in page sends a signed-in user on instead of showing the form

The sign-up page already checks the session and redirects, so Start free
on the marketing site took a signed-in customer straight into the app
while Login showed them a form they had no use for. Sign-in now does the
same check and honours a safe redirect parameter.

* Sign-up form keeps the redirect when switching to sign-in

The pricing page sends Pro and Enterprise buyers to sign-up with a
redirect to the subscription page. An existing customer who switched to
sign-in from there lost it and landed on the dashboard after signing in.

* E2E re-sign-in drops the session first, and a PR template

The admin-only spec signs up, then signs in again to pick up a role
change. With the sign-in page now sending a signed-in user on, the
helper has to clear cookies before opening the form.
Bernt Christian Egeland 2 weeks ago
parent
commit
7ebf58e712

+ 4 - 0
.github/PULL_REQUEST_TEMPLATE.md

@@ -0,0 +1,4 @@
+## Summary
+<!-- What changed, in a short short descriptoin-->
+## Why
+<!--  in a short short why -->

+ 3 - 0
e2e/specs/security/admin-only.spec.ts

@@ -40,6 +40,9 @@ let organizationId = ''
 let roleId = ''
 let roleId = ''
 
 
 async function signIn(page: Page, email: string, password: string) {
 async function signIn(page: Page, email: string, password: string) {
+  // The sign-in page sends anyone with a session straight on, so a re-sign-in
+  // after a role change has to drop the old session first.
+  await page.context().clearCookies()
   await page.goto('/auth/sign-in')
   await page.goto('/auth/sign-in')
   await page.locator('#email').fill(email)
   await page.locator('#email').fill(email)
   await page.locator('#password').fill(password)
   await page.locator('#password').fill(password)

+ 28 - 7
src/app/(public)/auth/sign-in/page.tsx

@@ -1,4 +1,8 @@
+import { redirect } from 'next/navigation'
+import { headers } from 'next/headers'
+import { safeRedirectPath } from '@/lib/safe-redirect'
 import { db } from '@/lib/db'
 import { db } from '@/lib/db'
+import { auth } from '@/lib/auth'
 import { SignInForm } from './sign-in-form'
 import { SignInForm } from './sign-in-form'
 import { isDemoMode } from '@/lib/demo'
 import { isDemoMode } from '@/lib/demo'
 import { isCloudMode } from '@/lib/features'
 import { isCloudMode } from '@/lib/features'
@@ -6,13 +10,30 @@ import { isGoogleSignInEnabled } from '@/lib/auth-providers'
 
 
 export const dynamic = 'force-dynamic'
 export const dynamic = 'force-dynamic'
 
 
-export default async function SignInPage() {
-  const regSetting = isDemoMode
-    ? null
-    : await db.systemSetting.findUnique({
-        where: { key: 'registration.disabled' },
-        select: { value: true },
-      })
+export default async function SignInPage({
+  searchParams,
+}: {
+  searchParams: Promise<{ redirect?: string }>
+}) {
+  const params = await searchParams
+  const redirectTo = params.redirect ? safeRedirectPath(params.redirect) : undefined
+
+  const [session, regSetting] = await Promise.all([
+    auth.api.getSession({ headers: await headers() }),
+    isDemoMode
+      ? null
+      : db.systemSetting.findUnique({
+          where: { key: 'registration.disabled' },
+          select: { value: true },
+        }),
+  ])
+
+  // Someone who already has a session gets sent on, the same as sign-up does.
+  // Without this the marketing site's Login link lands a signed-in customer
+  // on a form they have no use for, while Start free takes them straight in.
+  if (session?.user?.id) {
+    redirect(redirectTo || '/')
+  }
 
 
   return (
   return (
     <SignInForm
     <SignInForm

+ 7 - 2
src/app/(public)/auth/sign-up/sign-up-form.tsx

@@ -36,6 +36,11 @@ export function SignUpForm({
   const t = useTranslations('auth.signUp')
   const t = useTranslations('auth.signUp')
   const tc = useTranslations('common')
   const tc = useTranslations('common')
   const tSocial = useTranslations('auth.social')
   const tSocial = useTranslations('auth.social')
+  // Someone who came here with a destination keeps it when they switch to
+  // sign-in; the sign-in form does the same in the other direction.
+  const signInHref = `/auth/sign-in${
+    redirectTo ? `?redirect=${encodeURIComponent(safeRedirectPath(redirectTo))}` : ''
+  }`
   const [googleLoading, setGoogleLoading] = useState(false)
   const [googleLoading, setGoogleLoading] = useState(false)
   const [name, setName] = useState('')
   const [name, setName] = useState('')
   const [email, setEmail] = useState('')
   const [email, setEmail] = useState('')
@@ -177,7 +182,7 @@ export function SignUpForm({
               <p>{error}</p>
               <p>{error}</p>
               {emailAlreadyExists && (
               {emailAlreadyExists && (
                 <p className="mt-1 text-xs">
                 <p className="mt-1 text-xs">
-                  <Link href="/auth/sign-in" className="font-medium underline">
+                  <Link href={signInHref} className="font-medium underline">
                     {t('signInInstead')}
                     {t('signInInstead')}
                   </Link>
                   </Link>
                 </p>
                 </p>
@@ -300,7 +305,7 @@ export function SignUpForm({
 
 
         <p className="mt-6 text-center text-sm text-muted-foreground">
         <p className="mt-6 text-center text-sm text-muted-foreground">
           {t('alreadyHaveAccount')}{' '}
           {t('alreadyHaveAccount')}{' '}
-          <Link href="/auth/sign-in" className="font-medium text-primary hover:underline">
+          <Link href={signInHref} className="font-medium text-primary hover:underline">
             {tc('buttons.signIn')}
             {tc('buttons.signIn')}
           </Link>
           </Link>
         </p>
         </p>