Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
120 changes: 120 additions & 0 deletions packages/application/src/api/http/controllers/authutils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
import { describe, expect, it } from 'vitest';
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added tests for the authutils because I want to understand how it works in order to test the endpoint.

import { getPublicKey, etc } from '@noble/secp256k1';
import { hmac } from '@noble/hashes/hmac';
import { sha256 } from '@noble/hashes/sha256';
import cbor from 'cbor';
import { verifyLedgerPoP } from './authutils';
import { FilecoinTxBuilder } from '@src/testing/mocks/builders';

// Ensure noble-secp256k1 HMAC is set in case setup didn't run yet
if (!etc.hmacSha256Sync) {
etc.hmacSha256Sync = (key: Uint8Array, ...msgs: Uint8Array[]) =>
hmac(sha256, key, etc.concatBytes(...msgs));
}

describe('verifyLedgerPoP (integration)', () => {
it('returns true for a valid signed transaction and matching challenge', async () => {
const challenge = 'challenge';
const { address, pubKeyBase64, transaction } = await new FilecoinTxBuilder()
.withChallenge(challenge)
.build();

const ok = await verifyLedgerPoP(address, pubKeyBase64, transaction, challenge);
expect(ok).toBe(true);
});

it("throws when To/From/Nonce don't match expected (replay guard)", async () => {
const challenge = 'challenge';
const { address, pubKeyBase64, transaction } = await new FilecoinTxBuilder()
.withChallenge(challenge)
.build();

const parsed = JSON.parse(transaction);
parsed.Message.From = 'f1different';

await expect(
verifyLedgerPoP(address, pubKeyBase64, JSON.stringify(parsed), challenge),
).rejects.toThrow("addresses don't match");
});

it('throws when derived address from pubkey does not match provided address', async () => {
const challenge = 'challenge';
const { address, transaction } = await new FilecoinTxBuilder().withChallenge(challenge).build();

// Provide a mismatching pubkey (different private key)
const otherPriv = Uint8Array.from(
Buffer.from('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', 'hex'),
);
const otherPub = getPublicKey(otherPriv, false);
const otherPubB64 = Buffer.from(otherPub).toString('base64');

await expect(verifyLedgerPoP(address, otherPubB64, transaction, challenge)).rejects.toThrow(
'wrong key for address',
);
});

it("throws when pre-image doesn't match", async () => {
const challenge = 'challenge';
const { address, pubKeyBase64, transaction } = await new FilecoinTxBuilder()
.withChallenge(challenge)
.build();
await expect(
verifyLedgerPoP(address, pubKeyBase64, transaction, 'different-challenge'),
).rejects.toThrow("pre-images don't match");
});

it("throws when signature doesn't exist", async () => {
const challenge = 'challenge';
const { address, pubKeyBase64, transaction } = await new FilecoinTxBuilder()
.withChallenge(challenge)
.build();
const parsed = JSON.parse(transaction);
parsed.Signature.Data = '';

await expect(
verifyLedgerPoP(address, pubKeyBase64, JSON.stringify(parsed), challenge),
).rejects.toThrow("signature doesn't exist");
});

it('throws when signature has wrong length', async () => {
const challenge = 'challenge';
const { address, pubKeyBase64, transaction } = await new FilecoinTxBuilder()
.withChallenge(challenge)
.build();
const parsed = JSON.parse(transaction);
const sigBytes = Buffer.from(parsed.Signature.Data, 'base64');
// Drop recovery byte to make it 64
const wrongLen = sigBytes.subarray(0, 64);
parsed.Signature.Data = Buffer.from(wrongLen).toString('base64');

await expect(
verifyLedgerPoP(address, pubKeyBase64, JSON.stringify(parsed), challenge),
).rejects.toThrow('Bad signature length: 64');
});

it('should throw "addresses don\'t match" when nonce is not 0', async () => {
const challenge = 'challenge';
const { address, pubKeyBase64, transaction } = await new FilecoinTxBuilder()
.withCustomMessage({ Nonce: 1 })
.withChallenge(challenge)
.build();

await expect(verifyLedgerPoP(address, pubKeyBase64, transaction, challenge)).rejects.toThrow(
"addresses don't match",
);
});

it('should throw "addresses don\'t match" when address does not match', async () => {
const challenge = 'challenge';
const { address, pubKeyBase64, transaction } = await new FilecoinTxBuilder()
.withChallenge(challenge)
.build();

const parsed = JSON.parse(transaction);
parsed.Message.To = 'f1evc3p45ke4apzvi5ix25mniemuva6umggusmdif';

await expect(
verifyLedgerPoP(address, pubKeyBase64, JSON.stringify(parsed), challenge),
).rejects.toThrow("addresses don't match");
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@ import {
httpPut,
request,
requestBody,
requestParam,
response,
} from 'inversify-express-utils';

import { badRequest, ok } from '@src/api/http/processors/response';
import { badPermissions, badRequest, ok } from '@src/api/http/processors/response';
import { TYPES } from '@src/types';
import { RefreshIssuesCommand } from '@src/application/use-cases/refresh-issues/refresh-issues.command';
import { GetRefreshesQuery } from '@src/application/queries/get-refreshes/get-refreshes.query';
Expand All @@ -21,6 +22,14 @@ import { UpsertIssueCommand } from '@src/application/use-cases/refresh-issues/up
import { IssuesWebhookPayload } from '@src/infrastructure/clients/github';
import { validateIssueUpsert, validateRefreshesQuery } from '@src/api/http/validators';
import { RESPONSE_MESSAGES } from '@src/constants';
import { validateGovernanceReview } from '../validators';
import { validateRequest } from '../middleware/validate-request.middleware';
import { GovernanceReviewDto } from '@src/application/dtos/GovernanceReviewDto';
import { RoleService } from '@src/application/services/role.service';
import { SignatureType } from '@src/patterns/decorators/signature-guard.decorator';
import { SignatureGuard } from '@src/patterns/decorators/signature-guard.decorator';
import { RejectRefreshCommand } from '@src/application/use-cases/refresh-issues/reject-refesh.command';
import { ApproveRefreshCommand } from '@src/application/use-cases/refresh-issues/approve-refresh.command';

const RES = RESPONSE_MESSAGES.REFRESH_CONTROLLER;

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

@httpGet('', ...validateRefreshesQuery)
Expand Down Expand Up @@ -83,4 +93,36 @@ export class RefreshController {

return res.json(ok(RES.REFRESH_SUCCESS, result));
}

@httpPost('/:githubIssueNumber/review', validateRequest(validateGovernanceReview))
@SignatureGuard(SignatureType.RefreshReview)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

reusable NestJS like guard to handle signature

async approveRefresh(
@requestParam('githubIssueNumber') githubIssueNumber: string,
@requestBody() approveRefreshDto: GovernanceReviewDto,
@response() res: Response,
) {
const id = parseInt(githubIssueNumber);
const address = approveRefreshDto.details.reviewerAddress;
const role = this._roleService.getRole(address);
if (role !== 'GOVERNANCE_TEAM') {
console.log(`Not a governance team member: ${role}`);
return res.status(403).json(badPermissions());
}

const { result } = approveRefreshDto;

const command =
result === 'approve'
? new ApproveRefreshCommand(id, approveRefreshDto.details.finalDataCap)
: new RejectRefreshCommand(id);

const refreshResult = await this._commandBus.send(command);
if (!refreshResult.success) {
return res
.status(400)
.json(badRequest(RES.FAILED_TO_UPSERT_ISSUE, [refreshResult.error.message]));
}

return res.json(ok(RES.REFRESH_SUCCESS, result));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { Request, Response, NextFunction } from 'express';
import { validationResult, ValidationChain } from 'express-validator';
import { badRequest } from '@src/api/http/processors/response';

export function validateRequest(validators: ValidationChain[]) {
return async (req: Request, res: Response, next: NextFunction) => {
await Promise.all(validators.map(validator => validator.run(req)));

const errors = validationResult(req);

if (!errors.isEmpty()) {
const errorMessages = errors.array().map(error => error.msg);
return res.status(400).json(badRequest('Validation failed', errorMessages));
}

next();
};
}
4 changes: 2 additions & 2 deletions packages/application/src/api/http/processors/response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ export const ok = (message: string, data?: any) => ({
data,
});

export const badRequest = (message: string, errors: any) => ({
export const badRequest = (message: string, errors?: any) => ({
status: '400',
message: message || 'Bad Request',
errors,
});

export const badPermissions = (message?: string) => ({
status: '400',
status: '403',
message: message || 'Bad Permissions',
});

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { VALIDATION_MESSAGES } from '@src/constants/validation-messages';
import { body, param } from 'express-validator';

export const validateGovernanceReview = [
param('githubIssueNumber')
.isInt({ min: 1, max: Number.MAX_SAFE_INTEGER })
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_GITHUB_ISSUE_NUMBER.INVALID)
.bail(),

body('result')
.exists()
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_RESULT.REQUIRED)
.bail()
.isIn(['approve', 'reject'])
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_RESULT.INVALID)
.bail(),

body('details')
.exists()
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_DETAILS.REQUIRED)
.bail()
.isObject()
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_DETAILS.INVALID),

body('details.reviewerAddress')
.exists()
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_DETAILS_REVIEWER_ADDRESS.REQUIRED)
.bail()
.isString()
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_DETAILS_REVIEWER_ADDRESS.INVALID)
.bail(),

body('details.reviewerPublicKey')
.exists()
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_DETAILS_REVIEWER_PUBLIC_KEY.REQUIRED)
.bail()
.isString()
.bail()
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_DETAILS_REVIEWER_PUBLIC_KEY.INVALID)
.bail(),

body('details.finalDataCap')
.exists()
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_DETAILS_FINAL_DATACAP.REQUIRED)
.bail()
.isNumeric()
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_DETAILS_FINAL_DATACAP.INVALID)
.bail(),

body('details.allocatorType')
.exists()
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_DETAILS_ALLOCATOR_TYPE.REQUIRED)
.bail()
.isString()
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_DETAILS_ALLOCATOR_TYPE.INVALID)
.bail(),

body('signature')
.exists()
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_DETAILS_SIGNATURE.REQUIRED)
.bail()
.isString()
.withMessage(VALIDATION_MESSAGES.GOVERNANCE_REVIEW_DETAILS_SIGNATURE.INVALID)
.bail(),
];
1 change: 1 addition & 0 deletions packages/application/src/api/http/validators/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export * from './github-issue.validator';
export * from './governance-review.validator';
12 changes: 12 additions & 0 deletions packages/application/src/application/dtos/GovernanceReviewDto.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export interface GovernanceReviewDetailsDto {
reviewerAddress: string;
reviewerPublicKey: string;
finalDataCap: number;
allocatorType: string;
}

export interface GovernanceReviewDto {
result: string;
details: GovernanceReviewDetailsDto;
signature: string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ import { GithubClient } from '@src/infrastructure/clients/github';
import { TYPES } from '@src/types';
import { GithubConfig } from '@src/domain/types';
import { ICommandBus } from '@filecoin-plus/core/src/interfaces/ICommandBus';
import { AuditMapper, IAuditMapper } from '@src/infrastructure/mappers/audit-mapper';
import { IAuditMapper } from '@src/infrastructure/mappers/audit-mapper';
import { Logger } from '@filecoin-plus/core';

describe('RefreshAuditPublisher', () => {
let container: Container;
Expand Down Expand Up @@ -37,6 +38,7 @@ describe('RefreshAuditPublisher', () => {
datacap_amount: a.datacapAmount as number,
})),
};
const loggerMock = { info: vi.fn(), error: vi.fn() };

const baseAllocator = {
application_number: 123,
Expand Down Expand Up @@ -66,6 +68,7 @@ describe('RefreshAuditPublisher', () => {
container
.bind<IAuditMapper>(TYPES.AuditMapper)
.toConstantValue(auditMapperMock as unknown as IAuditMapper);
container.bind<Logger>(TYPES.Logger).toConstantValue(loggerMock as unknown as Logger);
container
.bind(TYPES.AllocatorRegistryConfig)
.toConstantValue(configMock as unknown as GithubConfig);
Expand All @@ -82,7 +85,7 @@ describe('RefreshAuditPublisher', () => {
githubMock.mergePullRequest.mockResolvedValue({});
githubMock.deleteBranch.mockResolvedValue({});

commandBusMock.send.mockResolvedValue({ success: true, data: structuredClone(baseAllocator) });
commandBusMock.send.mockResolvedValue({ success: true, data: baseAllocator });
});

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

Expand Down
Loading