fix(security): security client aliases are not unique (#208)

ConduitPlatform · Jun 16, 2022 · 90b4545 · 90b4545
1 parent 012dff3
commit 90b4545
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 45 deletions.
diff --git a/packages/security/src/admin/routes/CreateSecurityClient.route.ts b/packages/security/src/admin/routes/CreateSecurityClient.route.ts
@@ -25,23 +25,30 @@ export function getCreateSecurityClientRoute() {
         notes: ConduitString.Optional,
       },
     },
-    new ConduitRouteReturnDefinition('CreateSecurityClient', {
-      id: ConduitString.Required,
-      clientId: ConduitString.Required,
-      clientSecret: ConduitString.Required,
-      platform: ConduitString.Required,
-      domain: ConduitString.Optional,
-      alias: ConduitString.Optional,
-      notes: ConduitString.Optional,
-    }),
+    new ConduitRouteReturnDefinition('CreateSecurityClient', Client.getInstance().fields),
     async (params: ConduitRouteParameters) => {
-      const { platform, domain, alias, notes } = params.params!;
+      const { platform, domain, notes } = params.params!;
+      let { alias } = params.params!;
       if (!Object.values(PlatformTypesEnum).includes(platform)) {
         throw new ConduitError('INVALID_ARGUMENTS', 400, 'Platform not supported');
       }
-      const clientId = randomBytes(15).toString('hex');
-      const clientSecret = randomBytes(64).toString('hex');
-      const hash = await bcrypt.hash(clientSecret, 10);
+      if (alias === '') {
+        throw new ConduitError(
+          'INVALID_ARGUMENTS',
+          400,
+          'Non-null alias field should not be an empty string',
+        );
+      }
+      if (alias) {
+        const existingClient = await Client.getInstance().findOne({ alias });
+        if (existingClient) {
+          throw new ConduitError(
+            'ALREADY_EXISTS',
+            409,
+            `A security client with an alias of '${alias}' already exists`,
+          );
+        }
+      }
       if (platform === PlatformTypesEnum.WEB) {
         if (!domain || domain === '')
           throw new ConduitError(
@@ -66,6 +73,14 @@ export function getCreateSecurityClientRoute() {
         if (!domainPattern.test(comparedDomain) && domain !== '*')
           throw new ConduitError('INVALID_ARGUMENTS', 400, 'Invalid domain argument');
       }
+      const clientId = randomBytes(15).toString('hex');
+      const clientSecret = randomBytes(64).toString('hex');
+      const hash = await bcrypt.hash(clientSecret, 10);
+      if (!alias) {
+        alias = `${platform.toLowerCase()}:${
+          platform === 'WEB' ? `${domain}:${clientId}` : clientId
+        }`;
+      }
       const client = await Client.getInstance().create({
         clientId,
         clientSecret: hash,
@@ -75,15 +90,7 @@ export function getCreateSecurityClientRoute() {
         notes,
       });
       return {
-        result: {
-          id: client._id,
-          clientId,
-          clientSecret,
-          platform,
-          domain,
-          alias,
-          notes,
-        },
+        result: { ...client, clientSecret },
       }; // unnested from result in Rest.addConduitRoute, grpc routes avoid this using wrapRouterGrpcFunction
     },
   );

diff --git a/packages/security/src/admin/routes/GetSecurityClients.route.ts b/packages/security/src/admin/routes/GetSecurityClients.route.ts
@@ -3,13 +3,14 @@ import { Client } from '../../models';
 import { ConduitRouteActions, ConduitRouteParameters } from '@conduitplatform/grpc-sdk';
 
 export function getGetSecurityClientsRoute() {
+  const { clientSecret, ...returnTypeFields } = Client.getInstance().fields;
   return new ConduitRoute(
     {
       path: '/security/client',
       action: ConduitRouteActions.GET,
     },
     new ConduitRouteReturnDefinition('GetSecurityClient', {
-      clients: [Client.getInstance().fields],
+      clients: [returnTypeFields],
     }),
     async (params: ConduitRouteParameters) => {
       const clients = await Client.getInstance().findMany({});

diff --git a/packages/security/src/admin/routes/UpdateSecurityClient.route.ts b/packages/security/src/admin/routes/UpdateSecurityClient.route.ts
@@ -8,12 +8,12 @@ import {
   ConduitRouteActions,
   ConduitRouteParameters,
   ConduitString,
-  RouteOptionType,
 } from '@conduitplatform/grpc-sdk';
 import { Client } from '../../models';
 import { isNil } from 'lodash';
 
 export function getUpdateSecurityClientRoute() {
+  const { clientSecret, ...returnTypeFields } = Client.getInstance().fields;
   return new ConduitRoute(
     {
       path: '/security/client/:id',
@@ -27,14 +27,7 @@ export function getUpdateSecurityClientRoute() {
         notes: ConduitString.Optional,
       },
     },
-    new ConduitRouteReturnDefinition('UpdateSecurityClient', {
-      id: ConduitString.Required,
-      clientId: ConduitString.Required,
-      platform: ConduitString.Optional,
-      domain: ConduitString.Optional,
-      alias: ConduitString.Optional,
-      notes: ConduitString.Optional,
-    }),
+    new ConduitRouteReturnDefinition('UpdateSecurityClient', returnTypeFields),
     async (params: ConduitRouteParameters) => {
       const { domain, alias, notes } = params.params!;
       let client = await Client.getInstance().findOne({
@@ -43,6 +36,23 @@ export function getUpdateSecurityClientRoute() {
       if (isNil(client)) {
         throw new ConduitError('INVALID_PARAMS', 400, 'Security client not found');
       }
+      if (alias === '') {
+        throw new ConduitError(
+          'INVALID_ARGUMENTS',
+          400,
+          'Non-null alias field should not be an empty string',
+        );
+      }
+      if (alias && alias !== client.alias) {
+        const existingClient = await Client.getInstance().findOne({ alias });
+        if (existingClient) {
+          throw new ConduitError(
+            'ALREADY_EXISTS',
+            409,
+            `A security client with an alias of '${alias}' already exists`,
+          );
+        }
+      }
       if (client.platform === PlatformTypesEnum.WEB) {
         if (!domain || domain === '')
           throw new ConduitError(
@@ -67,21 +77,14 @@ export function getUpdateSecurityClientRoute() {
         if (!domainPattern.test(comparedDomain) && domain !== '*')
           throw new ConduitError('INVALID_ARGUMENTS', 400, 'Invalid domain argument');
       }
-
       client = await Client.getInstance().findByIdAndUpdate(client._id, {
         domain: client.platform === PlatformTypesEnum.WEB ? domain : undefined,
         alias,
         notes,
       });
       return {
-        result: {
-          id: client!._id,
-          clientId: client!.clientId,
-          domain: client!.platform === PlatformTypesEnum.WEB ? domain : undefined,
-          alias,
-          notes,
-        },
-      };
+        result: client,
+      }; // unnested from result in Rest.addConduitRoute, grpc routes avoid this using wrapRouterGrpcFunction
     },
   );
 }
diff --git a/packages/security/src/interfaces/ValidationInterface.ts b/packages/security/src/interfaces/ValidationInterface.ts
@@ -2,7 +2,7 @@ export interface ValidationInterface {
   validated?: boolean;
   client?: {
     platform: string;
-    domain: string;
+    domain?: string;
     clientSecret: string;
   };
 }
diff --git a/packages/security/src/models/Client.schema.ts b/packages/security/src/models/Client.schema.ts
@@ -15,7 +15,8 @@ const schema = {
   },
   alias: {
     type: TYPE.String,
-    required: false,
+    unique: true,
+    required: true,
   },
   notes: {
     type: TYPE.String,
@@ -51,7 +52,9 @@ export class Client extends ConduitActiveSchema<Client> {
   _id!: string;
   clientId!: string;
   clientSecret!: string;
-  domain!: string;
+  alias?: string;
+  notes?: string;
+  domain?: string;
   platform!: string;
   createdAt!: Date;
   updatedAt!: Date;

diff --git a/packages/security/src/utils/security.ts b/packages/security/src/utils/security.ts
@@ -6,7 +6,7 @@ export async function validateClient(
   req: Request,
   client: {
     platform: string;
-    domain: string;
+    domain?: string;
     clientSecret: string;
   },
   fromRedis: boolean,