feat(provider): make client secret non mandatory for public clients (#…

…154) * feat(provider): added clientType to auth client making client secret non mandatory for public clients GH-153
sourcefuse · Apr 3, 2023 · 039fa8c · 039fa8c
1 parent 13b67ac
commit 039fa8c
Show file tree

Hide file tree

Showing 8 changed files with 230 additions and 30 deletions.
diff --git 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": [

diff --git 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,
@@ -114,6 +115,14 @@ export class AuthenticationComponent implements Component {
       [Strategies.Passport.AZURE_AD_VERIFIER.key]: AzureADAuthVerifyProvider,
       [Strategies.Passport.KEYCLOAK_VERIFIER.key]: KeycloakVerifyProvider,
     };
+
+    if (this.config?.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/...strategies/passport/passport-client-password/client-password-strategy-factory-provider.ts b/...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,60 +27,52 @@ export class ClientPasswordStrategyFactoryProvider
       this.getClientPasswordVerifier(options, verifier);
   }
 
+  clientPasswordVerifierHelper(
+    client: IAuthClient | null,
+    clientSecret: string | undefined,
+  ) {
+    if (!client?.clientSecret || client.clientSecret !== clientSecret) {
+      throw new HttpErrors.Unauthorized(AuthErrorKeys.ClientVerificationFailed);
+    } else {
+      // do nothing
+    }
+  }
+
   getClientPasswordVerifier(
     options?: ClientPasswordStrategy.StrategyOptionsWithRequestInterface,
     verifierPassed?: VerifyFunction.OauthClientPasswordFn,
   ): ClientPasswordStrategy.Strategy {
     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 (
-              !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 (
-              !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
@@ -0,0 +1,75 @@
+// Type definitions for passport-oauth2-client-password 0.1.2
+// Project: https://github.com/jaredhanson/passport-oauth2-client-password
+// Definitions by: Ivan Zubok <https://github.com/akaNightmare>
+// 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 (!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
@@ -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/...ies/passport/passport-client-password/secure-client-password-strategy-factory-provider.ts b/...ies/passport/passport-client-password/secure-client-password-strategy-factory-provider.ts
@@ -0,0 +1,91 @@
+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, 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<SecureClientPasswordStrategyFactory>
+{
+  constructor(
+    @inject(Strategies.Passport.OAUTH2_CLIENT_PASSWORD_VERIFIER)
+    private readonly verifier: VerifyFunction.OauthSecureClientPasswordFn,
+  ) {}
+
+  value(): SecureClientPasswordStrategyFactory {
+    return (options, verifier) =>
+      this.getSecureClientPasswordVerifier(options, verifier);
+  }
+
+  secureClientPasswordVerifierHelper(
+    client: IAuthSecureClient | null,
+    clientSecret: string | undefined,
+  ) {
+    if (
+      !client ||
+      (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,
+  ): 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 | null) => void,
+          req: Request | undefined,
+        ) => {
+          try {
+            const client = await verifyFn(clientId, clientSecret, req);
+            this.secureClientPasswordVerifierHelper(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 | undefined,
+          cb: (err: Error | null, client?: IAuthSecureClient | null) => void,
+        ) => {
+          try {
+            const client = await verifyFn(clientId, clientSecret);
+
+            this.secureClientPasswordVerifierHelper(client, clientSecret);
+
+            cb(null, client);
+          } catch (err) {
+            cb(err);
+          }
+        },
+      );
+    }
+  }
+}
diff --git 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<T | null>;
   }
 
+  export interface OauthSecureClientPasswordFn<T = IAuthSecureClient>
+    extends GenericAuthFn<T> {
+    (clientId: string, clientSecret: string, req?: Request): Promise<T | null>;
+  }
+
   export interface LocalPasswordFn<T = IAuthUser> extends GenericAuthFn<T> {
     (username: string, password: string, req?: Request): Promise<T | null>;
   }
@@ -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<T = IAuthUser> extends GenericAuthFn<T> {
     (
       accessToken: string,

diff --git a/src/types.ts b/src/types.ts
@@ -14,6 +14,13 @@ export interface IAuthClient {
   redirectUrl?: string;
 }
 
+export interface IAuthSecureClient {
+  clientId: string;
+  clientSecret: string;
+  clientType: ClientType;
+  redirectUrl?: string;
+}
+
 export interface IAuthUser {
   id?: number | string;
   username: string;
@@ -49,4 +56,10 @@ export interface ClientAuthCode<T extends IAuthUser, ID = number> {
 export interface AuthenticationConfig {
   useClientAuthenticationMiddleware?: boolean;
   useUserAuthenticationMiddleware?: boolean;
+  secureClient?: boolean;
+}
+
+export enum ClientType {
+  public = 'public',
+  private = 'private',
 }