From f66e7478964bf476bc724b276502ca6103669206 Mon Sep 17 00:00:00 2001 From: Sunny Date: Sat, 25 Mar 2023 15:34:51 +0530 Subject: [PATCH 1/7] feat(provider): added clientType to auth client making client secret non mandatory for public clients GH-153 --- .../client-password-verify.integration.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/__tests__/integration/action-sequence/passport-client-password/client-password-verify.integration.ts b/src/__tests__/integration/action-sequence/passport-client-password/client-password-verify.integration.ts index 4781ca9..4b47b69 100644 --- a/src/__tests__/integration/action-sequence/passport-client-password/client-password-verify.integration.ts +++ b/src/__tests__/integration/action-sequence/passport-client-password/client-password-verify.integration.ts @@ -69,9 +69,9 @@ describe('Client-password strategy', () => { .expect(200); expect(client.body).to.have.property('clientId'); - expect(client.body).to.have.property('clientSecret'); + // expect(client.body).to.have.property('clientSecret'); expect(client.body.clientId).to.equal('some id'); - expect(client.body.clientSecret).to.equal('some secret'); + // expect(client.body.clientSecret).to.equal('some secret'); }); it('should return status 401 when options.passRequestToCallback is set true', async () => { @@ -170,7 +170,7 @@ describe('integration test for client-password and no verifier', () => { @requestBody() body: { client_id: string; - client_secret: string; + // client_secret: string; }, ) { return this.client; From 0e51de728b82b2653465e2f979ea281a426be1b2 Mon Sep 17 00:00:00 2001 From: Sunny Date: Thu, 30 Mar 2023 15:08:50 +0530 Subject: [PATCH 2/7] feat(provider): make client secret non mandatory for public clients make client secret non mandatory for publlic clients GH-153 --- .../client-password-verify.integration.ts | 6 +- src/component.ts | 13 +- src/keys.ts | 5 + ...ient-password-strategy-factory-provider.ts | 8 ++ ...ient-password-strategy-factory-provider.ts | 112 ++++++++++++++++++ src/strategies/types/types.ts | 20 +++- src/types.ts | 11 ++ 7 files changed, 170 insertions(+), 5 deletions(-) create mode 100644 src/strategies/passport/passport-client-password/secure-client-password-strategy-factory-provider.ts diff --git a/src/__tests__/integration/action-sequence/passport-client-password/client-password-verify.integration.ts b/src/__tests__/integration/action-sequence/passport-client-password/client-password-verify.integration.ts index 4b47b69..4781ca9 100644 --- a/src/__tests__/integration/action-sequence/passport-client-password/client-password-verify.integration.ts +++ b/src/__tests__/integration/action-sequence/passport-client-password/client-password-verify.integration.ts @@ -69,9 +69,9 @@ describe('Client-password strategy', () => { .expect(200); expect(client.body).to.have.property('clientId'); - // expect(client.body).to.have.property('clientSecret'); + expect(client.body).to.have.property('clientSecret'); expect(client.body.clientId).to.equal('some id'); - // expect(client.body.clientSecret).to.equal('some secret'); + expect(client.body.clientSecret).to.equal('some secret'); }); it('should return status 401 when options.passRequestToCallback is set true', async () => { @@ -170,7 +170,7 @@ describe('integration test for client-password and no verifier', () => { @requestBody() body: { client_id: string; - // client_secret: string; + client_secret: string; }, ) { return this.client; diff --git a/src/component.ts b/src/component.ts index e746da8..4d8b371 100644 --- a/src/component.ts +++ b/src/component.ts @@ -39,6 +39,7 @@ import { OtpVerifyProvider, } from './strategies'; import {Strategies} from './strategies/keys'; +import {SecureClientPasswordStrategyFactoryProvider} from './strategies/passport/passport-client-password/secure-client-password-strategy-factory-provider'; import { CognitoAuthVerifyProvider, CognitoStrategyFactoryProvider, @@ -47,12 +48,14 @@ import { SamlStrategyFactoryProvider, SamlVerifyProvider, } from './strategies/SAML'; -import {AuthenticationConfig} from './types'; +import {AuthenticationConfig, IAuthSecureClientConfig} from './types'; export class AuthenticationComponent implements Component { constructor( @inject(AuthenticationBindings.CONFIG, {optional: true}) private readonly config?: AuthenticationConfig, + @inject(AuthenticationBindings.SecureClientConfig, {optional: true}) + private readonly secureClientConfig?: IAuthSecureClientConfig, ) { this.providers = { [AuthenticationBindings.USER_AUTH_ACTION.key]: AuthenticateActionProvider, @@ -114,6 +117,14 @@ export class AuthenticationComponent implements Component { [Strategies.Passport.AZURE_AD_VERIFIER.key]: AzureADAuthVerifyProvider, [Strategies.Passport.KEYCLOAK_VERIFIER.key]: KeycloakVerifyProvider, }; + + if (this.secureClientConfig?.secureClient) { + this.providers = { + ...this.providers, + [Strategies.Passport.CLIENT_PASSWORD_STRATEGY_FACTORY.key]: + SecureClientPasswordStrategyFactoryProvider, + }; + } this.bindings = []; if (this.config?.useClientAuthenticationMiddleware) { this.bindings.push( diff --git a/src/keys.ts b/src/keys.ts index e0d5a6f..9c39535 100644 --- a/src/keys.ts +++ b/src/keys.ts @@ -8,6 +8,7 @@ import { AuthenticationMetadata, EntityWithIdentifier, IAuthClient, + IAuthSecureClientConfig, IAuthUser, } from './types'; @@ -17,6 +18,10 @@ export * from './strategies/keys'; * Binding keys used by this component. */ export namespace AuthenticationBindings { + export const SecureClientConfig = + BindingKey.create( + `auth.secure.client.config`, + ); export const USER_STRATEGY = BindingKey.create( 'sf.userAuthentication.strategy', ); diff --git a/src/strategies/passport/passport-client-password/client-password-strategy-factory-provider.ts b/src/strategies/passport/passport-client-password/client-password-strategy-factory-provider.ts index 7505280..6d02b3b 100644 --- a/src/strategies/passport/passport-client-password/client-password-strategy-factory-provider.ts +++ b/src/strategies/passport/passport-client-password/client-password-strategy-factory-provider.ts @@ -47,6 +47,10 @@ export class ClientPasswordStrategyFactoryProvider const client = await verifyFn(clientId, clientSecret, req); if (!client) { throw new HttpErrors.Unauthorized(AuthErrorKeys.ClientInvalid); + } else if (!clientSecret) { + throw new HttpErrors.Unauthorized( + AuthErrorKeys.ClientSecretMissing, + ); } else if ( !client.clientSecret || client.clientSecret !== clientSecret @@ -73,6 +77,10 @@ export class ClientPasswordStrategyFactoryProvider const client = await verifyFn(clientId, clientSecret); if (!client) { throw new HttpErrors.Unauthorized(AuthErrorKeys.ClientInvalid); + } else if (!clientSecret) { + throw new HttpErrors.Unauthorized( + AuthErrorKeys.ClientSecretMissing, + ); } else if ( !client.clientSecret || client.clientSecret !== clientSecret diff --git a/src/strategies/passport/passport-client-password/secure-client-password-strategy-factory-provider.ts b/src/strategies/passport/passport-client-password/secure-client-password-strategy-factory-provider.ts new file mode 100644 index 0000000..5f9c81c --- /dev/null +++ b/src/strategies/passport/passport-client-password/secure-client-password-strategy-factory-provider.ts @@ -0,0 +1,112 @@ +import {inject, Provider} from '@loopback/core'; +import {HttpErrors, Request} from '@loopback/rest'; +import * as ClientPasswordStrategy from './client-password-strategy'; + +import {AuthErrorKeys} from '../../../error-keys'; +import {ClientType, IAuthClient, IAuthSecureClient} from '../../../types'; +import {Strategies} from '../../keys'; +import {VerifyFunction} from '../../types'; + +export interface SecureClientPasswordStrategyFactory { + ( + options?: ClientPasswordStrategy.StrategyOptionsWithRequestInterface, + verifierPassed?: VerifyFunction.OauthSecureClientPasswordFn, + ): ClientPasswordStrategy.Strategy; +} + +export class SecureClientPasswordStrategyFactoryProvider + implements Provider +{ + constructor( + @inject(Strategies.Passport.OAUTH2_CLIENT_PASSWORD_VERIFIER) + private readonly verifier: VerifyFunction.OauthSecureClientPasswordFn, + ) {} + + value(): SecureClientPasswordStrategyFactory { + return (options, verifier) => + this.getSecureClientPasswordVerifier(options, verifier); + } + + getSecureClientPasswordVerifier( + options?: ClientPasswordStrategy.StrategyOptionsWithRequestInterface, + verifierPassed?: VerifyFunction.OauthSecureClientPasswordFn, + ): ClientPasswordStrategy.Strategy { + const verifyFn = verifierPassed ?? this.verifier; + if (options?.passReqToCallback) { + return new ClientPasswordStrategy.Strategy( + // eslint-disable-next-line @typescript-eslint/no-misused-promises + async ( + clientId: string, + clientSecret: string | undefined, + cb: (err: Error | null, client?: IAuthSecureClient | false) => void, + req: Request | undefined, + ) => { + try { + const client = await verifyFn(clientId, clientSecret, req); + if (!client) { + throw new HttpErrors.Unauthorized(AuthErrorKeys.ClientInvalid); + } else if ( + client.clientType !== ClientType.public && + !clientSecret + ) { + throw new HttpErrors.Unauthorized( + AuthErrorKeys.ConfidentialClientSecretMissing, + ); + } else if ( + (client.clientType !== ClientType.public && + !client.clientSecret) || + client.clientSecret !== clientSecret + ) { + throw new HttpErrors.Unauthorized( + AuthErrorKeys.ClientVerificationFailed, + ); + } else { + // do nothing + } + + cb(null, client); + } catch (err) { + cb(err); + } + }, + options, + ); + } else { + return new ClientPasswordStrategy.Strategy( + // eslint-disable-next-line @typescript-eslint/no-misused-promises + async ( + clientId: string, + clientSecret: string | undefined, + cb: (err: Error | null, client?: IAuthClient | false) => void, + ) => { + try { + const client = await verifyFn(clientId, clientSecret); + + if (!client) { + throw new HttpErrors.Unauthorized(AuthErrorKeys.ClientInvalid); + } else if ( + client.clientType !== ClientType.public && + !clientSecret + ) { + throw new HttpErrors.Unauthorized( + AuthErrorKeys.ConfidentialClientSecretMissing, + ); + } else if ( + client.clientType !== ClientType.public && + (!client.clientSecret || client.clientSecret !== clientSecret) + ) { + throw new HttpErrors.Unauthorized( + AuthErrorKeys.ClientVerificationFailed, + ); + } else { + // do nothing + } + cb(null, client); + } catch (err) { + cb(err); + } + }, + ); + } + } +} diff --git a/src/strategies/types/types.ts b/src/strategies/types/types.ts index fcd3f77..7f2a996 100644 --- a/src/strategies/types/types.ts +++ b/src/strategies/types/types.ts @@ -6,7 +6,7 @@ import * as FacebookStrategy from 'passport-facebook'; import * as AppleStrategy from 'passport-apple'; import * as SamlStrategy from '@node-saml/passport-saml'; import {DecodedIdToken} from 'passport-apple'; -import {Cognito, IAuthClient, IAuthUser} from '../../types'; +import {Cognito, IAuthClient, IAuthSecureClient, IAuthUser} from '../../types'; import {Keycloak} from './keycloak.types'; import {Otp} from '../passport'; @@ -23,6 +23,11 @@ export namespace VerifyFunction { (clientId: string, clientSecret: string, req?: Request): Promise; } + export interface OauthSecureClientPasswordFn + extends GenericAuthFn { + (clientId: string, clientSecret: string, req?: Request): Promise; + } + export interface LocalPasswordFn extends GenericAuthFn { (username: string, password: string, req?: Request): Promise; } @@ -45,6 +50,19 @@ export namespace VerifyFunction { ): Promise<{client: T; user: S} | null>; } + export interface SecureResourceOwnerPasswordFn< + T = IAuthSecureClient, + S = IAuthUser, + > { + ( + clientId: string, + clientSecret: string, + username: string, + password: string, + req?: Request, + ): Promise<{client: T; user: S} | null>; + } + export interface GoogleAuthFn extends GenericAuthFn { ( accessToken: string, diff --git a/src/types.ts b/src/types.ts index 5799d89..2b3e4c4 100644 --- a/src/types.ts +++ b/src/types.ts @@ -14,6 +14,17 @@ export interface IAuthClient { redirectUrl?: string; } +export interface IAuthSecureClient { + clientId: string; + clientSecret: string; + clientType: ClientType; + redirectUrl?: string; +} + +export interface IAuthSecureClientConfig { + secureClient: boolean; +} + export interface IAuthUser { id?: number | string; username: string; From a9969515e625ca9a56e4dceda2b870f3ad938423 Mon Sep 17 00:00:00 2001 From: Sunny Date: Fri, 31 Mar 2023 15:21:50 +0530 Subject: [PATCH 3/7] feat(provider): make client secret non mandatory for public clients make client secret non mandatory for public clients GH-153 --- src/error-keys.ts | 1 + ...ient-password-strategy-factory-provider.ts | 60 ++++++-------- .../client-password-strategy.ts | 79 +++++++++++++++++++ .../passport-client-password/index.ts | 1 + ...ient-password-strategy-factory-provider.ts | 68 ++++++---------- src/types.ts | 5 ++ 6 files changed, 136 insertions(+), 78 deletions(-) create mode 100644 src/strategies/passport/passport-client-password/client-password-strategy.ts diff --git a/src/error-keys.ts b/src/error-keys.ts index f5a071f..f861a1f 100644 --- a/src/error-keys.ts +++ b/src/error-keys.ts @@ -13,4 +13,5 @@ export const enum AuthErrorKeys { KeyInvalid = 'Key Invalid', OtpInvalid = 'Otp Invalid', OtpExpired = 'Otp Token Incorrect or Expired', + ConfidentialClientSecretMissing = 'Confidential Client Secret Missing', } diff --git a/src/strategies/passport/passport-client-password/client-password-strategy-factory-provider.ts b/src/strategies/passport/passport-client-password/client-password-strategy-factory-provider.ts index 6d02b3b..005e75e 100644 --- a/src/strategies/passport/passport-client-password/client-password-strategy-factory-provider.ts +++ b/src/strategies/passport/passport-client-password/client-password-strategy-factory-provider.ts @@ -1,6 +1,6 @@ import {inject, Provider} from '@loopback/core'; import {HttpErrors, Request} from '@loopback/rest'; -import * as ClientPasswordStrategy from 'passport-oauth2-client-password'; +import * as ClientPasswordStrategy from './client-password-strategy'; import {AuthErrorKeys} from '../../../error-keys'; import {IAuthClient} from '../../../types'; @@ -27,6 +27,21 @@ export class ClientPasswordStrategyFactoryProvider this.getClientPasswordVerifier(options, verifier); } + clientPasswordVerifierHelper( + client: IAuthClient | null, + clientSecret: string | undefined, + ) { + if (!client) { + throw new HttpErrors.Unauthorized(AuthErrorKeys.ClientInvalid); + } else if (!clientSecret) { + throw new HttpErrors.Unauthorized(AuthErrorKeys.ClientSecretMissing); + } else if (!client.clientSecret || client.clientSecret !== clientSecret) { + throw new HttpErrors.Unauthorized(AuthErrorKeys.ClientVerificationFailed); + } else { + // do nothing + } + } + getClientPasswordVerifier( options?: ClientPasswordStrategy.StrategyOptionsWithRequestInterface, verifierPassed?: VerifyFunction.OauthClientPasswordFn, @@ -34,61 +49,34 @@ export class ClientPasswordStrategyFactoryProvider const verifyFn = verifierPassed ?? this.verifier; if (options?.passReqToCallback) { return new ClientPasswordStrategy.Strategy( - options, - // eslint-disable-next-line @typescript-eslint/no-misused-promises async ( - req: Request, clientId: string, - clientSecret: string, - cb: (err: Error | null, client?: IAuthClient | false) => void, + clientSecret: string | undefined, + cb: (err: Error | null, client?: IAuthClient | null) => void, + req: Request | undefined, ) => { try { const client = await verifyFn(clientId, clientSecret, req); - if (!client) { - throw new HttpErrors.Unauthorized(AuthErrorKeys.ClientInvalid); - } else if (!clientSecret) { - throw new HttpErrors.Unauthorized( - AuthErrorKeys.ClientSecretMissing, - ); - } else if ( - !client.clientSecret || - client.clientSecret !== clientSecret - ) { - throw new HttpErrors.Unauthorized( - AuthErrorKeys.ClientVerificationFailed, - ); - } + this.clientPasswordVerifierHelper(client, clientSecret); cb(null, client); } catch (err) { cb(err); } }, + options, ); } else { return new ClientPasswordStrategy.Strategy( // eslint-disable-next-line @typescript-eslint/no-misused-promises async ( clientId: string, - clientSecret: string, - cb: (err: Error | null, client?: IAuthClient | false) => void, + clientSecret: string | undefined, + cb: (err: Error | null, client?: IAuthClient | null) => void, ) => { try { const client = await verifyFn(clientId, clientSecret); - if (!client) { - throw new HttpErrors.Unauthorized(AuthErrorKeys.ClientInvalid); - } else if (!clientSecret) { - throw new HttpErrors.Unauthorized( - AuthErrorKeys.ClientSecretMissing, - ); - } else if ( - !client.clientSecret || - client.clientSecret !== clientSecret - ) { - throw new HttpErrors.Unauthorized( - AuthErrorKeys.ClientVerificationFailed, - ); - } + this.clientPasswordVerifierHelper(client, clientSecret); cb(null, client); } catch (err) { cb(err); diff --git a/src/strategies/passport/passport-client-password/client-password-strategy.ts b/src/strategies/passport/passport-client-password/client-password-strategy.ts new file mode 100644 index 0000000..e9f3253 --- /dev/null +++ b/src/strategies/passport/passport-client-password/client-password-strategy.ts @@ -0,0 +1,79 @@ +// Type definitions for passport-oauth2-client-password 0.1.2 +// Project: https://github.com/jaredhanson/passport-oauth2-client-password +// Definitions by: Ivan Zubok +// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped +// TypeScript Version: 2.3 + +import * as passport from 'passport'; +import * as express from 'express'; +import {IAuthClient, IAuthSecureClient} from '../../../types'; + +export interface StrategyOptionsWithRequestInterface { + passReqToCallback: boolean; +} + +export interface VerifyFunctionWithRequest { + ( + clientId: string, + clientSecret: string | undefined, + done: ( + error: Error | null, + client?: IAuthSecureClient | IAuthClient | null, + info?: Object | undefined, + ) => void, + req?: express.Request, + ): void; +} + +export class Strategy extends passport.Strategy { + constructor( + verify: VerifyFunctionWithRequest, + options?: StrategyOptionsWithRequestInterface, + ) { + super(); + if (!verify) + throw new Error( + 'OAuth 2.0 client password strategy requires a verify function', + ); + + this.verify = verify; + if (options) this.passReqToCallback = options.passReqToCallback; + this.name = 'oauth2-client-password'; + } + + private readonly verify: VerifyFunctionWithRequest; + private readonly passReqToCallback: boolean; + name: string; + authenticate(req: express.Request, options?: {}): void { + if ( + /* eslint-disable @typescript-eslint/prefer-optional-chain */ + !req.body || + !req.body['client_id'] + ) { + return this.fail(); + } + + const clientId = req.body['client_id']; + const clientSecret = req.body['client_secret']; + + const verified = ( + err: Error | null, + client: IAuthSecureClient | IAuthClient | null | undefined, + info: Object | undefined, + ) => { + if (err) { + return this.error(err); + } + if (!client) { + return this.fail(); + } + this.success(client, info); + }; + + if (this.passReqToCallback) { + this.verify(clientId, clientSecret, verified, req); + } else { + this.verify(clientId, clientSecret, verified); + } + } +} diff --git a/src/strategies/passport/passport-client-password/index.ts b/src/strategies/passport/passport-client-password/index.ts index 9e23b38..bb291b1 100644 --- a/src/strategies/passport/passport-client-password/index.ts +++ b/src/strategies/passport/passport-client-password/index.ts @@ -1,2 +1,3 @@ export * from './client-password-verify.provider'; export * from './client-password-strategy-factory-provider'; +export * from './client-password-strategy'; diff --git a/src/strategies/passport/passport-client-password/secure-client-password-strategy-factory-provider.ts b/src/strategies/passport/passport-client-password/secure-client-password-strategy-factory-provider.ts index 5f9c81c..686101b 100644 --- a/src/strategies/passport/passport-client-password/secure-client-password-strategy-factory-provider.ts +++ b/src/strategies/passport/passport-client-password/secure-client-password-strategy-factory-provider.ts @@ -3,7 +3,7 @@ import {HttpErrors, Request} from '@loopback/rest'; import * as ClientPasswordStrategy from './client-password-strategy'; import {AuthErrorKeys} from '../../../error-keys'; -import {ClientType, IAuthClient, IAuthSecureClient} from '../../../types'; +import {ClientType, IAuthSecureClient} from '../../../types'; import {Strategies} from '../../keys'; import {VerifyFunction} from '../../types'; @@ -27,6 +27,26 @@ export class SecureClientPasswordStrategyFactoryProvider this.getSecureClientPasswordVerifier(options, verifier); } + secureClientPasswordVerifierHelper( + client: IAuthSecureClient | null, + clientSecret: string | undefined, + ) { + if (!client) { + throw new HttpErrors.Unauthorized(AuthErrorKeys.ClientInvalid); + } else if (client.clientType !== ClientType.public && !clientSecret) { + throw new HttpErrors.Unauthorized( + AuthErrorKeys.ConfidentialClientSecretMissing, + ); + } else if ( + client.clientType !== ClientType.public && + (!client.clientSecret || client.clientSecret !== clientSecret) + ) { + throw new HttpErrors.Unauthorized(AuthErrorKeys.ClientVerificationFailed); + } else { + // do nothing + } + } + getSecureClientPasswordVerifier( options?: ClientPasswordStrategy.StrategyOptionsWithRequestInterface, verifierPassed?: VerifyFunction.OauthSecureClientPasswordFn, @@ -38,31 +58,12 @@ export class SecureClientPasswordStrategyFactoryProvider async ( clientId: string, clientSecret: string | undefined, - cb: (err: Error | null, client?: IAuthSecureClient | false) => void, + cb: (err: Error | null, client?: IAuthSecureClient | null) => void, req: Request | undefined, ) => { try { const client = await verifyFn(clientId, clientSecret, req); - if (!client) { - throw new HttpErrors.Unauthorized(AuthErrorKeys.ClientInvalid); - } else if ( - client.clientType !== ClientType.public && - !clientSecret - ) { - throw new HttpErrors.Unauthorized( - AuthErrorKeys.ConfidentialClientSecretMissing, - ); - } else if ( - (client.clientType !== ClientType.public && - !client.clientSecret) || - client.clientSecret !== clientSecret - ) { - throw new HttpErrors.Unauthorized( - AuthErrorKeys.ClientVerificationFailed, - ); - } else { - // do nothing - } + this.secureClientPasswordVerifierHelper(client, clientSecret); cb(null, client); } catch (err) { @@ -77,30 +78,13 @@ export class SecureClientPasswordStrategyFactoryProvider async ( clientId: string, clientSecret: string | undefined, - cb: (err: Error | null, client?: IAuthClient | false) => void, + cb: (err: Error | null, client?: IAuthSecureClient | null) => void, ) => { try { const client = await verifyFn(clientId, clientSecret); - if (!client) { - throw new HttpErrors.Unauthorized(AuthErrorKeys.ClientInvalid); - } else if ( - client.clientType !== ClientType.public && - !clientSecret - ) { - throw new HttpErrors.Unauthorized( - AuthErrorKeys.ConfidentialClientSecretMissing, - ); - } else if ( - client.clientType !== ClientType.public && - (!client.clientSecret || client.clientSecret !== clientSecret) - ) { - throw new HttpErrors.Unauthorized( - AuthErrorKeys.ClientVerificationFailed, - ); - } else { - // do nothing - } + this.secureClientPasswordVerifierHelper(client, clientSecret); + cb(null, client); } catch (err) { cb(err); diff --git a/src/types.ts b/src/types.ts index 2b3e4c4..29906d9 100644 --- a/src/types.ts +++ b/src/types.ts @@ -61,3 +61,8 @@ export interface AuthenticationConfig { useClientAuthenticationMiddleware?: boolean; useUserAuthenticationMiddleware?: boolean; } + +export enum ClientType { + public = 'public', + private = 'private', +} From 81a5d65689ad7e26630f8ea724a5970c04077072 Mon Sep 17 00:00:00 2001 From: Sunny Date: Fri, 31 Mar 2023 16:29:53 +0530 Subject: [PATCH 4/7] feat(provider): make client secret non mandatory for public clients non blocking change make client secret non mandatory for public clients non blocking change GH-153 --- package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 8b06d98..a37fc02 100644 --- a/package.json +++ b/package.json @@ -141,7 +141,8 @@ }, "@semantic-release/npm": { "npm": "^9.4.2" - } + }, + "@openapi-contrib/openapi-schema-to-json-schema": "3.2.0" }, "release": { "branches": [ From 7c634f20bef6999eca2393187c96706723d17aab Mon Sep 17 00:00:00 2001 From: Sunny Date: Mon, 3 Apr 2023 12:21:37 +0530 Subject: [PATCH 5/7] feat(provider): make single config for secure client add securClient config to AuthenticationConifg GH-153 --- src/component.ts | 6 ++---- src/keys.ts | 5 ----- src/types.ts | 5 +---- 3 files changed, 3 insertions(+), 13 deletions(-) diff --git a/src/component.ts b/src/component.ts index 4d8b371..c6ff614 100644 --- a/src/component.ts +++ b/src/component.ts @@ -48,14 +48,12 @@ import { SamlStrategyFactoryProvider, SamlVerifyProvider, } from './strategies/SAML'; -import {AuthenticationConfig, IAuthSecureClientConfig} from './types'; +import {AuthenticationConfig} from './types'; export class AuthenticationComponent implements Component { constructor( @inject(AuthenticationBindings.CONFIG, {optional: true}) private readonly config?: AuthenticationConfig, - @inject(AuthenticationBindings.SecureClientConfig, {optional: true}) - private readonly secureClientConfig?: IAuthSecureClientConfig, ) { this.providers = { [AuthenticationBindings.USER_AUTH_ACTION.key]: AuthenticateActionProvider, @@ -118,7 +116,7 @@ export class AuthenticationComponent implements Component { [Strategies.Passport.KEYCLOAK_VERIFIER.key]: KeycloakVerifyProvider, }; - if (this.secureClientConfig?.secureClient) { + if (this.config?.secureClient) { this.providers = { ...this.providers, [Strategies.Passport.CLIENT_PASSWORD_STRATEGY_FACTORY.key]: diff --git a/src/keys.ts b/src/keys.ts index 9c39535..e0d5a6f 100644 --- a/src/keys.ts +++ b/src/keys.ts @@ -8,7 +8,6 @@ import { AuthenticationMetadata, EntityWithIdentifier, IAuthClient, - IAuthSecureClientConfig, IAuthUser, } from './types'; @@ -18,10 +17,6 @@ export * from './strategies/keys'; * Binding keys used by this component. */ export namespace AuthenticationBindings { - export const SecureClientConfig = - BindingKey.create( - `auth.secure.client.config`, - ); export const USER_STRATEGY = BindingKey.create( 'sf.userAuthentication.strategy', ); diff --git a/src/types.ts b/src/types.ts index 29906d9..023cb82 100644 --- a/src/types.ts +++ b/src/types.ts @@ -21,10 +21,6 @@ export interface IAuthSecureClient { redirectUrl?: string; } -export interface IAuthSecureClientConfig { - secureClient: boolean; -} - export interface IAuthUser { id?: number | string; username: string; @@ -60,6 +56,7 @@ export interface ClientAuthCode { export interface AuthenticationConfig { useClientAuthenticationMiddleware?: boolean; useUserAuthenticationMiddleware?: boolean; + secureClient?: boolean; } export enum ClientType { From 9c6d8307b006f48f38a8f7076c06bddcab95f66b Mon Sep 17 00:00:00 2001 From: Sunny Date: Mon, 3 Apr 2023 13:17:46 +0530 Subject: [PATCH 6/7] feat(provider): some minor fixes eslint and error messages modifications some minor fixes eslint and error messages modifications GH-153 --- src/error-keys.ts | 1 - .../client-password-strategy-factory-provider.ts | 2 -- .../passport-client-password/client-password-strategy.ts | 6 +----- .../secure-client-password-strategy-factory-provider.ts | 4 ---- 4 files changed, 1 insertion(+), 12 deletions(-) diff --git a/src/error-keys.ts b/src/error-keys.ts index f861a1f..f5a071f 100644 --- a/src/error-keys.ts +++ b/src/error-keys.ts @@ -13,5 +13,4 @@ export const enum AuthErrorKeys { KeyInvalid = 'Key Invalid', OtpInvalid = 'Otp Invalid', OtpExpired = 'Otp Token Incorrect or Expired', - ConfidentialClientSecretMissing = 'Confidential Client Secret Missing', } diff --git a/src/strategies/passport/passport-client-password/client-password-strategy-factory-provider.ts b/src/strategies/passport/passport-client-password/client-password-strategy-factory-provider.ts index 005e75e..c451a49 100644 --- a/src/strategies/passport/passport-client-password/client-password-strategy-factory-provider.ts +++ b/src/strategies/passport/passport-client-password/client-password-strategy-factory-provider.ts @@ -33,8 +33,6 @@ export class ClientPasswordStrategyFactoryProvider ) { if (!client) { throw new HttpErrors.Unauthorized(AuthErrorKeys.ClientInvalid); - } else if (!clientSecret) { - throw new HttpErrors.Unauthorized(AuthErrorKeys.ClientSecretMissing); } else if (!client.clientSecret || client.clientSecret !== clientSecret) { throw new HttpErrors.Unauthorized(AuthErrorKeys.ClientVerificationFailed); } else { diff --git a/src/strategies/passport/passport-client-password/client-password-strategy.ts b/src/strategies/passport/passport-client-password/client-password-strategy.ts index e9f3253..76ce956 100644 --- a/src/strategies/passport/passport-client-password/client-password-strategy.ts +++ b/src/strategies/passport/passport-client-password/client-password-strategy.ts @@ -45,11 +45,7 @@ export class Strategy extends passport.Strategy { private readonly passReqToCallback: boolean; name: string; authenticate(req: express.Request, options?: {}): void { - if ( - /* eslint-disable @typescript-eslint/prefer-optional-chain */ - !req.body || - !req.body['client_id'] - ) { + if (!req?.body?.client_id) { return this.fail(); } diff --git a/src/strategies/passport/passport-client-password/secure-client-password-strategy-factory-provider.ts b/src/strategies/passport/passport-client-password/secure-client-password-strategy-factory-provider.ts index 686101b..7410c19 100644 --- a/src/strategies/passport/passport-client-password/secure-client-password-strategy-factory-provider.ts +++ b/src/strategies/passport/passport-client-password/secure-client-password-strategy-factory-provider.ts @@ -33,10 +33,6 @@ export class SecureClientPasswordStrategyFactoryProvider ) { if (!client) { throw new HttpErrors.Unauthorized(AuthErrorKeys.ClientInvalid); - } else if (client.clientType !== ClientType.public && !clientSecret) { - throw new HttpErrors.Unauthorized( - AuthErrorKeys.ConfidentialClientSecretMissing, - ); } else if ( client.clientType !== ClientType.public && (!client.clientSecret || client.clientSecret !== clientSecret) From f5ca8f59eb8d8ab35ff13657e119684802ecd42f Mon Sep 17 00:00:00 2001 From: Sunny Date: Mon, 3 Apr 2023 14:58:40 +0530 Subject: [PATCH 7/7] feat(provider): some minor changes some minor changes GH-153 --- .../client-password-strategy-factory-provider.ts | 4 +--- .../secure-client-password-strategy-factory-provider.ts | 9 ++++----- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/strategies/passport/passport-client-password/client-password-strategy-factory-provider.ts b/src/strategies/passport/passport-client-password/client-password-strategy-factory-provider.ts index c451a49..9788431 100644 --- a/src/strategies/passport/passport-client-password/client-password-strategy-factory-provider.ts +++ b/src/strategies/passport/passport-client-password/client-password-strategy-factory-provider.ts @@ -31,9 +31,7 @@ export class ClientPasswordStrategyFactoryProvider client: IAuthClient | null, clientSecret: string | undefined, ) { - if (!client) { - throw new HttpErrors.Unauthorized(AuthErrorKeys.ClientInvalid); - } else if (!client.clientSecret || client.clientSecret !== clientSecret) { + if (!client?.clientSecret || client.clientSecret !== clientSecret) { throw new HttpErrors.Unauthorized(AuthErrorKeys.ClientVerificationFailed); } else { // do nothing diff --git a/src/strategies/passport/passport-client-password/secure-client-password-strategy-factory-provider.ts b/src/strategies/passport/passport-client-password/secure-client-password-strategy-factory-provider.ts index 7410c19..4c99660 100644 --- a/src/strategies/passport/passport-client-password/secure-client-password-strategy-factory-provider.ts +++ b/src/strategies/passport/passport-client-password/secure-client-password-strategy-factory-provider.ts @@ -31,11 +31,10 @@ export class SecureClientPasswordStrategyFactoryProvider client: IAuthSecureClient | null, clientSecret: string | undefined, ) { - if (!client) { - throw new HttpErrors.Unauthorized(AuthErrorKeys.ClientInvalid); - } else if ( - client.clientType !== ClientType.public && - (!client.clientSecret || client.clientSecret !== clientSecret) + if ( + !client || + (client.clientType !== ClientType.public && + (!client.clientSecret || client.clientSecret !== clientSecret)) ) { throw new HttpErrors.Unauthorized(AuthErrorKeys.ClientVerificationFailed); } else {