Merge pull request #6038 from espoon-voltti/remove-csrf-tokens

Poistetaan tilallinen CSRF-tokenien käyttö ja luotetaan headerin olemassaoloon OWASP-suosituksen mukaisesti

apigw/src/enduser/__tests__/csrf.ts

@@ -3,19 +3,18 @@
 // SPDX-License-Identifier: LGPL-2.1-or-later
 
 import {
-  describe,
-  beforeAll,
-  afterEach,
   afterAll,
+  afterEach,
+  beforeAll,
   beforeEach,
+  describe,
   expect,
   it
 } from '@jest/globals'
 
 import { configFromEnv } from '../../shared/config.js'
 import { CitizenUser } from '../../shared/service-client.js'
 import { GatewayTester } from '../../shared/test/gateway-tester.js'
-import { AuthStatus } from '../routes/auth-status.js'
 
 const mockUser: CitizenUser = {
   id: '4f73e4f8-8759-46c6-9b9d-4da860138ce2'
@@ -30,16 +29,6 @@ describe('CSRF middleware and cookie handling in enduser-gw', () => {
   afterEach(async () => tester.afterEach())
   afterAll(async () => tester?.stop())
 
-  async function setupAntiCsrfToken() {
-    tester.nockScope.get(`/system/citizen/${mockUser.id}`).reply(200, mockUser)
-    const response = await tester.client.get('/api/application/auth/status')
-    const authStatus = response.data as AuthStatus
-    tester.nockScope.done()
-
-    expect(authStatus.antiCsrfToken).toBeTruthy()
-    tester.antiCsrfToken = authStatus.antiCsrfToken
-  }
-
   it('should fail a POST to a proxied API when there is no CSRF token', async () => {
     const res = await tester.client.post(
       '/api/application/citizen/applications',
@@ -56,27 +45,13 @@ describe('CSRF middleware and cookie handling in enduser-gw', () => {
     tester.nockScope.done()
     expect(res.status).toBe(200)
   })
-  it('should pass a POST to a proxied API after CSRF token has been set up by the auth/status endpoint', async () => {
-    await setupAntiCsrfToken()
-
+  it('should pass POST to a proxied API when CSRF header is present', async () => {
+    tester.setCsrfHeader = true
     tester.nockScope.post('/citizen/applications').reply(200)
     const res = await tester.client.post(
       '/api/application/citizen/applications'
     )
     tester.nockScope.done()
     expect(res.status).toBe(200)
   })
-  it('should not check CSRF if a session is not available', async () => {
-    await setupAntiCsrfToken()
-    await tester.expireSession()
-
-    const res = await tester.client.post(
-      '/api/application/citizen/applications',
-      undefined,
-      {
-        validateStatus: () => true
-      }
-    )
-    expect(res.status).toBe(401)
-  })
 })

apigw/src/enduser/routes/auth-status.ts

@@ -12,7 +12,6 @@ import {
 
 export interface AuthStatus {
   loggedIn: boolean
-  antiCsrfToken?: string
   user?: CitizenUserResponse
   apiVersion: string
   authLevel?: 'STRONG' | 'WEAK'
@@ -35,7 +34,6 @@ export default toRequestHandler(async (req, res) => {
     const data = await getCitizenDetails(req, req.user.id)
     status = {
       loggedIn: true,
-      antiCsrfToken: req.session.antiCsrfToken,
       user: data,
       apiVersion: appCommit,
       authLevel: getAuthLevel(req.user)

apigw/src/internal/__tests__/csrf.ts

@@ -3,19 +3,18 @@
 // SPDX-License-Identifier: LGPL-2.1-or-later
 
 import {
-  describe,
-  beforeAll,
-  afterEach,
   afterAll,
+  afterEach,
+  beforeAll,
   beforeEach,
+  describe,
   expect,
   it
 } from '@jest/globals'
 
 import { configFromEnv } from '../../shared/config.js'
 import { EmployeeUser } from '../../shared/service-client.js'
 import { GatewayTester } from '../../shared/test/gateway-tester.js'
-import { AuthStatus } from '../routes/auth-status.js'
 
 const mockUser: EmployeeUser = {
   id: '8fc11215-6d55-4059-bd59-038bfa36f294',
@@ -34,17 +33,7 @@ describe('CSRF middleware and cookie handling in internal-gw', () => {
   afterEach(async () => tester.afterEach())
   afterAll(async () => tester?.stop())
 
-  async function setupAntiCsrfToken() {
-    tester.nockScope.get(`/system/employee/${mockUser.id}`).reply(200, mockUser)
-    const response = await tester.client.get('/api/internal/auth/status')
-    const authStatus = response.data as AuthStatus
-    tester.nockScope.done()
-
-    expect(authStatus.antiCsrfToken).toBeTruthy()
-    tester.antiCsrfToken = authStatus.antiCsrfToken
-  }
-
-  it('should fail POST to a proxied API when there is no CSRF token', async () => {
+  it('should fail POST to a proxied API when there is no CSRF header', async () => {
     const res = await tester.client.post(
       '/api/internal/employee/some-proxied-api',
       undefined,
@@ -62,27 +51,13 @@ describe('CSRF middleware and cookie handling in internal-gw', () => {
     tester.nockScope.done()
     expect(res.status).toBe(200)
   })
-  it('should pass POST to a proxied API after CSRF token has been set up by the auth/status endpoint', async () => {
-    await setupAntiCsrfToken()
-
+  it('should pass POST to a proxied API when CSRF header is present', async () => {
+    tester.setCsrfHeader = true
     tester.nockScope.post('/employee/some-proxied-api').reply(200)
     const res = await tester.client.post(
       '/api/internal/employee/some-proxied-api'
     )
     tester.nockScope.done()
     expect(res.status).toBe(200)
   })
-  it('should not check CSRF if a session is not available', async () => {
-    await setupAntiCsrfToken()
-    await tester.expireSession()
-
-    const res = await tester.client.post(
-      '/api/internal/employee/some-proxied-api',
-      undefined,
-      {
-        validateStatus: () => true
-      }
-    )
-    expect(res.status).toBe(401)
-  })
 })

apigw/src/internal/routes/auth-status.ts

@@ -18,7 +18,6 @@ import { Sessions } from '../../shared/session.js'
 
 export interface AuthStatus {
   loggedIn: boolean
-  antiCsrfToken?: string
   user?: EmployeeUser | MobileUser
   roles?: string[]
   globalRoles?: string[]
@@ -147,7 +146,6 @@ export default (sessions: Sessions) =>
       }
       status = {
         loggedIn: true,
-        antiCsrfToken: req.session.antiCsrfToken,
         user,
         globalRoles,
         allScopedRoles,

apigw/src/shared/auth/index.ts

@@ -2,8 +2,6 @@
 //
 // SPDX-License-Identifier: LGPL-2.1-or-later
 
-import { randomBytes } from 'node:crypto'
-
 import { Profile } from '@node-saml/node-saml'
 import express, { NextFunction, Request, Response } from 'express'
 
@@ -79,10 +77,6 @@ export const login = async (
   await fromCallback<void>((cb) => req.logIn(user, cb))
   // Passport has now regenerated the active session and saved it, so we have a
   // guarantee that the session ID has changed and Redis has stored the new session data
-
-  req.session.antiCsrfToken = (
-    await fromCallback<Buffer>((cb) => randomBytes(32, cb))
-  ).toString('base64')
 }
 
 export const logout = async (

apigw/src/shared/express.ts

@@ -56,7 +56,6 @@ export class InvalidRequest extends BaseError {}
 // Use TS interface merging to add fields to express req.session
 declare module 'express-session' {
   interface SessionData {
-    antiCsrfToken?: string
     idpProvider?: string | null
     logoutToken?: LogoutToken
     employeeIdToken?: string

apigw/src/shared/test/gateway-tester.ts

@@ -28,7 +28,7 @@ export class GatewayTester {
   public readonly nockScope: nock.Scope
 
   private readonly baseUrl: string
-  public antiCsrfToken: string | undefined
+  public setCsrfHeader = false
 
   private constructor(
     private readonly server: http.Server,
@@ -44,8 +44,8 @@ export class GatewayTester {
       includeCookiesInRequest(this.baseUrl, this.cookies, config)
     )
     this.client.interceptors.request.use((config) => {
-      if (this.antiCsrfToken) {
-        config.headers.set('x-evaka-csrf', this.antiCsrfToken)
+      if (this.setCsrfHeader) {
+        config.headers.set('x-evaka-csrf', '1')
       }
       return config
     })
@@ -87,7 +87,7 @@ export class GatewayTester {
   public async afterEach(): Promise<void> {
     nock.cleanAll()
     await this.cookies.removeAllCookies()
-    delete this.antiCsrfToken
+    this.setCsrfHeader = false
   }
 
   public async stop(): Promise<void> {

frontend/src/citizen-frontend/api-client.ts

@@ -13,12 +13,6 @@ export const client = axios.create({
 })
 client.defaults.headers.common['x-evaka-csrf'] = '1'
 
-export const setAntiCsrfToken = (value: string | undefined) => {
-  for (const method of ['delete', 'patch', 'post', 'put'] as const) {
-    client.defaults.headers[method]['x-evaka-csrf'] = value
-  }
-}
-
 if (isAutomatedTest) {
   client.interceptors.request.use((config) => {
     const mockedTime =

frontend/src/citizen-frontend/auth/api.ts

@@ -12,7 +12,6 @@ export type AuthStatus =
   | { loggedIn: false; apiVersion: string }
   | {
       loggedIn: true
-      antiCsrfToken: string
       user: CitizenUserResponse
       apiVersion: string
       authLevel: CitizenAuthLevel

frontend/src/citizen-frontend/auth/state.tsx

@@ -19,8 +19,6 @@ import {
 } from 'lib-common/generated/api-types/shared'
 import { query, useQueryResult } from 'lib-common/query'
 
-import { setAntiCsrfToken } from '../api-client'
-
 import { getAuthStatus } from './api'
 
 export interface User extends CitizenUserDetails {
@@ -43,11 +41,7 @@ const defaultState: AuthState = {
 export const AuthContext = createContext<AuthState>(defaultState)
 
 const authStatusQuery = query({
-  api: () =>
-    getAuthStatus().then((status) => {
-      setAntiCsrfToken(status.loggedIn ? status.antiCsrfToken : undefined)
-      return status
-    }),
+  api: () => getAuthStatus(),
   queryKey: () => ['auth-status']
 })
 

frontend/src/employee-frontend/api/client.ts

@@ -13,12 +13,6 @@ export const client = axios.create({
 })
 client.defaults.headers.common['x-evaka-csrf'] = '1'
 
-export const setAntiCsrfToken = (value: string | undefined) => {
-  for (const method of ['delete', 'patch', 'post', 'put'] as const) {
-    client.defaults.headers[method]['x-evaka-csrf'] = value
-  }
-}
-
 if (isAutomatedTest) {
   client.interceptors.request.use((config) => {
     const evakaMockedTime =

frontend/src/employee-frontend/state/user.tsx

@@ -4,7 +4,6 @@
 
 import React, { createContext, useCallback, useMemo } from 'react'
 
-import { setAntiCsrfToken } from 'employee-frontend/api/client'
 import { AdRole, User } from 'lib-common/api-types/employee-auth'
 import { query, useQuery } from 'lib-common/query'
 
@@ -29,11 +28,7 @@ export const UserContext = createContext<UserState>({
 })
 
 const authStatusQuery = query({
-  api: () =>
-    getAuthStatus().then((status) => {
-      setAntiCsrfToken(status.antiCsrfToken)
-      return status
-    }),
+  api: () => getAuthStatus(),
   queryKey: () => ['auth-status']
 })
 

frontend/src/employee-mobile-frontend/auth/state.tsx

@@ -10,7 +10,6 @@ import { MobileUser } from 'lib-common/api-types/employee-auth'
 import { query, useQueryResult } from 'lib-common/query'
 
 import { renderResult } from '../async-rendering'
-import { setAntiCsrfToken } from '../client'
 
 import { getAuthStatus } from './api'
 
@@ -29,11 +28,7 @@ export const UserContext = createContext<UserState>({
 })
 
 const authStatusQuery = query({
-  api: () =>
-    getAuthStatus().then((status) => {
-      setAntiCsrfToken(status.antiCsrfToken)
-      return status
-    }),
+  api: () => getAuthStatus(),
   queryKey: () => ['auth-status']
 })
 

frontend/src/employee-mobile-frontend/client.ts

@@ -13,12 +13,6 @@ export const client = axios.create({
 })
 client.defaults.headers.common['x-evaka-csrf'] = '1'
 
-export const setAntiCsrfToken = (value: string | undefined) => {
-  for (const method of ['delete', 'patch', 'post', 'put'] as const) {
-    client.defaults.headers[method]['x-evaka-csrf'] = value
-  }
-}
-
 if (isAutomatedTest) {
   client.interceptors.request.use((config) => {
     const evakaMockedTime =

frontend/src/lib-common/api-types/employee-auth.ts

@@ -46,7 +46,6 @@ export type AdRole = GlobalRole | ScopedRole
 
 export interface AuthStatus<U extends User | MobileUser> {
   loggedIn: boolean
-  antiCsrfToken?: string
   user?: U
   roles?: AdRole[]
   apiVersion: string