Skip to content

fix(webex-core): re-evaluate credentials on app level redirects - #5150

Draft
chrisadubois wants to merge 1 commit into
nextfrom
fix/redirect-interceptor-credentials
Draft

fix(webex-core): re-evaluate credentials on app level redirects#5150
chrisadubois wants to merge 1 commit into
nextfrom
fix/redirect-interceptor-credentials

Conversation

@chrisadubois

Copy link
Copy Markdown
Collaborator

COMPLETES #https://jira-eng-gpk2.cisco.com/jira/browse/FPV-747

This pull request addresses

RedirectInterceptor.onResponse() re-issues a request against a uri taken from the previous response. It copied the request options with lodash clone(), which is shallow, so the re-issued request kept the same headers object as the request it replaced.

AuthInterceptor.onRequest() returns early when a request already carries an authorization header. A carried-over header therefore meant requiresCredentials() never ran on the re-issued request, so neither the service catalog nor the allowed-domain list was evaluated against the uri that was about to be requested. All three redirect branches were affected: the cisco-location response header, the Locus errorCode body, and the AppAPI code / siteFullUrl body.

Separately, the two body-based branches keyed only off the response body and not off its status code, so they could act on a response that is not a redirect.

This is a different mechanism from #5144. That change made the allowed-domain comparison exact; this one is about that check not being reached. Neither fixes the other.

by making the following changes

All three branches now copy headers as well, and drop the authorization inherited from the previous request, through a single shared helper:

options = cloneOptionsForRedirect(options);

AuthInterceptor then makes its normal decision against the uri that is actually about to be requested — the catalog lookup and the allowed-domain check both run — instead of returning early. Switching clone() to cloneDeep() would not have been sufficient: it copies the header through unchanged.

The two body-based branches additionally require the 404 they are documented to arrive with. This is the same status HttpStatusInterceptor already pairs with each of the two error codes.

The existing preJoin / webex-appapi-service case still sets authorization: false explicitly, so that request stays unauthenticated — AuthInterceptor treats a falsy value as "delete it and do not add one".

Compatibility

Legitimate redirects continue to be authorized. Their targets are Locus hosts under wbx2.com and Webex sites under webex.com, both of which are in COMMERCIAL_ALLOWED_DOMAINS. Foreign-cluster Locus hosts are additionally present in the service catalog, because _formatReceivedHostmap() collects every host that shares a service id, including those with homeCluster: false, and findServiceUrlFromUrl() matches against all of them. The same holds for FedRAMP, the narrowest allowlist, whose catalog and allowed domains are both gov hosts. On a legitimate redirect the header is re-attached by AuthInterceptor rather than carried over.

A caller that sets headers.authorization by hand, rather than letting AuthInterceptor attach it, no longer has that exact value forwarded across a redirect; the interceptor re-derives the header for the new uri. No in-tree caller that sets the header by hand is on a redirecting path, and those that do set it (webinar, user, support) set the user token, which is what the interceptor attaches. Callers that use options.auth are unaffected, because AuthInterceptor still returns early for those.

For the status code: HttpStatusInterceptor runs before RedirectInterceptor on responses, and already rejects a response of 400 or above carrying either body unless it is a 404. The new condition therefore only changes what happens to responses below 400, which are not redirects.

Change Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Tooling change
  • Internal code refactor

The following scenarios were tested

  • Automated: yarn workspace @webex/webex-core test:unit — 33 suites, 758 tests passing.
  • Automated: each of the three branches is now exercised end to end against a real Services and a real ServicesV2 catalog, following the pattern added in fix(webex-core): match allowed domains on DNS label boundaries #5144. The real hasAllowedDomains() / isAllowedDomainUrl() are kept rather than stubbed, and waitForService is made to reject so that an allowed domain is the only thing that can authorize the request. Each branch asserts that the header is not forwarded, that a redirect to a uri under an allowed domain is still authorized with the user token, and that a redirect to a uri that is under neither the catalog nor an allowed domain is issued with no authorization header and does not request a token. The options of the request being redirected are asserted to be left untouched.
  • Automated: both body-based branches assert that a redirect-shaped body on a non-404 response is returned rather than followed.
  • Verified that all 22 new and changed assertions fail when the change is reverted.
  • Automated: yarn workspace @webex/plugin-meetings test:unit — 3698 passing. No plugin-meetings changes; run because it is the largest consumer of the redirecting services.
  • yarn workspace @webex/webex-core test:style — no new findings.

The GAI Coding Policy And Copyright Annotation Best Practices

  • GAI was not used (or, no additional notation is required)
  • Code was generated entirely by GAI
  • GAI was used to create a draft that was subsequently customized or modified
  • Coder created a draft manually that was non-substantively modified by GAI (e.g., refactoring was performed by GAI on manually written code)
  • Tool used for AI assistance (GitHub Copilot / Other - specify)
    • Github Copilot
    • Other - Claude Code
  • This PR is related to
    • Feature
    • Defect fix
    • Tech Debt
    • Automation

I certified that

  • I have read and followed contributing guidelines
  • I discussed changes with code owners prior to submitting this pull request
  • I have not skipped any automated checks
  • All existing and new tests passed

Copy the headers when re-issuing a request against a uri taken from the
previous response, and drop the authorization header inherited from the
previous request, so AuthInterceptor makes its normal decision against the
new uri instead of returning early. Applies to all three redirect branches.

Also require the documented 404 status code on the two body based branches,
matching the status HttpStatusInterceptor already pairs with each of the two
error codes.

Ref: FPV-747

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@chrisadubois
chrisadubois requested review from a team as code owners August 5, 2026 22:46
@chrisadubois
chrisadubois marked this pull request as draft August 5, 2026 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant