Skip to content

Commit 0f54253

Browse files
JAG-UKJAG-UK
authored
Merge pull request #125 from RafalMagrys/FIL-1005-rkhp-extend-refreshes-with-the-gov-teams-approval-rejection-step
[Fil 1005] extended refreshes with the gov teams approval/rejection step
2 parents 0f25b2c + 7b6ff6e commit 0f54253

File tree

51 files changed

+2109
-437
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

51 files changed

+2109
-437
lines changed

‎packages/application/src/api/http/controllers/authutils.test.ts‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
import { describe, expect, it } from 'vitest';
2+
import { getPublicKey, etc } from '@noble/secp256k1';
3+
import { hmac } from '@noble/hashes/hmac';
4+
import { sha256 } from '@noble/hashes/sha256';
5+
import cbor from 'cbor';
6+
import { verifyLedgerPoP } from './authutils';
7+
import { FilecoinTxBuilder } from '@src/testing/mocks/builders';
8+
9+
// Ensure noble-secp256k1 HMAC is set in case setup didn't run yet
10+
if (!etc.hmacSha256Sync) {
11+
etc.hmacSha256Sync = (key: Uint8Array, ...msgs: Uint8Array[]) =>
12+
hmac(sha256, key, etc.concatBytes(...msgs));
13+
}
14+
15+
describe('verifyLedgerPoP (integration)', () => {
16+
it('returns true for a valid signed transaction and matching challenge', async () => {
17+
const challenge = 'challenge';
18+
const { address, pubKeyBase64, transaction } = await new FilecoinTxBuilder()
19+
.withChallenge(challenge)
20+
.build();
21+
22+
const ok = await verifyLedgerPoP(address, pubKeyBase64, transaction, challenge);
23+
expect(ok).toBe(true);
24+
});
25+
26+
it("throws when To/From/Nonce don't match expected (replay guard)", async () => {
27+
const challenge = 'challenge';
28+
const { address, pubKeyBase64, transaction } = await new FilecoinTxBuilder()
29+
.withChallenge(challenge)
30+
.build();
31+
32+
const parsed = JSON.parse(transaction);
33+
parsed.Message.From = 'f1different';
34+
35+
await expect(
36+
verifyLedgerPoP(address, pubKeyBase64, JSON.stringify(parsed), challenge),
37+
).rejects.toThrow("addresses don't match");
38+
});
39+
40+
it('throws when derived address from pubkey does not match provided address', async () => {
41+
const challenge = 'challenge';
42+
const { address, transaction } = await new FilecoinTxBuilder().withChallenge(challenge).build();
43+
44+
// Provide a mismatching pubkey (different private key)
45+
const otherPriv = Uint8Array.from(
46+
Buffer.from('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', 'hex'),
47+
);
48+
const otherPub = getPublicKey(otherPriv, false);
49+
const otherPubB64 = Buffer.from(otherPub).toString('base64');
50+
51+
await expect(verifyLedgerPoP(address, otherPubB64, transaction, challenge)).rejects.toThrow(
52+
'wrong key for address',
53+
);
54+
});
55+
56+
it("throws when pre-image doesn't match", async () => {
57+
const challenge = 'challenge';
58+
const { address, pubKeyBase64, transaction } = await new FilecoinTxBuilder()
59+
.withChallenge(challenge)
60+
.build();
61+
await expect(
62+
verifyLedgerPoP(address, pubKeyBase64, transaction, 'different-challenge'),
63+
).rejects.toThrow("pre-images don't match");
64+
});
65+
66+
it("throws when signature doesn't exist", async () => {
67+
const challenge = 'challenge';
68+
const { address, pubKeyBase64, transaction } = await new FilecoinTxBuilder()
69+
.withChallenge(challenge)
70+
.build();
71+
const parsed = JSON.parse(transaction);
72+
parsed.Signature.Data = '';
73+
74+
await expect(
75+
verifyLedgerPoP(address, pubKeyBase64, JSON.stringify(parsed), challenge),
76+
).rejects.toThrow("signature doesn't exist");
77+
});
78+
79+
it('throws when signature has wrong length', async () => {
80+
const challenge = 'challenge';
81+
const { address, pubKeyBase64, transaction } = await new FilecoinTxBuilder()
82+
.withChallenge(challenge)
83+
.build();
84+
const parsed = JSON.parse(transaction);
85+
const sigBytes = Buffer.from(parsed.Signature.Data, 'base64');
86+
// Drop recovery byte to make it 64
87+
const wrongLen = sigBytes.subarray(0, 64);
88+
parsed.Signature.Data = Buffer.from(wrongLen).toString('base64');
89+
90+
await expect(
91+
verifyLedgerPoP(address, pubKeyBase64, JSON.stringify(parsed), challenge),
92+
).rejects.toThrow('Bad signature length: 64');
93+
});
94+
95+
it('should throw "addresses don\'t match" when nonce is not 0', async () => {
96+
const challenge = 'challenge';
97+
const { address, pubKeyBase64, transaction } = await new FilecoinTxBuilder()
98+
.withCustomMessage({ Nonce: 1 })
99+
.withChallenge(challenge)
100+
.build();
101+
102+
await expect(verifyLedgerPoP(address, pubKeyBase64, transaction, challenge)).rejects.toThrow(
103+
"addresses don't match",
104+
);
105+
});
106+
107+
it('should throw "addresses don\'t match" when address does not match', async () => {
108+
const challenge = 'challenge';
109+
const { address, pubKeyBase64, transaction } = await new FilecoinTxBuilder()
110+
.withChallenge(challenge)
111+
.build();
112+
113+
const parsed = JSON.parse(transaction);
114+
parsed.Message.To = 'f1evc3p45ke4apzvi5ix25mniemuva6umggusmdif';
115+
116+
await expect(
117+
verifyLedgerPoP(address, pubKeyBase64, JSON.stringify(parsed), challenge),
118+
).rejects.toThrow("addresses don't match");
119+
});
120+
});

‎packages/application/src/api/http/controllers/refresh.controller.ts‎

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,11 @@ import {
99
httpPut,
1010
request,
1111
requestBody,
12+
requestParam,
1213
response,
1314
} from 'inversify-express-utils';
1415

15-
import { badRequest, ok } from '@src/api/http/processors/response';
16+
import { badPermissions, badRequest, ok } from '@src/api/http/processors/response';
1617
import { TYPES } from '@src/types';
1718
import { RefreshIssuesCommand } from '@src/application/use-cases/refresh-issues/refresh-issues.command';
1819
import { GetRefreshesQuery } from '@src/application/queries/get-refreshes/get-refreshes.query';
@@ -21,6 +22,14 @@ import { UpsertIssueCommand } from '@src/application/use-cases/refresh-issues/up
2122
import { IssuesWebhookPayload } from '@src/infrastructure/clients/github';
2223
import { validateIssueUpsert, validateRefreshesQuery } from '@src/api/http/validators';
2324
import { RESPONSE_MESSAGES } from '@src/constants';
25+
import { validateGovernanceReview } from '../validators';
26+
import { validateRequest } from '../middleware/validate-request.middleware';
27+
import { GovernanceReviewDto } from '@src/application/dtos/GovernanceReviewDto';
28+
import { RoleService } from '@src/application/services/role.service';
29+
import { SignatureType } from '@src/patterns/decorators/signature-guard.decorator';
30+
import { SignatureGuard } from '@src/patterns/decorators/signature-guard.decorator';
31+
import { RejectRefreshCommand } from '@src/application/use-cases/refresh-issues/reject-refesh.command';
32+
import { ApproveRefreshCommand } from '@src/application/use-cases/refresh-issues/approve-refresh.command';
2433

2534
const RES = RESPONSE_MESSAGES.REFRESH_CONTROLLER;
2635

@@ -30,6 +39,7 @@ export class RefreshController {
3039
@inject(TYPES.QueryBus) private readonly _queryBus: IQueryBus,
3140
@inject(TYPES.CommandBus) private readonly _commandBus: ICommandBus,
3241
@inject(TYPES.IssueMapper) private readonly _issueMapper: IIssueMapper,
42+
@inject(TYPES.RoleService) private readonly _roleService: RoleService,
3343
) {}
3444

3545
@httpGet('', ...validateRefreshesQuery)
@@ -83,4 +93,36 @@ export class RefreshController {
8393

8494
return res.json(ok(RES.REFRESH_SUCCESS, result));
8595
}
96+
97+
@httpPost('/:githubIssueNumber/review', validateRequest(validateGovernanceReview))
98+
@SignatureGuard(SignatureType.RefreshReview)
99+
async approveRefresh(
100+
@requestParam('githubIssueNumber') githubIssueNumber: string,
101+
@requestBody() approveRefreshDto: GovernanceReviewDto,
102+
@response() res: Response,
103+
) {
104+
const id = parseInt(githubIssueNumber);
105+
const address = approveRefreshDto.details.reviewerAddress;
106+
const role = this._roleService.getRole(address);
107+
if (role !== 'GOVERNANCE_TEAM') {
108+
console.log(`Not a governance team member: ${role}`);
109+
return res.status(403).json(badPermissions());
110+
}
111+
112+
const { result } = approveRefreshDto;
113+
114+
const command =
115+
result === 'approve'
116+
? new ApproveRefreshCommand(id, approveRefreshDto.details.finalDataCap)
117+
: new RejectRefreshCommand(id);
118+
119+
const refreshResult = await this._commandBus.send(command);
120+
if (!refreshResult.success) {
121+
return res
122+
.status(400)
123+
.json(badRequest(RES.FAILED_TO_UPSERT_ISSUE, [refreshResult.error.message]));
124+
}
125+
126+
return res.json(ok(RES.REFRESH_SUCCESS, result));
127+
}
86128
}

‎packages/application/src/api/http/middleware/validate-request.middleware.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import { Request, Response, NextFunction } from 'express';
2+
import { validationResult, ValidationChain } from 'express-validator';
3+
import { badRequest } from '@src/api/http/processors/response';
4+
5+
export function validateRequest(validators: ValidationChain[]) {
6+
return async (req: Request, res: Response, next: NextFunction) => {
7+
await Promise.all(validators.map(validator => validator.run(req)));
8+
9+
const errors = validationResult(req);
10+
11+
if (!errors.isEmpty()) {
12+
const errorMessages = errors.array().map(error => error.msg);
13+
return res.status(400).json(badRequest('Validation failed', errorMessages));
14+
}
15+
16+
next();
17+
};
18+
}

‎packages/application/src/api/http/processors/response.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,14 @@ export const ok = (message: string, data?: any) => ({
44
data,
55
});
66

7-
export const badRequest = (message: string, errors: any) => ({
7+
export const badRequest = (message: string, errors?: any) => ({
88
status: '400',
99
message: message || 'Bad Request',
1010
errors,
1111
});
1212

1313
export const badPermissions = (message?: string) => ({
14-
status: '400',
14+
status: '403',
1515
message: message || 'Bad Permissions',
1616
});
1717

‎packages/application/src/api/http/validators/governance-review.validator.ts‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import { VALIDATION_MESSAGES } from '@src/constants/validation-messages';
2+
import { body, param } from 'express-validator';
3+
4+
export const validateGovernanceReview = [
5+
param('githubIssueNumber')
6+
.isInt({ min: 1, max: Number.MAX_SAFE_INTEGER })
7+
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_GITHUB_ISSUE_NUMBER.INVALID)
8+
.bail(),
9+
10+
body('result')
11+
.exists()
12+
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_RESULT.REQUIRED)
13+
.bail()
14+
.isIn(['approve', 'reject'])
15+
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_RESULT.INVALID)
16+
.bail(),
17+
18+
body('details')
19+
.exists()
20+
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_DETAILS.REQUIRED)
21+
.bail()
22+
.isObject()
23+
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_DETAILS.INVALID),
24+
25+
body('details.reviewerAddress')
26+
.exists()
27+
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_DETAILS_REVIEWER_ADDRESS.REQUIRED)
28+
.bail()
29+
.isString()
30+
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_DETAILS_REVIEWER_ADDRESS.INVALID)
31+
.bail(),
32+
33+
body('details.reviewerPublicKey')
34+
.exists()
35+
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_DETAILS_REVIEWER_PUBLIC_KEY.REQUIRED)
36+
.bail()
37+
.isString()
38+
.bail()
39+
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_DETAILS_REVIEWER_PUBLIC_KEY.INVALID)
40+
.bail(),
41+
42+
body('details.finalDataCap')
43+
.exists()
44+
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_DETAILS_FINAL_DATACAP.REQUIRED)
45+
.bail()
46+
.isNumeric()
47+
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_DETAILS_FINAL_DATACAP.INVALID)
48+
.bail(),
49+
50+
body('details.allocatorType')
51+
.exists()
52+
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_DETAILS_ALLOCATOR_TYPE.REQUIRED)
53+
.bail()
54+
.isString()
55+
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_DETAILS_ALLOCATOR_TYPE.INVALID)
56+
.bail(),
57+
58+
body('signature')
59+
.exists()
60+
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_DETAILS_SIGNATURE.REQUIRED)
61+
.bail()
62+
.isString()
63+
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_DETAILS_SIGNATURE.INVALID)
64+
.bail(),
65+
];

‎packages/application/src/api/http/validators/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
export * from './github-issue.validator';
2+
export * from './governance-review.validator';

‎packages/application/src/application/dtos/GovernanceReviewDto.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
export interface GovernanceReviewDetailsDto {
2+
reviewerAddress: string;
3+
reviewerPublicKey: string;
4+
finalDataCap: number;
5+
allocatorType: string;
6+
}
7+
8+
export interface GovernanceReviewDto {
9+
result: string;
10+
details: GovernanceReviewDetailsDto;
11+
signature: string;
12+
}

‎packages/application/src/application/publishers/refresh-audit-publisher.test.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ import { GithubClient } from '@src/infrastructure/clients/github';
77
import { TYPES } from '@src/types';
88
import { GithubConfig } from '@src/domain/types';
99
import { ICommandBus } from '@filecoin-plus/core/src/interfaces/ICommandBus';
10-
import { AuditMapper, IAuditMapper } from '@src/infrastructure/mappers/audit-mapper';
10+
import { IAuditMapper } from '@src/infrastructure/mappers/audit-mapper';
11+
import { Logger } from '@filecoin-plus/core';
1112

1213
describe('RefreshAuditPublisher', () => {
1314
let container: Container;
@@ -37,6 +38,7 @@ describe('RefreshAuditPublisher', () => {
3738
datacap_amount: a.datacapAmount as number,
3839
})),
3940
};
41+
const loggerMock = { info: vi.fn(), error: vi.fn() };
4042

4143
const baseAllocator = {
4244
application_number: 123,
@@ -66,6 +68,7 @@ describe('RefreshAuditPublisher', () => {
6668
container
6769
.bind<IAuditMapper>(TYPES.AuditMapper)
6870
.toConstantValue(auditMapperMock as unknown as IAuditMapper);
71+
container.bind<Logger>(TYPES.Logger).toConstantValue(loggerMock as unknown as Logger);
6972
container
7073
.bind(TYPES.AllocatorRegistryConfig)
7174
.toConstantValue(configMock as unknown as GithubConfig);
@@ -82,7 +85,7 @@ describe('RefreshAuditPublisher', () => {
8285
githubMock.mergePullRequest.mockResolvedValue({});
8386
githubMock.deleteBranch.mockResolvedValue({});
8487

85-
commandBusMock.send.mockResolvedValue({ success: true, data: structuredClone(baseAllocator) });
88+
commandBusMock.send.mockResolvedValue({ success: true, data: baseAllocator });
8689
});
8790

8891
it('newAudit creates a new pending audit and publishes PR', async () => {
@@ -103,7 +106,7 @@ describe('RefreshAuditPublisher', () => {
103106
it.each([AuditOutcome.PENDING, AuditOutcome.APPROVED])(
104107
'newAudit throws when last audit is %s',
105108
async outcome => {
106-
const pendingAllocator = structuredClone(baseAllocator);
109+
const pendingAllocator = baseAllocator;
107110
pendingAllocator.audits[pendingAllocator.audits.length - 1].outcome = outcome;
108111
commandBusMock.send.mockResolvedValueOnce({ success: true, data: pendingAllocator });
109112

0 commit comments

Comments
 (0)