feat: forward end-user token and expose bucket-policy GraphQL proxy - #34
feat: forward end-user token and expose bucket-policy GraphQL proxy#34iago1501 wants to merge 5 commits into
Conversation
|
Hi! I'm VTEX IO CI/CD Bot and I'll be helping you to publish your app! 🤖 Please select which version do you want to release:
And then you just need to merge your PR when you are ready! There is no need to create a release commit/tag.
|
|
Beep boop 🤖 I noticed you didn't make any changes at the
In order to keep track, I'll create an issue if you decide now is not a good time
|
US-1: FileManager's constructor stopped reading context.authToken and now
receives an explicit userToken argument, forwarded as VtexIdclientAutCookie
only when present (never sent empty). The three-way token resolution
already used by the @requiresAuth directive (adminUserAuthToken cookie ->
raw vtexidclientautcookie header -> storeUserAuthToken cookie) is extracted
into a shared, exported resolveUserToken() helper in node/directives/auth.ts
so both call sites can never diverge. All four file resolvers
(getFile/getFileUrl/uploadFile/deleteFile) now pass resolveUserToken(ctx)
to FileManager.
US-2/US-3: adds listPolicies/getPolicy/setAdminPolicy/deleteAdminPolicy to
FileManager (no 404-remapping, errors incl. 403 rethrown as-is) plus
toWireAccessLevel/fromWireAccessLevel/mapPolicyViewFromWire helpers to
convert between this app's upper-snake AccessLevel enum and file-manager's
lowercase/kebab wire format. Adds the matching listBucketPolicies (with
transparent nextMarker pagination)/getBucketPolicy/setBucketPolicy/
deleteBucketPolicy resolvers, reusing resolveUserToken(ctx). No
POST /policies/{bucket}/manifest exposure, no independent permission logic
(LicenseManager on file-manager's side remains the only authorization
source).
node/FileManager.test.ts covers header inclusion/omission, that
context.authToken is never read, access-level round-tripping, wire-format
request bodies, response mapping with null-policy preservation, and
unchanged error propagation for all four new methods.
Co-authored-by: Cursor <cursoragent@cursor.com>
…source policy Adds AccessLevel enum, BucketPolicy/BucketPolicyView types, and the four new @requiresAuth operations (listBucketPolicies, getBucketPolicy queries; setBucketPolicy, deleteBucketPolicy mutations) to the existing single Query/Mutation blocks in graphql/schema.graphql. Declares the vtex.file-manager:file-manager-bucket-config-rw resource policy in manifest.json -- without it, kube-router has no grant to let this app's /policies/* calls reach file-manager's controller at all, regardless of the caller's own LicenseManager permissions (Decision 6). Co-authored-by: Cursor <cursoragent@cursor.com>
2a966e4 to
a716ddc
Compare
…r route vtex.file-manager renamed its private policy routes' literal path segment from "policies" to "bucket-access-policies" to fix a routing collision (the platform's injected router was matching by trailing path segments, so a public GET to a bucket literally named "policies" got hijacked to the private policy-read action). Update listPolicies, getPolicy, setAdminPolicy and deleteAdminPolicy to call the new path, and update the matching test assertions. Co-authored-by: Cursor <cursoragent@cursor.com>
Mirrors vtex.file-manager's revert of the policy routes from /bucket-access-policies back to /policies -- keeps this client in sync with the upstream route names and the existing policies.json VRN grant. Co-authored-by: Cursor <cursoragent@cursor.com>
setBucketPolicy, deleteBucketPolicy, listBucketPolicies and getBucketPolicy were only gated on "user is logged in" at the GraphQL layer, same as any other authenticated query. file-manager still enforces the real file-manager-bucket-config License Manager resource downstream, so this was never an actual bypass, but a non-admin store-token request would reach file-manager just to be rejected there. Extend the existing deleteFile admin gate to these bucket-policy operations so non-admin requests are rejected in GraphQL, before ever calling file-manager. Co-authored-by: Cursor <cursoragent@cursor.com>
monteirogc
left a comment
monteirogc
left a comment
There was a problem hiding this comment.
US-1 e o proxy de /policies/* estão no rumo certo (token do end-user, header omitido no anônimo, sem /manifest, resource name bate com file-manager#94).
Dois pontos pra fechar antes do approve. CI (vtexio/build) também vai continuar vermelho até o file-manager publicar file-manager-bucket-config-rw.
| 'setBucketPolicy', | ||
| 'deleteBucketPolicy', | ||
| 'listBucketPolicies', | ||
| 'getBucketPolicy', |
There was a problem hiding this comment.
Isso contradiz a Decision 5 da spec (Approved): /policies/* só leva @requiresAuth + License Manager no file-manager — sem Sphinx.
sphinx.isAdmin ≠ resource file-manager-bucket-config. Role com o resource e sem ser admin Sphinx leva false deny, e o caller deixa de ver o 403 original.
Reverter, ou emendar a Decision 5.
There was a problem hiding this comment.
@requiresAuth só exige login — token de store passa. Sem um corte na borda, o customer chega no /policies/* só para o file-manager recusar no License Manager.
O isAdmin aqui é o mesmo filtro Admin vs store que deleteFile já usa neste app. Role no LM só se atribui a usuário Admin; store não tem o resource. Não é um oracle paralelo ao file-manager-bucket-config: Admin sem o resource ainda chega no file-manager e vê o 403 original. O Sphinx só corta a população (store) antes do round-trip.
O persona "tem o resource e não é admin Sphinx" não é um caso que a gente suporte — quem recebe a role já é Admin. Por isso não estamos emendando a Decision 5: o LM continua sendo quem autoriza qual admin gerencia policy.
| case 'account-administrator': | ||
| return 'ACCOUNT_ADMINISTRATOR' | ||
| default: | ||
| return 'PUBLIC' |
There was a problem hiding this comment.
Fail-open pra PUBLIC. O converter do file-manager throw em valor inválido; aqui um wire missing/futuro aparece como público no Admin.
Throw no default.
2 participants
Summary
Implements
specs/bucket-access-control-integration.md(US-1, US-2, US-3 — Approved):FileManager's outboundVtexIdclientAutCookieheader now carries the resolved end-user token (adminUserAuthTokencookie → rawvtexidclientautcookieheader →storeUserAuthTokencookie — same precedence as the existing@requiresAuthdirective) instead ofcontext.authToken(this app's own token). The header is omitted entirely for anonymous callers, never sent empty. This unblocksvtex.file-manager's upcoming per-bucket access-control enforcement (US-4) from correctly classifying callers instead of treating all proxied traffic as anonymous.vtex.file-manager's private/policies/*REST API —listBucketPolicies(transparently paginated),getBucketPolicy,setBucketPolicy,deleteBucketPolicy— with no independent permission logic in this app; every error (including403) from file-manager is surfaced to the caller unchanged.POST /policies/{bucket}/manifestis not exposed (exclusive to builder-hub's service-token flow).vtex.file-manager:file-manager-bucket-config-rwresource policy inmanifest.json, required for/policies/*calls to reach file-manager's controller at all (Decision 6).Depends on #33 (spec approval, already merged into this branch's history) and, for
/policies/*to actually authorize a caller in production, onvtex.file-managerpublishing that new resource policy grant (tracked in that repo's ownbucket-access-controlspec, US-2/US-4; routing itself is already fixed there, see vtex/file-manager#94).How to test
Run
vtex linkon this branch against a dev workspace wherevtex.file-manager'sbucket-access-controlbranch is also linked, then:(replace
{account}/{workspace}with the ones you linked to, e.g.storecomponents/cookiehop825;USER_TOKEN= your ownvtex loginsession token from~/.vtex/session/tokens.json)POST {GQL_URL}{ settings { maxFileSizeMB } }, no token401fromVtex.Kube.Router(Credential required...)@requiresAuthfieldsVtexIdclientAutCookie: $USER_TOKEN200,{"data":{"settings":{"maxFileSizeMB":4}}}{ getFile(path: "doesnotexist.json") }(public route), with$USER_TOKEN200GraphQL body with aFile Not Founderror,status: 404vtex.file-manager, which executed real business logic (not a gateway rejection) and returned a genuine 404. This route ispublic: trueon file-manager's side{ getFileUrl(path: "foo.json", bucket: "images") }(private route,public: false, already existed before this PR), with$USER_TOKENfile-manager-read-writepolicy{ listBucketPolicies { bucket effectivePolicy { readAccess writeAccess } } }(private, new route), with$USER_TOKEN, account not grantedfile-manager-bucket-config-rw200GraphQL envelope with a GraphQL-level error:"Request failed with status code 403"(occasionally surfaces as500— see note)LicenseManagerServicecheck and gets a genuine "not granted"403back through this proxy unchanged — not a gateway-level 404 anymore. Confirmed the exact same request against file-manager directly (bypassing this proxy) returns4033/3 times. The occasional500instead of403is this sandbox's intermittent DNS resolution failure reachinglicense-manager.vtex.comfrom file-manager's side (Name or service not known) — an environment/network characteristic of the dev sandbox, not a bug in either app's codemutation { setBucketPolicy(bucket: "images", readAccess: PUBLIC, writeAccess: AUTHENTICATED) { readAccess writeAccess } }, with$USER_TOKEN, same as #5"public"/"authenticated") before being sent200end-to-end through this proxy:listBucketPolicies/getBucketPolicyreturn the real vbase-backed policy,setBucketPolicy/deleteBucketPolicymutations succeed and are reflected on the next read{ __type(name: "AccessLevel") { enumValues { name } } }PUBLIC,AUTHENTICATED,ACCOUNT_ADMINISTRATORItem 5/6's
403(not404) is the concrete proof that the only remaining gap is the resource-policy grant itself (file-manager's LicenseManager check correctly says "not granted" for this account) -- not a routing/wiring problem on either side, which is what the previous test round (before file-manager#94's routing fix) couldn't distinguish. Item 7 removes that last variable by simulating the grant and shows the full proxy — schema, client, resolvers — working end-to-end with real data. Items 3/4 remain the strongest "no regression" signal: the exact sameFileManagerclient code, on a pre-existing private route, already worked before this PR and still does.