diff --git a/README.md b/README.md index 2c86f68..0bc67a0 100644 --- a/README.md +++ b/README.md @@ -174,7 +174,7 @@ The following configuration must be set in the OIDC provider, assuming Foreman i - Redirect URL: `https://foreman.example.com/api/auth/oidc/callback` It is assumed that your OIDC provider restricts which users can log in to Foreman, as no additional role checks are -performed by Foreman. +performed by Foreman. Additionally, the issuer must use HTTPS. ### GitLab integration diff --git a/backend/src/auth/oidc-strategy.ts b/backend/src/auth/oidc-strategy.ts index a551e83..122f688 100644 --- a/backend/src/auth/oidc-strategy.ts +++ b/backend/src/auth/oidc-strategy.ts @@ -1,4 +1,5 @@ -import { custom, Issuer, Strategy as OpenIdStrategy, TokenSet, type UserinfoResponse } from 'openid-client' +import * as openid from 'openid-client' +import { Strategy as OpenIdStrategy } from 'openid-client/passport' import { AuthStrategy, type User } from './common.js' import type { AuthenticateOptions } from '@fastify/passport/dist/AuthenticationRoute.js' import type { RouteHandlerMethod } from 'fastify' @@ -6,51 +7,66 @@ import { fastifyPassport } from '../fastifyPassport.js' export const authenticateOidc = (options?: AuthenticateOptions): RouteHandlerMethod => fastifyPassport.authenticate(AuthStrategy.OIDC, options) -export async function makeOidcStrategy (options: { +interface OidcOptions { issuer: string clientId: string clientSecret: string redirectUri: string -}): Promise> { - const issuerUrl = options.issuer.includes('.well-known') - ? options.issuer - : new URL('/.well-known/openid-configuration', options.issuer).toString() - const issuer = await Issuer.discover(issuerUrl) + /** + * @deprecated This option is only provided for testing purposes. + */ + allowInsecureRequests?: boolean +} + +export async function makeOidcStrategy (options: OidcOptions): Promise { + const issuerUrl = options.issuer.includes('.well-known') + ? new URL(options.issuer) + : new URL('/.well-known/openid-configuration', options.issuer) - const client = new issuer.Client({ - client_id: options.clientId, - client_secret: options.clientSecret, - redirect_uris: [options.redirectUri], - response_types: ['code'], - token_endpoint_auth_method: 'client_secret_basic' + const config = await openid.discovery(issuerUrl, options.clientId, options.clientSecret, undefined, { + // eslint-disable-next-line @typescript-eslint/no-deprecated + execute: options.allowInsecureRequests === true ? [openid.allowInsecureRequests] : undefined, + timeout: 30_000 }) - // There are some really slow OIDC providers out there. - client[custom.http_options] = () => { - return { timeout: 30_000 } - } + type TokenSet = openid.TokenEndpointResponse & openid.TokenEndpointResponseHelpers - return new OpenIdStrategy({ - client, - params: { - scope: 'openid email' - } - }, (tokenset: TokenSet, userinfo: UserinfoResponse, done: (err: any, user?: User) => void) => { - if (tokenset.expired()) { - done(new Error('Token expired')) - return + const verify = async (tokenset: TokenSet): Promise => { + const expiresIn = tokenset.expiresIn() + if (expiresIn != null && expiresIn < 1) { + throw new Error('Token expired') } + const claims = tokenset.claims() - const username = claims.email ?? userinfo.email + if (claims == null) { + throw new Error('No claims provided') + } + + let username = claims.email if (username == null) { - done(new Error('No username or email provided')) - return + const userInfo = await openid.fetchUserInfo(config, tokenset.access_token, claims.sub) + username = userInfo.email } - done(null, { + if (username == null || typeof username !== 'string') { + throw new Error('No username or email provided, or it is not a string') + } + + return { strategy: AuthStrategy.OIDC, username, createdAt: Date.now() - }) + } + } + + return new OpenIdStrategy({ + name: AuthStrategy.OIDC, + config, + callbackURL: options.redirectUri, + scope: 'openid email' + }, (tokenset: TokenSet, done: (err: any, user?: User) => void) => { + verify(tokenset) + .then((user: User) => done(null, user)) + .catch((err: unknown) => done(err)) }) } diff --git a/backend/src/fastifyPassport.ts b/backend/src/fastifyPassport.ts index 3788e53..1cea81a 100644 --- a/backend/src/fastifyPassport.ts +++ b/backend/src/fastifyPassport.ts @@ -2,3 +2,5 @@ import passport from '@fastify/passport' // Something is wrong with esModuleInterop... this is a workaround export const fastifyPassport = passport.default + +export const Authenticator = passport.Authenticator diff --git a/backend/src/index.ts b/backend/src/index.ts index 0532696..16edf62 100644 --- a/backend/src/index.ts +++ b/backend/src/index.ts @@ -90,7 +90,7 @@ export const backend = (kubeConfig: KubeConfig, config: BackendConfig): FastifyP fastifyPassport.use(AuthStrategy.OIDC, await makeOidcStrategy({ ...config.auth.oidc, redirectUri: new URL(`${app.prefix}/auth/oidc/callback`, config.auth.oidc.publicUrl).toString() - })) + }) as any) } await app.register(strategiesAuthRoute(enabledAuthStrategies), { prefix: '/auth/strategies' }) diff --git a/backend/test/auth/oidc-strategy.test.ts b/backend/test/auth/oidc-strategy.test.ts index 4e284c9..94268b3 100644 --- a/backend/test/auth/oidc-strategy.test.ts +++ b/backend/test/auth/oidc-strategy.test.ts @@ -1,7 +1,9 @@ import { fastify, type FastifyInstance } from 'fastify' import { makeOidcStrategy } from '../../src/auth/oidc-strategy.js' import assert from 'node:assert' -import { Strategy } from 'openid-client' +import { Strategy as OpenIdStrategy } from 'openid-client/passport' +import { Authenticator } from '../../src/fastifyPassport.js' +import { AuthStrategy } from '../../src/auth/common.js' describe('auth/oidc-strategy.ts', () => { describe('makeOidcStrategy()', () => { @@ -24,12 +26,28 @@ describe('auth/oidc-strategy.ts', () => { issuer: 'http://127.0.0.1:58080', clientId: 'foobar', clientSecret: 'bazqux', - redirectUri: 'http://localhost:3000/oidc/callback' + redirectUri: 'http://localhost:3000/oidc/callback', + allowInsecureRequests: true }) - assert.ok(strategy instanceof Strategy) + assert.ok(strategy instanceof OpenIdStrategy) assert.ok(called) }) + it('disallows insecure requests by default', async () => { + try { + await makeOidcStrategy({ + issuer: 'http://127.0.0.1:58080', + clientId: 'foobar', + clientSecret: 'bazqux', + redirectUri: 'http://localhost:3000/oidc/callback' + }) + } catch (err: unknown) { + assert.ok(err != null && (err as any).message.toLowerCase().includes('https')) + return + } + assert.fail('Expected an error') + }) + it('accepts issuer URL with full .well-known path', async () => { app = fastify() let called = false @@ -42,9 +60,10 @@ describe('auth/oidc-strategy.ts', () => { issuer: 'http://127.0.0.1:58080/.well-known/openid-configuration', clientId: 'foobar', clientSecret: 'bazqux', - redirectUri: 'http://localhost:3000/oidc/callback' + redirectUri: 'http://localhost:3000/oidc/callback', + allowInsecureRequests: true }) - assert.ok(strategy instanceof Strategy) + assert.ok(strategy instanceof OpenIdStrategy) assert.ok(called) }) @@ -61,25 +80,36 @@ describe('auth/oidc-strategy.ts', () => { } }) await app.listen({ host: '127.0.0.1', port: 58080 }) - const strategy = await makeOidcStrategy({ + const passport = new Authenticator() + passport.use(await makeOidcStrategy({ issuer: 'http://127.0.0.1:58080', clientId: 'foobar', clientSecret: 'bazqux', - redirectUri: 'http://localhost:3000/oidc/callback' - }) + redirectUri: 'http://localhost:3000/oidc/callback', + allowInsecureRequests: true + }) as any) + const strategy = passport.strategy(AuthStrategy.OIDC) + assert.ok(strategy != null) const mockRequest = { method: 'GET', url: '/', body: {}, - session: {} + session: {}, + res: { + redirect: (url: string) => { + assert.fail(`Unexpected redirect to ${url}`) + }, + status: (code: number) => { + assert.fail(`Unexpected status code ${code}`) + } + } } const promise = new Promise((resolve, reject) => { - // eslint-disable-next-line @typescript-eslint/prefer-promise-reject-errors - strategy.error = (err: any) => reject(err) - strategy.fail = () => reject(new Error('should not have failed')) - strategy.success = () => reject(new Error('should not have succeeded')) - strategy.pass = () => reject(new Error('should not have passed')) - strategy.redirect = (url: string) => { + (strategy as any).error = () => reject(new Error('should not have errored')); + (strategy as any).fail = () => reject(new Error('should not have failed')); + (strategy as any).success = () => reject(new Error('should not have succeeded')); + (strategy as any).pass = () => reject(new Error('should not have passed')); + (strategy as any).redirect = (url: string) => { const urlObj = new URL(url) assert.strictEqual(urlObj.origin, 'http://127.0.0.1:58080') assert.strictEqual(urlObj.pathname, '/authorize') @@ -89,7 +119,7 @@ describe('auth/oidc-strategy.ts', () => { resolve() } }) - strategy.authenticate(mockRequest as any) + strategy.authenticate.call(strategy as any, mockRequest as any, {}) await promise }) }) diff --git a/package-lock.json b/package-lock.json index 676c405..b08fafc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -22,7 +22,7 @@ "dotenv": "16.4.7", "fastify": "5.2.0", "ms": "2.1.3", - "openid-client": "5.7.1", + "openid-client": "6.1.7", "passport-local": "1.0.0", "pino": "9.6.0", "react-router-dom": "7.1.1", @@ -1924,28 +1924,6 @@ "ws": "^8.18.0" } }, - "node_modules/@kubernetes/client-node/node_modules/jose": { - "version": "5.9.6", - "resolved": "https://registry.npmjs.org/jose/-/jose-5.9.6.tgz", - "integrity": "sha512-AMlnetc9+CV9asI19zHmrgS/WYsWUwCn2R7RzlbJWD7F9eWYUTGyBmU9o6PxngtLGOiDGPRu+Uc4fhKzbpteZQ==", - "license": "MIT", - "funding": { - "url": "https://github.com/sponsors/panva" - } - }, - "node_modules/@kubernetes/client-node/node_modules/openid-client": { - "version": "6.1.7", - "resolved": "https://registry.npmjs.org/openid-client/-/openid-client-6.1.7.tgz", - "integrity": "sha512-JfY/KvQgOutmG2P+oVNKInE7zIh+im1MQOaO7g5CtNnTWMociA563WweiEMKfR9ry9XG3K2HGvj9wEqhCQkPMg==", - "license": "MIT", - "dependencies": { - "jose": "^5.9.6", - "oauth4webapi": "^3.1.4" - }, - "funding": { - "url": "https://github.com/sponsors/panva" - } - }, "node_modules/@lukeed/ms": { "version": "2.0.2", "resolved": "https://registry.npmjs.org/@lukeed/ms/-/ms-2.0.2.tgz", @@ -6793,9 +6771,9 @@ } }, "node_modules/jose": { - "version": "4.15.9", - "resolved": "https://registry.npmjs.org/jose/-/jose-4.15.9.tgz", - "integrity": "sha512-1vUQX+IdDMVPj4k8kOxgUqlcK518yluMuGZwqlr44FS1ppZB/5GWh4rZG89erpOBOJjU/OBsnCVFfapsRz6nEA==", + "version": "5.9.6", + "resolved": "https://registry.npmjs.org/jose/-/jose-5.9.6.tgz", + "integrity": "sha512-AMlnetc9+CV9asI19zHmrgS/WYsWUwCn2R7RzlbJWD7F9eWYUTGyBmU9o6PxngtLGOiDGPRu+Uc4fhKzbpteZQ==", "license": "MIT", "funding": { "url": "https://github.com/sponsors/panva" @@ -7071,18 +7049,6 @@ "loose-envify": "cli.js" } }, - "node_modules/lru-cache": { - "version": "6.0.0", - "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-6.0.0.tgz", - "integrity": "sha512-Jo6dJ04CmSjuznwJSS3pUeWmd/H0ffTlkXXgwZi+eq1UCmqQwCh+eLsYOYCwY991i2Fah4h1BEMCx4qThGbsiA==", - "license": "ISC", - "dependencies": { - "yallist": "^4.0.0" - }, - "engines": { - "node": ">=10" - } - }, "node_modules/luxon": { "version": "3.5.0", "resolved": "https://registry.npmjs.org/luxon/-/luxon-3.5.0.tgz", @@ -7547,15 +7513,6 @@ "node": ">=0.10.0" } }, - "node_modules/object-hash": { - "version": "2.2.0", - "resolved": "https://registry.npmjs.org/object-hash/-/object-hash-2.2.0.tgz", - "integrity": "sha512-gScRMn0bS5fH+IuwyIFgnh9zBdo4DV+6GhygmWM9HyNJSgS0hScp1f5vjtm7oIIOiT9trXrShAkLFSc2IqKNgw==", - "license": "MIT", - "engines": { - "node": ">= 6" - } - }, "node_modules/object-inspect": { "version": "1.13.3", "resolved": "https://registry.npmjs.org/object-inspect/-/object-inspect-1.13.3.tgz", @@ -7668,15 +7625,6 @@ "url": "https://github.com/sponsors/ljharb" } }, - "node_modules/oidc-token-hash": { - "version": "5.0.3", - "resolved": "https://registry.npmjs.org/oidc-token-hash/-/oidc-token-hash-5.0.3.tgz", - "integrity": "sha512-IF4PcGgzAr6XXSff26Sk/+P4KZFJVuHAJZj3wgO3vX2bMdNVp/QXTP3P7CEm9V1IdG8lDLY3HhiqpsE/nOwpPw==", - "license": "MIT", - "engines": { - "node": "^10.13.0 || >=12.0.0" - } - }, "node_modules/on-exit-leak-free": { "version": "2.1.2", "resolved": "https://registry.npmjs.org/on-exit-leak-free/-/on-exit-leak-free-2.1.2.tgz", @@ -7696,15 +7644,13 @@ } }, "node_modules/openid-client": { - "version": "5.7.1", - "resolved": "https://registry.npmjs.org/openid-client/-/openid-client-5.7.1.tgz", - "integrity": "sha512-jDBPgSVfTnkIh71Hg9pRvtJc6wTwqjRkN88+gCFtYWrlP4Yx2Dsrow8uPi3qLr/aeymPF3o2+dS+wOpglK04ew==", + "version": "6.1.7", + "resolved": "https://registry.npmjs.org/openid-client/-/openid-client-6.1.7.tgz", + "integrity": "sha512-JfY/KvQgOutmG2P+oVNKInE7zIh+im1MQOaO7g5CtNnTWMociA563WweiEMKfR9ry9XG3K2HGvj9wEqhCQkPMg==", "license": "MIT", "dependencies": { - "jose": "^4.15.9", - "lru-cache": "^6.0.0", - "object-hash": "^2.2.0", - "oidc-token-hash": "^5.0.3" + "jose": "^5.9.6", + "oauth4webapi": "^3.1.4" }, "funding": { "url": "https://github.com/sponsors/panva" @@ -11055,12 +11001,6 @@ "node": ">=10" } }, - "node_modules/yallist": { - "version": "4.0.0", - "resolved": "https://registry.npmjs.org/yallist/-/yallist-4.0.0.tgz", - "integrity": "sha512-3wdGidZyq5PB084XLES5TpOSRA3wjXAlIWMhum2kRcv/41Sn2emQ0dycQW4uZXLejwKvg6EsvbdlVL+FYEct7A==", - "license": "ISC" - }, "node_modules/yaml": { "version": "2.6.1", "resolved": "https://registry.npmjs.org/yaml/-/yaml-2.6.1.tgz", diff --git a/package.json b/package.json index 95aeafc..d5c896b 100644 --- a/package.json +++ b/package.json @@ -57,7 +57,7 @@ "dotenv": "16.4.7", "fastify": "5.2.0", "ms": "2.1.3", - "openid-client": "5.7.1", + "openid-client": "6.1.7", "passport-local": "1.0.0", "pino": "9.6.0", "react-router-dom": "7.1.1",