From 5291f2958155694252969f8bd0a92f26ee2a662e Mon Sep 17 00:00:00 2001 From: Johann150 Date: Mon, 7 Nov 2022 00:12:21 +0100 Subject: [PATCH] implement OAuth PKCE This implements Proof Key for Code Exchange a.k.a. RFC 7636. --- .../backend/migration/1667738304733-pkce.js | 12 ++++++++ .../src/models/entities/auth-session.ts | 6 ++++ .../backend/src/server/api/common/oauth.ts | 29 +++++++++++++++++++ .../api/endpoints/auth/session/generate.ts | 5 ++++ packages/client/src/pages/auth.vue | 15 ++++++++++ 5 files changed, 67 insertions(+) create mode 100644 packages/backend/migration/1667738304733-pkce.js diff --git a/packages/backend/migration/1667738304733-pkce.js b/packages/backend/migration/1667738304733-pkce.js new file mode 100644 index 000000000..4036bd649 --- /dev/null +++ b/packages/backend/migration/1667738304733-pkce.js @@ -0,0 +1,12 @@ +export class pkce1667738304733 { + name = 'pkce1667738304733' + + async up(queryRunner) { + await queryRunner.query(`ALTER TABLE "auth_session" ADD "pkceChallenge" text`); + await queryRunner.query(`COMMENT ON COLUMN "auth_session"."pkceChallenge" IS 'PKCE code_challenge value, if provided (OAuth only)'`); + } + + async down(queryRunner) { + await queryRunner.query(`ALTER TABLE "auth_session" DROP COLUMN "pkceChallenge"`); + } +} diff --git a/packages/backend/src/models/entities/auth-session.ts b/packages/backend/src/models/entities/auth-session.ts index a68ead2da..98031f2de 100644 --- a/packages/backend/src/models/entities/auth-session.ts +++ b/packages/backend/src/models/entities/auth-session.ts @@ -40,4 +40,10 @@ export class AuthSession { }) @JoinColumn() public app: App | null; + + @Column('text', { + nullable: true, + comment: 'PKCE code_challenge value, if provided (OAuth only)', + }) + pkceChallenge: string | null; } diff --git a/packages/backend/src/server/api/common/oauth.ts b/packages/backend/src/server/api/common/oauth.ts index bef5ee919..c09712e9b 100644 --- a/packages/backend/src/server/api/common/oauth.ts +++ b/packages/backend/src/server/api/common/oauth.ts @@ -10,6 +10,7 @@ export async function oauth(ctx: Koa.Context): void { grant_type, code, redirect_uri, + code_verifier, } = ctx.request.body; // check if any of the parameters are null or empty string @@ -79,6 +80,34 @@ export async function oauth(ctx: Koa.Context): void { return; } + // check PKCE challenge, if provided before + if (session.pkceChallenge) { + // Also checking the client's homework, the RFC says: + //> minimum length of 43 characters and a maximum length of 128 characters + if (!code_verifier || code_verifier.length < 43 || code_verifier.length > 128) { + ctx.response.status = 400; + ctx.response.body = { + error: 'invalid_grant', + error_description: 'invalid or missing PKCE code_verifier', + }; + return; + } else { + // verify that (from RFC 7636): + //> BASE64URL-ENCODE(SHA256(ASCII(code_verifier))) == code_challenge + const hash = crypto.createHash('sha256'); + hash.update(code_verifier); + + if (hash.digest('base64url') !== code_challenge) { + ctx.response.status = 400; + ctx.response.body = { + error: 'invalid_grant', + error_description: 'invalid PKCE code_verifier', + }; + return; + } + } + } + // check redirect URI if (!compareUrl(app.callbackUrl, redirect_uri)) { ctx.response.status = 400; diff --git a/packages/backend/src/server/api/endpoints/auth/session/generate.ts b/packages/backend/src/server/api/endpoints/auth/session/generate.ts index 0c06c5895..fe1286c03 100644 --- a/packages/backend/src/server/api/endpoints/auth/session/generate.ts +++ b/packages/backend/src/server/api/endpoints/auth/session/generate.ts @@ -52,6 +52,10 @@ export const paramDef = { type: 'string', minLength: 1, }, + pkceChallenge: { + type: 'string', + minLength: 1, + }, }, required: ['clientId'] }, { @@ -93,6 +97,7 @@ export default define(meta, paramDef, async (ps) => { createdAt: new Date(), appId: app.id, token, + pkceChallenge: ps.pkceChallenge, }).then(x => AuthSessions.findOneByOrFail(x.identifiers[0])); return { diff --git a/packages/client/src/pages/auth.vue b/packages/client/src/pages/auth.vue index a74f22a79..0f7b3582e 100644 --- a/packages/client/src/pages/auth.vue +++ b/packages/client/src/pages/auth.vue @@ -64,6 +64,20 @@ onMounted(async () => { if (params.get('response_type') === 'code') { // OAuth request detected! + // if PKCE is used, check that it is a supported method + // the default value for code_challenge_method if not supplied is 'plain', which is not supported. + if (params.has('code_challenge') && params.get('code_challenge_method') !== 'S256') { + if (params.has('redirect_uri')) { + location.href = appendQuery(params.get('redirect_uri'), query({ + error: 'invalid_request', + error_description: 'unsupported code_challenge_method, only "S256" is supported', + })); + } else { + state = 'oauth-error'; + } + return; + } + // as a kind of hack, we first have to start the session for the OAuth client const clientId = params.get('client_id'); if (!clientId) { @@ -75,6 +89,7 @@ onMounted(async () => { clientId, // make the server check the redirect, if provided callbackUrl: params.get('redirect_uri') ?? undefined, + pkceChallenge: params.get('code_challenge') ?? undefined, }).catch(e => { const response = { error: 'server_error',