Skip to content

Commit

Permalink
Merge pull request #5926 from espoon-voltti/new-citizen-weak-auth
Browse files Browse the repository at this point in the history
Alustava uusi kevytkirjautuminen kuntalaisille (ei vielä tuotantokäytössä)
  • Loading branch information
Gekkio authored Nov 27, 2024
2 parents f440d5f + 4933c71 commit e87b3e4
Show file tree
Hide file tree
Showing 50 changed files with 1,373 additions and 112 deletions.
2 changes: 2 additions & 0 deletions apigw/src/enduser/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { createDevSfiRouter } from './dev-sfi-auth.js'
import { authenticateKeycloakCitizen } from './keycloak-citizen-saml.js'
import mapRoutes from './mapRoutes.js'
import authStatus from './routes/auth-status.js'
import { authWeakLogin } from './routes/auth-weak-login.js'
import { authenticateSuomiFi } from './suomi-fi-saml.js'

export function enduserGwRouter(
Expand Down Expand Up @@ -89,6 +90,7 @@ export function enduserGwRouter(
})
)
router.get('/auth/status', authStatus)
router.post('/auth/weak-login', express.json(), authWeakLogin(redisClient))
router.use(requireAuthentication)
router.use(csrf)
router.all('/citizen/*', createProxy())
Expand Down
74 changes: 74 additions & 0 deletions apigw/src/enduser/routes/auth-weak-login.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
// SPDX-FileCopyrightText: 2017-2024 City of Espoo
//
// SPDX-License-Identifier: LGPL-2.1-or-later

import { getHours } from 'date-fns/getHours'
import { z } from 'zod'

import { EvakaSessionUser, login } from '../../shared/auth/index.js'
import { toRequestHandler } from '../../shared/express.js'
import { logAuditEvent } from '../../shared/logging.js'
import { RedisClient } from '../../shared/redis-client.js'
import { citizenWeakLogin } from '../../shared/service-client.js'

const Request = z.object({
username: z
.string()
.min(1)
.max(128)
.transform((email) => email.toLowerCase()),
password: z.string().min(1).max(128)
})

const eventCode = (name: string) => `evaka.citizen_weak.${name}`

const loginAttemptsPerHour = 20

export const authWeakLogin = (redis: RedisClient) =>
toRequestHandler(async (req, res) => {
logAuditEvent(eventCode('sign_in_requested'), req, 'Login endpoint called')
try {
const body = Request.parse(req.body)

// Apply rate limit (attempts per hour)
// Reference: Redis Rate Limiting Best Practices
// https://redis.io/glossary/rate-limiting/
const hour = getHours(new Date())
const key = `citizen-weak-login:${body.username}:${hour}`
const value = Number.parseInt((await redis.get(key)) ?? '', 10)
if (Number.isNaN(value) || value < loginAttemptsPerHour) {
// expire in 1 hour, so there's no old entry when the hours value repeats the next day
const expirySeconds = 60 * 60
await redis.multi().incr(key).expire(key, expirySeconds).exec()
} else {
res.sendStatus(429)
return
}

const { id } = await citizenWeakLogin(body)
const user: EvakaSessionUser = {
id,
userType: 'CITIZEN_WEAK',
globalRoles: [],
allScopedRoles: []
}
await login(req, user)
logAuditEvent(eventCode('sign_in'), req, 'User logged in successfully')
res.sendStatus(200)
} catch (err) {
logAuditEvent(
eventCode('sign_in_failed'),
req,
`Error logging user in. Error: ${err?.toString()}`
)
if (!res.headersSent) {
if (err instanceof z.ZodError) {
res.sendStatus(400)
} else {
res.sendStatus(403)
}
} else {
throw err
}
}
})
1 change: 0 additions & 1 deletion apigw/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ deprecatedEnvVariables.forEach((name) => {
logWarn(`Deprecated environment variable ${name} was specified`)
}
})

const redisClient = redis.createClient(toRedisClientOpts(config))
redisClient.on('error', (err) =>
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
Expand Down
46 changes: 46 additions & 0 deletions apigw/src/shared/__tests__/parse-url-with-origin.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// SPDX-FileCopyrightText: 2017-2024 City of Espoo
//
// SPDX-License-Identifier: LGPL-2.1-or-later

import { describe, expect, it } from '@jest/globals'

import { parseUrlWithOrigin } from '../parse-url-with-origin.js'

describe('parseUrlWithOrigin', () => {
const origin = 'https://example.com'
const base = { origin }
it('returns a parsed URL if the input URL is empty', () => {
const url = parseUrlWithOrigin(base, '')
expect(url?.toString()).toEqual(`${origin}/`)
})
it('returns a parsed URL if the input URL is /', () => {
const url = parseUrlWithOrigin(base, '/')
expect(url?.toString()).toEqual(`${origin}/`)
})
it('returns a parsed URL if the input URL is a relative path', () => {
const url = parseUrlWithOrigin(base, '/test')
expect(url?.toString()).toEqual(`${origin}/test`)
})
it('returns a parsed URL if the input URL has the correct origin', () => {
const url = parseUrlWithOrigin(base, `${origin}/valid`)
expect(url?.toString()).toEqual(`${origin}/valid`)
})
it('retains the query and hash, if present', () => {
const url = parseUrlWithOrigin(base, '/test?query=qvalue#hash')
expect(url?.toString()).toEqual(`${origin}/test?query=qvalue#hash`)
expect(url?.search).toEqual('?query=qvalue')
expect(url?.hash).toEqual('#hash')
})
it('returns undefined if the input URL is not relative and has the wrong origin', () => {
const url = parseUrlWithOrigin(base, 'https://other.example.com')
expect(url).toBeUndefined()
})
it('returns undefined if the input URL has a protocol-relative URL (two slashes)', () => {
const url = parseUrlWithOrigin(base, '//something')
expect(url).toBeUndefined()
})
it('returns undefined if the input URL has an unusual protocol + value combination', () => {
const url = parseUrlWithOrigin(base, 'x:https://example.com')
expect(url).toBeUndefined()
})
})
2 changes: 1 addition & 1 deletion apigw/src/shared/auth/dev-auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export function createDevAuthRouter({
res.redirect(`${root}?loginError=true`)
} else {
await login(req, user)
res.redirect(validateRelayStateUrl(req) ?? root)
res.redirect(validateRelayStateUrl(req)?.toString() ?? root)
}
} catch (err) {
if (!res.headersSent) {
Expand Down
4 changes: 2 additions & 2 deletions apigw/src/shared/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ function createLocalDevelopmentOverrides(): Partial<EnvVariables> {
JWT_PRIVATE_KEY: 'config/test-cert/jwt_private_key.pem',
JWT_REFRESH_ENABLED: !isTest,

EVAKA_BASE_URL: 'local',
EVAKA_BASE_URL: 'http://localhost:9099',
EVAKA_SERVICE_URL: 'http://localhost:8888',

AD_MOCK: true,
Expand Down Expand Up @@ -786,7 +786,7 @@ export const jwtRefreshEnabled = required('JWT_REFRESH_ENABLED', parseBoolean)
export const serviceName = 'evaka-api-gw'
export const jwtKid = required('JWT_KID', unchanged)

export const evakaBaseUrl = required('EVAKA_BASE_URL', unchanged)
export const evakaBaseUrl = new URL(required('EVAKA_BASE_URL', unchanged))
export const evakaServiceUrl = required('EVAKA_SERVICE_URL', unchanged)
export const useSecureCookies = required('USE_SECURE_COOKIES', parseBoolean)

Expand Down
21 changes: 21 additions & 0 deletions apigw/src/shared/parse-url-with-origin.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// SPDX-FileCopyrightText: 2017-2024 City of Espoo
//
// SPDX-License-Identifier: LGPL-2.1-or-later

/**
* Parses a string as a URL, requiring it to be either a relative URL, or to have exactly the correct origin.
*
* If the string does not pass the validation, undefined is returned.
*/
export function parseUrlWithOrigin(
base: { origin: string },
value: string
): URL | undefined {
try {
const url = new URL(value, base.origin)
return url.origin === base.origin ? url : undefined
// eslint-disable-next-line @typescript-eslint/no-unused-vars
} catch (err) {
return undefined
}
}
18 changes: 16 additions & 2 deletions apigw/src/shared/redis-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,36 @@
//
// SPDX-License-Identifier: LGPL-2.1-or-later

export interface RedisClient {
export interface RedisCommands {}

export interface RedisClient extends RedisCommands {
isReady: boolean

get(key: string): Promise<string | null>

set(
key: string,
value: string,
options: { EX: number }
options: { EX: number; GET?: true; NX?: true }
): Promise<string | null>

del(key: string | string[]): Promise<number>

expire(key: string, seconds: number): Promise<boolean>

incr(key: string): Promise<number>

ping(): Promise<string>

multi(): RedisTransaction
}

export interface RedisTransaction extends RedisCommands {
incr(key: string): RedisTransaction

expire(key: string, seconds: number): RedisTransaction

exec(): Promise<unknown[]>
}

export async function assertRedisConnection(
Expand Down
7 changes: 4 additions & 3 deletions apigw/src/shared/routes/saml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ export default function createSamlRouter(
// These errors can happen for example when the user browses back to the login callback after login
throw new SamlError('Login failed', {
redirectUrl: req.user
? validateRelayStateUrl(req) ?? defaultPageUrl
? validateRelayStateUrl(req)?.toString() ?? defaultPageUrl
: errorRedirectUrl(err),
cause: err,
// just ignore without logging to reduce noise in logs
Expand Down Expand Up @@ -191,7 +191,8 @@ export default function createSamlRouter(
req.session.idpProvider = strategyName
await sessions.saveLogoutToken(req, createLogoutToken(profile))

const redirectUrl = validateRelayStateUrl(req) ?? defaultPageUrl
const redirectUrl =
validateRelayStateUrl(req)?.toString() ?? defaultPageUrl
logDebug(`Redirecting to ${redirectUrl}`, req, { redirectUrl })
return res.redirect(redirectUrl)
} catch (err) {
Expand Down Expand Up @@ -287,7 +288,7 @@ export default function createSamlRouter(
success
)
} else {
url = validateRelayStateUrl(req) ?? defaultPageUrl
url = validateRelayStateUrl(req)?.toString() ?? defaultPageUrl
}
return res.redirect(url)
} catch (err) {
Expand Down
20 changes: 5 additions & 15 deletions apigw/src/shared/saml/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
// SPDX-License-Identifier: LGPL-2.1-or-later

import { readFileSync } from 'node:fs'
import path from 'node:path'

import { CacheProvider, Profile, SamlConfig } from '@node-saml/node-saml'
import express from 'express'
Expand All @@ -13,6 +12,7 @@ import { EvakaSessionUser } from '../auth/index.js'
import certificates, { TrustedCertificates } from '../certificates.js'
import { evakaBaseUrl, EvakaSamlConfig } from '../config.js'
import { logError } from '../logging.js'
import { parseUrlWithOrigin } from '../parse-url-with-origin.js'

export function createSamlConfig(
config: EvakaSamlConfig,
Expand Down Expand Up @@ -109,20 +109,10 @@ export function validateRelayStateUrl(
req: express.Request
): string | undefined {
const relayState = getRawUnvalidatedRelayState(req)

if (relayState && path.isAbsolute(relayState)) {
if (evakaBaseUrl === 'local') {
return relayState
} else {
const baseUrl = evakaBaseUrl.replace(/\/$/, '')
const redirect = new URL(relayState, baseUrl)
if (redirect.origin == baseUrl) {
return redirect.href
}
}
if (relayState) {
const url = parseUrlWithOrigin(evakaBaseUrl, relayState)
if (url) return url.toString()
logError('Invalid RelayState in request', req)
}

if (relayState) logError('Invalid RelayState in request', req)

return undefined
}
18 changes: 18 additions & 0 deletions apigw/src/shared/service-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,24 @@ export async function citizenLogin(
return data
}

interface CitizenWeakLoginRequest {
username: string
password: string
}

export async function citizenWeakLogin(
request: CitizenWeakLoginRequest
): Promise<CitizenUser> {
const { data } = await client.post<CitizenUser>(
`/system/citizen-weak-login`,
request,
{
headers: createServiceRequestHeaders(undefined, machineUser)
}
)
return data
}

export async function getCitizenDetails(
req: express.Request,
personId: string
Expand Down
38 changes: 37 additions & 1 deletion apigw/src/shared/test/mock-redis-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//
// SPDX-License-Identifier: LGPL-2.1-or-later

import { RedisClient } from '../redis-client.js'
import { RedisClient, RedisTransaction } from '../redis-client.js'

export class MockRedisClient implements RedisClient {
private time: number
Expand Down Expand Up @@ -60,7 +60,43 @@ export class MockRedisClient implements RedisClient {
return Promise.resolve(true)
}

incr(key: string): Promise<number> {
const record = this.db[key] ?? { value: '0', expires: null }
const value = Number.parseInt(record.value, 10) + 1
if (Number.isNaN(value)) throw new Error('Not a number')
this.db[key] = { ...record, value: value.toString() }
return Promise.resolve(value)
}

ping(): Promise<string> {
return Promise.resolve('PONG')
}

multi(): RedisTransaction {
return new MockTransaction(this)
}
}

class MockTransaction implements RedisTransaction {
constructor(private client: RedisClient) {}
#queue: (() => Promise<unknown>)[] = []
incr(key: string): RedisTransaction {
this.#queue.push(async () => {
await this.client.incr(key)
})
return this
}
expire(key: string, seconds: number): RedisTransaction {
this.#queue.push(async () => {
await this.client.expire(key, seconds)
})
return this
}
async exec(): Promise<unknown[]> {
const returnValues: unknown[] = []
for (const op of this.#queue) {
returnValues.push(await op())
}
return returnValues
}
}
13 changes: 13 additions & 0 deletions frontend/src/citizen-frontend/auth/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,16 @@ export type AuthStatus =
export async function getAuthStatus(): Promise<AuthStatus> {
return (await client.get<JsonOf<AuthStatus>>('/auth/status')).data
}

interface WeakLoginRequest {
username: string
password: string
}

export async function authWeakLogin(
username: string,
password: string
): Promise<void> {
const reqBody: WeakLoginRequest = { username, password }
await client.post('/auth/weak-login', reqBody)
}
Loading

0 comments on commit e87b3e4

Please sign in to comment.