Repository navigation
feat: make withCredentials configurable on the connection builder - #278
Merged
Merged
Conversation
tnschneider
previously approved these changes
Sep 21, 2026
SignalR sends credentials by default, and a credentialed cross-origin request forbids a wildcard Access-Control-Allow-Origin or Access-Control-Allow-Headers. Consumers whose hub is authenticated with a bearer token rather than a cookie gain nothing from credentials mode but still pay for it: their negotiate preflight is rejected unless the server echoes the request origin and enumerates every request header. Add withCredentials(include) so those consumers can opt out. The default stays true, matching SignalR's own default, so existing behaviour is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Branch protection on main requires a pull request, so GITHUB_TOKEN could not push the "Release [skip-ci]" commit. Every Merge on Main run since June failed on that step (GH006) even though npm publish had already succeeded, leaving the pipeline red and the release tags unpushed. pub.sh now bumps from the registry, so the version in git is no longer the source of truth and the commit-back is dead weight. Push the tag created by npm version and drop the forced push to main. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mcarey1590
force-pushed
the
forks/mcarey/configurable-with-credentials
branch
from
September 21, 2026 19:25
9732919 to
795d775
Compare
mcarey1590
marked this pull request as ready for review
September 21, 2026 19:26
JacobAtchley
approved these changes
Sep 21, 2026
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
3 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 changed
Adds
withCredentials(include: boolean)toRealTimeEventsConnectionBuilder, wired into theIHttpConnectionOptionspassed towithUrl.Default stays
true, matching SignalR's own default, so existing consumers see no behaviour change.Why
SignalR sends credentials by default. A credentialed cross-origin request forbids a wildcard
Access-Control-Allow-OriginorAccess-Control-Allow-Headers— the browser rejects the negotiate preflight, and the failure usually surfaces as an opaque network error rather than a readable CORS message.Consumers whose hub is authenticated with a bearer token rather than a cookie gain nothing from credentials mode, but still pay for it: their server has to echo the request origin and enumerate every request header the client sends. Opting out lets a permissive CORS configuration serve the preflight.
This came out of a live incident. A HAProxy ingress upgrade began emitting a literal
*where the previous version echoed request values, which broke every negotiate call in two environments. The infrastructure-side fix works but requires a hand-maintained header allowlist, and that list has already caused one outage of its own when it was incomplete. Being able to drop credentials mode removes the need for that list entirely.How to test
Two tests added, both watched failing before the implementation existed:
should send credentials by default— assertswithUrlreceiveswithCredentials: truewith no configuration.should omit credentials when the consumer opts out— assertswithCredentials(false)propagates.31 tests pass, lint clean, build clean.
Notes for reviewers
platform-uipins@firebend/control-tower-web-socket-clientand should move only after this publishes.Merge after #279
An earlier version of this description said the
feat:prefix would cut a minor via versionize. Both halves of that were wrong. This repo does not use versionize — it uses.github/scripts/pub.sh— and that script mappedfeatto major, withminorunreachable from any conventional prefix.#279 corrects the mapping. Merged before it, this PR publishes 2.0.0 and advertises a breaking change that does not exist. Merged after it, it publishes 1.8.0, which is what an additive, backward-compatible option should be.
Note also that the repo's checked-in version (1.7.3) trails the registry (1.7.4), which #279 also addresses.
🤖 Generated with Claude Code