Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
| if err != nil { | ||
| if db.IsNotFound(err) { | ||
| return nil, fault.New("invalid or expired portal session", | ||
| fault.Code(codes.Auth.Authentication.KeyNotFound.URN()), |
There was a problem hiding this comment.
We should define custom errors for this and not re-use Authentication.KeyNotFound aka we can add a new one for portal stuff in pkg/codes/unkey_auth.go
| row, err := db.Query.FindValidPortalSession(ctx, s.db.RO(), token) | ||
| if err != nil { | ||
| if db.IsNotFound(err) { | ||
| return nil, fault.New("invalid or expired portal session", | ||
| fault.Code(codes.Auth.Authentication.KeyNotFound.URN()), | ||
| fault.Internal("portal session not found or expired"), | ||
| fault.Public("The portal session is invalid or has expired."), | ||
| ) | ||
| } | ||
| return nil, fault.Wrap(err, | ||
| fault.Code(codes.App.Internal.ServiceUnavailable.URN()), | ||
| fault.Internal("database error finding portal session"), | ||
| fault.Public("Failed to validate portal session."), | ||
| ) | ||
| } | ||
|
|
There was a problem hiding this comment.
should be cached, see key verifier for reference
| if err := json.Unmarshal(row.Permissions, &permissions); err != nil { | ||
| return nil, fault.Wrap(err, | ||
| fault.Code(codes.App.Internal.UnexpectedError.URN()), | ||
| fault.Internal("failed to unmarshal portal session permissions"), | ||
| fault.Public("An internal error occurred."), | ||
| ) | ||
| } |
There was a problem hiding this comment.
we could just use our db.UnmarshalNullableJSONTo
| "401": | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: '#/components/schemas/UnauthorizedErrorResponse' | ||
| description: Unauthorized — session is invalid, expired, or already exchanged | ||
| "429": | ||
| content: | ||
| application/problem+json: | ||
| schema: | ||
| $ref: '#/components/schemas/TooManyRequestsErrorResponse' | ||
| description: Too Many Requests | ||
| "500": | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: '#/components/schemas/InternalServerErrorResponse' | ||
| description: Internal server error |
There was a problem hiding this comment.
we shouldnt mix application/problem+json and application/json
which I started but we really want application/problem+json for errors
| if req.ExternalID == "" { | ||
| return fault.New("externalId is required", | ||
| fault.Code(codes.App.Validation.InvalidInput.URN()), | ||
| fault.Internal("missing externalId"), | ||
| fault.Public("externalId is required."), | ||
| ) | ||
| } | ||
|
|
||
| if len(req.Permissions) == 0 { | ||
| return fault.New("permissions is required", | ||
| fault.Code(codes.App.Validation.InvalidInput.URN()), | ||
| fault.Internal("missing permissions"), | ||
| fault.Public("permissions array is required."), | ||
| ) | ||
| } |
There was a problem hiding this comment.
I dont think we need this, the openapi validation should take core of this
2 participants
What does this PR do?
Fixes # (issue)
If there is not an issue for this, please create one first. This is used to tracking purposes and also helps us understand why this PR exists
Type of change
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtmake fmton/godirectoryconsole.logsgit pull origin mainAppreciated