Repository navigation
pubsub: breaking changes to auth and tokens - #738
Open
owenpearson wants to merge 10 commits into
Open
owenpearson wants to merge 10 commits into
owenpearson wants to merge 10 commits into
Conversation
A client given a key alongside an auth_callback, auth_url, token or token_details authenticated with basic auth: Auth.__init__ chose the scheme from use_token_auth and the presence of a key secret alone, so the callback and auth URL were never consulted, and the HTTP client's credential chain discarded a token or token_details given with a key. RSA4 selects token auth whenever use_token_auth is unspecified and any of those four is given, and RSA4a/RSA4b then treat the key as a means of renewing the token. The client now does that. The key is kept, so a token given with it can be renewed. use_token_auth=False still forces basic auth when a key is present, which RSA4 allows, since it only applies when use_token_auth is unspecified. This is a breaking change. A client configured with both now calls its auth_callback or auth_url, which reaches the customer's auth server, or presents the token it was given. Callers who relied on the key winning pass use_token_auth=False, or drop the unused option. The RSA1 token-precedence UTS test now asserts the specification's Bearer header. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
token_request_from_auth_url passed the SDK's internal token params dict straight to the auth URL, as the query string for GET and the form body for POST. An auth server therefore received client_id where RSA8c1a and every other SDK send clientId, a capability dict as its Python repr, a timedelta ttl as "1:00:00", and an unset param as an empty value. Issue #736 reports the client_id half: an auth server written against the docs or another SDK never sees the client ID and issues a token without one. The params are now sent under their wire names and in their wire forms: client_id becomes clientId, a capability that is not already a JSON string is serialised as canonical JSON, a timedelta ttl becomes integer milliseconds, and None values are left out. timestamp and nonce already travel under their wire names, and auth_params and any other key pass through unchanged. Token params still take precedence over auth_params on a name conflict (RSA8c2), now under the wire name. This is a breaking change for auth servers: one that reads client_id from a 3.x client must read clientId instead. There is no transition period that sends both. The names callers use in token_params are unchanged. A mocked test covers GET and POST, and the RSA12b UTS test's deviation note drops the half this fixes; it stays gated on the RSA12a clientId check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Auth.__init__ set auth.client_id to None on a realtime client, deferring it to the clientId a CONNECTED message confirms. So client.client_id and client.auth.client_id disagreed until the connection was established (RTC17), and _ensure_valid_auth_credentials, which only puts a non-null client_id into the token params, called the first auth_callback or auth_url without the configured clientId (RTN2e, RSA7d, RSA12a). The REST client did neither. A realtime client now initialises its clientId as the REST client does: the configured clientId, or failing that the clientId of a supplied TokenDetails (RSA7b1, RSA7b2). CONNECTED still validates it through _configure_client_id, which keeps a configured clientId against a wildcard and rejects a conflicting one with 40102 (RSA15a). This is a breaking change. auth.client_id is no longer None before CONNECTED on a client with a configured clientId, and the first token request now carries that clientId, so an auth_callback receives client_id and an auth server receives clientId where 3.x sent neither. An auth server that issues tokens from the params it receives now issues them for that clientId. The RTN2e UTS test is un-gated, the RTC17 one asserts the specification's equality, and three legacy realtime auth tests that pinned None before CONNECTED now expect the clientId. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
_ensure_valid_auth_credentials wrote the client's clientId into the
token params whenever it had one, over any client_id the caller passed,
and did so in the caller's own dict. So authorize({'client_id': 'b'}) on
a client configured as 'a' silently requested a token for 'a', and the
caller's dict came back changed. RSA10b and RSA10j have explicit
TokenParams supersede the configured ones, and RSA10h makes
Auth#clientId the value used only by default.
authorize() now copies the params it is given and fills in the client's
clientId only when they name none. Params taken from
default_token_params still have the configured clientId written over
theirs, which RSA7a4 requires. A conflicting explicit clientId now
yields a token for that clientId, which _configure_client_id rejects
with 40102 as for any token whose clientId conflicts with the configured
one (RSA15a).
This is a breaking change for a caller that passed a clientId other
than the configured one: authorize() now fails with 40102 where it used
to succeed with a token for the configured clientId. Such callers drop
the explicit clientId or pass the configured one. The RSA10b UTS test's
deviation note now names only the RSA12a check that still gates it, and
two mocked tests cover the copy and the explicit clientId.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A client built with a client_id and a token_details naming a different, non-wildcard clientId was never checked: Auth.__init__ only fell back to the token's clientId when none was configured, and _configure_client_id, which raises on a conflict, runs only after a fetched token. An HTTP client then made requests under a token for the other clientId. RSA15a requires the configured clientId to match any non-wildcard clientId in the TokenDetails, and RSA15c requires an error response. Auth.__init__ now raises IncompatibleClientIdException with 40102, the code _configure_client_id already uses for the same conflict, when token auth is selected and the token's clientId is known and conflicts. The check runs at construction, which the RSA7 UTS test explicitly permits. A wildcard or absent token clientId passes (RSA15b). A bare token string carries no clientId the client can read, so it is checked where RSA15a says: against the CONNECTED connectionDetails on a realtime client, which already fails the connection with 40102. A REST client has nothing to check it against. This is a breaking change: such a client now fails to construct, and a realtime one no longer gets as far as connecting. Callers make the two clientIds agree or drop client_id. The RSA15a UTS test is un-gated, the RSA7 mismatch and RSA15c ones expect the error from the constructor, and the legacy realtime mismatch test expects it there too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TokenDetails parsed its capability into a Capability mapping, so a token's capability could not be read as the JSON string TD5 and RSA16a describe, and a missing one became an empty Capability rather than None. Separately, Capability.add_operation_to_resource always raised AttributeError: Capability stores each resource's operations as a set, and the method called .append on it. TokenDetails.capability is now a str holding the capability exactly as the server sent it, or None. A dict or Capability passed to the constructor is serialised as canonical JSON, and to_dict() returns the string. Capability stays the builder for the capability token param and is still canonicalised before a TokenRequest is signed (RSA9f). add_operation_to_resource now adds the operation to the set. Why as received rather than canonicalised: TD5 says the attribute "contains the capability JSON stringified" and TC2 that "a string representation of the JSON will suffice", and nothing in features.md asks a client to canonicalise what the server returns; c14n appears only for TokenRequest signing. The UTS specs assert the literal server string (rest/unit/RSA16a/reflects-capability-1, rest/unit/TD/ token-details-from-json-0), which json.dumps canonicalisation would fail on its separators alone. ably-js (TokenDetails.capability: string, the response passed through JSON.parse), ably-java (a String deserialised by gson, Capability.c14n applied only to outgoing params), ably-cocoa (NSString via artString) and ably-go (a string struct field) all store it as received. Only ably-dotnet parses and re-serialises it. This is a breaking change: code that indexed or compared token_details.capability as a Capability wraps it in Capability(...), and to_dict() now gives a string, so an auth server returning to_dict() from an auth_url sends the capability in the wire form rather than as a JSON object. The TD and RSA16a UTS tests now assert the specification's strings, and the legacy capability and token tests wrap the attribute. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TokenDetails defaulted expires to the local time plus an hour, read from time.time() rather than the clock, and issued to 0. So a TokenDetails known only by its token string, whether passed as token, returned as a bare string or JWT by an auth_callback or auth_url, or constructed directly, reported an expiry nobody had stated. RSA16b requires only the token attribute to be populated in that case. expires and issued now default to None, as capability already does since the previous commit. Every reader of expires handles None: token_details_has_expired returns False for it, as it does with no server time offset, which is the only check RSA4b1 permits and the one reauth_if_expired relies on; and the cached-token debug log now formats it with %s rather than %d, which would have raised inside logging. A bare token with a known time offset is therefore no longer renewed an hour after it was wrapped; the server's token error drives its renewal (RSA4b). TokenDetails no longer reads the time at all. This is a breaking change for code that read expires or issued from such a TokenDetails, which must handle None. Nothing on the wire changes. The two RSA16b UTS tests are un-gated. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
create_token_request() read only the token params handed to it, so a
direct call ignored default_token_params entirely and signed a request
with no ttl or capability (RSA5c, RSA6c). request_token(), meanwhile,
merged explicit params over the defaults key by key.
Both now take the same rule: with no token params the defaults apply,
and given params, even an empty dict, replace the defaults wholesale.
That is what RSA8e and RSA9h say ("the values of each attribute are not
merged with the configured client library defaults, but rather are used
instead of the stored values (even when null)"), what RSA10j says for
authorize(), and what authorize() here already did. Replace rather than
merge on the evidence of the other SDKs too: ably-js
(`tokenParams || Utils.copy(this.tokenParams)` in both
_requestTokenImpl and _createTokenRequestImpl), ably-java
(`params == null ? this.tokenParams : params.copy()` in requestToken and
createTokenRequest), ably-cocoa (`tokenParams ? : [_tokenParams copy]`,
commented "they're used instead of stored, don't merge them") and
ably-dotnet (`tokenParams ?? CurrentTokenParams ?? ...`) all replace.
ably-go replaces in requestToken and never reads DefaultTokenParams in
createTokenRequest. None merges key by key. request_token() also copies
the params it passes to an auth_callback, as the merge used to.
This is a breaking change in both directions. An auth server whose
client sets default_token_params and signs with create_token_request()
now issues tokens carrying those defaults, typically a shorter ttl or a
narrower capability. A request_token() call that names some params no
longer inherits the rest from the defaults, so a default capability that
restricted its tokens no longer applies. Callers pass every param they
want. The RSA5c and RSA6c UTS tests are un-gated, and a mocked test
covers both methods.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
_configure_client_id ran after every token fetch and read a token with no clientId as an attempt to change the configured one, raising 40102. A bare token string or JWT from an auth_callback or auth_url is wrapped without being parsed, so its clientId is always None: a client configured with clientId whose auth server returns a JWT for that very clientId failed its first attempt, and a realtime client then waited out a full disconnected_retry_timeout, fifteen seconds, before the retry took the cached-token branch and connected (deviations.md I.4, RSA7). A REST client failed the request outright. RSA15a constrains only a non-wildcard token clientId, and the RSA7 UTS case table has a configured clientId with a null token clientId succeed. _configure_client_id now leaves a configured clientId as it is when the token names none. The server still validates the token against the clientId: a realtime client sends it on connect (RSA7e1) and fails on a CONNECTED clientId that conflicts (RSA15a), and a mismatched JWT now reaches that failure on its first attempt. This is not a breaking change. The RSA12b, RSA10b and realtime integration RSA7 matching-clientId UTS tests are un-gated, the RSA12a test asserts the specification's success, and the mismatched-clientId integration test no longer needs a twenty-second wait. The header's REST root-cause count also drops the RSA10b cause, which closed with the explicit-clientId fix but whose test stayed gated on this one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A realtime client built from a key alone is told clientId "*" in its CONNECTED message. _configure_client_id opens with a branch that keeps a configured clientId against a server wildcard, and with no clientId configured that branch still fired, recording the clientId as validated and None. can_assume_client_id then refused every clientId, so presence.enter_client could never succeed on such a connection (deviations.md I.2; RSA7b4, RTP14, RTP15). The UTS integration presence tests and the repository's own presence suite worked around it by configuring client_id='*'. The branch now applies only when a clientId is configured, so with none the client takes the wildcard as its clientId, which RSA7b4 asks for. Because auth.client_id can now be "*", the Basic auth X-Ably-ClientId header is taken from the configured clientId, as RSA7e2 specifies, rather than from auth.client_id, so a wildcard the server reported is never sent back to it as a clientId. A later authorize() on such a client requests a wildcard token, which RSA10h asks for and the key already permits. This is not a breaking change. The three batch presence tests and the two realtime integration presence tests that configured client_id='*' as a workaround now build their clients as the specifications do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 42 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Oct 26, 2026
This branch was successfully deployed
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.
Summary
Breaking changes to authentication and tokens for 4.0.0, plus two small non-breaking fixes in the same code. Each commit is one change, passes
ruffandtest/utson its own, has anUPDATING.mdentry where it is breaking, and updatestest/uts/deviations.md: entries it closes are removed, tests that now pass are un-gated, and the header counts are moved with them.Several of these change what reaches a customer's auth server (an
auth_callbackorauth_url), not only their code. Those are called out below and inUPDATING.md.Commits
keyplus anauth_callback,auth_url,tokenortoken_details, the client used Basic auth and never called the callback. The HTTP client also dropped atoken/token_detailsgiven alongside a key. It now uses token auth and keeps the key for renewal. Migration: passuse_token_auth=Falseto keep Basic auth, which RSA4 allows. Auth servers: a client configured like this now calls them.auth_urlunder their wire names (RSA8c1a, 4.0.0 findings: presence type exports, ignoredrecover,auth_urlparam names #736). SendsclientId, notclient_id, with no transition period. A dict/Capabilitycapability goes as JSON, not a Python repr. Atimedeltattl goes as integer ms, not1:00:00.Nonevalues are left out.timestampandnoncewere already wire names. This covers GET query and POST body. Migration (auth servers): readclientId. The param names callers pass are unchanged.auth.client_idis set from the start, as on the HTTP client, so the firstauth_callback/auth_urlcall now carries the clientId. Migration: don't useauth.client_id is Noneas a not-connected check. Auth servers now receive the clientId on the first token request.authorize()and copy the params (RSA10b, RSA10h, RSA10j). An explicitclient_idis no longer overwritten, and the caller's dict is no longer mutated. The configured clientId still wins over one indefault_token_params(RSA7a4). Migration: a conflicting explicit clientId now fails with 40102 instead of silently getting a token for the configured one. Drop it or pass the configured one.token_detailswhose clientId conflicts withclient_id(RSA15a, RSA15c). RaisesIncompatibleClientIdException(40102) at construction. A bare token string has no readable clientId. For that case the spec's other check applies, against CONNECTEDconnectionDetails: the realtime client already does this and fails the connection with 40102. A REST client has nothing to compare it with. Migration: make the two agree, or dropclient_id.TokenDetails.capabilitythe server's JSON string (TD5, RSA16a). It holds astr, orNone.Capabilitystays the builder for token params. Also fixesCapability.add_operation_to_resource, which always raisedAttributeError. Migration: wrap it asCapability(td.capability).to_dict()now gives the string, so an auth server returningto_dict()sends the wire form.expiresandissuedfor a bare token string (RSA16b). Both are nowNone, andTokenDetailsno longer readstime.time(). Every reader ofexpireshandlesNone:token_details_has_expired,reauth_if_expired, and a%ddebug log that would have raised. Migration: handleNone.default_token_paramsincreate_token_request()(RSA5c, RSA6c). With no params, the defaults apply. Given params, even{}, replace them wholesale, inrequest_token()too. Migration (auth servers): a server withdefault_token_paramsthat signs withcreate_token_request()now issues tokens with those defaults, which usually means a shorter ttl or narrower capability. Arequest_token()call that names some params no longer inherits the rest.disconnected_retry_timeout, and on REST the request failed. Not breaking.NoneclientId, soenter_clientcould never succeed. It is now'*'. The BasicX-Ably-ClientIdheader now comes from the configured clientId, per RSA7e2, so the wildcard is never echoed back. Not breaking. The five UTS integration tests that worked around this withclient_id='*'now build their clients as the spec does.Decision:
TokenDetails.capabilityholds the server's string as isNot canonicalised. Evidence:
features.mdasks for canonicalisation only when signing a TokenRequest, never for what the server returns.rest/unit/RSA16a/reflects-capability-1andrest/unit/TD/token-details-from-json-0.json.dumpscanonicalisation would fail those on its separators alone.TokenDetails.capability: string, from a plainJSON.parse).Stringdeserialised by gson). It appliesCapability.c14nonly to outgoing params, inrequestTokenandcreateTokenRequest.NSStringviaartString).Capabilityand re-serialises it.Decision: given token params replace
default_token_params, not mergeEvidence:
requestToken) and RSA9h (createTokenRequest) both say that provided params "are not merged with the configured client library defaults, but rather are used instead of the stored values (even when null)".authorize(), which here already replaced.params ?: storedidiom, and none merges key by key.tokenParams || Utils.copy(this.tokenParams)in_requestTokenImpland_createTokenRequestImpl.params == null ? this.tokenParams : params.copy()inrequestTokenandcreateTokenRequest.tokenParams ? : [_tokenParams copy], commented "If options, params passed in, they're used instead of stored, don't merge them".tokenParams ?? CurrentTokenParams ?? TokenParams.WithDefaultsApplied()inRequestTokenAsync,AuthorizeAsyncandCreateTokenRequestAsync.requestToken, and never readsDefaultTokenParamsincreateTokenRequest.Skipped or scoped down
recover,auth_urlparam names #736's other half is not done.create_token_request/request_tokenstill silently ignore camelCase keys such as{'clientId': ...}. This PR only fixes the auth_url wire names.create_token_request()/request_token()does not overlay the configured clientId. That path covers no params given, anddefault_token_paramsthat name a clientId. RSA7a4's precedence is applied inauthorize()only.create_token_request()still falls back toauth.client_idwhen the params name none, as before.TokenDetails.DEFAULTSis left in place. The library no longer reads it, but a legacy test does.deviations.mdheader drops by one more in the I.4 commit, which says so.Conflicts
This will conflict with the
v4/http-client-optionsandv4/exceptionsbranches, which are being prepared in parallel offintegration/v4:ably/pubsub/http/auth.py: heaviest withv4/exceptions, around the authCallback error wrapping (RSA4d/RSA4e) and theauth_urlfetch.test/uts/deviations.md: its header counts are changed by all three branches.UPDATING.md: each branch appends entries at the end of the 4.0.0 section.ably/pubsub/server/__init__.py: this PR changes the factory docstrings foruse_token_authand for the newIncompatibleClientIdExceptionraise.Whichever merges second will need its header counts re-measured, not added up by hand.
Tests
test/uts, measured. Each figure matches the header ofdeviations.mdat that commit:RUN_DEVIATIONS=1integration/v4default_token_paramscommitThe 11 tests un-gated are exactly the 11 that left the gated failure list, and no new failure appears. Every commit was also checked on its own with
ruffand the unit tiers, in both modes.New mocked tests:
test/ably/http/authurlparams_test.pyandtest/ably/http/authorizeparams_test.py. They need no sandbox and pass locally.Legacy tests edited but not run, because they hit the sandbox:
test/ably/realtime/realtimeauth_test.py: the three clientId inheritance/mismatch tests.test/ably/http/httpcapability_test.pyandtest/ably/http/httptoken_test.py: thetd.capabilityassertions now wrap the string inCapability(...).🤖 Generated with Claude Code