Skip to content

Commit

Permalink
graph-api: added more validation (#515)
Browse files Browse the repository at this point in the history
# Problem

Adds extra validation on graph-api dtos and request/path params

Closes #496 

# Discussion
- Since we are going to overhaul our way of e2e testing it didn't make
sense to add tests which would be replaced in the future.

## Steps to Verify:

1. Run the api
2. Go to swagger
3. Try some invalid values in requests
  • Loading branch information
aramikm committed Sep 18, 2024
1 parent 2953b89 commit 097b7af
Show file tree
Hide file tree
Showing 23 changed files with 4,814 additions and 4,724 deletions.
1 change: 1 addition & 0 deletions .github/workflows/e2e-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ jobs:
- 'docker-compose.yaml'
- 'docker-compose-e2e.*.yaml'
- 'libs/types'
- 'libs/utils'
account:
- 'apps/account-api/**'
- 'apps/account-worker/**'
Expand Down
4 changes: 2 additions & 2 deletions apps/graph-api/src/controllers/v1/graph-v1.controller.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { ApiService } from '#graph-api/api.service';
import { ReadOnlyGuard } from '#graph-api/guards/read-only.guard';
import { ReadOnlyDeploymentGuard } from '#graph-api/guards/read-only-deployment-guard.service';
import { UserGraphDto, GraphsQueryParamsDto, GraphChangeRepsonseDto, ProviderGraphDto } from '#types/dtos/graph';
import { Controller, Post, HttpCode, HttpStatus, Logger, Body, Put, UseGuards } from '@nestjs/common';
import { ApiBody, ApiCreatedResponse, ApiOkResponse, ApiOperation, ApiTags } from '@nestjs/swagger';
Expand Down Expand Up @@ -31,7 +31,7 @@ export class GraphControllerV1 {

// Enqueue a request to update a user graph
@Put()
@UseGuards(ReadOnlyGuard)
@UseGuards(ReadOnlyDeploymentGuard)
@HttpCode(HttpStatus.CREATED)
@ApiOperation({ summary: "Request an update to a given user's graph" })
@ApiCreatedResponse({ description: 'Graph update request created successfully', type: GraphChangeRepsonseDto })
Expand Down
33 changes: 5 additions & 28 deletions apps/graph-api/src/controllers/v1/webhooks-v1.controller.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { ApiService } from '#graph-api/api.service';
import { WatchGraphsDto } from '#types/dtos/graph';
import { MsaIdDto, UrlDto } from '#types/dtos/common';
import {
Controller,
HttpCode,
Expand Down Expand Up @@ -38,34 +39,22 @@ export class WebhooksControllerV1 {
@HttpCode(HttpStatus.OK)
@ApiOperation({ summary: 'Get all registered webhooks for a specific MSA Id' })
@ApiOkResponse({ description: 'Retrieved all registered webhooks for the given MSA Id' })
@ApiParam({
name: 'msaId',
example: '2',
type: String,
description: 'MSA Id for which to request registered webhooks ',
})
@ApiQuery({
name: 'includeAll',
type: Boolean,
example: true,
required: false,
description: "Boolean whether to include webhooks registered for 'all' MSA Ids (default: true)",
})
async getWebhooksForMsa(@Param('msaId') msaId: string, @Query('includeAll') includeAll = 'true'): Promise<string[]> {
async getWebhooksForMsa(@Param() { msaId }: MsaIdDto, @Query('includeAll') includeAll = 'true'): Promise<string[]> {
return this.apiService.getWebhooksForMsa(msaId, JSON.parse(includeAll ?? 'true'));
}

@Get('urls')
@HttpCode(HttpStatus.OK)
@ApiOperation({ summary: 'Get all webhooks registered to the specified URL' })
@ApiOkResponse({ description: 'Retrieved all webhooks registered to the specified URL' })
@ApiQuery({
name: 'url',
type: String,
example: 'http://localhost/webhook',
description: 'URL for which to fetch all watched MSAs',
})
async getWebhooksForUrl(@Query('url') url: string): Promise<string[]> {
async getWebhooksForUrl(@Query() { url }: UrlDto): Promise<string[]> {
return this.apiService.getWatchedGraphsForUrl(url);
}

Expand Down Expand Up @@ -100,27 +89,15 @@ export class WebhooksControllerV1 {
@HttpCode(HttpStatus.OK)
@ApiOperation({ summary: 'Delete all webhooks registered for a specific MSA' })
@ApiOkResponse({ description: 'Removed all registered webhooks for the specified MSA' })
@ApiParam({
name: 'msaId',
type: String,
example: '2',
description: 'MSA for which to remove all webhook registrations',
})
deleteWebhooksForMsa(@Param('msaId') msaId: string): Promise<void> {
deleteWebhooksForMsa(@Param() { msaId }: MsaIdDto): Promise<void> {
return this.apiService.deleteWebhooksForUser(msaId);
}

@Delete('urls')
@HttpCode(HttpStatus.OK)
@ApiOperation({ summary: 'Delete all MSA webhooks registered with the given URL' })
@ApiOkResponse({ description: 'Removed all webhooks registered to the specified URL' })
@ApiQuery({
name: 'url',
type: String,
example: 'http://localhost/webhook',
description: 'URL for which to un-watch all MSAs',
})
deleteAllWebhooksForUrl(@Query('url') url: string): Promise<void> {
deleteAllWebhooksForUrl(@Query() { url }: UrlDto): Promise<void> {
return this.apiService.deleteWebhooksForUrl(url);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { ConfigService } from '#graph-lib/config';
import { Injectable, CanActivate, ExecutionContext } from '@nestjs/common';

@Injectable()
export class ReadOnlyGuard implements CanActivate {
export class ReadOnlyDeploymentGuard implements CanActivate {
// eslint-disable-next-line no-useless-constructor, no-empty-function
constructor(private configService: ConfigService) {}

Expand Down
2 changes: 1 addition & 1 deletion apps/graph-api/src/metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@ export default async () => {
["../../../libs/types/src/dtos/graph/user-graph.dto"]: await import("../../../libs/types/src/dtos/graph/user-graph.dto"),
["../../../libs/types/src/dtos/graph/graph-change-response.dto"]: await import("../../../libs/types/src/dtos/graph/graph-change-response.dto")
};
return { "@nestjs/swagger": { "models": [[import("../../../libs/types/src/dtos/graph/connection.dto"), { "ConnectionDto": { dsnpId: { required: true, type: () => String }, privacyType: { required: true, enum: t["../../../libs/types/src/dtos/graph/privacy-type.enum"].PrivacyType }, direction: { required: true, enum: t["../../../libs/types/src/dtos/graph/direction.enum"].Direction }, connectionType: { required: true, enum: t["../../../libs/types/src/dtos/graph/connection-type.enum"].ConnectionType } }, "ConnectionDtoWrapper": { data: { required: true, type: () => [t["../../../libs/types/src/dtos/graph/connection.dto"].ConnectionDto] } } }], [import("../../../libs/types/src/dtos/graph/dsnp-graph-edge.dto"), { "DsnpGraphEdgeDto": { userId: { required: true, type: () => String }, since: { required: true, type: () => Number } } }], [import("../../../libs/types/src/dtos/graph/graph-change-response.dto"), { "GraphChangeRepsonseDto": { referenceId: { required: true, type: () => String } } }], [import("../../../libs/types/src/dtos/graph/graph-key-pair.dto"), { "GraphKeyPairDto": { publicKey: { required: true, type: () => String }, privateKey: { required: true, type: () => String }, keyType: { required: true, type: () => String, enum: t["../../../libs/types/src/dtos/graph/key-type.enum"].KeyType } } }], [import("../../../libs/types/src/dtos/graph/graph-query-params.dto"), { "GraphsQueryParamsDto": { dsnpIds: { required: true, type: () => [String] }, privacyType: { required: true, enum: t["../../../libs/types/src/dtos/graph/privacy-type.enum"].PrivacyType }, graphKeyPairs: { required: false, type: () => [t["../../../libs/types/src/dtos/graph/graph-key-pair.dto"].GraphKeyPairDto] } } }], [import("../../../libs/types/src/dtos/graph/provider-graph.dto"), { "ProviderGraphDto": { dsnpId: { required: true, type: () => String }, connections: { required: true, type: () => ({ data: { required: true, type: () => [t["../../../libs/types/src/dtos/graph/connection.dto"].ConnectionDto] } }) }, graphKeyPairs: { required: false, type: () => [t["../../../libs/types/src/dtos/graph/graph-key-pair.dto"].GraphKeyPairDto] }, webhookUrl: { required: false, type: () => String } } }], [import("../../../libs/types/src/dtos/graph/user-graph.dto"), { "UserGraphDto": { dsnpId: { required: true, type: () => String }, dsnpGraphEdges: { required: false, type: () => [t["../../../libs/types/src/dtos/graph/dsnp-graph-edge.dto"].DsnpGraphEdgeDto] } } }], [import("../../../libs/types/src/dtos/graph/watch-graphs.dto"), { "WatchGraphsDto": { dsnpIds: { required: false, type: () => [String] }, webhookEndpoint: { required: true, type: () => String } } }]], "controllers": [[import("./controllers/v1/graph-v1.controller"), { "GraphControllerV1": { "getGraphs": { type: [t["../../../libs/types/src/dtos/graph/user-graph.dto"].UserGraphDto] }, "updateGraph": { type: t["../../../libs/types/src/dtos/graph/graph-change-response.dto"].GraphChangeRepsonseDto } } }], [import("./controllers/health.controller"), { "HealthController": { "healthz": {}, "livez": {}, "readyz": {} } }], [import("./controllers/v1/webhooks-v1.controller"), { "WebhooksControllerV1": { "getAllWebhooks": { type: Object }, "getWebhooksForMsa": { type: [String] }, "getWebhooksForUrl": { type: [String] }, "watchGraphs": {}, "deleteAllWebhooks": {}, "deleteWebhooksForMsa": {}, "deleteAllWebhooksForUrl": {} } }]] } };
return { "@nestjs/swagger": { "models": [[import("../../../libs/types/src/dtos/graph/connection.dto"), { "ConnectionDto": { dsnpId: { required: true, type: () => String }, privacyType: { required: true, enum: t["../../../libs/types/src/dtos/graph/privacy-type.enum"].PrivacyType }, direction: { required: true, enum: t["../../../libs/types/src/dtos/graph/direction.enum"].Direction }, connectionType: { required: true, enum: t["../../../libs/types/src/dtos/graph/connection-type.enum"].ConnectionType } }, "ConnectionDtoWrapper": { data: { required: true, type: () => [t["../../../libs/types/src/dtos/graph/connection.dto"].ConnectionDto] } } }], [import("../../../libs/types/src/dtos/graph/dsnp-graph-edge.dto"), { "DsnpGraphEdgeDto": { userId: { required: true, type: () => String }, since: { required: true, type: () => Number } } }], [import("../../../libs/types/src/dtos/graph/graph-change-response.dto"), { "GraphChangeRepsonseDto": { referenceId: { required: true, type: () => String } } }], [import("../../../libs/types/src/dtos/graph/graph-key-pair.dto"), { "GraphKeyPairDto": { publicKey: { required: true, type: () => String }, privateKey: { required: true, type: () => String }, keyType: { required: true, type: () => String, enum: t["../../../libs/types/src/dtos/graph/key-type.enum"].KeyType } } }], [import("../../../libs/types/src/dtos/graph/graph-query-params.dto"), { "GraphsQueryParamsDto": { dsnpIds: { required: true, type: () => [String] }, privacyType: { required: true, enum: t["../../../libs/types/src/dtos/graph/privacy-type.enum"].PrivacyType }, graphKeyPairs: { required: false, type: () => [t["../../../libs/types/src/dtos/graph/graph-key-pair.dto"].GraphKeyPairDto] } } }], [import("../../../libs/types/src/dtos/graph/provider-graph.dto"), { "ProviderGraphDto": { dsnpId: { required: true, type: () => String }, connections: { required: true, type: () => t["../../../libs/types/src/dtos/graph/connection.dto"].ConnectionDtoWrapper }, graphKeyPairs: { required: false, type: () => [t["../../../libs/types/src/dtos/graph/graph-key-pair.dto"].GraphKeyPairDto] }, webhookUrl: { required: false, type: () => String } } }], [import("../../../libs/types/src/dtos/graph/user-graph.dto"), { "UserGraphDto": { dsnpId: { required: true, type: () => String }, dsnpGraphEdges: { required: false, type: () => [t["../../../libs/types/src/dtos/graph/dsnp-graph-edge.dto"].DsnpGraphEdgeDto] } } }], [import("../../../libs/types/src/dtos/graph/watch-graphs.dto"), { "WatchGraphsDto": { dsnpIds: { required: false, type: () => [String] }, webhookEndpoint: { required: true, type: () => String } } }], [import("../../../libs/types/src/dtos/common/params.dto"), { "MsaIdDto": { msaId: { required: true, type: () => String } }, "UrlDto": { url: { required: true, type: () => String } } }]], "controllers": [[import("./controllers/v1/graph-v1.controller"), { "GraphControllerV1": { "getGraphs": { type: [t["../../../libs/types/src/dtos/graph/user-graph.dto"].UserGraphDto] }, "updateGraph": { type: t["../../../libs/types/src/dtos/graph/graph-change-response.dto"].GraphChangeRepsonseDto } } }], [import("./controllers/health.controller"), { "HealthController": { "healthz": {}, "livez": {}, "readyz": {} } }], [import("./controllers/v1/webhooks-v1.controller"), { "WebhooksControllerV1": { "getAllWebhooks": { type: Object }, "getWebhooksForMsa": { type: [String] }, "getWebhooksForUrl": { type: [String] }, "watchGraphs": {}, "deleteAllWebhooks": {}, "deleteWebhooksForMsa": {}, "deleteAllWebhooksForUrl": {} } }]] } };
};
Loading

0 comments on commit 097b7af

Please sign in to comment.