feat!: Update openid-client to v6, require HTTPS for issuer (#190)

BREAKING CHANGE: Foreman now requires the OIDC issuer to use HTTPS.
Plain HTTP is no longer supported for OpenID Connect authentication.

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
 

backend/src/auth/oidc-strategy.ts

@@ -1,56 +1,72 @@
-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'
 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<OpenIdStrategy<User>> {
-  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<OpenIdStrategy> {
+  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<User> => {
+    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))
   })
 }

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

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' })

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<void>((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
     })
   })

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",