From 7d61b57b282826cef5511ad28506399a9cdd71da Mon Sep 17 00:00:00 2001 From: Simon Murray Date: Wed, 19 Feb 2025 13:40:25 +0000 Subject: [PATCH] Better OIDC Support (#175) Now the identity service requires a client secret, we need to keep it secret by passing it in as a private environment variable. This means all authorization functionality needs to be moved server-side rather than exposing it in an SPA. --- charts/ui/Chart.yaml | 4 +- charts/ui/templates/deployment.yaml | 21 +++- charts/ui/values.yaml | 5 + src/lib/clients/index.ts | 38 ++----- src/lib/oidc/index.ts | 4 +- src/routes/(shell)/+layout.ts | 44 +------- src/routes/oauth2/callback/+page.server.ts | 121 +++++++++++++++++++++ src/routes/oauth2/callback/+page.ts | 111 ++----------------- src/routes/oauth2/login/+page.server.ts | 23 ++++ src/routes/oauth2/login/+page.ts | 10 ++ src/routes/oauth2/login2/+page.server.ts | 48 ++++++++ src/routes/oauth2/refresh/+page.server.ts | 53 +++++++++ src/routes/oauth2/refresh/+page.svelte | 1 + 13 files changed, 305 insertions(+), 178 deletions(-) create mode 100644 src/routes/oauth2/callback/+page.server.ts create mode 100644 src/routes/oauth2/login/+page.server.ts create mode 100644 src/routes/oauth2/login/+page.ts create mode 100644 src/routes/oauth2/login2/+page.server.ts create mode 100644 src/routes/oauth2/refresh/+page.server.ts create mode 100644 src/routes/oauth2/refresh/+page.svelte diff --git a/charts/ui/Chart.yaml b/charts/ui/Chart.yaml index ca9e136..5b51a80 100644 --- a/charts/ui/Chart.yaml +++ b/charts/ui/Chart.yaml @@ -4,8 +4,8 @@ description: A Helm chart for deploying Unikorn UI type: application -version: v0.3.9-rc1 -appVersion: v0.3.9-rc1 +version: v0.3.9-rc2 +appVersion: v0.3.9-rc2 icon: https://assets.unikorn-cloud.org/assets/images/logos/dark-on-light/icon.png diff --git a/charts/ui/templates/deployment.yaml b/charts/ui/templates/deployment.yaml index a113675..1b6dd9a 100644 --- a/charts/ui/templates/deployment.yaml +++ b/charts/ui/templates/deployment.yaml @@ -30,7 +30,7 @@ spec: env: - name: PUBLIC_APPLICATION_VERSION value: {{ .Chart.Version }} - - name: PUBLIC_OAUTH2_ISSUER + - name: PUBLIC_IDENTITY_HOST value: https://{{ include "unikorn.identity.host" . }} - name: PUBLIC_REGION_HOST value: https://{{ include "unikorn.region.host" . }} @@ -40,16 +40,31 @@ spec: value: https://{{ include "unikorn.application.host" . }} - name: PUBLIC_COMPUTE_HOST value: https://{{ include "unikorn.compute.host" . }} - - name: PUBLIC_OAUTH2_CLIENT_ID + - name: OIDC_CLIENT_ID {{- if .Values.oauth2.clientName }} value: {{ include "resource.id" .Values.oauth2.clientName }} {{- else }} value: {{ .Values.oauth2.clientID }} {{- end }} - - name: PUBLIC_OAUTH2_CLIENT_SECRET + - name: OIDC_CLIENT_SECRET value: {{ .Values.oauth2.clientSecret }} + {{- if .Values.tls.private }} + - name: NODE_EXTRA_CA_CERTS + value: /var/run/secrets/unikorn-cloud.org/ca.crt + {{- end }} securityContext: readOnlyRootFilesystem: true + {{- if .Values.tls.private }} + volumeMounts: + - name: unikorn-ui-ingress-tls + mountPath: /var/run/secrets/unikorn-cloud.org + {{- end }} serviceAccountName: unikorn-ui securityContext: runAsNonRoot: true + {{- if .Values.tls.private }} + volumes: + - name: unikorn-ui-ingress-tls + secret: + secretName: unikorn-ui-ingress-tls + {{- end }} diff --git a/charts/ui/values.yaml b/charts/ui/values.yaml index 7bc0730..de61897 100644 --- a/charts/ui/values.yaml +++ b/charts/ui/values.yaml @@ -55,6 +55,11 @@ oauth2: # and then hashed into a predictlable ID. Overrides the ID. # clientName: foo +tls: + # When marked as private, this will mount the ingress secret into the + # pod and expose the CA certificate to Node. + private: false + ingress: # Sets the ingress class to use. class: ~ diff --git a/src/lib/clients/index.ts b/src/lib/clients/index.ts index 6d5fa86..c4c84c2 100644 --- a/src/lib/clients/index.ts +++ b/src/lib/clients/index.ts @@ -76,40 +76,26 @@ async function accessToken(tokens: InternalToken, fetchImpl?: typeof fetch): Pro // we are repeating the operation, be nice if we could handle this // somehow. if (new Date(Date.now()).toJSON() > tokens.expiry) { - const discovery = await OIDC.discovery(fetchImpl || fetch); - - const form = new URLSearchParams({ - grant_type: 'refresh_token', - client_id: OIDC.clientID, - client_secret: OIDC.clientSecret, + const query = new URLSearchParams({ refresh_token: tokens.refresh_token }); - const options = { - method: 'POST', - headers: { - 'Content-Type': 'application/x-www-form-urlencoded' - }, - body: form.toString() - }; - - const response = await fetch(discovery.token_endpoint, options); - - const result = await response.json(); + const target = new URL(`${window.location.protocol}//${window.location.host}/oauth2/refresh`); + target.search = query.toString(); + const response = await fetch(target.toString()); if (!response.ok) { - const err = result as Identity.ModelError; - - if (err.error == Identity.ModelErrorErrorEnum.InvalidGrant) { - removeCredentials(); - } + removeCredentials(); + return ''; + } - // This will utimately turn into a 400 due to missing headers. - // TODO: Not the best way to handle things to be honest. + const tokenRaw = response.headers.get('X-Unikorn-Token'); + if (!tokenRaw) { + console.log('token header missing'); return ''; } - const new_token = result as InternalToken; + const new_token = JSON.parse(tokenRaw) as InternalToken; // Set the expiry time so we know when to perform a rotation. // Add a little wiggle room in there to account for any latency. @@ -165,7 +151,7 @@ export function application( export function identity(tokens: InternalToken, fetchImpl?: typeof fetch): Identity.DefaultApi { const config = new Identity.Configuration({ - basePath: env.PUBLIC_OAUTH2_ISSUER, + basePath: env.PUBLIC_IDENTITY_HOST, accessToken: async () => accessToken(tokens, fetchImpl), middleware: [authenticationMiddleware(), traceContextMiddleware()], fetchApi: fetchImpl diff --git a/src/lib/oidc/index.ts b/src/lib/oidc/index.ts index 6d33a64..b46fce8 100644 --- a/src/lib/oidc/index.ts +++ b/src/lib/oidc/index.ts @@ -6,9 +6,7 @@ import type { JWTVerifyResult } from 'jose'; import { env } from '$env/dynamic/public'; // These are required variables from the environment. -export const issuer = env.PUBLIC_OAUTH2_ISSUER || ''; -export const clientID = env.PUBLIC_OAUTH2_CLIENT_ID || ''; -export const clientSecret = env.PUBLIC_OAUTH2_CLIENT_SECRET || ''; +export const issuer = env.PUBLIC_IDENTITY_HOST || ''; export type IDToken = { // openid scope. diff --git a/src/routes/(shell)/+layout.ts b/src/routes/(shell)/+layout.ts index d3fec0b..27d3265 100644 --- a/src/routes/(shell)/+layout.ts +++ b/src/routes/(shell)/+layout.ts @@ -1,8 +1,5 @@ export const ssr = false; -import Base64url from 'crypto-js/enc-base64url'; -import SHA256 from 'crypto-js/sha256'; - import type { LayoutLoad } from './$types'; import { error, redirect } from '@sveltejs/kit'; @@ -23,45 +20,12 @@ export const load: LayoutLoad = async ({ fetch, depends }) => { const token = getSessionData('token'); const profile = getSessionData('profile'); + // Not logged in, redirect to the start of the login flow, remembering where + // we were. if (!token) { - const oidc = await OIDC.discovery(fetch); - - const nonceBytes = new Uint8Array(16); - crypto.getRandomValues(nonceBytes); - - const nonce = btoa(nonceBytes.toString()); - const nonceHash = SHA256(nonce).toString(Base64url); - - window.sessionStorage.setItem('oidc_nonce', nonce); - - /* Kck off the oauth2/oidc authentication code flow */ - const codeChallengeVerifierBytes = new Uint8Array(32); - crypto.getRandomValues(codeChallengeVerifierBytes); - - const codeChallengeVerifier = btoa(codeChallengeVerifierBytes.toString()); - const codeChallenge = SHA256(codeChallengeVerifier).toString(Base64url); - - window.sessionStorage.setItem('oauth2_code_challenge_verifier', codeChallengeVerifier); - window.sessionStorage.setItem('oauth2_location', window.location.pathname); - - const query = new URLSearchParams({ - response_type: 'code', - client_id: OIDC.clientID, - redirect_uri: `${window.location.protocol}//${window.location.host}/oauth2/callback`, - code_challenge_method: 'S256', - code_challenge: codeChallenge, - scope: 'openid email profile', - nonce: nonceHash - }); - - if (profile?.email) { - query.set('login_hint', profile.email); - } - - const url = new URL(oidc.authorization_endpoint); - url.search = query.toString(); + window.sessionStorage.setItem('oidc_location', window.location.pathname); - redirect(307, url.toString()); + redirect(307, '/oauth2/login'); } if (!profile) { diff --git a/src/routes/oauth2/callback/+page.server.ts b/src/routes/oauth2/callback/+page.server.ts new file mode 100644 index 0000000..0067449 --- /dev/null +++ b/src/routes/oauth2/callback/+page.server.ts @@ -0,0 +1,121 @@ +export const ssr = false; + +import Base64url from 'crypto-js/enc-base64url'; +import SHA256 from 'crypto-js/sha256'; +import { createRemoteJWKSet, jwtVerify } from 'jose'; + +import type { PageServerLoad } from './$types'; +import { error } from '@sveltejs/kit'; +import { env } from '$env/dynamic/private'; + +import type { InternalToken } from '$lib/oauth2'; +import * as OIDC from '$lib/oidc'; + +type OAuth2Error = { + error: string; + error_description: string; +}; + +// All has gone well so far and the OIDC server has either given us an error or +// an authorization code that we can exchange for some tokens. For that we need +// our PKCE cookies from the browser session and the server-side client secret. +export const load: PageServerLoad = async ({ fetch, url, cookies }) => { + if (url.searchParams.has('error')) { + error( + 400, + `oauth2 authentication failed with error ${url.searchParams.get('error')}: ${url.searchParams.get('error_description')}` + ); + } + + const nonce = cookies.get('oidc_nonce'); + if (!nonce) { + error(400, 'OIDC nonce not set'); + } + + const codeChallengeVerifier = cookies.get('oidc_code_challenge_verifier'); + if (!codeChallengeVerifier) { + error(400, 'OIDC code challenge verifier not set'); + } + + const clientID = env.OIDC_CLIENT_ID; + if (!clientID) { + error(400, 'OIDC client ID not set'); + } + + const clientSecret = env.OIDC_CLIENT_SECRET; + if (!clientSecret) { + error(400, 'OIDC client secret not set'); + } + + const code = url.searchParams.get('code'); + if (!code) { + error(400, `oauth2 authentication did not respond with an authorization code`); + } + + // Code exchange... + const discovery = await OIDC.discovery(fetch); + + const body = new URLSearchParams({ + grant_type: 'authorization_code', + client_id: clientID, + client_secret: clientSecret, + redirect_uri: `${url.protocol}//${url.host}/oauth2/callback`, + code: code, + code_verifier: codeChallengeVerifier + }); + + const options = { + method: 'POST', + headers: { + 'Content-Type': 'application/x-www-form-urlencoded' + }, + body: body.toString() + }; + + const response = await fetch(discovery.token_endpoint, options); + + const result = await response.json(); + + if (!response.ok) { + const e = result as OAuth2Error; + + error(400, `oauth2 token exchange failed with error ${e.error}: ${e.error_description}`); + } + + // Verify the ID token against the OIDC JWKs etc. + const jwks = createRemoteJWKSet(new URL(discovery.jwks_uri)); + + const jwt = await jwtVerify(result.id_token, jwks, { + issuer: discovery.issuer, + audience: clientID + }); + + const idToken = jwt.payload as OIDC.IDToken; + if (idToken.nonce != SHA256(nonce).toString(Base64url)) { + error(400, 'OIDC ID token nonce does not match, possible replay attack'); + } + + // Verify the access token matches the hash in the signed ID token. + try { + OIDC.compareAccessTokenHash(jwt, result.access_token); + } catch (err) { + const e = err as Error; + + error(400, `oauth2 access token error: ${e.message}`); + } + + const token = result as InternalToken; + + // Set the expiry time so we know when to perform a rotation. + // Add a little wiggle room in there to account for any latency. + const expiry = new Date(Date.now()); + expiry.setSeconds(expiry.getSeconds() + token.expires_in - 60); + + token.expiry = expiry.toJSON(); + + // Pass the tokens back to the client for persistence in session storage. + return { + token: token, + idToken: idToken + }; +}; diff --git a/src/routes/oauth2/callback/+page.ts b/src/routes/oauth2/callback/+page.ts index 0deb640..c44138b 100644 --- a/src/routes/oauth2/callback/+page.ts +++ b/src/routes/oauth2/callback/+page.ts @@ -1,109 +1,12 @@ -export const ssr = false; - -import Base64url from 'crypto-js/enc-base64url'; -import SHA256 from 'crypto-js/sha256'; -import { createRemoteJWKSet, jwtVerify } from 'jose'; - import type { PageLoad } from './$types'; -import { error, redirect } from '@sveltejs/kit'; - -import type { InternalToken } from '$lib/oauth2'; -import * as OIDC from '$lib/oidc'; - -type OAuth2Error = { - error: string; - error_description: string; -}; - -export const load: PageLoad = async ({ url, fetch }) => { - // Error checking... - const nonce = window.sessionStorage.getItem('oidc_nonce'); - if (!nonce) { - error(400, 'OIDC nonce not present in session store'); - } - - window.sessionStorage.removeItem('oidc_nonce'); - - const code_verifier = window.sessionStorage.getItem('oauth2_code_challenge_verifier'); - if (!code_verifier) { - error(400, 'Code verifier not present in session store'); - } - - window.sessionStorage.removeItem('oauth2_code_challenge_verifier'); - - if (url.searchParams.has('error')) { - error( - 400, - `oauth2 authentication failed with error ${url.searchParams.get('error')}: ${url.searchParams.get('error_description')}` - ); - } - - const code = url.searchParams.get('code'); - if (!code) { - error(400, `oauth2 authentication did not respond with an authorization code`); - } - - // Code exchange... - const discovery = await OIDC.discovery(fetch); - - const body = new URLSearchParams({ - grant_type: 'authorization_code', - client_id: OIDC.clientID, - client_secret: OIDC.clientSecret, - redirect_uri: `${window.location.protocol}//${window.location.host}/oauth2/callback`, - code: code, - code_verifier: code_verifier - }); - - const options = { - method: 'POST', - headers: { - 'Content-Type': 'application/x-www-form-urlencoded' - }, - body: body.toString() - }; - - const response = await fetch(discovery.token_endpoint, options); - - const result = await response.json(); - - if (!response.ok) { - const e = result as OAuth2Error; - - error(400, `oauth2 token exchange failed with error ${e.error}: ${e.error_description}`); - } - - const jwks = createRemoteJWKSet(new URL(discovery.jwks_uri)); - - const jwt = await jwtVerify(result.id_token, jwks, { - issuer: discovery.issuer, - audience: OIDC.clientID - }); - - const id_token = jwt.payload as OIDC.IDToken; - if (id_token.nonce != SHA256(nonce).toString(Base64url)) { - error(400, 'OIDC ID token nonce does not match, possible replay attack'); - } - - try { - OIDC.compareAccessTokenHash(jwt, result.access_token); - } catch (err) { - const e = err as Error; - - error(400, `oauth2 access token error: ${e.message}`); - } - - const token = result as InternalToken; - - // Set the expiry time so we know when to perform a rotation. - // Add a little wiggle room in there to account for any latency. - const expiry = new Date(Date.now()); - expiry.setSeconds(expiry.getSeconds() + token.expires_in - 60); - token.expiry = expiry.toJSON(); +import { redirect } from '@sveltejs/kit'; - window.sessionStorage.setItem('token', JSON.stringify(token)); - window.sessionStorage.setItem('profile', JSON.stringify(id_token)); +// Server-side code exchange has occurred now and we are passed back our tokens +// that can be used against the various APIs. +export const load: PageLoad = ({ data }) => { + window.sessionStorage.setItem('token', JSON.stringify(data.token)); + window.sessionStorage.setItem('profile', JSON.stringify(data.idToken)); - redirect(307, window.sessionStorage.getItem('oauth2_location') || '/'); + redirect(307, window.sessionStorage.getItem('oidc_location') || '/'); }; diff --git a/src/routes/oauth2/login/+page.server.ts b/src/routes/oauth2/login/+page.server.ts new file mode 100644 index 0000000..d36ed76 --- /dev/null +++ b/src/routes/oauth2/login/+page.server.ts @@ -0,0 +1,23 @@ +import type { PageServerLoad } from './$types'; + +// This is the first step in authentication, we are going to generate an OIDC +// nonce and PKCE code verifier, then set them as cookies in the client so we +// can retrieve them later in the callback. This is vitally important for the +// code verifier as we need to use this server-side during code exchange which +// uses the client secret. +export const load: PageServerLoad = ({ cookies }) => { + const nonceBytes = new Uint8Array(16); + crypto.getRandomValues(nonceBytes); + + const nonce = btoa(nonceBytes.toString()); + + const codeChallengeVerifierBytes = new Uint8Array(32); + crypto.getRandomValues(codeChallengeVerifierBytes); + + const codeChallengeVerifier = btoa(codeChallengeVerifierBytes.toString()); + + cookies.set('oidc_nonce', nonce, { path: '/' }); + cookies.set('oidc_code_challenge_verifier', codeChallengeVerifier, { path: '/' }); + + return {}; +}; diff --git a/src/routes/oauth2/login/+page.ts b/src/routes/oauth2/login/+page.ts new file mode 100644 index 0000000..5e3a01d --- /dev/null +++ b/src/routes/oauth2/login/+page.ts @@ -0,0 +1,10 @@ +import type { PageLoad } from './$types'; + +import { redirect } from '@sveltejs/kit'; + +// Second step in authentication, the server has generated a nonce and PKCE code +// verifier and saved them to the browser as cookies. We need to now redirect +// to the OIDC server. +export const load: PageLoad = () => { + redirect(307, '/oauth2/login2'); +}; diff --git a/src/routes/oauth2/login2/+page.server.ts b/src/routes/oauth2/login2/+page.server.ts new file mode 100644 index 0000000..20ec18e --- /dev/null +++ b/src/routes/oauth2/login2/+page.server.ts @@ -0,0 +1,48 @@ +import type { PageServerLoad } from './$types'; + +import { redirect, error } from '@sveltejs/kit'; +import { env } from '$env/dynamic/private'; + +import Base64url from 'crypto-js/enc-base64url'; +import SHA256 from 'crypto-js/sha256'; + +import * as OIDC from '$lib/oidc'; + +// We hit /oauth2/login which has set our cookies for the nonce and PKCE, now we +// can begin the oauth2 flow for real... +export const load: PageServerLoad = async ({ fetch, url, cookies }) => { + const oidc = await OIDC.discovery(fetch); + + const nonce = cookies.get('oidc_nonce'); + if (!nonce) { + error(400, 'OIDC nonce not set'); + } + + const codeChallengeVerifier = cookies.get('oidc_code_challenge_verifier'); + if (!codeChallengeVerifier) { + error(400, 'OIDC code challenge verifier not set'); + } + + const clientID = env.OIDC_CLIENT_ID; + if (!clientID) { + error(400, 'OIDC client ID not set'); + } + + const nonceHash = SHA256(nonce).toString(Base64url); + const codeChallenge = SHA256(codeChallengeVerifier).toString(Base64url); + + const query = new URLSearchParams({ + response_type: 'code', + client_id: clientID, + redirect_uri: `${url.protocol}//${url.host}/oauth2/callback`, + code_challenge_method: 'S256', + code_challenge: codeChallenge, + scope: 'openid email profile', + nonce: nonceHash + }); + + const target = new URL(oidc.authorization_endpoint); + target.search = query.toString(); + + redirect(307, target.toString()); +}; diff --git a/src/routes/oauth2/refresh/+page.server.ts b/src/routes/oauth2/refresh/+page.server.ts new file mode 100644 index 0000000..833cbc8 --- /dev/null +++ b/src/routes/oauth2/refresh/+page.server.ts @@ -0,0 +1,53 @@ +import type { PageServerLoad } from './$types'; + +import { error } from '@sveltejs/kit'; +import { env } from '$env/dynamic/private'; + +import * as OIDC from '$lib/oidc'; + +// This forms the basis of a GET /oauth2/refresh?refresh_token=foo handler. Given +// the client ID and secret are secret we need to do this. We return the new token +// to the client in a header. +export const load: PageServerLoad = async ({ fetch, url, setHeaders }) => { + const refreshToken = url.searchParams.get('refresh_token'); + if (!refreshToken) { + error(400, 'OIDC refresh token not set'); + } + + const clientID = env.OIDC_CLIENT_ID; + if (!clientID) { + error(400, 'OIDC client ID not set'); + } + + const clientSecret = env.OIDC_CLIENT_SECRET; + if (!clientSecret) { + error(400, 'OIDC client secret not set'); + } + + const discovery = await OIDC.discovery(fetch); + + const form = new URLSearchParams({ + grant_type: 'refresh_token', + client_id: clientID, + client_secret: clientSecret, + refresh_token: refreshToken + }); + + const options = { + method: 'POST', + headers: { + 'Content-Type': 'application/x-www-form-urlencoded' + }, + body: form.toString() + }; + + const response = await fetch(discovery.token_endpoint, options); + + if (!response.ok) { + error(400, 'OIDC token refresh failed'); + } + + const token = await response.json(); + + setHeaders({ 'X-Unikorn-Token': JSON.stringify(token) }); +}; diff --git a/src/routes/oauth2/refresh/+page.svelte b/src/routes/oauth2/refresh/+page.svelte new file mode 100644 index 0000000..944c593 --- /dev/null +++ b/src/routes/oauth2/refresh/+page.svelte @@ -0,0 +1 @@ +Nothing to see here.