From c3c297a9a59e3bcf12dbccd167a8efb48c6c485c Mon Sep 17 00:00:00 2001 From: Evert Prants Date: Sat, 3 Dec 2022 10:02:58 +0200 Subject: [PATCH] actually implement oauth2 pcke, drop image scope --- package-lock.json | 14 +++++----- package.json | 2 +- src/app.controller.ts | 3 +- src/migration/1670052416869-pcke.ts | 17 +++++++++++ src/modules/api/api.controller.ts | 11 ++------ src/modules/oauth2/adapter/code.adapter.ts | 28 +++++++++++++++++++ src/modules/oauth2/adapter/jwt.adapter.ts | 7 +---- src/modules/oauth2/oauth2.service.ts | 7 ++--- .../oauth2-client/oauth2-client.service.ts | 1 - .../oauth2-token/oauth2-token.entity.ts | 3 ++ .../oauth2-token/oauth2-token.service.ts | 2 ++ 11 files changed, 66 insertions(+), 29 deletions(-) create mode 100644 src/migration/1670052416869-pcke.ts diff --git a/package-lock.json b/package-lock.json index 65a9f5e..3af4698 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,7 +9,7 @@ "version": "0.0.1", "license": "UNLICENSED", "dependencies": { - "@icynet/oauth2-provider": "^1.0.6", + "@icynet/oauth2-provider": "^1.0.7", "@nestjs/common": "^9.0.11", "@nestjs/core": "^9.0.11", "@nestjs/platform-express": "^9.0.11", @@ -2142,9 +2142,9 @@ "dev": true }, "node_modules/@icynet/oauth2-provider": { - "version": "1.0.6", - "resolved": "https://registry.npmjs.org/@icynet/oauth2-provider/-/oauth2-provider-1.0.6.tgz", - "integrity": "sha512-CsPQZB0Jbzxll4re34aPtZFVNkeeWtC4aW9UZCg8U57fPL8/Xe/2dfnigxdn4r9jVdd6d/qiFSh5x7wrFUmYrw==", + "version": "1.0.7", + "resolved": "https://registry.npmjs.org/@icynet/oauth2-provider/-/oauth2-provider-1.0.7.tgz", + "integrity": "sha512-YdzkB8c/7BOUZaiKpeEFbLfttfH6kztDm+qUG3zqgZ6J+CXJMqtLnJtFu++bn8/okYikU1ErdZq2/4fetD1C+Q==", "dependencies": { "express": "^4.17.3", "express-session": "^1.17.2" @@ -14155,9 +14155,9 @@ "dev": true }, "@icynet/oauth2-provider": { - "version": "1.0.6", - "resolved": "https://registry.npmjs.org/@icynet/oauth2-provider/-/oauth2-provider-1.0.6.tgz", - "integrity": "sha512-CsPQZB0Jbzxll4re34aPtZFVNkeeWtC4aW9UZCg8U57fPL8/Xe/2dfnigxdn4r9jVdd6d/qiFSh5x7wrFUmYrw==", + "version": "1.0.7", + "resolved": "https://registry.npmjs.org/@icynet/oauth2-provider/-/oauth2-provider-1.0.7.tgz", + "integrity": "sha512-YdzkB8c/7BOUZaiKpeEFbLfttfH6kztDm+qUG3zqgZ6J+CXJMqtLnJtFu++bn8/okYikU1ErdZq2/4fetD1C+Q==", "requires": { "express": "^4.17.3", "express-session": "^1.17.2" diff --git a/package.json b/package.json index c8e2538..6b45963 100644 --- a/package.json +++ b/package.json @@ -24,7 +24,7 @@ "test:e2e": "jest --config ./test/jest-e2e.json" }, "dependencies": { - "@icynet/oauth2-provider": "^1.0.6", + "@icynet/oauth2-provider": "^1.0.7", "@nestjs/common": "^9.0.11", "@nestjs/core": "^9.0.11", "@nestjs/platform-express": "^9.0.11", diff --git a/src/app.controller.ts b/src/app.controller.ts index f2899ba..c766c2b 100644 --- a/src/app.controller.ts +++ b/src/app.controller.ts @@ -24,7 +24,7 @@ export class AppController { response_types_supported: ['code', 'id_token'], id_token_signing_alg_values_supported: [this.config.get('jwt.algorithm')], subject_types_supported: ['public'], - scopes_supported: ['openid', 'profile', 'email'], + scopes_supported: ['openid', 'profile', 'picture', 'email'], claims_supported: [ 'aud', 'exp', @@ -33,6 +33,7 @@ export class AppController { 'sub', 'name', 'preferred_username', + 'nickname', 'profile', 'picture', 'updated_at', diff --git a/src/migration/1670052416869-pcke.ts b/src/migration/1670052416869-pcke.ts new file mode 100644 index 0000000..d73dcba --- /dev/null +++ b/src/migration/1670052416869-pcke.ts @@ -0,0 +1,17 @@ +import { MigrationInterface, QueryRunner } from 'typeorm'; + +export class pcke1670052416869 implements MigrationInterface { + name = 'pcke1670052416869'; + + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query( + `ALTER TABLE \`o_auth2_token\` ADD \`pcke\` text NULL`, + ); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.query( + `ALTER TABLE \`o_auth2_token\` DROP COLUMN \`pcke\``, + ); + } +} diff --git a/src/modules/api/api.controller.ts b/src/modules/api/api.controller.ts index 26b6060..616ce8c 100644 --- a/src/modules/api/api.controller.ts +++ b/src/modules/api/api.controller.ts @@ -73,16 +73,11 @@ export class ApiController { userData.email_verified = true; } - if ( - (scope.includes('image') || - scope.includes('picture') || - scopelessAccess) && - user.picture - ) { - userData.image = `${this._config.get('app.base_url')}/uploads/${ + if ((scope.includes('picture') || scopelessAccess) && user.picture) { + userData.picture = `${this._config.get('app.base_url')}/uploads/${ user.picture.file }`; - userData.image_file = user.picture.file; + userData.picture_file = user.picture.file; } if ( diff --git a/src/modules/oauth2/adapter/code.adapter.ts b/src/modules/oauth2/adapter/code.adapter.ts index 4d04493..6158ea7 100644 --- a/src/modules/oauth2/adapter/code.adapter.ts +++ b/src/modules/oauth2/adapter/code.adapter.ts @@ -6,6 +6,7 @@ export class CodeAdapter implements OAuth2CodeAdapter { constructor(private _service: OAuth2Service) {} ttl = 3600; + challengeMethods = ['plain', 'S256']; async create( userId: number, @@ -13,6 +14,8 @@ export class CodeAdapter implements OAuth2CodeAdapter { scope: string | string[], ttl: number, nonce?: string, + codeChallenge?: string, + codeChallengeMethod?: 'plain' | 'S256', ): Promise { const client = await this._service.clientService.getById(clientId); const user = await this._service.userService.getById(userId); @@ -24,6 +27,12 @@ export class CodeAdapter implements OAuth2CodeAdapter { ).join(' '); const expiresAt = new Date(Date.now() + ttl * 1000); + const pcke = + codeChallenge && codeChallengeMethod + ? `${this.challengeMethods.indexOf( + codeChallengeMethod, + )}:${codeChallenge}` + : null; this._service.tokenService.insertToken( accessToken, @@ -33,6 +42,7 @@ export class CodeAdapter implements OAuth2CodeAdapter { expiresAt, user, nonce, + pcke, ); return accessToken; @@ -49,11 +59,22 @@ export class CodeAdapter implements OAuth2CodeAdapter { return null; } + let codeChallenge: string; + let codeChallengeMethod: 'plain' | 'S256'; + if (find.pcke) { + codeChallengeMethod = this.challengeMethods[ + Number(find.pcke.substring(0, 1)) + ] as 'plain' | 'S256'; + codeChallenge = find.pcke.substring(2); + } + return { ...find, code: find.token, client_id: find.client.client_id, user_id: find.user.id, + code_challenge: codeChallenge, + code_challenge_method: codeChallengeMethod, }; } @@ -82,4 +103,11 @@ export class CodeAdapter implements OAuth2CodeAdapter { checkTTL(code: OAuth2Code): boolean { return code.expires_at.getTime() > Date.now(); } + + getCodeChallenge(code: OAuth2Code) { + return { + method: code.code_challenge_method, + challenge: code.code_challenge, + }; + } } diff --git a/src/modules/oauth2/adapter/jwt.adapter.ts b/src/modules/oauth2/adapter/jwt.adapter.ts index f78b563..e3985dc 100644 --- a/src/modules/oauth2/adapter/jwt.adapter.ts +++ b/src/modules/oauth2/adapter/jwt.adapter.ts @@ -25,12 +25,7 @@ export class IcyJWTAdapter implements JWTAdapter { userData.email_verified = true; } - if ( - (scope.includes('image') || - scope.includes('picture') || - scope.includes('profile')) && - user.picture - ) { + if (scope.includes('picture') && user.picture) { userData.picture = `${this._client.config.get('app.base_url')}/uploads/${ user.picture.file }`; diff --git a/src/modules/oauth2/oauth2.service.ts b/src/modules/oauth2/oauth2.service.ts index 274cb15..66d0e3e 100644 --- a/src/modules/oauth2/oauth2.service.ts +++ b/src/modules/oauth2/oauth2.service.ts @@ -15,7 +15,7 @@ import { UserAdapter } from './adapter/user.adapter'; const SCOPE_DESCRIPTION: Record = { email: 'Email address', - image: 'Profile picture', + picture: 'Profile picture', }; const ALWAYS_AVAILABLE = ['Username and display name']; @@ -40,10 +40,7 @@ export class OAuth2Service { let disallowedScopes = [...ALWAYS_UNAVAILABLE]; Object.keys(SCOPE_DESCRIPTION).forEach((item) => { - if ( - scope.includes(item) || - (item === 'image' && scope.includes('picture')) - ) { + if (scope.includes(item)) { allowedScopes.push(SCOPE_DESCRIPTION[item]); } else { disallowedScopes.push(SCOPE_DESCRIPTION[item]); diff --git a/src/modules/objects/oauth2-client/oauth2-client.service.ts b/src/modules/objects/oauth2-client/oauth2-client.service.ts index 3138480..cde195a 100644 --- a/src/modules/objects/oauth2-client/oauth2-client.service.ts +++ b/src/modules/objects/oauth2-client/oauth2-client.service.ts @@ -21,7 +21,6 @@ export class OAuth2ClientService { ]; public availableScopes = [ - 'image', 'picture', 'profile', 'email', diff --git a/src/modules/objects/oauth2-token/oauth2-token.entity.ts b/src/modules/objects/oauth2-token/oauth2-token.entity.ts index 6a4899c..021a8c7 100644 --- a/src/modules/objects/oauth2-token/oauth2-token.entity.ts +++ b/src/modules/objects/oauth2-token/oauth2-token.entity.ts @@ -29,6 +29,9 @@ export class OAuth2Token { @Column({ nullable: true, type: 'text' }) nonce: string; + @Column({ nullable: true, type: 'text' }) + pcke: string; + @Column({ type: 'text', nullable: true }) scope: string; diff --git a/src/modules/objects/oauth2-token/oauth2-token.service.ts b/src/modules/objects/oauth2-token/oauth2-token.service.ts index 5074d50..5e5c950 100644 --- a/src/modules/objects/oauth2-token/oauth2-token.service.ts +++ b/src/modules/objects/oauth2-token/oauth2-token.service.ts @@ -19,6 +19,7 @@ export class OAuth2TokenService { expiry: Date, user?: User, nonce?: string, + pcke?: string, ): Promise { const newToken = new OAuth2Token(); newToken.client = client; @@ -28,6 +29,7 @@ export class OAuth2TokenService { newToken.user = user; newToken.expires_at = expiry; newToken.nonce = nonce; + newToken.pcke = pcke; await this.tokenRepository.save(newToken);