feat(http-utils): add composite primary-resource resolver hook to facsWrapper - #1883
Open
ravverma wants to merge 2 commits into
Open
feat(http-utils): add composite primary-resource resolver hook to facsWrapper#1883ravverma wants to merge 2 commits into
ravverma wants to merge 2 commits into
Conversation
…sWrapper Adds an opt-in PRODUCTS_FACS_COMPOSITE_RESOURCE config + compositeResolvers registry (mirroring secondaryResolvers). On the primary path, a composite-keyed product delegates the grant decision to its registered resolver, returning a tri-state: true=grant, 'defer'=set context.attributes.facs + defer to the controller (collection routes that ReBAC-filter), else deny; fail-closed on throw. Keeps the shared wrapper product/route-agnostic — all qualifier logic lives in the consuming service's resolver. Inert until a service registers one. Design: adobe/mysticat-architecture#249 (rebac-composite-resource-key.md D3/D4). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
This PR will trigger a minor release when merged. |
4 tasks
MysticatBot
approved these changes
Aug 24, 2026
There was a problem hiding this comment.
Hey @ravverma,
Verdict: Approve - well-structured opt-in hook with correct fail-closed semantics and thorough test coverage.
Complexity: MEDIUM - medium diff; ReBAC + auth signal.
Changes: Adds an opt-in composite primary-resource resolver hook to facsWrapper, enabling products to scope their primary resource by an extra qualifier with tri-state grant/defer/deny semantics (3 files).
Non-blocking (3): minor issues and suggestions
- nit: Missing
orgfield in the composite-resolver-error log entry - the grant and deny logs includeorg: normalizedOrgIdbut the error/fail-closed catch block omits it, reducing log correlation on resolver failures -packages/spacecat-shared-http-utils/src/auth/facs-wrapper.js:652 - suggestion: The "defer" test should assert the resolver args shape (specifically that
routeParamsis{ siteId: 'site-abc' }with noopportunityIdon the collection route) to match the thoroughness of the grant test -packages/spacecat-shared-http-utils/test/auth/facs-wrapper.test.js:1213 - nit: Code comments reference
platform/decisions/rebac-composite-resource-key.mdwhich resolves to companion PR adobe/mysticat-architecture#249 (not yet merged to main) - the path will be valid once that ADR lands, but is currently a dead link for readers checking the architecture repo -packages/spacecat-shared-http-utils/src/auth/facs-wrapper.js:256
Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 1m 40s | Cost: $6.83 | Commit: 29af8f2b747c728b10d3a5c4cf434ec87482e801
If this code review was useful, please react with 👍. Otherwise, react with 👎.
ravverma
added a commit
to adobe/spacecat-api-service
that referenced
this pull request
Aug 24, 2026
…ring Adds src/support/facs-composite-resolvers.js (asoOpportunityComposite), registered as compositeResolvers and wired into facsWrapper (src/index.js), plus PRODUCTS_FACS_COMPOSITE_RESOURCE.ASO in facs-capabilities.js. The resolver classifies ASO site routes: opportunity item -> 'all' OR opp.type (fail-closed on unresolved opp / site-mismatch); opportunity list -> defer (controller filters); every other ASO site route -> any active site binding with the capability. Inert until http-utils (adobe/spacecat-shared#1883) releases + is bumped. Design: adobe/mysticat-architecture#249 (D3/D4). Follow-ups: write path (D6), collection filter (D4), OpenAPI, IT tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What (PR 2 of 3)
Adds a composite primary-resource resolver hook to
facsWrapperso a product can scope its PRIMARY resource by an extra qualifier (ASO's (site × opportunity-type)) — while the shared wrapper stays product/route-agnostic.Design: ReBAC Composite Resource Key ADR — adobe/mysticat-architecture#249 (
rebac-composite-resource-key.md, D3/D4).Change
PRODUCTS_FACS_COMPOSITE_RESOURCE+ acompositeResolversregistry (construction validation mirrorssecondaryResolvers).true→ grant'defer'→ setcontext.attributes.facs+ defer to the controller (collection routes that ReBAC-filter, D4).d.tsupdated (FacsCompositeResolvertype + config + opts).All ASO specifics (
'all'short-circuit,Opportunity→type fetch, collection defer) live in the api-service resolver (PR 3) — this hook is inert until a service registers a resolver, so it's a no-op for existing consumers.Tests
Lint clean; 504 passing;
facs-wrapper.jsat 100% stmts/branch/funcs/lines (package 100%/99.18%, above the 97% branch gate). 7 new composite tests: creation guards + grant / defer / deny / throw (fail-closed) / JWT short-circuit / resource-type-mismatch fall-through.Sequence: (1) mysticat-data-service migration (#923) → (2) this → (3) spacecat-api-service write/collection/OpenAPI.
🤖 Generated with Claude Code