Fix APIKey in search resolver for get role#16540
Conversation
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
| @AuthUserWorkspaceId() userWorkspaceId: string | undefined, | ||
| @AuthApiKey() apiKey: string | undefined, | ||
| @AuthApiKey() apiKey: ApiKeyEntity | undefined, |
There was a problem hiding this comment.
Bug: The type of apiKey from @AuthApiKey() was updated in one resolver but not in four other files using the same decorator, causing a type mismatch.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
The pull request correctly changes the type of the apiKey parameter from the @AuthApiKey() decorator to ApiKeyEntity in search.resolver.ts. However, this change was not propagated to other resolvers (workspace.resolver.ts, user.resolver.ts, billing.resolver.ts) and controllers (mcp-core.controller.ts) that still declare apiKey as a string. These modules will now receive an ApiKeyEntity object at runtime instead of the expected string. This will cause type mismatches when the apiKey object is passed to downstream services like permissionsService, leading to permission errors or silent failures in workspace and user operations.
💡 Suggested Fix
Update the type of the apiKey parameter from string to ApiKeyEntity in all resolvers and controllers using the @AuthApiKey() decorator (workspace.resolver.ts, user.resolver.ts, billing.resolver.ts, and mcp-core.controller.ts). Ensure that apiKey.id is used to access the string ID where a string is expected by downstream services.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location:
packages/twenty-server/src/engine/core-modules/search/search.resolver.ts#L39-L40
Potential issue: The pull request correctly changes the type of the `apiKey` parameter
from the `@AuthApiKey()` decorator to `ApiKeyEntity` in `search.resolver.ts`. However,
this change was not propagated to other resolvers (`workspace.resolver.ts`,
`user.resolver.ts`, `billing.resolver.ts`) and controllers (`mcp-core.controller.ts`)
that still declare `apiKey` as a `string`. These modules will now receive an
`ApiKeyEntity` object at runtime instead of the expected string. This will cause type
mismatches when the `apiKey` object is passed to downstream services like
`permissionsService`, leading to permission errors or silent failures in workspace and
user operations.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7458321
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:1110 This environment will automatically shut down when the PR is closed or after 5 hours. |
Following this #16540 Those were not failing (yet) but were wrongly typed and error prone --------- Co-authored-by: Guillim <guillim@users.noreply.github.com>
Fixes #16534
Typing was wrong, apiKey from request object is ApiKeyEntity and not a string which was then failing when using
error
Before
After