Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
6f1ae95
feat: add optional name and dataLimit prop for createNewAccessKey method
murka Jan 24, 2023
9a5b0e6
fix: add error catching
murka Jan 24, 2023
1ab3d5f
test: name and limit/dataLimit optional props
murka Jan 24, 2023
e2c45d3
feat: use interface instead of params
murka Jan 26, 2023
09f88bf
fix: validate only if limit is defined
murka Jan 26, 2023
f34c841
ci: fix query-filters
murka Mar 8, 2023
9512ca3
ci: fix codeql workflow
murka Mar 8, 2023
f39b453
ci(codeql): security and quality queries
murka Mar 8, 2023
baf0843
Merge branch 'Jigsaw-Code:master' into feat-createNewAccessKey-name-d…
murka Jul 19, 2023
7384a91
Merge branch 'Jigsaw-Code:master' into master
murka Jul 19, 2023
e29d82f
Merge branch 'feat-createNewAccessKey-name-datalimit'
murka Jul 19, 2023
032b0fd
Merge branch 'feat-createNewAccessKey-name-datalimit'
murka Jul 27, 2023
62eab04
fix(shadowbox): use optional chaining for check params defined
murka Jul 27, 2023
9c295b4
fix(shadowbox): check if encryptionMethod is empty string
murka Jul 27, 2023
a5e45b2
Merge branch 'Jigsaw-Code:master' into master
murka Dec 26, 2023
22554db
feat(shadowbox): provide the password prop on createNewAccessKey
murka Dec 27, 2023
aacd645
Merge pull request #1 from murka/feat-password-createNewAccessKey
murka Dec 27, 2023
450d58c
Update src/shadowbox/server/manager_service.ts
murka Dec 30, 2023
49b13a6
Update src/shadowbox/server/manager_service.ts
murka Dec 30, 2023
199d5f6
fix: remove odd static props
murka Dec 30, 2023
476bd23
fix: dataLimit validation function to get the types of object
murka Dec 30, 2023
d024091
fix: validatePassword prop maybe is non-string
murka Dec 30, 2023
455991a
fix: revert removed prop
murka Dec 30, 2023
ada6d38
style: use condition chain for check prop
murka Dec 30, 2023
36c6a0c
fix: params check for dataLimit prop
murka Dec 30, 2023
f0fc931
fix: remove double password prop validation
murka Dec 30, 2023
21fb92c
docs: add properties for api schema
murka Dec 30, 2023
09cda03
Update manager_service.ts
murka Jan 2, 2024
ab03340
Update src/shadowbox/server/manager_service.ts
murka Jan 10, 2024
727bcd0
perf(shadownbox): remove checking password bytes lenght
murka Jan 11, 2024
6a5b8c3
refactor: validating params system
murka Jan 11, 2024
c92f489
refactor: validate unknown string params
murka Jan 11, 2024
d53542d
chore: remove unusable test
murka Jan 11, 2024
9afefff
fix: skip to validating undefined param
murka Jan 11, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 12 additions & 1 deletion src/shadowbox/model/access_key.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,20 @@ export interface AccessKey {
readonly dataLimit?: DataLimit;
}

export interface AccessKeyCreateParams {
// The encryption method to use for the access key.
fortuna marked this conversation as resolved.
Show resolved Hide resolved
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.
Expand Down
10 changes: 9 additions & 1 deletion src/shadowbox/server/api.yml
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,19 @@ paths:
schema:
type: object
properties:
name:
type: string
method:
type: string
murka marked this conversation as resolved.
Show resolved Hide resolved
password:
type: string
port:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think port was not implemented as part of this PR

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jadolg I think you are right. You can still set the port for new access keys if you want to do that, but we should add it to the key creation API as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll send a PR to add this.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also review the PUT method. I think it's wired to the same method so it should not work either but the docs claim it does.

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
Expand Down
119 changes: 119 additions & 0 deletions src/shadowbox/server/manager_service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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) => {
Expand Down
57 changes: 42 additions & 15 deletions src/shadowbox/server/manager_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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.
Expand Down Expand Up @@ -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({
fortuna marked this conversation as resolved.
Show resolved Hide resolved
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());
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/shadowbox/server/server_access_key.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
22 changes: 14 additions & 8 deletions src/shadowbox/server/server_access_key.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import * as logging from '../infrastructure/logging';
import {PrometheusClient} from '../infrastructure/prometheus_scraper';
import {
AccessKey,
AccessKeyCreateParams,
AccessKeyId,
AccessKeyMetricsId,
AccessKeyRepository,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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();
Expand Down
Loading