Skip to content

[wrangler] Fix Access Service Token authentication for service-auth-only apps - #14008

Merged
petebacondarwin merged 1 commit into
mainfrom
fix/access-service-token-403
May 22, 2026
Merged

[wrangler] Fix Access Service Token authentication for service-auth-only apps#14008
petebacondarwin merged 1 commit into
mainfrom
fix/access-service-token-403

Conversation

@petebacondarwin

@petebacondarwin petebacondarwin commented May 22, 2026

Copy link
Copy Markdown
Contributor

When using remote bindings against a Worker behind a Cloudflare Access application configured to only allow Service Auth tokens (no interactive user authentication), Wrangler previously ignored the CLOUDFLARE_ACCESS_CLIENT_ID and CLOUDFLARE_ACCESS_CLIENT_SECRET env vars and the request would fail with a 403. This came up via reports from users using Wrangler for OpenNext pre-render uploads against a Worker behind Access, where the documented workaround of setting those env vars wasn't taking effect.

The detection method in domainUsesAccess() looks for a 302 redirect to cloudflareaccess.com. A service-auth-only Access app has no interactive login path, so it responds with a hard 403 instead of redirecting. Wrangler concluded the domain was not behind Access and skipped attaching the service token headers entirely.

The env-var check now runs before the Access detection step, so the configured service token credentials are always used when present. A code comment has been added explaining why the order matters to prevent regression.


  • Tests
    • Tests included/updated
    • Automated tests not possible - manual testing has been completed as follows:
    • Additional testing not necessary because:
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: this is a bug fix that restores documented behaviour of CLOUDFLARE_ACCESS_CLIENT_ID/CLOUDFLARE_ACCESS_CLIENT_SECRET env vars for an additional Access configuration.

A picture of a cute animal (not mandatory, but encouraged)


Open in Devin Review

…nly apps

When using remote bindings against a Worker behind a Cloudflare Access
application configured to only allow Service Auth tokens (no interactive
user authentication), Wrangler previously ignored the
CLOUDFLARE_ACCESS_CLIENT_ID and CLOUDFLARE_ACCESS_CLIENT_SECRET env vars
and the request would fail with a 403.

The detection method in domainUsesAccess() looks for a 302 redirect to
cloudflareaccess.com. A service-auth-only Access app has no interactive
login path, so it responds with a hard 403 instead of redirecting.
Wrangler concluded the domain was not behind Access and skipped attaching
the service token headers entirely.

The env-var check now runs before the Access detection step, so the
configured service token credentials are always used when present.
@changeset-bot

changeset-bot Bot commented May 22, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 00a4325

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
wrangler Patch
@cloudflare/vite-plugin Patch
@cloudflare/vitest-pool-workers Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-project-automation github-project-automation Bot moved this to Untriaged in workers-sdk May 22, 2026
@workers-devprod
workers-devprod requested review from a team and dario-piotrowicz and removed request for a team May 22, 2026 11:22
@workers-devprod

workers-devprod commented May 22, 2026

Copy link
Copy Markdown
Contributor

Codeowners approval required for this PR:

  • ✅ @cloudflare/wrangler
Show detailed file reviewers

@ask-bonk

ask-bonk Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

I've now thoroughly reviewed the changes. Let me analyze:

  1. The fix: Moves the env-var check for CLOUDFLARE_ACCESS_CLIENT_ID / CLOUDFLARE_ACCESS_CLIENT_SECRET before the domainUsesAccess() call, so service token credentials are always attached when present — even when the domain returns a 403 instead of a 302 redirect.

  2. Correctness: The reordering is sound. When both env vars are set, headers are returned immediately regardless of domainUsesAccess() result. When neither or only one is set, the code falls through to domainUsesAccess() as before. The "warn if only one is set" path still works correctly because it falls through to domainUsesAccess() which will still detect the 302-redirect case.

  3. Tests: Well-structured regression tests covering the new scenario, proper MSW handler for 403, comments explaining the rationale.

  4. Changeset: Correct format, patch level, good description.

  5. One behavioral consideration: When both env vars are set and the domain is not behind Access at all, the old code would return {} (because domainUsesAccess returned false before checking env vars). The new code will return the service token headers. This seems intentional and correct — if a user explicitly sets the env vars, they want them used. The headers will just be ignored by non-Access endpoints.

Everything looks correct. The fix is minimal, well-tested, and well-documented.

LGTM

github run

@github-actions

Copy link
Copy Markdown
Contributor

✅ All changesets look good

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

Open in Devin Review

@pkg-pr-new

pkg-pr-new Bot commented May 22, 2026

Copy link
Copy Markdown
create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@14008

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@14008

miniflare

npm i https://pkg.pr.new/miniflare@14008

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@14008

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@14008

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@14008

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@14008

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@14008

@cloudflare/workers-utils

npm i https://pkg.pr.new/@cloudflare/workers-utils@14008

wrangler

npm i https://pkg.pr.new/wrangler@14008

commit: 00a4325

@emily-shen emily-shen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i wonder if the probe can check for 403s as well and suggest setting the access env vars? (as a followup)

@petebacondarwin
petebacondarwin enabled auto-merge (squash) May 22, 2026 11:49

@workers-devprod workers-devprod left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codeowners reviews satisfied

@petebacondarwin
petebacondarwin merged commit fc1f7b9 into main May 22, 2026
58 of 59 checks passed
@github-project-automation github-project-automation Bot moved this from Untriaged to Approved in workers-sdk May 22, 2026
@petebacondarwin
petebacondarwin deleted the fix/access-service-token-403 branch May 22, 2026 11:49
@github-project-automation github-project-automation Bot moved this from Approved to Done in workers-sdk May 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants