feat(middleware): forward product on user-flow authorization#114
Conversation
WalkthroughThe ChangesProduct parameter in authorization middleware
Comment |
f25394e to
a313044
Compare
a313044 to
b164be9
Compare
checkAuthorization now takes the product owning the route and derives the subject internally: M2M tokens map to the product's editor role, while normal users are identified by their JWT (owner/userId). The product is forwarded as "product" for normal-user tokens only, so the auth service can isolate permissions by product. Empty product preserves the previous behavior, enabling incremental adoption. X-Lerian-Ref: 0x1
b164be9 to
e597ae2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
auth/middleware/middleware_test.go (1)
118-166: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd a regression test for empty-product non-normal-user behavior.
Current tests only exercise non-empty product for M2M. Please add a case with
product == ""for non-normal-user tokens to lock the intended compatibility behavior and prevent regressions like malformedsubvalues.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@auth/middleware/middleware_test.go` around lines 118 - 166, The current test TestCheckAuthorization_ApplicationUser_SubjectConstruction only covers non-normal-user tokens with a product value, leaving a gap in test coverage. Add a new regression test function that exercises the same authorization flow and subject construction logic but with an empty product value (product == "") in the JWT claims. This test should verify that the subject is constructed correctly and that the product field is not forwarded even when empty, ensuring the intended behavior is preserved and preventing future regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@auth/middleware/middleware.go`:
- Around line 301-307: The M2M subject derivation logic in the userType check is
not compatibility-safe. When product is empty, the sub assignment creates
malformed values like admin/-editor-role, and it doesn't preserve legacy
sub-style values that current callers may still be passing. Additionally, this
reconstruction can leak unintended product values in normal-user authorization
flows at lines 329-332. Validate that product is not empty before constructing
the M2M subject with the admin/<product>-editor-role format, and ensure that
when product is empty or when dealing with normal users, the original or
properly preserved sub value is used instead of overwriting it with a
potentially invalid constructed value.
---
Outside diff comments:
In `@auth/middleware/middleware_test.go`:
- Around line 118-166: The current test
TestCheckAuthorization_ApplicationUser_SubjectConstruction only covers
non-normal-user tokens with a product value, leaving a gap in test coverage. Add
a new regression test function that exercises the same authorization flow and
subject construction logic but with an empty product value (product == "") in
the JWT claims. This test should verify that the subject is constructed
correctly and that the product field is not forwarded even when empty, ensuring
the intended behavior is preserved and preventing future regressions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 80c9ff3e-bad0-489e-abd1-7ff8ab21bf9c
📒 Files selected for processing (2)
auth/middleware/middleware.goauth/middleware/middleware_test.go
|
🎉 This PR is included in version 2.9.0-beta.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 2.9.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Contexto
Lado produtor do contrato de isolamento de permissão por produto no fluxo de usuário. O lado receptor está em
plugin-access-managerPR #134 (consome o campoproductquando presente; sem ele, comportamento legado).Problema
O 1º argumento do
Authorize(sub, resource, action)identifica o produto dono da rota (no M2M viraadmin/<sub>-editor-role). Mas no token de usuário normal ele era descartado: osubé sobrescrito porowner/userIddo JWT, e ocheckAuthorizationmandava pro serviço de auth só{sub, resource, action}— sem discriminador de produto. Logo, o serviço não tinha como isolar permissões por produto no fluxo de usuário.Mudança
checkAuthorizationrecebe umproducte o encaminha como"product"no body apenas para tokens de usuário normal.Authorize: passa o 1º argumento (o produto).""(aPolicyainda não carrega produto) → comportamento inalterado lá; isolação por produto no gRPC fica pra quando aPolicyganhar esse campo.productvazio preserva o comportamento anterior → o serviço de auth adota a isolação incrementalmente, serviço a serviço, conforme cada um sobe esta versão. Sem flag, sem cutover atômico.Testes
producté encaminhado no body.productnão é encaminhado.checkAuthorizationatualizados pro novo parâmetro. 72 testes do pacote passam,go vetlimpo.Rollout
Após release,
plugin-access-manager(e demais consumidores) bumpam olib-authe passam a enviar o produto. Antes disso, tudo segue no caminho legado.🤖 Generated with Claude Code