feat(preflight): mint IMS token via dedicated client (SITES-46699)#2641
Merged
GeezerPelletier merged 4 commits intoJun 18, 2026
Merged
Conversation
…GHT_IMS_* client (SITES-46699) Supersedes PR #2627's scope-only hardcode. Replaces the custom-env ImsClient that only overrode IMS_SCOPE='system' on the default `aem-project-collab-service` client with a full custom-env that swaps all three credentials (CLIENT_ID, CLIENT_SECRET, SCOPE) to a dedicated PREFLIGHT_IMS_* client. Why: Ravi confirmed on Slack (2026-06-17) that `aem-project-collab-service` is NOT registered for the client_credentials grant type at all, so v3 service- token minting cannot succeed against it regardless of scope. The previous hardcode was building on a wrong foundation. SITES-46699 covers the dedicated- client registration + Vault provisioning; this PR is the spacecat-api-service- side companion (staged in advance; held until Vault credentials land). Source changes (src/controllers/preflight.js): - Replace IMS_SCOPE-only override with full credential swap matching the email-service.js / trial-users.js / cloud-manager-client / brand-client precedent: { IMS_CLIENT_ID, IMS_CLIENT_SECRET, IMS_SCOPE } all sourced from env.PREFLIGHT_IMS_*. - Build preflightImsEnv from the factory-captured env (closure), consistent with env.AWS_ENV usage elsewhere in the controller. - Drop the "transitional bypass" / "reversibility note" framing from the comment block — this IS the permanent shape now, mirroring established spacecat one-IMS-client-per-purpose convention. - Keep all defensive guards from #2611/#2627: post-condition hasText(access_token), structured error logging, fail-closed posture, mint-before-DB-writes ordering. Test changes (test/controllers/preflight.test.js): - Factory env now provides test PREFLIGHT_IMS_CLIENT_ID/SECRET/SCOPE values. - Load-bearing contract assertion updated to verify all three credentials reach ImsClient.createFrom (previously only checked IMS_SCOPE='system'; now checks all three to pin the dedicated-client wiring). Staged ahead of Vault provisioning (SITES-46699 scope item 2). Branch parked until credentials land; PR opens then. 90/90 tests pass locally. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… env ImsClient.createFrom's constructor validates IMS_CLIENT_CODE as required (hasText check) even though v3 client_credentials doesn't send it to IMS. Without an explicit override, our dedicated PREFLIGHT_IMS_* setup would inherit CLIENT_CODE from the default env's IMS_CLIENT_CODE — works (content-ai does this) but leaves a hidden dependency on the default client's value. Binding to our own PREFLIGHT_IMS_CLIENT_CODE makes the dedicated client fully self-contained: no inherited values, all four credentials sourced from the PREFLIGHT_IMS_* prefix. Source: add IMS_CLIENT_CODE override in callMysticatAnalyze. Tests: provide PREFLIGHT_IMS_CLIENT_CODE in factory env; assertion now pins all four credentials reach ImsClient.createFrom. 90/90 tests pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…S_ORG_ID (SITES-46699) Adapts to ownerless IMS client registration. IMSS doesn't expose owner-org edit-after-create for the realm we registered in, so the dedicated aem_site_optimizer_preflight client is ownerless — every v3 mint must explicitly carry org_id per the IMS Client Credentials Token spec (https://wiki.corp.adobe.com/x/Olwxn). Source: - Switch from getServiceAccessTokenV3() (no org) to getServicePrincipalAccessToken(env.PREFLIGHT_IMS_ORG_ID). - Add comment explaining the ownerless-client rationale + Service Principal Binding (provisioned out-of-band per the SITES-46699 acceptance criterion). Tests: - mockImsClient.getServicePrincipalAccessToken instead of getServiceAccessTokenV3. - PREFLIGHT_IMS_ORG_ID added to factory env (test value: 'test-preflight-org@AdobeOrg' — shape matches the @AdobeOrg IMS org_id convention). - Load-bearing assertion now verifies BOTH the credential swap AND the org_id threading — if a future refactor swaps back to v3-without-org or drops the org_id, one of the assertions fails. 90/90 tests pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…n (SITES-46699) The dedicated `aem_site_optimizer_preflight` IMS client is provisioned in IMSS with a v2 Service Token (permanent authorization code) bound to a synthetic service account. v2 `authorization_code` mint via that code needs no org_id and no Service Principal Binding — simpler than the v3 client_credentials + SP-binding path we'd pivoted to. The CGW-Flex edge gate validates the `client_id` claim, which is identical across v2 and v3 tokens for this client, so v2 satisfies the gate. The custom-env ImsClient wrapper stays in place; only the mint method changes. Drops PREFLIGHT_IMS_ORG_ID (never populated). IMS_SCOPE mapping is retained — unused by v2, kept for forward-compat if/when this client moves to v3. Tests updated to stub `getServiceAccessToken` (no args) instead of `getServicePrincipalAccessToken(orgId)`. 90/90 controller tests pass.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
MysticatBot
approved these changes
Jun 18, 2026
There was a problem hiding this comment.
Hey @GeezerPelletier,
Verdict: Approve
Changes: Swaps the preflight IMS token mint from a borrowed client with hardcoded scope (v3 client_credentials) to a dedicated aem_site_optimizer_preflight client using v2 authorization_code (2 files).
Non-blocking (2): minor issues and suggestions
- suggestion: Early validation of
PREFLIGHT_IMS_*env vars before constructing the IMS client would surface misconfiguration faster during initial Vault rollout (today the try/catch returns a generic 500 and the IMS SDK error body reveals the root cause, but a pre-flight assertion like['PREFLIGHT_IMS_CLIENT_ID', ...].filter(k => !env[k])would fail immediately with an actionable message) -src/controllers/preflight.js:519 - nit:
IMS_SCOPE: env.PREFLIGHT_IMS_SCOPEoverwrites the base env'sIMS_SCOPEwithundefinedwhen the Vault key is absent (the spread of...envruns first, then the override clobbers it); since the comment anticipates a future v3 migration that would read this field, consider a conditional spread...(env.PREFLIGHT_IMS_SCOPE && { IMS_SCOPE: env.PREFLIGHT_IMS_SCOPE })to preserve the base value as a safety net -src/controllers/preflight.js:527
Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 8m 54s | Cost: $4.13 | Commit: 2a64c8447476597c49229c23a2b7b0b04c99b851
If this code review was useful, please react with 👍. Otherwise, react with 👎.
GeezerPelletier
deleted the
feat/sites-46699-preflight-ims-client-refactor
branch
solaris007
pushed a commit
that referenced
this pull request
Jun 18, 2026
Member
|
🎉 This PR is included in version 1.583.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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
4 participants
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 & why
SITES-46699 -
callMysticatAnalyzenow mints its IMS service token via a dedicatedaem_sites_optimizer_preflight_serviceIMS client (ASO-team-managed), following the established spacecat one-IMS-client-per-purpose convention (email-service, trial-users, cloud-manager-client, brand-client all swap IMS env at construction for their own identity).The previous attempt (#2611 + scope-hardcode #2627) tried to reuse the borrowed
aem-project-collab-serviceIMS client viagetServiceAccessTokenV3()(v3 client_credentials). Ravi confirmed via Slack that client isn't registered forclient_credentialsgrant at all - the IMS 400 we were chasing wasunauthorized_client, which the new IMS-body-in-error-message diagnostic (SITES-46698, adobe/spacecat-shared#1689, published as@adobe/spacecat-shared-ims-client@1.14.0) now surfaces directly.Hindsight
In retrospect,
aem-project-collab-servicedoes have anauthorization_codegrant + IMSS-provisioned Service Token - so a v2getServiceAccessToken()against it would have worked from day one. We never tried it because PR #2611 set v3 as the path and we accepted that constraint, then chased scope/grant issues inside the v3 box without seeing IMS'sunauthorized_clientresponse. SITES-46698 closes that diagnostic loop.A dedicated client is still the right architecture, not a consolation prize: independent rotation + scope + CGW allowlisting, cleaner Splunk/IMS audit-trail attribution, and
allowedClientIDsis more meaningful per-consumer than one bucket of shared traffic.v2, not v3
Mint uses
getServiceAccessToken()(v2authorization_code) against the IMSS-provisioned permanent authorization code on the client's Service Token. v2 needs no org_id and no Service Principal Binding - the synthetic service-account identity (aem_sites_optimizer_preflight_service@AdobeService) is encoded in the permanent code at IMSS registration time.We initially refactored to v3
getServicePrincipalAccessToken(orgId)(the path for ownerless clients), then pivoted to v2 when the IMSS console surfaced an auto-provisioned Service Token on the client. The CGW-Flex edge gate keys on theclient_idclaim, which is identical across v2 and v3 tokens for this client, so v2 satisfies the gate.IMS_SCOPEmapping is retained in the env spread (unused by v2, but forward-compat if/when this client moves to v3).Files
src/controllers/preflight.js- custom-envImsClientwrapper incallMysticatAnalyze; swap to v2 minttest/controllers/preflight.test.js- factory env carries 4PREFLIGHT_IMS_*test values; load-bearing assertions pin both the credential threading throughImsClient.createFromand the v2 method on the stubVault state (dev only)
PREFLIGHT_IMS_CLIENT_ID/PREFLIGHT_IMS_CLIENT_SECRET/PREFLIGHT_IMS_CLIENT_CODE/PREFLIGHT_IMS_SCOPEprovisioned indx_mysticat/dev/api-service. Production rollout is tracked separately under SITES-46770 - explicitly blocked on this ticket landing + dev empirical verification so we don't burn the prod IMS-client + Vault provisioning effort until the v2 flow is proven on dev.Sequencing
This PR doesn't unblock end-to-end on its own. The companion change on the deploy side is mystique-deploy #470, which adds
aem_sites_optimizer_preflight_servicetom-dev.adobe.io'sallowedClientIDsfor/v1/preflight/analyze. (Supersedes closed-unmerged mystique-deploy #466 which targeted the old borrowed client.) Edge gate remainsoptional: truein Phase 1 - once both PRs land and dev redeploys, empirical verification can flip Phase 2'soptional: false.Test plan
npx mocha test/controllers/preflight.test.js- 90/90 passx-ims-authorizationbearing a validaem_sites_optimizer_preflight_service-issued v2 token)optional: false+tokenType: service, this becomes load-bearingRelated