Skip to content

Commit 9a54afe

Browse files
JoeCap08055aramikm
andauthored
fix: do not queue multiple jobs when same SIWF payload is posted multiple times (#752)
# Problem If the same authorizationCode was posted to the `/v2/accounts/siwf` endpoint, multiple queue tasks would be created for (essentially) the same payload (at least, the same sequence of extrinsics). # Root Cause The work queue in BullMQ is intended to be unique on `jobId`. The `jobId` was being created from a hash of the payload retrieved from SIWF (Frequency Access). However, when a payload is retrieved from SIWF using an authorization code, that payload is *SIGNED ON DEMAND*--meaning the signed extrinsics within the payload differ; different contents => different hash, different job ID, hence multiple jobs. # Solution If an authorization code is being used to retrieve a payload, use the authorization code as the job ID instead of the retrieved payload hash Note, it's still possible that the same operations could be submitted by an external agent who handles their own signing/payload creation, or handles fetching the payload from SIWF themselves and submits the actual payload to Gateway instead of the authorization code. This solution does not prevent that scenario from causing multiple jobs to be queued for the same sequence of user creation transactions. (Tracking as #753) Closes #674 --------- Co-authored-by: Aramik <[email protected]>
1 parent 8d94071 commit 9a54afe

File tree

13 files changed

+139
-57
lines changed

13 files changed

+139
-57
lines changed

apps/account-api/src/controllers/v2/accounts-v2.controller.ts

+6-2
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,13 @@ export class AccountsControllerV2 {
116116
}
117117

118118
// Trigger chain submissions, if any
119-
await this.siwfV2Service.queueChainActions(payload);
119+
const jobStatus = await this.siwfV2Service.queueChainActions(payload, callbackRequest);
120120

121-
const response = this.siwfV2Service.getSiwfV2LoginResponse(payload);
121+
const response = await this.siwfV2Service.getSiwfV2LoginResponse(payload);
122+
if (jobStatus) {
123+
response.signUpReferenceId = jobStatus.referenceId;
124+
response.signUpStatus = jobStatus.state;
125+
}
122126

123127
return response;
124128
}

apps/account-api/src/services/siwfV2.service.spec.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,7 @@ describe('SiwfV2Service', () => {
349349

350350
it('should do nothing if there are no chain submissions', async () => {
351351
const enqueueSpy = jest.spyOn(enqueueService, 'enqueueRequest');
352-
const result = await siwfV2Service.queueChainActions(validSiwfLoginResponsePayload);
352+
const result = await siwfV2Service.queueChainActions(validSiwfLoginResponsePayload, {});
353353

354354
expect(result).toBeNull();
355355
expect(enqueueSpy).not.toHaveBeenCalled();
@@ -367,7 +367,7 @@ describe('SiwfV2Service', () => {
367367
]) as ApiPromise,
368368
);
369369

370-
await expect(siwfV2Service.queueChainActions(validSiwfAddDelegationResponsePayload)).resolves.not.toThrow();
370+
await expect(siwfV2Service.queueChainActions(validSiwfAddDelegationResponsePayload, {})).resolves.not.toThrow();
371371
expect(enqueueSpy).toHaveBeenCalledWith({
372372
calls: [
373373
{
@@ -397,7 +397,7 @@ describe('SiwfV2Service', () => {
397397
);
398398

399399
try {
400-
await siwfV2Service.queueChainActions(validSiwfNewUserResponse);
400+
await siwfV2Service.queueChainActions(validSiwfNewUserResponse, {});
401401
} catch (err: any) {
402402
console.error(err);
403403
throw err;

apps/account-api/src/services/siwfV2.service.ts

+12-7
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@ import { isNotNull } from '#utils/common/common.utils';
3030
import { chainSignature, statefulStoragePayload } from '#utils/common/signature.util';
3131
import { ApiPromise } from '@polkadot/api';
3232

33+
interface IAuthCode {
34+
authorizationCode?: string;
35+
}
36+
3337
@Injectable()
3438
export class SiwfV2Service {
3539
private readonly logger: Logger;
@@ -177,12 +181,9 @@ export class SiwfV2Service {
177181

178182
response.controlKey = payload.userPublicKey.encodedValue;
179183

180-
// Try to look up the MSA id, if there is no createSponsoredAccountWithDelegation request
181-
if (payload.payloads.every((x) => x.endpoint?.extrinsic !== 'createSponsoredAccountWithDelegation')) {
182-
// Get the MSA Id from the chain
183-
const msaId = await this.blockchainService.publicKeyToMsaId(response.controlKey);
184-
if (msaId) response.msaId = msaId;
185-
}
184+
// Get the MSA Id from the chain, if it exists
185+
const msaId = await this.blockchainService.publicKeyToMsaId(response.controlKey);
186+
if (msaId) response.msaId = msaId;
186187

187188
// Parse out the email, phone, and graph
188189
const email = payload.credentials.find(isCredentialEmail)?.credentialSubject.emailAddress;
@@ -198,7 +199,10 @@ export class SiwfV2Service {
198199
return response;
199200
}
200201

201-
async queueChainActions(response: SiwfResponse): Promise<TransactionResponse | null> {
202+
async queueChainActions(
203+
response: SiwfResponse,
204+
{ authorizationCode }: IAuthCode,
205+
): Promise<TransactionResponse | null> {
202206
// Don't do anything if there is nothing to do
203207
if (!hasChainSubmissions(response)) return null;
204208

@@ -212,6 +216,7 @@ export class SiwfV2Service {
212216
return this.enqueueService.enqueueRequest<PublishSIWFSignupRequestDto>({
213217
calls,
214218
type: TransactionType.SIWF_SIGNUP,
219+
authorizationCode,
215220
});
216221
} catch (e) {
217222
this.logger.warn('Error during SIWF V2 Chain Action Queuing', { error: e.toString() });

docs/account/index.html

+8-8
Large diffs are not rendered by default.

libs/account-lib/src/services/enqueue-request.service.ts

+23-5
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
/* eslint-disable class-methods-use-this */
21
import { InjectQueue } from '@nestjs/bullmq';
32
import { Inject, Injectable, Logger } from '@nestjs/common';
43
import { Queue } from 'bullmq';
@@ -19,30 +18,49 @@ export class EnqueueService {
1918
this.logger = new Logger(this.constructor.name);
2019
}
2120

22-
private calculateJobId<RequestType>(jobWithoutId: RequestType): string {
21+
private static calculateJobId<RequestType>(jobWithoutId: RequestType): string {
2322
const stringVal = JSON.stringify(jobWithoutId);
2423
return createHash('sha1').update(stringVal).digest('base64url');
2524
}
2625

27-
async enqueueRequest<RequestType>(request: RequestType): Promise<TransactionResponse> {
26+
async enqueueRequest<RequestType extends Object>(request: RequestType): Promise<TransactionResponse> {
2827
const { providerId } = this.config;
28+
// Best-effort attempt at de-duping payloads that are submitted multiple times.
29+
// For payloads submitted via authorization code, we can de-dupe on that.
30+
// For payloads submitted directly, we can de-dupe IFF the exact same payload
31+
// is submitted multiple times; however, it is possible that the same set of extrinsic
32+
// calls could be submitted multiple times, but with different signatures. In that case
33+
// the only way to de-dupe would be to decode the extrinsics in the payload and de-dupe
34+
// based on examination of the public key and other parameters of the extrinsics. While
35+
// technically possible, deemed overkill for now.
36+
const authorizationCode =
37+
'authorizationCode' in request && typeof request.authorizationCode === 'string'
38+
? request.authorizationCode
39+
: undefined;
40+
const referenceId = authorizationCode || EnqueueService.calculateJobId(request);
41+
2942
const data: TransactionData<RequestType> = {
3043
...request,
3144
providerId: providerId.toString(),
32-
referenceId: this.calculateJobId(request),
45+
referenceId,
3346
};
3447

3548
// add() will not fail if Redis is down, it will keep waiting for a reconnection to occur
3649
// and then add the job to the queue.
3750
// Configuring enableOfflineQueue: false will not queue the job if the connection is lost.
3851
// The REST API will return a 500 error if the connection is lost.
52+
//
53+
// Also: if this job already exists in the queue (whether waiting, completed, failed, etc),
54+
// then '.add()' will simply return the existing job, not add a new one.
3955
const job = await this.transactionPublishQueue.add(`Transaction Job - ${data.referenceId}`, data, {
4056
jobId: data.referenceId,
4157
});
58+
const jobState = await job.getState();
4259

43-
this.logger.log('Job enqueued: ', JSON.stringify(job));
60+
this.logger.log('Job enqueued or retrieved: ', JSON.stringify(job));
4461
return {
4562
referenceId: data.referenceId,
63+
state: jobState,
4664
};
4765
}
4866
}

libs/types/src/account-webhook/types.gen.ts

+25
Original file line numberDiff line numberDiff line change
@@ -72,3 +72,28 @@ export type TxWebhookRsp =
7272
| PublishGraphKeysWebhookRsp
7373
| RetireMsaWebhookRsp
7474
| RevokeDelegationWebhookRsp;
75+
76+
export type PostTransactionNotify_Data = {
77+
body: TxWebhookRsp;
78+
path?: never;
79+
query?: never;
80+
url: '/transaction-notify';
81+
};
82+
83+
export type PostTransactionNotify_Errors = {
84+
/**
85+
* Bad request
86+
*/
87+
400: unknown;
88+
};
89+
90+
export type PostTransactionNotify_Responses = {
91+
/**
92+
* Successful notification
93+
*/
94+
200: unknown;
95+
};
96+
97+
export type ClientOptions = {
98+
baseUrl: `${string}://openapi-specs` | (string & {});
99+
};

libs/types/src/dtos/account/transaction.request.dto.ts

+1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ export interface SIWFEncodedExtrinsic {
1616
export type PublishSIWFSignupRequestDto = {
1717
calls: SIWFEncodedExtrinsic[];
1818
type: TransactionType.SIWF_SIGNUP;
19+
authorizationCode?: string;
1920
};
2021

2122
export type TransactionData<
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
1-
import { IsNotEmpty } from 'class-validator';
1+
import { ApiProperty } from '@nestjs/swagger';
2+
import { JobState } from 'bullmq';
23

34
export class TransactionResponse {
4-
@IsNotEmpty()
55
referenceId: string;
6+
7+
// JobState is a string union type; it's not possible to extract the specific values here
8+
// without hard-coding them, and since it's a response only, we don't need to validate, so it
9+
// can just be 'String'
10+
@ApiProperty({ description: 'Job state', name: 'state', type: String, example: 'waiting' })
11+
state?: JobState | 'unknown';
612
}

libs/types/src/dtos/account/wallet.v2.login.response.dto.ts

+16-17
Original file line numberDiff line numberDiff line change
@@ -1,62 +1,54 @@
11
/* eslint-disable max-classes-per-file */
22
import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger';
3-
import { IsArray, IsOptional, IsString } from 'class-validator';
43

54
export class GraphKeySubject {
65
@ApiProperty({
76
description: 'The id type of the VerifiedGraphKeyCredential.',
87
required: true,
98
example: 'did:key:z6QNucQV4AF1XMQV4kngbmnBHwYa6mVswPEGrkFrUayhttT1',
109
})
11-
@IsString()
1210
id: string;
1311

1412
@ApiProperty({
1513
description: 'The encoded public key.',
1614
required: true,
1715
example: '0xb5032900293f1c9e5822fd9c120b253cb4a4dfe94c214e688e01f32db9eedf17',
1816
})
19-
@IsString()
2017
encodedPublicKeyValue: string;
2118

2219
@ApiProperty({
2320
description: 'The encoded private key. WARNING: This is sensitive user information!',
2421
required: true,
2522
example: '0xd0910c853563723253c4ed105c08614fc8aaaf1b0871375520d72251496e8d87',
2623
})
27-
@IsString()
2824
encodedPrivateKeyValue: string;
2925

3026
@ApiProperty({
3127
description: 'How the encoded keys are encoded. Only "base16" (aka hex) currently.',
3228
required: true,
3329
example: 'base16',
3430
})
35-
@IsString()
3631
encoding: string;
3732

3833
@ApiProperty({
3934
description: 'Any addition formatting options. Only: "bare" currently.',
4035
required: true,
4136
example: 'bare',
4237
})
43-
@IsString()
4438
format: string;
4539

4640
@ApiProperty({
4741
description: 'The encryption key algorithm.',
4842
required: true,
4943
example: 'X25519',
5044
})
51-
@IsString()
5245
type: string;
5346

5447
@ApiProperty({
5548
description: 'The DSNP key type.',
5649
required: true,
5750
example: 'dsnp.public-key-key-agreement',
5851
})
59-
@IsString()
6052
keyType: string;
6153
}
6254

@@ -67,42 +59,50 @@ export class WalletV2LoginResponseDto {
6759
type: String,
6860
example: 'f6cL4wq1HUNx11TcvdABNf9UNXXoyH47mVUwT59tzSFRW8yDH',
6961
})
70-
@IsString()
7162
controlKey: string;
7263

64+
@ApiPropertyOptional({
65+
description: 'ReferenceId of an associated sign-up request queued task, if applicable',
66+
required: false,
67+
type: String,
68+
example: 'MjY3MjI3NWZlMGM0NTZmYjY3MWU0ZjQxN2ZiMmY5ODkyYzc1NzNiYQo',
69+
})
70+
signUpReferenceId?: string;
71+
72+
@ApiPropertyOptional({
73+
description: 'Status of associated sign-up request queued task, if applicable',
74+
required: false,
75+
type: String,
76+
example: 'waiting',
77+
})
78+
signUpStatus?: string;
79+
7380
@ApiPropertyOptional({
7481
description: "The user's MSA Id, if one is already created. Will be empty if it is still being processed.",
7582
type: String,
7683
example: '314159265358979323846264338',
7784
})
78-
@IsString()
79-
@IsOptional()
8085
msaId?: string;
8186

8287
@ApiPropertyOptional({
8388
description: "The users's validated email",
8489
type: String,
8590
example: '[email protected]',
8691
})
87-
@IsString()
88-
@IsOptional()
8992
email?: string;
9093

9194
@ApiPropertyOptional({
9295
description: "The users's validated SMS/Phone Number",
9396
type: String,
9497
example: '555-867-5309',
9598
})
96-
@IsString()
97-
@IsOptional()
9899
phoneNumber?: string;
99100

100101
@ApiPropertyOptional({
101102
description: "The users's Private Graph encryption key.",
102103
type: GraphKeySubject,
103104
example: '555-867-5309',
104105
})
105-
@IsOptional()
106106
graphKey?: GraphKeySubject;
107107

108108
@ApiPropertyOptional({
@@ -159,6 +159,5 @@ export class WalletV2LoginResponseDto {
159159
},
160160
],
161161
})
162-
@IsArray()
163162
rawCredentials?: Object[];
164163
}

openapi-specs/account.openapi.json

+15
Original file line numberDiff line numberDiff line change
@@ -902,6 +902,16 @@
902902
"description": "The ss58 encoded MSA Control Key of the login.",
903903
"example": "f6cL4wq1HUNx11TcvdABNf9UNXXoyH47mVUwT59tzSFRW8yDH"
904904
},
905+
"signUpReferenceId": {
906+
"type": "string",
907+
"description": "ReferenceId of an associated sign-up request queued task, if applicable",
908+
"example": "MjY3MjI3NWZlMGM0NTZmYjY3MWU0ZjQxN2ZiMmY5ODkyYzc1NzNiYQo"
909+
},
910+
"signUpStatus": {
911+
"type": "string",
912+
"description": "Status of associated sign-up request queued task, if applicable",
913+
"example": "waiting"
914+
},
905915
"msaId": {
906916
"type": "string",
907917
"description": "The user's MSA Id, if one is already created. Will be empty if it is still being processed.",
@@ -1231,6 +1241,11 @@
12311241
"TransactionResponse": {
12321242
"type": "object",
12331243
"properties": {
1244+
"state": {
1245+
"type": "string",
1246+
"description": "Job state",
1247+
"example": "waiting"
1248+
},
12341249
"referenceId": {
12351250
"type": "string"
12361251
}

openapi-ts.config.ts

+13-4
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,25 @@
22
import { defineConfig, UserConfig } from '@hey-api/openapi-ts';
33

44
export default defineConfig({
5-
client: 'axios',
5+
input: '',
66
exportCore: false,
77
output: {
88
format: 'prettier',
99
lint: 'eslint',
1010
path: 'types',
1111
},
12+
plugins: [
13+
{
14+
dates: true,
15+
name: '@hey-api/transformers',
16+
},
17+
{
18+
identifierCase: 'preserve',
19+
enums: 'typescript',
20+
enumsCase: 'preserve',
21+
name: '@hey-api/typescript',
22+
},
23+
],
1224
schemas: false,
1325
services: false,
14-
types: {
15-
enums: 'typescript',
16-
},
1726
} as UserConfig);

0 commit comments

Comments
 (0)