Skip to content

Commit 8764776

Browse files
committed
Simplify API key.
An API key optionally belong to a service.
1 parent 3b16519 commit 8764776

File tree

13 files changed

+135
-175
lines changed

13 files changed

+135
-175
lines changed

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
"test:e2e": "jest --config ./test/jest-e2e.json"
2121
},
2222
"dependencies": {
23-
"@letsflow/core": "^1.0.4",
23+
"@letsflow/core": "^1.2.4",
2424
"@nestjs/common": "^10.3.9",
2525
"@nestjs/core": "^10.3.9",
2626
"@nestjs/event-emitter": "^3.0.0",

src/apikey/apikey.dto.ts

Lines changed: 5 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -34,35 +34,13 @@ export class ApiKey {
3434
revoked?: Date;
3535

3636
@ApiProperty({
37-
enum: privileges,
3837
type: 'array',
39-
items: { type: 'string' },
38+
items: { type: 'string', enum: privileges as any },
4039
})
4140
privileges: Array<Privilege>;
4241

43-
@ApiProperty({
44-
type: 'array',
45-
items: {
46-
type: 'object',
47-
properties: {
48-
scenario: { type: 'string', format: 'uuid' },
49-
actors: {
50-
type: 'array',
51-
items: { type: 'string' },
52-
},
53-
actions: {
54-
type: 'array',
55-
items: { type: 'string' },
56-
},
57-
},
58-
required: ['scenario'],
59-
},
60-
})
61-
processes?: Array<{
62-
scenario: string;
63-
actors?: string[];
64-
actions?: string[];
65-
}>;
42+
@ApiProperty()
43+
service?: string;
6644

6745
isActive(): boolean {
6846
return !this.revoked && (!this.expiration || this.expiration > new Date());
@@ -83,26 +61,6 @@ export class IssueApiKeyDto {
8361
})
8462
privileges: Array<Privilege>;
8563

86-
@ApiProperty({
87-
type: 'array',
88-
items: {
89-
type: 'object',
90-
properties: {
91-
scenario: { type: 'string', format: 'uuid' },
92-
actors: {
93-
type: 'array',
94-
items: { type: 'string' },
95-
},
96-
actions: {
97-
type: 'array',
98-
items: { type: 'string' },
99-
},
100-
},
101-
},
102-
})
103-
processes?: Array<{
104-
scenario: string;
105-
actors?: string[];
106-
actions?: string[];
107-
}>;
64+
@ApiProperty()
65+
service?: string;
10866
}

src/apikey/apikey.service.spec.ts

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,7 @@ describe('ApiKeyService', () => {
3939
description: 'Test description',
4040
expirationDays: 1,
4141
privileges: ['process:start', 'process:step'],
42-
processes: [
43-
{
44-
scenario: '43dec73d-7e4c-4f48-aacc-f913ff16e14f',
45-
actions: ['complete'],
46-
actors: ['admin'],
47-
},
48-
],
42+
service: 'test',
4943
};
5044

5145
const insertOne = collection.insertOne.mockResolvedValue({
@@ -71,13 +65,7 @@ describe('ApiKeyService', () => {
7165
expect(insertedDoc.expiration.toISOString()).toEqual(expiration.toISOString());
7266

7367
expect(insertedDoc).toHaveProperty('privileges', ['process:start', 'process:step']);
74-
expect(insertedDoc).toHaveProperty('processes', [
75-
{
76-
scenario: '43dec73d-7e4c-4f48-aacc-f913ff16e14f',
77-
actions: ['complete'],
78-
actors: ['admin'],
79-
},
80-
]);
68+
expect(insertedDoc).toHaveProperty('service', 'test');
8169
});
8270
});
8371

@@ -121,13 +109,7 @@ describe('ApiKeyService', () => {
121109
issued: new Date('2023-01-01'),
122110
expiration: new Date('2024-01-01'),
123111
privileges: ['process:step'],
124-
processes: [
125-
{
126-
scenario: '43dec73d-7e4c-4f48-aacc-f913ff16e14f',
127-
actions: ['complete'],
128-
actors: ['admin'],
129-
},
130-
],
112+
service: 'test',
131113
});
132114

133115
const apiKey = await service.get('123456789012345678901234');
@@ -143,6 +125,7 @@ describe('ApiKeyService', () => {
143125
expect(apiKey.issued.toISOString()).toEqual(new Date('2023-01-01').toISOString());
144126
expect(apiKey.expiration.toISOString()).toEqual(new Date('2024-01-01').toISOString());
145127
expect(apiKey.privileges).toEqual(['process:step']);
128+
expect(apiKey.service).toEqual('test');
146129
});
147130
});
148131

src/apikey/apikey.service.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ export class ApiKeyService implements OnModuleInit {
3838
? new Date(input.expiration)
3939
: this.determineExpiration(issued, input.expirationDays),
4040
privileges: input.privileges ?? [],
41+
service: input.service,
4142
};
42-
if (input.processes) apiKey.processes = input.processes;
4343

4444
const result = await this.collection.insertOne(apiKey);
4545

src/auth/auth.service.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,11 +116,11 @@ export class AuthService implements OnModuleInit {
116116
};
117117
}
118118

119-
async verifyApiKey(token: string): Promise<Pick<ApiKey, 'privileges' | 'processes'> | null> {
119+
async verifyApiKey(token: string): Promise<Pick<ApiKey, 'privileges' | 'service'> | null> {
120120
const doc = await this.apiKeys.findOne(
121121
{ token, expiration: { $gt: new Date() }, revoked: { $exists: false } },
122122
{
123-
projection: { _id: 0, privileges: 1, processes: 1 },
123+
projection: { _id: 0, privileges: 1, service: 1 },
124124
},
125125
);
126126

src/auth/types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { ApiKey as FullApiKey } from '@/apikey';
22

3-
export type ApiKey = Pick<FullApiKey, 'privileges' | 'processes'>;
3+
export type ApiKey = Pick<FullApiKey, 'privileges' | 'service'>;
44

55
export interface Account {
66
id: string;

src/common/utils/clean.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export function clean<T extends Record<string, any>>(input: T): T {
2+
return Object.fromEntries(Object.entries(input).filter(([_, v]) => v !== undefined)) as T;
3+
}

src/notify/notify.service.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { Injectable, Logger } from '@nestjs/common';
22
import { ZeromqService } from './zeromq/zeromq.service';
3-
import { Notify, Process } from '@letsflow/core/process';
3+
import { determineTrigger, Notify, Process } from '@letsflow/core/process';
44
import { ConfigService } from '@/common/config/config.service';
55
import { NotifyProvider } from './notify-provider.interface';
66
import { OnEvent } from '@nestjs/event-emitter';
@@ -38,14 +38,24 @@ export class NotifyService implements NotifyProvider {
3838
try {
3939
const response = await this.getProvider(args.service).notify(process, args);
4040

41-
if (args.trigger && typeof response !== 'undefined') {
42-
await this.processes.step(process, args.trigger, { key: `service:${args.service}` }, response);
41+
if (typeof response !== 'undefined') {
42+
await this.step(process, args.service, response);
4343
}
4444
} catch (err) {
4545
this.logger.error(err.message);
4646
}
4747
}
4848

49+
private async step(process: Process, service: string, response: any): Promise<void> {
50+
const action = determineTrigger(process, service, response);
51+
52+
if (!action) {
53+
throw new Error(`Service '${service}' gave a response, but unable to determine which action was executed`);
54+
}
55+
56+
await this.processes.step(process, action, { key: `service:${service}` }, response);
57+
}
58+
4959
private getProvider(service: string): NotifyProvider {
5060
const settings = this.config.get('services')[service];
5161

src/process/process.controller.spec.ts

Lines changed: 46 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -102,52 +102,59 @@ describe('ProcessController', () => {
102102
];
103103

104104
const user = { id: '2', roles: ['admin', 'team', 'assistant'] } as Account;
105-
const apiKey = {
106-
processes: [{ scenario: 'test' }, { scenario: 'test2', actors: ['system'] }],
107-
} as ApiKey;
105+
const apiKey = { service: 'system' } as ApiKey;
108106

109107
it('should return a list of processes', async () => {
110108
jest.spyOn(processService, 'list').mockResolvedValue(mockProcesses as any);
111109

112-
await controller.list(undefined, undefined, undefined, undefined, mockResponse);
110+
await controller.list(undefined, undefined, undefined, undefined, undefined, mockResponse);
113111

114-
expect(processService.list).toHaveBeenCalledWith({ limit: 100, page: 1 });
112+
expect(processService.list).toHaveBeenCalledWith({}, { limit: 100, page: 1 });
115113
expect(mockResponse.status).toHaveBeenCalledWith(HttpStatus.OK);
116114
expect(mockResponse.json).toHaveBeenCalledWith(mockProcesses);
117115
});
118116

119117
it('should return page 2', async () => {
120118
jest.spyOn(processService, 'list').mockResolvedValue(mockProcesses as any);
121119

122-
await controller.list(2, undefined, undefined, undefined, mockResponse);
120+
await controller.list(2, undefined, undefined, undefined, undefined, mockResponse);
123121

124-
expect(processService.list).toHaveBeenCalledWith({ limit: 100, page: 2 });
122+
expect(processService.list).toHaveBeenCalledWith({}, { limit: 100, page: 2 });
125123
});
126124

127125
it('should filter processes by actor for user', async () => {
128126
jest.spyOn(processService, 'list').mockResolvedValue(mockProcesses as any);
129127

130-
await controller.list(undefined, undefined, user, undefined, mockResponse);
128+
await controller.list(undefined, undefined, undefined, user, undefined, mockResponse);
131129

132-
expect(processService.list).toHaveBeenCalledWith({
133-
limit: 100,
134-
page: 1,
135-
actors: [{ id: '2' }, { role: 'admin' }, { role: 'team' }, { role: 'assistant' }],
136-
});
130+
expect(processService.list).toHaveBeenCalledWith(
131+
{ actors: [{ id: '2' }, { role: 'admin' }, { role: 'team' }, { role: 'assistant' }] },
132+
{ limit: 100, page: 1 },
133+
);
137134
});
138135

139136
it('should not filter processes by actor for admin user requesting all processes', async () => {
140137
jest.spyOn(processService, 'list').mockResolvedValue(mockProcesses as any);
141138

142-
await controller.list(undefined, true, user, undefined, mockResponse);
139+
await controller.list(undefined, undefined, true, user, undefined, mockResponse);
143140

144-
expect(processService.list).toHaveBeenCalledWith({ limit: 100, page: 1 });
141+
expect(processService.list).toHaveBeenCalledWith({}, { limit: 100, page: 1 });
145142
});
146143

147144
it('should not allow user with super privileges to list all processes', async () => {
148145
authService.hasPrivilege.mockReturnValue(false);
149146

150-
await controller.list(undefined, true, { id: '2', roles: ['team'] } as Account, undefined, mockResponse);
147+
await controller.list(
148+
undefined,
149+
undefined,
150+
true,
151+
{
152+
id: '2',
153+
roles: ['team'],
154+
} as Account,
155+
undefined,
156+
mockResponse,
157+
);
151158

152159
expect(mockResponse.status).toHaveBeenCalledWith(HttpStatus.FORBIDDEN);
153160
expect(mockResponse.send).toHaveBeenCalledWith('Not allowed to list all processes');
@@ -159,21 +166,17 @@ describe('ProcessController', () => {
159166
it('should filter processes by scenario for API key', async () => {
160167
jest.spyOn(processService, 'list').mockResolvedValue(mockProcesses as any);
161168

162-
await controller.list(undefined, undefined, undefined, apiKey, mockResponse);
169+
await controller.list(undefined, undefined, undefined, undefined, apiKey, mockResponse);
163170

164-
expect(processService.list).toHaveBeenCalledWith({
165-
limit: 100,
166-
page: 1,
167-
scenarios: ['test', 'test2'],
168-
});
171+
expect(processService.list).toHaveBeenCalledWith({ service: 'system' }, { limit: 100, page: 1 });
169172
});
170173

171174
it('should not filter processes for an API key without process limitations', async () => {
172175
jest.spyOn(processService, 'list').mockResolvedValue(mockProcesses as any);
173176

174-
await controller.list(undefined, true, undefined, {} as ApiKey, mockResponse);
177+
await controller.list(undefined, undefined, true, undefined, {} as ApiKey, mockResponse);
175178

176-
expect(processService.list).toHaveBeenCalledWith({ limit: 100, page: 1 });
179+
expect(processService.list).toHaveBeenCalledWith({}, { limit: 100, page: 1 });
177180
});
178181
});
179182

@@ -186,6 +189,15 @@ describe('ProcessController', () => {
186189
client: { id: '3' },
187190
assistant: { role: 'assistant' },
188191
},
192+
current: {
193+
key: 'foo',
194+
actions: [
195+
{
196+
key: 'generate',
197+
actor: ['service:system'],
198+
},
199+
],
200+
},
189201
created: new Date(),
190202
lastUpdated: new Date(),
191203
};
@@ -194,13 +206,13 @@ describe('ProcessController', () => {
194206
['admin user', { id: '1', roles: ['admin'], token: '' }, undefined],
195207
['user by role', { id: '2', roles: ['assistant'], token: '' }, undefined],
196208
['user by id', { id: '3', roles: [], token: '' }, undefined],
197-
['API key', undefined, { privileges: ['process:read'] }],
198-
['API key with scenario', undefined, { privileges: ['process:read'], processes: [{ scenario: 'test' }] }],
209+
['API key of service', undefined, { privileges: ['process:read'], service: 'system' }],
210+
['API key with super privs', undefined, { privileges: ['process:read', 'process:super'] }],
199211
])('should return a process for %s', async (_, user, apiKey) => {
200212
processService.has.mockResolvedValue(true);
201213
processService.get.mockResolvedValue(mockProcess as any);
202214

203-
await controller.get('00000000-0000-0000-0001-000000000001', user, apiKey, mockResponse);
215+
await controller.get('00000000-0000-0000-0001-000000000001', user, apiKey, false, mockResponse);
204216

205217
expect(processService.has).toHaveBeenCalledWith('00000000-0000-0000-0001-000000000001');
206218
expect(processService.get).toHaveBeenCalledWith('00000000-0000-0000-0001-000000000001');
@@ -211,7 +223,7 @@ describe('ProcessController', () => {
211223
it('should return 404 if process not found', async () => {
212224
processService.has.mockResolvedValue(false);
213225

214-
await controller.get('00000000-0000-0000-0001-000000000001', undefined, undefined, mockResponse);
226+
await controller.get('00000000-0000-0000-0001-000000000001', undefined, undefined, false, mockResponse);
215227

216228
expect(processService.has).toHaveBeenCalledWith('00000000-0000-0000-0001-000000000001');
217229
expect(mockResponse.status).toHaveBeenCalledWith(HttpStatus.NOT_FOUND);
@@ -225,7 +237,7 @@ describe('ProcessController', () => {
225237
authService.hasPrivilege.mockReturnValue(false);
226238

227239
const user = { id: '99', roles: [], token: '', info: {} };
228-
await controller.get('00000000-0000-0000-0001-000000000001', user, undefined, mockResponse);
240+
await controller.get('00000000-0000-0000-0001-000000000001', user, undefined, false, mockResponse);
229241

230242
expect(mockResponse.status).toHaveBeenCalledWith(HttpStatus.FORBIDDEN);
231243

@@ -242,7 +254,7 @@ describe('ProcessController', () => {
242254

243255
const apiKey = { processes: [{ scenario: 'foo' }], privileges: [] };
244256

245-
await controller.get('00000000-0000-0000-0001-000000000001', undefined, apiKey, mockResponse);
257+
await controller.get('00000000-0000-0000-0001-000000000001', undefined, apiKey, false, mockResponse);
246258

247259
expect(mockResponse.status).toHaveBeenCalledWith(HttpStatus.FORBIDDEN);
248260
});
@@ -281,7 +293,7 @@ describe('ProcessController', () => {
281293
scenarioService.getStatus.mockResolvedValue('available');
282294
scenarioService.get.mockResolvedValue(scenario);
283295
processService.determineActor.mockReturnValue(actor);
284-
processService.instantiate.mockResolvedValue(process);
296+
processService.instantiate.mockReturnValue(process);
285297
});
286298

287299
it('should start a new process', async () => {
@@ -293,7 +305,7 @@ describe('ProcessController', () => {
293305
const steppedProcess = step(process, 'init', actor);
294306
processService.step.mockResolvedValue(steppedProcess);
295307

296-
await controller.start(undefined, user, undefined, mockInstructions, mockResponse);
308+
await controller.start(undefined, user, undefined, false, mockInstructions, mockResponse);
297309

298310
expect(mockResponse.status).toHaveBeenCalledWith(HttpStatus.CREATED);
299311
expect(mockResponse.header).toHaveBeenCalledWith('Location', `/processes/${process.id}`);
@@ -316,7 +328,7 @@ describe('ProcessController', () => {
316328
const steppedProcess = step(process, 'init', actor, 'hello');
317329
processService.step.mockResolvedValue(steppedProcess);
318330

319-
await controller.start(undefined, user, undefined, mockInstructions, mockResponse);
331+
await controller.start(undefined, user, undefined, false, mockInstructions, mockResponse);
320332

321333
expect(mockResponse.status).toHaveBeenCalledWith(HttpStatus.CREATED);
322334
expect(mockResponse.header).toHaveBeenCalledWith('Location', `/processes/${process.id}`);
@@ -335,7 +347,7 @@ describe('ProcessController', () => {
335347
const steppedProcess = step(process, 'wrongAction', actor, {});
336348
processService.step.mockResolvedValue(steppedProcess);
337349

338-
await controller.start(undefined, user, undefined, mockInstructions, mockResponse);
350+
await controller.start(undefined, user, undefined, false, mockInstructions, mockResponse);
339351

340352
expect(mockResponse.status).toHaveBeenCalledWith(HttpStatus.BAD_REQUEST);
341353
expect(mockResponse.json).toHaveBeenCalledWith({

0 commit comments

Comments
 (0)