fix(medusa): resolve user_id from user linked to secret key on draft order edit with api-key auth #14053
Conversation
🦋 Changeset detectedLatest commit: 6a6b0b8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 74 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 8 Skipped Deployments
|
| let userId = req.auth_context.actor_id | ||
| const authType = req.auth_context.actor_type as AuthType | ||
|
|
||
| const shouldResolveUser = authType === 'api-key' | ||
| if (shouldResolveUser) { | ||
| const {data: [apiKey]} = await query.graph({ | ||
| entity: 'api_key', | ||
| fields: ['created_by'], | ||
| filters: { id: userId }, | ||
| }) | ||
| userId = apiKey.created_by | ||
| } |
There was a problem hiding this comment.
thought: This seems like something that should live in a middleware, as this is not really specific to draft orders ... think we should consider doing that instead
Also, this code does not follow our linting conventions
There was a problem hiding this comment.
Agreed I think we could follow something similar to existent publishable_key_context for secret_key_context and add the created_by prop in the new middleware, I started with this approach. Oh yes, forgot to run the formatter, will do :)
There was a problem hiding this comment.
I think there is an issue with your linter configuration ;)
There was a problem hiding this comment.
Pushed commit with middleware, applied it to all /admin routes, could you let me know:
- if the middleware looks good to you
- if the way i applied to all admin routes is the correct way
integration-tests/http/__tests__/draft-order/admin/draft-order.spec.ts
Outdated
Show resolved
Hide resolved
integration-tests/http/__tests__/draft-order/admin/draft-order.spec.ts
Outdated
Show resolved
Hide resolved
| let userId = req.auth_context.actor_id | ||
| const authType = req.auth_context.actor_type as AuthType | ||
|
|
||
| const shouldResolveUser = authType === 'api-key' | ||
| if (shouldResolveUser) { | ||
| const {data: [apiKey]} = await query.graph({ | ||
| entity: 'api_key', | ||
| fields: ['created_by'], | ||
| filters: { id: userId }, | ||
| }) | ||
| userId = apiKey.created_by | ||
| } |
….spec.ts Co-authored-by: Adrien de Peretti <[email protected]>
….spec.ts Co-authored-by: Adrien de Peretti <[email protected]>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on December 17
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
packages/core/framework/src/http/middlewares/set-secret-api-key-context.ts
Show resolved
Hide resolved
| const shouldSkip = req.auth_context.actor_type !== "api-key" | ||
| if (shouldSkip) { | ||
| next() | ||
| } |
There was a problem hiding this comment.
Bug: Middleware: API Key Logic Runs Unchecked
Missing return statement after calling next() when skipping non-api-key requests. This causes the middleware to continue executing the api_key query logic for all request types, not just api-key authenticated requests. Non-api-key requests will query the api_key table unnecessarily and potentially call next() twice.
packages/core/framework/src/http/middlewares/set-secret-api-key-context.ts
Outdated
Show resolved
Hide resolved
packages/core/framework/src/http/middlewares/set-secret-api-key-context.ts
Show resolved
Hide resolved
packages/core/framework/src/http/middlewares/set-secret-api-key-context.ts
Show resolved
Hide resolved
| filters: { id: userId }, | ||
| }) | ||
| userId = apiKey.created_by | ||
| } |
There was a problem hiding this comment.
Bug: Route handler duplicates API key query unnecessarily
The route handler queries the API key to resolve created_by even though the setSecretApiKeyContext middleware already sets this value in req.secret_key_context.created_by. This duplicates the database query and could cause inconsistencies since the route query lacks withDeleted: true while the middleware includes it. The route should use req.secret_key_context?.created_by instead of performing its own query.
packages/core/framework/src/http/middlewares/set-secret-api-key-context.ts
Show resolved
Hide resolved
| middlewares: [ | ||
| setSecretApiKeyContext, | ||
| ] | ||
| }, |
There was a problem hiding this comment.
Bug: Middleware breaks unauthenticated admin routes
The setSecretApiKeyContext middleware is registered for all /admin* routes but throws an error when req.auth_context is missing. Some admin routes have opted out of authentication via export const AUTHENTICATE = false (e.g., /admin/feature-flags). For these routes, the global auth middleware skips setting auth_context, causing setSecretApiKeyContext to throw "No auth_context found in request" and breaking unauthenticated admin endpoints.
Summary
What — What changes are introduced in this PR?
Resolve the
user_idfrom the user linked to secret api key when updating a draft order with api-key auth type and set ti tocreated_byfield of order change.Why — Why are these changes relevant or necessary?
Currently for this scenario, we incorrectly set the secret api key id as the
created_byfield of the correspondingorder_changewhich causes authentication issues when that same user tries to interact with the change.How — How have these changes been implemented?
Check the auth type in the draft order edit endpoint and if we identify it is
api-keythen instead of resolving theuser_idfromreq.auth_context.actor_idthat would point to the api key id, we search for the user that created the api key and set its id.Testing — How have these changes been tested, or how can the reviewer test the feature?
Added tests.
Examples
Provide examples or code snippets that demonstrate how this feature works, or how it can be used in practice.
This helps with documentation and ensures maintainers can quickly understand and verify the change.
// Example usageChecklist
Please ensure the following before requesting a review:
yarn changesetand follow the promptsAdditional Context
Add any additional context, related issues, or references that might help the reviewer understand this PR.
fixes #13859
closes SUP-2560
Note
Ensure draft order edits authenticated via API key use the key owner’s user_id for created_by, adding middleware to populate context and tests to verify.
POST /admin/draft-orders/:idto resolveuser_idfrom the secret API key’screated_bywhenactor_typeisapi-key.set-secret-api-key-contextand apply to"/admin*"to populatereq.secret_key_context; export inframework/src/http/middlewares/index.tsand extendframework/src/http/types.tswithSecretKeyContext.order_change.created_bymatches the user linked to the secret API key.@medusajs/medusa.Written by Cursor Bugbot for commit 6a6b0b8. This will update automatically on new commits. Configure here.