Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

schemas for serialized requests and usage in engine #238

Merged
merged 3 commits into from
May 2, 2024
Merged

schemas for serialized requests and usage in engine #238

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7ad1c9a
schemas for serialized requests and usage in engine
Ptroger May 1, 2024
638c591
Zod interceptor to return correct DTO from controller
Ptroger May 2, 2024
093666e
added e2e test for serialization
Ptroger May 2, 2024
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
11 changes: 9 additions & 2 deletions apps/policy-engine/src/engine/http/rest/controller/evaluation.controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { SerializedEvaluationResponse } from '@narval/policy-engine-shared'
import { Body, Controller, HttpCode, HttpStatus, Post, UseGuards } from '@nestjs/common'
import { ClientId } from '../../../../shared/decorator/client-id.decorator'
import { ClientSecretGuard } from '../../../../shared/guard/client-secret.guard'
import { EvaluationService } from '../../../core/service/evaluation.service'
import { EvaluationRequestDto } from '../dto/evaluation-request.dto'
import { SerializedEvaluationResponseDto } from '../dto/serialized-evaluation-response.dto'

@Controller('/evaluations')
@UseGuards(ClientSecretGuard)
Expand All @@ -11,7 +13,12 @@ export class EvaluationController {

@Post()
@HttpCode(HttpStatus.OK)
async evaluate(@ClientId() clientId: string, @Body() body: EvaluationRequestDto) {
return this.evaluationService.evaluate(clientId, body)
async evaluate(
@ClientId() clientId: string,
@Body() body: EvaluationRequestDto
): Promise<SerializedEvaluationResponseDto> {
return this.evaluationService.evaluate(clientId, body).then((response) => {
return SerializedEvaluationResponse.parse(response)
})
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's step up your game on the controller implementation by using DTO for documentation and serialization.

The way you implemented the DTO today isn't adding much value because Nest doesn't know how to use it.

  @Post()
  @HttpCode(HttpStatus.OK)
  // Let's start by adding Swagger decorators for documentation.
  @ApiOperation({
    summary: 'Evaluates a request into a decision'
  })
  @ApiHeader({
    name: REQUEST_HEADER_CLIENT_ID
  })
  @ApiResponse({
    status: HttpStatus.OK,
    type: SerializedEvaluationResponseDto
  })
  async evaluate(@ClientId() clientId: string, @Body() body: EvaluationRequestDto) {
    const response = await this.evaluationService.evaluate(clientId, body)

    // Now let's use the DTO to serialize the response.
    return new SerializedEvaluationResponseDto(response)
  }

Let me know if that doesn't work

The serialization must be configured by either module and/or application. As good practice, we always add it to the application level, so all modules have it in production.

armory/apps/policy-engine/src/main.ts

Lines 17 to 21 in cdd72ef

const withGlobalPipes = (app: INestApplication): INestApplication => {
app.useGlobalPipes(new ValidationPipe({ transform: true }))
return app
}

Note the { transform: true } option.

You'll notice we also add it per module. The reason is that modules are standalone functionalities, and we test them in a mock server instead of the real one.

Since we're moving away from the standard DTOs for validation and serialization, we'll need to use nest-zod utilities.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the link. I've implemented it following the documentation you linked. Meaning:

  • ZodSerializer interceptor set at the module level, not at the whole app level.
  • Define ZodSerializer schemas on controller if you want to ensure serialization

As you asked 'let me know if that doesn't work', your code snippet as is didnt. Also, from what I understood of the documentation and what I've tried, you need to specifically add @ZodSerializerDto decorator with the schema you want to use on every route that uses the interceptor. Did you wanted to pass the SerializerDto as part of the @ApiResponse decorator ? If so, I didn't managed to configure that, nor do I think it is possible looking at the documentation.

}
}
4 changes: 4 additions & 0 deletions apps/policy-engine/src/engine/http/rest/dto/serialized-evaluation-response.dto.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { SerializedEvaluationResponse } from '@narval/policy-engine-shared'
import { createZodDto } from 'nestjs-zod'

export class SerializedEvaluationResponseDto extends createZodDto(SerializedEvaluationResponse) {}
4 changes: 2 additions & 2 deletions apps/policy-engine/src/open-policy-agent/core/type/open-policy-agent.type.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
Address,
CredentialEntity,
Feed,
TransactionRequest,
SerializedTransactionRequest,
UserRole
} from '@narval/policy-engine-shared'
import { Intent } from '@narval/transaction-request-intent'
Expand All @@ -20,7 +20,7 @@ export type OpenPolicyAgentInstance = PromiseType<ReturnType<typeof loadPolicy>>
export type Input = {
action: Action
intent?: Intent
transactionRequest?: TransactionRequest
transactionRequest?: SerializedTransactionRequest
principal: CredentialEntity
resource?: { uid: string }
approvals?: CredentialEntity[]
Expand Down
3 changes: 2 additions & 1 deletion apps/policy-engine/src/open-policy-agent/core/util/evaluation.util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
EvaluationRequest,
Feed,
Request,
SerializedTransactionRequest,
SignMessageAction,
SignRawAction,
SignTransactionAction,
Expand Down Expand Up @@ -37,7 +38,7 @@ const toSignTransaction: Mapping<SignTransactionAction> = (request, principal, a
principal,
approvals,
intent: result.intent,
transactionRequest: request.transactionRequest,
transactionRequest: SerializedTransactionRequest.parse(request.transactionRequest),
resource: { uid: request.resourceId },
feeds
}
Expand Down
12 changes: 12 additions & 0 deletions packages/policy-engine-shared/src/lib/type/action.type.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@ export const TransactionRequest = z.object({
})
export type TransactionRequest = z.infer<typeof TransactionRequest>

export const SerializedTransactionRequest = TransactionRequest.extend({
gas: z.coerce.string().optional(),
maxFeePerGas: z.coerce.string().optional(),
maxPriorityFeePerGas: z.coerce.string().optional()
})
export type SerializedTransactionRequest = z.infer<typeof SerializedTransactionRequest>

export const Eip712Domain = z.object({
name: z.string().optional(),
version: z.string().optional(),
Expand Down Expand Up @@ -76,6 +83,11 @@ export const SignTransactionAction = BaseAction.merge(
)
export type SignTransactionAction = z.infer<typeof SignTransactionAction>

export const SerializedTransactionAction = SignTransactionAction.extend({
transactionRequest: SerializedTransactionRequest
})
export type SerializedTransactionAction = z.infer<typeof SerializedTransactionAction>

// Matching viem's SignableMessage options
// See https://viem.sh/docs/actions/wallet/signMessage#message
export const SignableMessage = z.union([
Expand Down
59 changes: 43 additions & 16 deletions packages/policy-engine-shared/src/lib/type/domain.type.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import { ZodTypeAny, z } from 'zod'
import { AccountId } from '../util/caip.util'
import { SignMessageAction, SignRawAction, SignTransactionAction, SignTypedDataAction } from './action.type'
import {
SerializedTransactionAction,
SignMessageAction,
SignRawAction,
SignTransactionAction,
SignTypedDataAction
} from './action.type'

export enum Decision {
PERMIT = 'Permit',
Expand Down Expand Up @@ -92,6 +98,14 @@ export const Request = z.discriminatedUnion('action', [
])
export type Request = z.infer<typeof Request>

export const SerializedRequest = z.discriminatedUnion('action', [
SerializedTransactionAction,
SignMessageAction,
SignTypedDataAction,
SignRawAction
])
export type SerializedRequest = z.infer<typeof SerializedRequest>

export const Feed = <Data extends ZodTypeAny>(dataSchema: Data) =>
z.object({
source: z.string(),
Expand Down Expand Up @@ -137,6 +151,11 @@ export const EvaluationRequest = z
.describe('The action being authorized')
export type EvaluationRequest = z.infer<typeof EvaluationRequest>

export const SerializedEvaluationRequest = EvaluationRequest.extend({
request: SerializedRequest
})
export type SerializedEvaluationRequest = z.infer<typeof SerializedEvaluationRequest>

export const ApprovalRequirement = z.object({
approvalCount: z.number().min(0),
approvalEntityType: z.nativeEnum(EntityType).describe('The number of requried approvals'),
Expand All @@ -145,22 +164,30 @@ export const ApprovalRequirement = z.object({
})
export type ApprovalRequirement = z.infer<typeof ApprovalRequirement>

export type AccessToken = {
value: string // JWT
// could include a key-proof
}
export const AccessToken = z.object({
value: JwtString
})
export type AccessToken = z.infer<typeof AccessToken>

export const EvaluationResponse = z.object({
decision: z.nativeEnum(Decision),
request: Request.optional(),
approvals: z
.object({
required: z.array(ApprovalRequirement).optional(),
missing: z.array(ApprovalRequirement).optional(),
satisfied: z.array(ApprovalRequirement).optional()
})
.optional(),
accessToken: AccessToken.optional(),
transactionRequestIntent: z.unknown().optional()
})
export type EvaluationResponse = z.infer<typeof EvaluationResponse>

export type EvaluationResponse = {
decision: Decision
request?: Request
approvals?: {
required: ApprovalRequirement[]
missing: ApprovalRequirement[]
satisfied: ApprovalRequirement[]
}
accessToken?: AccessToken
transactionRequestIntent?: unknown
}
export const SerializedEvaluationResponse = EvaluationResponse.extend({
request: SerializedRequest.optional()
})
export type SerializedEvaluationResponse = z.infer<typeof SerializedEvaluationResponse>

export type Hex = `0x${string}`

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for refactoring it.

Expand Down
Loading