Instead of forcing login, display a message offering users the chance…

… to login (#1082) * failing * updates * unexport * format * update * remove tests we no longer want * testing * Update index.spec.ts * update test * updates from code review * update * format * a space * fix test
loculus-project · Feb 22, 2024 · dfff93e · dfff93e
1 parent d12664d
commit dfff93e
Show file tree

Hide file tree

Showing 9 changed files with 75 additions and 25 deletions.
diff --git a/website/src/components/common/NeedToLogin.astro b/website/src/components/common/NeedToLogin.astro
@@ -0,0 +1,24 @@
+---
+import { getAuthUrl } from '../../middleware/authMiddleware';
+import IcOutlineLogin from '~icons/ic/outline-login';
+
+const loginUrl = await getAuthUrl(Astro.url.toString());
+
+interface Props {
+    message?: string;
+}
+
+let { message } = Astro.props;
+if (message === undefined) {
+    message = 'You need to log in to access this page.';
+}
+---
+
+<div class='mt-6 alert max-w-4xl mx-auto'>
+    <IcOutlineLogin className='w-12 h-12 inline-block mr-2' />
+    <div>
+        <p>{message}</p>
+        <a href={loginUrl} class='btn mt-3 bg-white hover:bg-gray-50'>Log in</a>
+        <p class='mt-3'></p>
+    </div>
+</div>
diff --git a/website/src/middleware/authMiddleware.ts b/website/src/middleware/authMiddleware.ts
@@ -48,8 +48,24 @@ export async function getKeycloakClient() {
     return _keycloakClient;
 }
 
+export const getAuthUrl = async (redirectUrl: string) => {
+    const authUrl = (await getKeycloakClient()).authorizationUrl({
+        redirect_uri: redirectUrl,
+        scope: 'openid',
+        response_type: 'code',
+    });
+    return authUrl;
+};
+
 export const authMiddleware = defineMiddleware(async (context, next) => {
     let token = await getTokenFromCookie(context);
+    if (token === undefined) {
+        token = await getTokenFromParams(context);
+        if (token !== undefined) {
+            logger.debug(`Token found in params, setting cookie`);
+            setCookie(context, token);
+        }
+    }
 
     const enforceLogin = shouldMiddlewareEnforceLogin(
         context.url.pathname,
@@ -88,14 +104,8 @@ export const authMiddleware = defineMiddleware(async (context, next) => {
     }
 
     if (token === undefined) {
-        token = await getTokenFromParams(context);
-        if (token !== undefined) {
-            logger.debug(`Token found in params, setting cookie`);
-            setCookie(context, token);
-        } else {
-            logger.debug(`No token found, redirecting to auth`);
-            return redirectToAuth(context);
-        }
+        logger.debug(`No token found, redirecting to auth`);
+        return redirectToAuth(context);
     }
 
     const userInfo = await getUserInfo(token);
@@ -255,11 +265,7 @@ const redirectToAuth = async (context: APIContext) => {
     const redirectUrl = removeTokenCodeFromSearchParams(currentUrl);
 
     logger.debug(`Redirecting to auth with redirect url: ${redirectUrl}`);
-    const authUrl = (await getKeycloakClient()).authorizationUrl({
-        redirect_uri: redirectUrl,
-        scope: 'openid',
-        response_type: 'code',
-    });
+    const authUrl = await getAuthUrl(redirectUrl);
 
     deleteCookie(context);
     return createRedirectWithModifiableHeaders(authUrl);

diff --git a/website/src/pages/[organism]/revise/index.astro b/website/src/pages/[organism]/revise/index.astro
@@ -1,5 +1,6 @@
 ---
 import { RevisionForm } from '../../../components/Submission/RevisionForm';
+import NeedToLogin from '../../../components/common/NeedToLogin.astro';
 import { getRuntimeConfig } from '../../../config';
 import BaseLayout from '../../../layouts/BaseLayout.astro';
 import { GroupManagementClient } from '../../../services/groupManagementClient';
@@ -16,14 +17,16 @@ const clientConfig = getRuntimeConfig().public;
 <BaseLayout title='Submit'>
     <h1 class='title'>Revise sequences</h1>
     {
-        groupsResult.isOk() ? (
+        groupsResult.isOk() && accessToken ? (
             <RevisionForm
                 accessToken={accessToken}
                 organism={organism}
                 clientConfig={clientConfig}
                 groupsOfUser={groupsResult.value}
                 client:load
             />
+        ) : !accessToken ? (
+            <NeedToLogin />
         ) : (
             <p class='text-red-500'>Failed to load groups</p>
         )

diff --git a/website/src/pages/[organism]/submit/index.astro b/website/src/pages/[organism]/submit/index.astro
@@ -1,10 +1,10 @@
 ---
 import { SubmissionForm } from '../../../components/Submission/SubmissionForm';
+import NeedToLogin from '../../../components/common/NeedToLogin.astro';
 import { getRuntimeConfig } from '../../../config';
 import BaseLayout from '../../../layouts/BaseLayout.astro';
 import { GroupManagementClient } from '../../../services/groupManagementClient';
 import { getAccessToken } from '../../../utils/getAccessToken';
-
 const organism = Astro.params.organism!;
 const accessToken = getAccessToken(Astro.locals.session)!;
 
@@ -21,7 +21,9 @@ Astro.response.headers.append('Expires', '0');
 <BaseLayout title='Submit'>
     <h1 class='title'>Submit sequences</h1>
     {
-        groupsResult.isOk() ? (
+        !accessToken ? (
+            <NeedToLogin message='You need to be logged in to submit sequences.' />
+        ) : groupsResult.isOk() ? (
             <SubmissionForm
                 accessToken={accessToken}
                 organism={organism}

diff --git a/website/src/pages/[organism]/submit/review.astro b/website/src/pages/[organism]/submit/review.astro
@@ -1,16 +1,24 @@
 ---
 import { ReviewPage } from '../../../components/ReviewPage/ReviewPage';
+import NeedToLogin from '../../../components/common/NeedToLogin.astro';
 import { getRuntimeConfig } from '../../../config';
 import BaseLayout from '../../../layouts/BaseLayout.astro';
 import { type ClientConfig } from '../../../types/runtimeConfig';
 import { getAccessToken } from '../../../utils/getAccessToken';
-
 const organism = Astro.params.organism!;
 const accessToken = getAccessToken(Astro.locals.session)!;
 const clientConfig: ClientConfig = getRuntimeConfig().public;
 ---
 
-<BaseLayout title='Current submissions'>
-    <h1 class='title'>Current submissions</h1>
-    <ReviewPage clientConfig={clientConfig} organism={organism} accessToken={accessToken} client:load />
-</BaseLayout>
+{
+    accessToken ? (
+        <BaseLayout title='Current submissions'>
+            <h1 class='title'>Current submissions</h1>
+            <ReviewPage clientConfig={clientConfig} organism={organism} accessToken={accessToken} client:load />
+        </BaseLayout>
+    ) : (
+        <BaseLayout title='Current submissions'>
+            <NeedToLogin />
+        </BaseLayout>
+    )
+}
diff --git a/website/src/utils/shouldMiddlewareEnforceLogin.spec.ts b/website/src/utils/shouldMiddlewareEnforceLogin.spec.ts
@@ -19,8 +19,6 @@ describe('shouldMiddlewareEnforceLogin', () => {
     test('should return true on routes which should force login', () => {
         expectForceLogin('/user');
         expectForceLogin('/user/someUsername');
-        expectForceLogin(`/${testOrganism}/revise`);
-        expectForceLogin(`/${testOrganism}/submit`);
     });
 
     test('should return false for various public routes', () => {

diff --git a/website/src/utils/shouldMiddlewareEnforceLogin.ts b/website/src/utils/shouldMiddlewareEnforceLogin.ts
@@ -4,9 +4,8 @@ function getEnforcedLoginRoutes(configuredOrganisms: string[]) {
     const cacheKey = configuredOrganisms.join('');
     if (!(cacheKey in enforcedLoginRoutesCache)) {
         const organismSpecificRoutes = configuredOrganisms.flatMap((organism) => [
-            new RegExp(`^/${organism}/revise`),
-            new RegExp(`^/${organism}/submit`),
             new RegExp(`^/${organism}/user`),
+            new RegExp(`^/${organism}/my_sequences`),
         ]);
 
         enforcedLoginRoutesCache[cacheKey] = [

diff --git a/website/tests/pages/submit/index.spec.ts b/website/tests/pages/submit/index.spec.ts
@@ -4,6 +4,14 @@ import { dateTimeInMonths } from '../../../src/utils/DateTimeInMonths.tsx';
 import { baseUrl, dummyOrganism, expect, test, testSequenceCount } from '../../e2e.fixture';
 
 test.describe('The submit page', () => {
+    test('should ask to login if not logged in', async ({ submitPage }) => {
+        await submitPage.goto();
+
+        await submitPage.loginButton.click();
+
+        await expect(submitPage.page.url()).toContain('loculusRealm');
+    });
+
     test('should upload files and submit', async ({ submitPage, loginAsTestUser }) => {
         await loginAsTestUser();
         await submitPage.goto();

diff --git a/website/tests/pages/submit/submit.page.ts b/website/tests/pages/submit/submit.page.ts
@@ -15,10 +15,12 @@ import { expect } from '../../e2e.fixture.ts';
 export class SubmitPage {
     public readonly submitButton: Locator;
     public readonly dataUseTermsDropdown: Locator;
+    public readonly loginButton: Locator;
 
     constructor(public readonly page: Page) {
         this.submitButton = page.getByRole('button', { name: 'submit' });
         this.dataUseTermsDropdown = page.locator('#dataUseTermsDropdown');
+        this.loginButton = page.locator('a', { hasText: 'Log in' });
     }
 
     public async goto() {