Update authentication-backend, prevent open redirect attacks

davidje13 · davidje13 · commit a83e9481e996 · 2025-11-21T13:09:03.000Z
diff --git a/backend/.npmrc b/backend/.npmrc
@@ -0,0 +1,2 @@
+package-lock=true
+ignore-scripts=true
diff --git a/backend/package-lock.json b/backend/package-lock.json
diff --git a/backend/package.json b/backend/package.json
@@ -10,7 +10,7 @@
     "test": "lean-test --preprocess tsc --parallel"
   },
   "dependencies": {
-    "authentication-backend": "1.x",
+    "authentication-backend": "1.3.x",
     "collection-storage": "3.x",
     "express": "5.x",
     "express-static-gzip": "3.x",
diff --git a/backend/src/api-tests/testConfig.ts b/backend/src/api-tests/testConfig.ts
@@ -40,7 +40,7 @@ const baseTestConfig: ConfigT = {
       accessTokenUrl: '',
       userUrl: '',
     },
-    gitlab: { clientId: '', authUrl: '', tokenInfoUrl: '' },
+    gitlab: { clientId: '', authUrl: '', accessTokenUrl: '', tokenInfoUrl: '' },
   },
   giphy: { baseUrl: '', apiKey: '' },
 };
diff --git a/backend/src/auth.ts b/backend/src/auth.ts
@@ -35,7 +35,7 @@ export function getAuthBackend(
     userAuthService.grantLoginToken,
   );
   return {
-    addRoutes: (app) => app.useHTTP('/api/sso', sso.router),
+    addRoutes: (app) => app.useHTTP('/api/sso', sso.router()),
     clientConfig: sso.service.clientConfig,
   };
 }
diff --git a/backend/src/config/default.ts b/backend/src/config/default.ts
@@ -52,6 +52,7 @@ export default {
     gitlab: {
       clientId: '',
       authUrl: 'https://gitlab.com/oauth/authorize',
+      accessTokenUrl: 'https://gitlab.com/oauth/token',
       tokenInfoUrl: 'https://gitlab.com/oauth/token/info',
     },
   },
diff --git a/docs/SERVICES.md b/docs/SERVICES.md
@@ -116,12 +116,12 @@ SSO_GITHUB_CLIENT_ID="idhere" SSO_GITHUB_CLIENT_SECRET="secrethere" ./index.js
 
 You will need a GitLab client ID:
 
-1. Go to <https://gitlab.com/profile/applications>
+1. Go to <https://gitlab.com/-/user_settings/applications>
 2. Set the "Redirect URI" to match your deployment with
    `/sso/gitlab` appended to the end. e.g. for local
    testing, this could be `http://localhost:5000/sso/gitlab`
-3. Untick the "confidential" option. You do not need to enable
-   any scopes.
+3. Untick the "confidential" option and select the "email"
+   scope (this is the closest we can get to no scopes)
 4. Record the application ID (you will not need the secret).
 
 You can now invoke the application with the `SSO_GITLAB_CLIENT_ID`
@@ -137,6 +137,7 @@ the auth and token info URLs:
 
 ```sh
 SSO_GITLAB_AUTH_URL="https://gitlab.example.com/oauth/authorize" \
+SSO_GITLAB_ACCESS_TOKEN_URL="https://gitlab.example.com/oauth/token" \
 SSO_GITLAB_TOKEN_INFO_URL="https://gitlab.example.com/oauth/token/info" \
 SSO_GITLAB_CLIENT_ID="idhere" \
 ./index.js
diff --git a/frontend/src/api/PasswordService.ts b/frontend/src/api/PasswordService.ts
@@ -1,4 +1,4 @@
-import { sha1 } from '../helpers/crypto';
+import { digest, toHex } from '../helpers/crypto';
 
 export class PasswordService {
   public constructor(private readonly apiBase: string) {}
@@ -7,7 +7,7 @@ export class PasswordService {
     password: string,
     signal: AbortSignal,
   ): Promise<number> {
-    const passwordHash = (await sha1(password)).toUpperCase();
+    const passwordHash = toHex(await digest(password, 'SHA-1')).toUpperCase();
     const key = passwordHash.substring(0, 5);
     const rest = passwordHash.substring(5);
 
diff --git a/frontend/src/api/UserTokenService.ts b/frontend/src/api/UserTokenService.ts
@@ -6,6 +6,8 @@ export class UserTokenService {
   public async login(
     service: string,
     externalToken: string,
+    redirectUri: string,
+    codeVerifier: string | undefined,
     signal: AbortSignal,
   ): Promise<string> {
     const body = await jsonFetch<{ userToken: string }>(
@@ -14,7 +16,7 @@ export class UserTokenService {
         method: 'POST',
         cache: 'no-cache',
         headers: { 'Content-Type': 'application/json' },
-        body: JSON.stringify({ externalToken }),
+        body: JSON.stringify({ externalToken, redirectUri, codeVerifier }),
         signal,
       },
     );
diff --git a/frontend/src/components/login/LoginCallback.tsx b/frontend/src/components/login/LoginCallback.tsx
@@ -19,27 +19,35 @@ export const LoginCallback = memo(({ service }: PropsT) => {
   useEffect(() => {
     const ac = new AbortController();
     const { search, hash } = document.location;
-    const nonce = storage.getItem('login-nonce');
-    handleLogin(service, nonce, { search, hash }, ac.signal)
+    const redirectUri = document.location.href.split('?')[0]!;
+    const localState = storage.getItem('login-state');
+    handleLogin(service, localState, { search, hash, redirectUri }, ac.signal)
       .then((redirect) => {
         if (ac.signal.aborted) {
           return;
         }
-        storage.removeItem('login-nonce');
+        storage.removeItem('login-state');
+        if (
+          new URL(redirect, document.location.href).host !==
+          document.location.host
+        ) {
+          // possibly a malicious redirect - ignore it and substitute a safe one
+          redirect = '/';
+        }
         stableSetLocation(redirect, { replace: true });
       })
       .catch((err) => {
         if (ac.signal.aborted) {
           return;
         }
         if (!(err instanceof Error)) {
-          storage.removeItem('login-nonce');
+          storage.removeItem('login-state');
           setError(String(err));
         } else if (err.message === 'unrecognised login details') {
           // GitLab shows a bare link to the /sso/login URL on the confirmation page
           stableSetLocation('/', { replace: true });
         } else {
-          storage.removeItem('login-nonce');
+          storage.removeItem('login-state');
           setError(err.message);
         }
       });
diff --git a/frontend/src/components/login/LoginForm.tsx b/frontend/src/components/login/LoginForm.tsx
@@ -1,18 +1,16 @@
 import { memo, useLayoutEffect } from 'react';
 import { useConfig } from '../../hooks/data/useConfig';
-import { toHex, randomBytes } from '../../helpers/crypto';
+import { digest, randomBytes, toB64 } from '../../helpers/crypto';
 import { storage } from './storage';
 import './LoginForm.css';
 
-function makeState(redirect: string): string {
-  const nonce = toHex(randomBytes(10));
-  if (!storage.setItem('login-nonce', nonce)) {
+function storeState(state: unknown) {
+  if (!storage.setItem('login-state', JSON.stringify(state))) {
     window.alert(
       'You must enable session cookies or storage in your browser settings to log in',
     );
     throw new Error('insufficient browser permissions to log in');
   }
-  return JSON.stringify({ nonce, redirect });
 }
 
 interface PropsT {
@@ -34,10 +32,15 @@ export const LoginForm = memo(({ message, redirect }: PropsT) => {
 
   useLayoutEffect(() => {
     if (publicConfig) {
+      const nonce = toB64(randomBytes(15));
+      storeState({ nonce });
       const targetUrl = new URL('/sso/public', domain);
       const url = new URL(publicConfig.authUrl, document.location.href);
       url.searchParams.set('redirect_uri', targetUrl.toString());
-      url.searchParams.set('state', makeState(resolvedRedirect));
+      url.searchParams.set(
+        'state',
+        JSON.stringify({ nonce, redirect: resolvedRedirect }),
+      );
       document.location.href = url.toString();
     }
   }, [publicConfig]);
@@ -51,10 +54,15 @@ export const LoginForm = memo(({ message, redirect }: PropsT) => {
             type="button"
             className="global-button primary sso-google"
             onClick={() => {
+              const nonce = toB64(randomBytes(15));
+              storeState({ nonce });
               const targetUrl = new URL('/sso/google', domain);
               const url = new URL(googleConfig.authUrl);
               url.searchParams.set('redirect_uri', targetUrl.toString());
-              url.searchParams.set('state', makeState(resolvedRedirect));
+              url.searchParams.set(
+                'state',
+                JSON.stringify({ nonce, redirect: resolvedRedirect }),
+              );
               url.searchParams.set('response_type', 'id_token');
               url.searchParams.set('scope', 'openid');
               url.searchParams.set('client_id', googleConfig.clientId);
@@ -71,10 +79,15 @@ export const LoginForm = memo(({ message, redirect }: PropsT) => {
             type="button"
             className="global-button primary sso-github"
             onClick={() => {
+              const nonce = toB64(randomBytes(15));
+              storeState({ nonce });
               const targetUrl = new URL('/sso/github', domain);
               const url = new URL(githubConfig.authUrl);
               url.searchParams.set('redirect_uri', targetUrl.toString());
-              url.searchParams.set('state', makeState(resolvedRedirect));
+              url.searchParams.set(
+                'state',
+                JSON.stringify({ nonce, redirect: resolvedRedirect }),
+              );
               url.searchParams.set('scope', '');
               url.searchParams.set('client_id', githubConfig.clientId);
               document.location.href = url.toString();
@@ -87,13 +100,24 @@ export const LoginForm = memo(({ message, redirect }: PropsT) => {
           <button
             type="button"
             className="global-button primary sso-gitlab"
-            onClick={() => {
+            onClick={async () => {
+              const nonce = toB64(randomBytes(15));
+              const codeVerifier = toB64(randomBytes(36));
+              const codeChallenge = toB64(
+                await digest(codeVerifier, 'SHA-256'),
+              );
+              storeState({ nonce, codeVerifier });
               const targetUrl = new URL('/sso/gitlab', domain);
               const url = new URL(gitlabConfig.authUrl);
               url.searchParams.set('redirect_uri', targetUrl.toString());
-              url.searchParams.set('state', makeState(resolvedRedirect));
-              url.searchParams.set('response_type', 'token');
-              url.searchParams.set('scope', 'openid');
+              url.searchParams.set(
+                'state',
+                JSON.stringify({ nonce, redirect: resolvedRedirect }),
+              );
+              url.searchParams.set('response_type', 'code');
+              url.searchParams.set('code_challenge', codeChallenge);
+              url.searchParams.set('code_challenge_method', 'S256');
+              url.searchParams.set('scope', 'email');
               url.searchParams.set('client_id', gitlabConfig.clientId);
               document.location.href = url.toString();
             }}
diff --git a/frontend/src/components/login/handleLogin.ts b/frontend/src/components/login/handleLogin.ts
@@ -3,12 +3,13 @@ import { userTokenService, userTokenTracker } from '../../api/api';
 interface LocationT {
   search: string;
   hash: string;
+  redirectUri: string;
 }
 
 export async function handleLogin(
   service: string,
-  localNonce: string | null,
-  { search, hash }: LocationT,
+  localState: string | null,
+  { search, hash, redirectUri }: LocationT,
   signal: AbortSignal,
 ): Promise<string> {
   let externalToken: string | null = null;
@@ -27,30 +28,33 @@ export async function handleLogin(
     externalToken = searchParams.get('code');
     state = searchParams.get('state');
   } else if (service === 'gitlab') {
-    externalToken = hashParams.get('access_token');
-    state = hashParams.get('state');
+    externalToken = searchParams.get('code');
+    state = searchParams.get('state');
   }
 
   if (!externalToken || state === null) {
     throw new Error('unrecognised login details');
   }
 
-  const { redirect, nonce } = JSON.parse(state);
-  if (!localNonce) {
+  if (!localState) {
     // nonce checking ensures the token we receive is actually the result of our initial request
     // (i.e. an attacker has not invoked the redirect URI with their own token so that the user
     // is logged in as the wrong account). If we cannot validate this, the login cannot continue.
     throw new Error(
       'possible cross-site request forgery (you may need to enable local storage)',
     );
   }
-  if (nonce !== localNonce) {
+  const { redirect, nonce } = JSON.parse(state);
+  const local = JSON.parse(localState);
+  if (nonce !== local.nonce) {
     throw new Error('possible cross-site request forgery');
   }
 
   const userToken = await userTokenService.login(
     service,
     externalToken,
+    redirectUri,
+    local.codeVerifier,
     signal,
   );
   userTokenTracker.set(userToken);
diff --git a/frontend/src/helpers/crypto.ts b/frontend/src/helpers/crypto.ts
@@ -8,8 +8,33 @@ export function toHex(array: Uint8Array): string {
   return [...array].map((v) => v.toString(16).padStart(2, '0')).join('');
 }
 
-export async function sha1(data: string): Promise<string> {
+export function toB64(array: Uint8Array): string {
+  if (array.toBase64) {
+    return array.toBase64({ alphabet: 'base64url', omitPadding: true });
+  } else {
+    // fallback toBase64 for old browsers
+    return btoa(String.fromCharCode(...array))
+      .replaceAll('+', '-')
+      .replaceAll('/', '_')
+      .replaceAll('=', '');
+  }
+}
+
+export async function digest(
+  data: string,
+  algorithm: AlgorithmIdentifier,
+): Promise<Uint8Array> {
   const bytes = new TextEncoder().encode(data);
-  const hashBytes = await window.crypto.subtle.digest('SHA-1', bytes);
-  return toHex(new Uint8Array(hashBytes));
+  const hashBytes = await window.crypto.subtle.digest(algorithm, bytes);
+  return new Uint8Array(hashBytes);
+}
+
+declare global {
+  interface Uint8Array {
+    // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Uint8Array/toBase64
+    toBase64?: (options?: {
+      alphabet?: 'base64' | 'base64url';
+      omitPadding?: boolean;
+    }) => string;
+  }
 }

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+package-lock=true`
	`2`	`+ignore-scripts=true`
Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,7 @@ export function getAuthBackend(`
`35`	`35`	`userAuthService.grantLoginToken,`
`36`	`36`	`);`
`37`	`37`	`return {`
`38`		`- addRoutes: (app) => app.useHTTP('/api/sso', sso.router),`
	`38`	`+ addRoutes: (app) => app.useHTTP('/api/sso', sso.router()),`
`39`	`39`	`clientConfig: sso.service.clientConfig,`
`40`	`40`	`};`
`41`	`41`	`}`