Skip to content

Commit

Permalink
Account api context checks (#563)
Browse files Browse the repository at this point in the history
# Problem

closes #493 

# Changes
- Added extra verification fro signatures and accountIds
- Checks provided signatures in some endpoints
- removed wrapped `<Bytes></Bytes>` in endpoints that made sense.


## Steps to Verify:
1. run account-api
2. open swagger
3. try incorrect signatures or payloads
  • Loading branch information
aramikm authored Sep 27, 2024
1 parent d2afe7f commit b685a08
Show file tree
Hide file tree
Showing 27 changed files with 1,763 additions and 1,495 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -131,3 +131,5 @@ dist
.yarn/install-state.gz
.pnp.*
.idea/

.DS_Store
17 changes: 15 additions & 2 deletions apps/account-api/src/controllers/v1/handles-v1.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
HttpException,
Body,
UseGuards,
BadRequestException,
} from '@nestjs/common';
import { ApiBody, ApiOkResponse, ApiOperation, ApiTags } from '@nestjs/swagger';
import { HandlesService } from '#account-api/services/handles.service';
Expand All @@ -22,7 +23,7 @@ import {
import { TransactionResponse } from '#types/dtos/account/transaction.response.dto';
import { HandleResponseDto } from '#types/dtos/account/accounts.response.dto';
import { ReadOnlyGuard } from '#account-api/guards/read-only.guard';
import { u8aToHex, u8aWrapBytes } from '@polkadot/util';
import { u8aToHex } from '@polkadot/util';
import { TransactionType } from '#types/account-webhook';
import { HandleDto, MsaIdDto } from '#types/dtos/common';

Expand Down Expand Up @@ -52,13 +53,19 @@ export class HandlesControllerV1 {
*/
async createHandle(@Body() createHandleRequest: HandleRequestDto): Promise<TransactionResponse> {
try {
if (!this.handlesService.verifyHandleRequestSignature(createHandleRequest)) {
throw new BadRequestException('Provided signature is not valid for the payload!');
}
const response = await this.enqueueService.enqueueRequest<CreateHandleRequest>({
...createHandleRequest,
type: TransactionType.CREATE_HANDLE,
});
this.logger.log(`createHandle in progress. referenceId: ${response.referenceId}`);
return response;
} catch (error) {
if (error instanceof BadRequestException) {
throw error;
}
this.logger.error(error);
throw new Error('Failed to create handle');
}
Expand All @@ -77,13 +84,19 @@ export class HandlesControllerV1 {
*/
async changeHandle(@Body() changeHandleRequest: HandleRequestDto): Promise<TransactionResponse> {
try {
if (!this.handlesService.verifyHandleRequestSignature(changeHandleRequest)) {
throw new BadRequestException('Provided signature is not valid for the payload!');
}
const response = await this.enqueueService.enqueueRequest<ChangeHandleRequest>({
...changeHandleRequest,
type: TransactionType.CHANGE_HANDLE,
});
this.logger.log(`changeHandle in progress. referenceId: ${response.referenceId}`);
return response;
} catch (error) {
if (error instanceof BadRequestException) {
throw error;
}
this.logger.error(error);
throw new Error('Failed to change handle');
}
Expand All @@ -103,7 +116,7 @@ export class HandlesControllerV1 {
try {
const expiration = await this.handlesService.getExpiration();
const payload = { baseHandle: newHandle, expiration };
const encodedPayload = u8aToHex(u8aWrapBytes(this.handlesService.encodePayload(payload).toU8a()));
const encodedPayload = u8aToHex(this.handlesService.encodePayload(payload).toU8a());

return {
payload,
Expand Down
13 changes: 13 additions & 0 deletions apps/account-api/src/controllers/v1/keys-v1.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
Post,
UseGuards,
Query,
BadRequestException,
} from '@nestjs/common';
import { ApiBody, ApiOkResponse, ApiOperation, ApiTags } from '@nestjs/swagger';
import { KeysRequestDto, AddKeyRequestDto } from '#types/dtos/account/keys.request.dto';
Expand Down Expand Up @@ -53,13 +54,19 @@ export class KeysControllerV1 {
*/
async addKey(@Body() addKeysRequest: KeysRequestDto): Promise<TransactionResponse> {
try {
if (!this.keysService.verifyAddKeySignature(addKeysRequest)) {
throw new BadRequestException('Provided signature is not valid for the payload!');
}
const response = await this.enqueueService.enqueueRequest<AddKeyRequestDto>({
...addKeysRequest,
type: TransactionType.ADD_KEY,
});
this.logger.log(`AddKey in progress. referenceId: ${response.referenceId}`);
return response;
} catch (error) {
if (error instanceof BadRequestException) {
throw error;
}
this.logger.error(error);
throw new HttpException('Failed to find public keys for the given MSA Id', HttpStatus.BAD_REQUEST);
}
Expand Down Expand Up @@ -114,13 +121,19 @@ export class KeysControllerV1 {
*/
async AddNewPublicKeyAgreements(@Body() request: AddNewPublicKeyAgreementRequestDto): Promise<TransactionResponse> {
try {
if (!this.keysService.verifyPublicKeyAgreementSignature(request)) {
throw new BadRequestException('Proof is not valid for the payload!');
}
const response = await this.enqueueService.enqueueRequest<PublicKeyAgreementRequestDto>({
...request,
type: TransactionType.ADD_PUBLIC_KEY_AGREEMENT,
});
this.logger.log(`Add graph key in progress. referenceId: ${response.referenceId}`);
return response;
} catch (error) {
if (error instanceof BadRequestException) {
throw error;
}
this.logger.error(error);
throw new Error('Failed to add new key');
}
Expand Down
10 changes: 9 additions & 1 deletion apps/account-api/src/metadata.ts

Large diffs are not rendered by default.

19 changes: 9 additions & 10 deletions apps/account-api/src/services/delegation.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
} from '#types/dtos/account';
import { DelegationResponse, DelegationResponseV2 } from '#types/dtos/account/delegation.response.dto';
import { HttpException, HttpStatus, Inject, Injectable, Logger } from '@nestjs/common';
import { isAddress } from '@polkadot/util-crypto';
import blockchainConfig, { IBlockchainConfig } from '#account-lib/blockchain/blockchain.config';

@Injectable()
Expand Down Expand Up @@ -47,36 +46,36 @@ export class DelegationService {
throw new Error('Invalid msaId.');
}

async getRevokeDelegationPayload(accountId: string, providerId: string): Promise<RevokeDelegationPayloadResponseDto> {
if (!isAddress(accountId)) {
throw new HttpException('Invalid accountId', HttpStatus.BAD_REQUEST);
}
async getRevokeDelegationPayload(
accountId: string,
providerMsaId: string,
): Promise<RevokeDelegationPayloadResponseDto> {
const msaId = await this.blockchainService.publicKeyToMsaId(accountId);
if (!msaId) {
throw new HttpException('MSA ID for account not found', HttpStatus.NOT_FOUND);
}

if (providerId) {
const isValidProviderId = await this.blockchainService.isValidMsaId(providerId);
if (providerMsaId) {
const isValidProviderId = await this.blockchainService.isValidMsaId(providerMsaId);
if (!isValidProviderId) {
throw new HttpException('Invalid provider', HttpStatus.BAD_REQUEST);
}

const providerInfo = await this.blockchainService.getProviderToRegistryEntry(providerId);
const providerInfo = await this.blockchainService.getProviderToRegistryEntry(providerMsaId);
if (!providerInfo) {
throw new HttpException('Supplied ID not a Provider', HttpStatus.BAD_REQUEST);
}
}

// Validate that delegations exist for this msaId
const delegations = await this.blockchainService.getProviderDelegationForMsa(msaId, providerId);
const delegations = await this.blockchainService.getProviderDelegationForMsa(msaId, providerMsaId);
if (!delegations) {
throw new HttpException('No delegations found', HttpStatus.NOT_FOUND);
}
if (delegations.revokedAtBlock !== 0) {
throw new HttpException('Delegation already revoked', HttpStatus.NOT_FOUND);
}
return this.blockchainService.createRevokedDelegationPayload(accountId, providerId);
return this.blockchainService.createRevokedDelegationPayload(accountId, providerMsaId);
}

async postRevokeDelegation(revokeDelegationRequest: RevokeDelegationPayloadRequestDto): Promise<TransactionResponse> {
Expand Down
7 changes: 7 additions & 0 deletions apps/account-api/src/services/handles.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { BlockchainService } from '#account-lib/blockchain/blockchain.service';
import { HandleResponseDto } from '#types/dtos/account/accounts.response.dto';
import { BlockchainConstants } from '#account-lib/blockchain/blockchain-constants';
import { HandleRequestDto } from '#types/dtos/account';
import { u8aToHex, u8aWrapBytes } from '@polkadot/util';
import { verifySignature } from '#account-lib/utils/utility';

@Injectable()
export class HandlesService {
Expand All @@ -29,4 +31,9 @@ export class HandlesService {
encodePayload(payload: HandleRequestDto['payload']) {
return this.blockchainService.createClaimHandPayloadType(payload.baseHandle, payload.expiration);
}

verifyHandleRequestSignature(request: HandleRequestDto): boolean {
const encodedPayload = u8aToHex(u8aWrapBytes(this.encodePayload(request.payload).toU8a()));
return verifySignature(encodedPayload, request.proof, request.accountId).isValid;
}
}
27 changes: 24 additions & 3 deletions apps/account-api/src/services/keys.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,15 @@ import { EnvironmentInterface, EnvironmentType, Graph } from '@dsnp/graph-sdk';
import { HexString } from '@polkadot/util/types';
import {
AddNewPublicKeyAgreementPayloadRequest,
AddNewPublicKeyAgreementRequestDto,
ItemActionType,
ItemizedSignaturePayloadDto,
} from '#types/dtos/account/graphs.request.dto';
import { u8aToHex, u8aWrapBytes } from '@polkadot/util';
import { BlockchainConstants } from '#account-lib/blockchain/blockchain-constants';
import apiConfig, { IAccountApiConfig } from '#account-api/api.config';
import { verifySignature } from '#account-lib/utils/utility';
import { KeysRequestDto } from '#types/dtos/account';

@Injectable()
export class KeysService {
Expand Down Expand Up @@ -71,9 +74,7 @@ export class KeysService {
],
};

const encodedPayload = u8aToHex(
u8aWrapBytes(this.blockchainService.createItemizedSignaturePayloadV2Type(payload).toU8a()),
);
const encodedPayload = u8aToHex(this.blockchainService.createItemizedSignaturePayloadV2Type(payload).toU8a());
return {
payload,
encodedPayload,
Expand All @@ -92,4 +93,24 @@ export class KeysService {
// standard expiration in SIWF is 10 minutes
return lastFinalizedBlockNumber + 600 / BlockchainConstants.SECONDS_PER_BLOCK;
}

verifyAddKeySignature(request: KeysRequestDto): boolean {
const encodedPayload = u8aToHex(
u8aWrapBytes(this.blockchainService.createAddPublicKeyToMsaPayload(request.payload).toU8a()),
);
const msaOwnerVerification = verifySignature(encodedPayload, request.msaOwnerSignature, request.msaOwnerAddress);
const keyOwnerVerification = verifySignature(
encodedPayload,
request.newKeyOwnerSignature,
request.payload.newPublicKey,
);
return msaOwnerVerification.isValid && keyOwnerVerification.isValid;
}

verifyPublicKeyAgreementSignature(request: AddNewPublicKeyAgreementRequestDto): boolean {
const encodedPayload = u8aToHex(
u8aWrapBytes(this.blockchainService.createItemizedSignaturePayloadV2Type(request.payload).toU8a()),
);
return verifySignature(encodedPayload, request.proof, request.accountId).isValid;
}
}
14 changes: 10 additions & 4 deletions apps/account-api/test/delegations.controller.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,16 @@ describe('Delegation Controller', () => {
const { keypair } = users[1];
const invalidAccountId = `${keypair.address.slice(0, -1)}5H`;
const getPath: string = `/v1/delegation/revokeDelegation/${invalidAccountId}/${providerId}`;
await request(httpServer).get(getPath).expect(HttpStatus.BAD_REQUEST).expect({
statusCode: HttpStatus.BAD_REQUEST,
message: 'Invalid accountId',
});
await request(httpServer)
.get(getPath)
.expect(HttpStatus.BAD_REQUEST)
.expect({
statusCode: HttpStatus.BAD_REQUEST,
error: 'Bad Request',
message: [
'accountId should be a valid 32 bytes representing an account Id or address in Hex or SS58 format!',
],
});
});

it('(GET) /v1/delegation/revokeDelegation/:accountId/:providerId with valid accountId: no msa', async () => {
Expand Down
Loading

0 comments on commit b685a08

Please sign in to comment.