From dfff93e046217dcc15025d8734aaaf93287ee3eb Mon Sep 17 00:00:00 2001 From: Theo Sanderson Date: Thu, 22 Feb 2024 19:33:38 +0000 Subject: [PATCH] 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 --- .../src/components/common/NeedToLogin.astro | 24 ++++++++++++++ website/src/middleware/authMiddleware.ts | 32 +++++++++++-------- .../src/pages/[organism]/revise/index.astro | 5 ++- .../src/pages/[organism]/submit/index.astro | 6 ++-- .../src/pages/[organism]/submit/review.astro | 18 ++++++++--- .../shouldMiddlewareEnforceLogin.spec.ts | 2 -- .../src/utils/shouldMiddlewareEnforceLogin.ts | 3 +- website/tests/pages/submit/index.spec.ts | 8 +++++ website/tests/pages/submit/submit.page.ts | 2 ++ 9 files changed, 75 insertions(+), 25 deletions(-) create mode 100644 website/src/components/common/NeedToLogin.astro diff --git a/website/src/components/common/NeedToLogin.astro b/website/src/components/common/NeedToLogin.astro new file mode 100644 index 000000000..1677c420b --- /dev/null +++ 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.'; +} +--- + +
+ +
+

{message}

+ Log in +

+
+
diff --git a/website/src/middleware/authMiddleware.ts b/website/src/middleware/authMiddleware.ts index 8c72a6e3b..73aafa20b 100644 --- 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 index a20250d09..ca249ec52 100644 --- 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,7 +17,7 @@ const clientConfig = getRuntimeConfig().public;

Revise sequences

{ - groupsResult.isOk() ? ( + groupsResult.isOk() && accessToken ? ( + ) : !accessToken ? ( + ) : (

Failed to load groups

) diff --git a/website/src/pages/[organism]/submit/index.astro b/website/src/pages/[organism]/submit/index.astro index 987ba2650..5eefe6a59 100644 --- 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');

Submit sequences

{ - groupsResult.isOk() ? ( + !accessToken ? ( + + ) : groupsResult.isOk() ? ( -

Current submissions

- - +{ + accessToken ? ( + +

Current submissions

+ + + ) : ( + + + + ) +} diff --git a/website/src/utils/shouldMiddlewareEnforceLogin.spec.ts b/website/src/utils/shouldMiddlewareEnforceLogin.spec.ts index 337077ecd..5ae5838e9 100644 --- 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 index f7fa52926..468dc616c 100644 --- 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 index dd5c94dfb..47e86e750 100644 --- 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 index ef8c3a6b9..174ce83b0 100644 --- 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() {