feat(server): add optional name/limit/password props for createNewAccessKey method (#1273)

murka · web-flow · commit 57a30a2fafe7 · 2024-01-12T11:43:29.000-06:00
diff --git a/src/shadowbox/model/access_key.ts b/src/shadowbox/model/access_key.ts
@@ -48,9 +48,20 @@ export interface AccessKey {
   readonly dataLimit?: DataLimit;
 }
 
+export interface AccessKeyCreateParams {
+  // The encryption method to use for the access key.
+  readonly encryptionMethod?: string;
+  // The name to give the access key.
+  readonly name?: string;
+  // The password to use for the access key.
+  readonly password?: string;
+  // The data transfer limit to apply to the access key.
+  readonly dataLimit?: DataLimit;
+}
+
 export interface AccessKeyRepository {
   // Creates a new access key. Parameters are chosen automatically.
-  createNewAccessKey(encryptionMethod?: string): Promise<AccessKey>;
+  createNewAccessKey(params?: AccessKeyCreateParams): Promise<AccessKey>;
   // Removes the access key given its id. Throws on failure.
   removeAccessKey(id: AccessKeyId);
   // Returns the access key with the given id. Throws on failure.
diff --git a/src/shadowbox/server/api.yml b/src/shadowbox/server/api.yml
@@ -146,11 +146,19 @@ paths:
             schema:
               type: object
               properties:
+                name:
+                  type: string
                 method:
                   type: string
+                password:
+                  type: string
+                port:
+                  type: integer
+                dataLimit:
+                  $ref: "#/components/schemas/DataLimit"
             examples:
               '0':
-                value: '{"method":"aes-192-gcm"}'
+                value: '{"method":"aes-192-gcm","name":"First","password":"8iu8V8EeoFVpwQvQeS9wiD","limit":{"bytes":10000}}'
       responses:
         '201':
           description: The newly created access key
diff --git a/src/shadowbox/server/manager_service.spec.ts b/src/shadowbox/server/manager_service.spec.ts
@@ -337,6 +337,84 @@ describe('ShadowsocksManagerService', () => {
       };
       service.createNewAccessKey({params: {method: 'aes-256-gcm'}}, res, done);
     });
+    it('use default name is params is not defined', (done) => {
+      const repo = getAccessKeyRepository();
+      const service = new ShadowsocksManagerServiceBuilder().accessKeys(repo).build();
+
+      const res = {
+        send: (httpCode, data) => {
+          expect(httpCode).toEqual(201);
+          expect(data.name).toEqual('');
+          responseProcessed = true; // required for afterEach to pass.
+        },
+      };
+      service.createNewAccessKey({params: {}}, res, done);
+    });
+    it('rejects non-string name', (done) => {
+      const repo = getAccessKeyRepository();
+      const service = new ShadowsocksManagerServiceBuilder().accessKeys(repo).build();
+
+      const res = {send: (_httpCode, _data) => {}};
+      service.createNewAccessKey({params: {name: Number('9876')}}, res, (error) => {
+        expect(error.statusCode).toEqual(400);
+        responseProcessed = true; // required for afterEach to pass.
+        done();
+      });
+    });
+    it('defined name is equal to stored', (done) => {
+      const repo = getAccessKeyRepository();
+      const service = new ShadowsocksManagerServiceBuilder().accessKeys(repo).build();
+
+      const ACCESSKEY_NAME = 'accesskeyname';
+      const res = {
+        send: (httpCode, data) => {
+          expect(httpCode).toEqual(201);
+          expect(data.name).toEqual(ACCESSKEY_NAME);
+          responseProcessed = true; // required for afterEach to pass.
+        },
+      };
+      service.createNewAccessKey({params: {name: ACCESSKEY_NAME}}, res, done);
+    });
+    it('limit can be undefined', (done) => {
+      const repo = getAccessKeyRepository();
+      const service = new ShadowsocksManagerServiceBuilder().accessKeys(repo).build();
+
+      const res = {
+        send: (httpCode, data) => {
+          expect(httpCode).toEqual(201);
+          expect(data.limit).toBeUndefined();
+          responseProcessed = true; // required for afterEach to pass.
+        },
+      };
+      service.createNewAccessKey({params: {}}, res, done);
+    });
+    it('rejects non-numeric limits', (done) => {
+      const repo = getAccessKeyRepository();
+      const service = new ShadowsocksManagerServiceBuilder().accessKeys(repo).build();
+
+      const ACCESSKEY_LIMIT = {bytes: '9876'};
+
+      const res = {send: (_httpCode, _data) => {}};
+      service.createNewAccessKey({params: {limit: ACCESSKEY_LIMIT}}, res, (error) => {
+        expect(error.statusCode).toEqual(400);
+        responseProcessed = true; // required for afterEach to pass.
+        done();
+      });
+    });
+    it('defined limit is equal to stored', (done) => {
+      const repo = getAccessKeyRepository();
+      const service = new ShadowsocksManagerServiceBuilder().accessKeys(repo).build();
+
+      const ACCESSKEY_LIMIT = {bytes: 9876};
+      const res = {
+        send: (httpCode, data) => {
+          expect(httpCode).toEqual(201);
+          expect(data.dataLimit).toEqual(ACCESSKEY_LIMIT);
+          responseProcessed = true; // required for afterEach to pass.
+        },
+      };
+      service.createNewAccessKey({params: {limit: ACCESSKEY_LIMIT}}, res, done);
+    });
     it('method must be of type string', (done) => {
       const repo = getAccessKeyRepository();
       const service = new ShadowsocksManagerServiceBuilder().accessKeys(repo).build();
@@ -371,6 +449,47 @@ describe('ShadowsocksManagerService', () => {
         done();
       });
     });
+
+    it('generates a new password when no password is provided', async (done) => {
+      const repo = getAccessKeyRepository();
+      const service = new ShadowsocksManagerServiceBuilder().accessKeys(repo).build();
+
+      const res = {
+        send: (httpCode, data) => {
+          expect(httpCode).toEqual(201);
+          expect(data.password).toBeDefined();
+          responseProcessed = true; // required for afterEach to pass.
+        },
+      };
+      await service.createNewAccessKey({params: {}}, res, done);
+    });
+
+    it('uses the provided password when one is provided', async (done) => {
+      const repo = getAccessKeyRepository();
+      const service = new ShadowsocksManagerServiceBuilder().accessKeys(repo).build();
+
+      const PASSWORD = '8iu8V8EeoFVpwQvQeS9wiD';
+      const res = {
+        send: (httpCode, data) => {
+          expect(httpCode).toEqual(201);
+          expect(data.password).toEqual(PASSWORD);
+          responseProcessed = true; // required for afterEach to pass.
+        },
+      };
+      await service.createNewAccessKey({params: {password: PASSWORD}}, res, done);
+    });
+
+    it('rejects a password that is not a string', async (done) => {
+      const repo = getAccessKeyRepository();
+      const service = new ShadowsocksManagerServiceBuilder().accessKeys(repo).build();
+      const PASSWORD = Number.MAX_SAFE_INTEGER;
+      const res = {send: SEND_NOTHING};
+      await service.createNewAccessKey({params: {password: PASSWORD}}, res, (error) => {
+        expect(error.statusCode).toEqual(400);
+        responseProcessed = true; // required for afterEach to pass.
+        done();
+      });
+    });
   });
   describe('setPortForNewAccessKeys', () => {
     it('changes ports for new access keys', async (done) => {
diff --git a/src/shadowbox/server/manager_service.ts b/src/shadowbox/server/manager_service.ts
@@ -179,9 +179,10 @@ function validateAccessKeyId(accessKeyId: unknown): string {
 }
 
 function validateDataLimit(limit: unknown): DataLimit {
-  if (!limit) {
-    throw new restifyErrors.MissingParameterError({statusCode: 400}, 'Missing `limit` parameter');
+  if (typeof limit === 'undefined') {
+    return undefined;
   }
+
   const bytes = (limit as DataLimit).bytes;
   if (!(Number.isInteger(bytes) && bytes >= 0)) {
     throw new restifyErrors.InvalidArgumentError(
@@ -192,6 +193,20 @@ function validateDataLimit(limit: unknown): DataLimit {
   return limit as DataLimit;
 }
 
+function validateStringParam(param: unknown, paramName: string): string {
+  if (typeof param === 'undefined') {
+    return undefined;
+  }
+
+  if (typeof param !== 'string') {
+    throw new restifyErrors.InvalidArgumentError(
+      {statusCode: 400},
+      `Expected a string for ${paramName}, instead got ${param} of type ${typeof param}`
+    );
+  }
+  return param;
+}
+
 // The ShadowsocksManagerService manages the access keys that can use the server
 // as a proxy using Shadowsocks. It runs an instance of the Shadowsocks server
 // for each existing access key, with the port and password assigned for that access key.
@@ -311,28 +326,40 @@ export class ShadowsocksManagerService {
   }
 
   // Creates a new access key
-  public async createNewAccessKey(req: RequestType, res: ResponseType, next: restify.Next): Promise<void> {
+  public async createNewAccessKey(
+    req: RequestType,
+    res: ResponseType,
+    next: restify.Next
+  ): Promise<void> {
     try {
       logging.debug(`createNewAccessKey request ${JSON.stringify(req.params)}`);
-      let encryptionMethod = req.params.method;
-      if (!encryptionMethod) {
-        encryptionMethod = '';
-      }
-      if (typeof encryptionMethod !== 'string') {
-        return next(new restifyErrors.InvalidArgumentError(
-            {statusCode: 400},
-            `Expected a string encryptionMethod, instead got ${encryptionMethod} of type ${
-                typeof encryptionMethod}`));
-      }
-      const accessKeyJson = accessKeyToApiJson(await this.accessKeys.createNewAccessKey(encryptionMethod));
+      const encryptionMethod = validateStringParam(req.params.method || '', 'encryptionMethod');
+      const name = validateStringParam(req.params.name || '', 'name');
+      const dataLimit = validateDataLimit(req.params.limit);
+      const password = validateStringParam(req.params.password, 'password');
+
+      const accessKeyJson = accessKeyToApiJson(
+        await this.accessKeys.createNewAccessKey({
+          encryptionMethod,
+          name,
+          dataLimit,
+          password,
+        })
+      );
       res.send(201, accessKeyJson);
       logging.debug(`createNewAccessKey response ${JSON.stringify(accessKeyJson)}`);
       return next();
-    } catch(error) {
+    } catch (error) {
       logging.error(error);
       if (error instanceof errors.InvalidCipher) {
         return next(new restifyErrors.InvalidArgumentError({statusCode: 400}, error.message));
       }
+      if (
+        error instanceof restifyErrors.InvalidArgumentError ||
+        error instanceof restifyErrors.MissingParameterError
+      ) {
+        return next(error);
+      }
       return next(new restifyErrors.InternalServerError());
     }
   }
diff --git a/src/shadowbox/server/server_access_key.spec.ts b/src/shadowbox/server/server_access_key.spec.ts
@@ -60,7 +60,7 @@ describe('ServerAccessKeyRepository', () => {
 
   it('New access keys sees the encryption method correctly', (done) => {
     const repo = new RepoBuilder().build();
-    repo.createNewAccessKey('aes-256-gcm').then((accessKey) => {
+    repo.createNewAccessKey({encryptionMethod: 'aes-256-gcm'}).then((accessKey) => {
       expect(accessKey).toBeDefined();
       expect(accessKey.proxyParams.encryptionMethod).toEqual('aes-256-gcm');
       done();
diff --git a/src/shadowbox/server/server_access_key.ts b/src/shadowbox/server/server_access_key.ts
@@ -22,6 +22,7 @@ import * as logging from '../infrastructure/logging';
 import {PrometheusClient} from '../infrastructure/prometheus_scraper';
 import {
   AccessKey,
+  AccessKeyCreateParams,
   AccessKeyId,
   AccessKeyMetricsId,
   AccessKeyRepository,
@@ -97,10 +98,12 @@ function accessKeyToStorageJson(accessKey: AccessKey): AccessKeyStorageJson {
 }
 
 function isValidCipher(cipher: string): boolean {
-    if (["aes-256-gcm", "aes-192-gcm", "aes-128-gcm", "chacha20-ietf-poly1305"].indexOf(cipher) === -1) {
-      return false;
-    }
-    return true;
+  if (
+    ['aes-256-gcm', 'aes-192-gcm', 'aes-128-gcm', 'chacha20-ietf-poly1305'].indexOf(cipher) === -1
+  ) {
+    return false;
+  }
+  return true;
 }
 
 // AccessKeyRepository that keeps its state in a config file and uses ShadowsocksServer
@@ -166,12 +169,13 @@ export class ServerAccessKeyRepository implements AccessKeyRepository {
     this.portForNewAccessKeys = port;
   }
 
-  async createNewAccessKey(encryptionMethod?: string): Promise<AccessKey> {
+  async createNewAccessKey(params?: AccessKeyCreateParams): Promise<AccessKey> {
     const id = this.keyConfig.data().nextId.toString();
     this.keyConfig.data().nextId += 1;
     const metricsId = uuidv4();
-    const password = generatePassword();
-    encryptionMethod = encryptionMethod || this.NEW_USER_ENCRYPTION_METHOD;
+    const password = params?.password ?? generatePassword();
+    const encryptionMethod = params?.encryptionMethod || this.NEW_USER_ENCRYPTION_METHOD;
+
     // Validate encryption method.
     if (!isValidCipher(encryptionMethod)) {
       throw new errors.InvalidCipher(encryptionMethod);
@@ -182,7 +186,9 @@ export class ServerAccessKeyRepository implements AccessKeyRepository {
       encryptionMethod,
       password,
     };
-    const accessKey = new ServerAccessKey(id, '', metricsId, proxyParams);
+    const name = params?.name ?? '';
+    const dataLimit = params?.dataLimit;
+    const accessKey = new ServerAccessKey(id, name, metricsId, proxyParams, dataLimit);
     this.accessKeys.push(accessKey);
     this.saveAccessKeys();
     await this.updateServer();
