Summary
Every REST call made before a Login now fails locally, inside the SDK, without any request reaching the server. This breaks account registration and password recovery for every new user.
Api.request refuses the call:
if (auth && !this.loggedIn()) {
throw new Error(`API ${ method } ${ endpoint } requires a login`)
}
auth defaults to true, so this guard applies to every call unless a caller opts out.
Why it started
loggedIn() was corrected in #341 from Object.keys(this.currentLogin || {}).every(e => e) — which returned true even with no Current login — to this.currentLogin !== null.
That correction is right and should stand. It exposed the guard, which had been dead code for as long as loggedIn() was vacuously true. The fix is not a revert of #341.
Impact
Every Endpoint called before a Login throws. This includes registration, password recovery, and confirmation email — the whole pre-login surface of a consuming app. The Endpoint never reaches the REST client, so there is no network attempt and no server response to diagnose from; the consumer sees only requires a login.
No user is affected today: the React Native app resolves the SDK to an older commit than mobile HEAD. This lands the moment that pin moves.
Root cause
auth does not control the auth headers, despite its name and its doc comment (@param auth Require auth headers for endpoint, default true).
Auth headers are attached by setLogin, from the Current login, independently of this flag:
[authTokenHeader]: login.authToken,
Inside request, auth has exactly one effect: it enables the guard above. It has no effect on the outgoing request.
So the guard is a client-side pre-flight check that duplicates a decision only the server can make. The server already rejects an Endpoint that requires a session. The guard can therefore only produce false negatives — refusing calls that would have succeeded — and can never add safety.
Fix
Remove the guard and the auth parameter. A dead positional parameter named auth, documented as controlling auth headers, is a footgun: the next caller will pass it in good faith and get nothing.
- In
lib/api/api.ts, delete the three guard lines from request, and drop the auth parameter from its signature and its doc comment.
- Drop the
auth slot from the four verb wrappers (post, get, put, del) and their forwarding calls to request.
- Drop
auth?: boolean from IAPIRequest in interfaces/index.ts, and the matching @param auth line above it.
- Sweep the 15 call sites that pass the slot positionally (below).
loggedIn() stays public and unchanged. Callers that genuinely want to branch on Current login state still can.
Call sites to sweep
All 15 pass a literal, except one:
lib/api/api.ts — post('login', …, false), post('logout', {}, true)
lib/api/RocketChat.ts — 13 sites: users.info, rooms.info, channels.join, chat.sendMessage, chat.getRoomIdByNameOrId, chat.getRoomNameById, chat.find, im.create, chat.react, chat.syncMessages, channels.info, groups.info, and info
info() passes this.loggedIn() rather than a literal. That argument becomes meaningless with the flag gone; drop it like the others.
No call site anywhere passes an argument after the auth slot — verified across the SDK and the consuming app. So ignore, options and apiVersion are never bound positionally past it, and this sweep is a deletion of arguments, not a renumbering. Any site missed becomes a compile error, not a silent misbinding.
Two commented-out lines in lib/api/api.ts also show the old signature. Leave them or delete them; they are not live code.
Consumer impact
None. The React Native app's own get/post wrappers take (endpoint, params) only and forward two arguments, and no app call site passes a third positional argument.
Steps to reproduce
From a user's perspective, against an app built on mobile HEAD:
- Install the app fresh, so there is no stored
Current login.
- Add a server.
- Tap Register (or Forgot password).
- Fill the form and submit.
- The request fails immediately. No network request is made.
Deterministic reproduction, which is what the fix should be verified against:
- Construct an
Api with no Login, so currentLogin is null.
- Call
post on any Endpoint, e.g. users.forgotPassword.
- It throws
requires a login, and the REST client is never called.
Tests
Add a spec in the API suite asserting the corrected behaviour:
- With
currentLogin === null, post on a pre-login Endpoint reaches the REST client with the Endpoint and payload intact, instead of throwing requires a login.
- With a
Current login set, a call still carries the auth headers — confirming the removal did not disturb credential attachment.
Use a stubbed IClient to observe the call. Do not edit existing specs to accommodate the change.
Summary
Every REST call made before a
Loginnow fails locally, inside the SDK, without any request reaching the server. This breaks account registration and password recovery for every new user.Api.requestrefuses the call:authdefaults totrue, so this guard applies to every call unless a caller opts out.Why it started
loggedIn()was corrected in #341 fromObject.keys(this.currentLogin || {}).every(e => e)— which returnedtrueeven with noCurrent login— tothis.currentLogin !== null.That correction is right and should stand. It exposed the guard, which had been dead code for as long as
loggedIn()was vacuously true. The fix is not a revert of #341.Impact
Every
Endpointcalled before aLoginthrows. This includes registration, password recovery, and confirmation email — the whole pre-login surface of a consuming app. TheEndpointnever reaches theREST client, so there is no network attempt and no server response to diagnose from; the consumer sees onlyrequires a login.No user is affected today: the React Native app resolves the SDK to an older commit than
mobileHEAD. This lands the moment that pin moves.Root cause
authdoes not control the auth headers, despite its name and its doc comment (@param auth Require auth headers for endpoint, default true).Auth headers are attached by
setLogin, from theCurrent login, independently of this flag:Inside
request,authhas exactly one effect: it enables the guard above. It has no effect on the outgoing request.So the guard is a client-side pre-flight check that duplicates a decision only the server can make. The server already rejects an
Endpointthat requires a session. The guard can therefore only produce false negatives — refusing calls that would have succeeded — and can never add safety.Fix
Remove the guard and the
authparameter. A dead positional parameter namedauth, documented as controlling auth headers, is a footgun: the next caller will pass it in good faith and get nothing.lib/api/api.ts, delete the three guard lines fromrequest, and drop theauthparameter from its signature and its doc comment.authslot from the four verb wrappers (post,get,put,del) and their forwarding calls torequest.auth?: booleanfromIAPIRequestininterfaces/index.ts, and the matching@param authline above it.loggedIn()stays public and unchanged. Callers that genuinely want to branch onCurrent loginstate still can.Call sites to sweep
All 15 pass a literal, except one:
lib/api/api.ts—post('login', …, false),post('logout', {}, true)lib/api/RocketChat.ts— 13 sites:users.info,rooms.info,channels.join,chat.sendMessage,chat.getRoomIdByNameOrId,chat.getRoomNameById,chat.find,im.create,chat.react,chat.syncMessages,channels.info,groups.info, andinfoinfo()passesthis.loggedIn()rather than a literal. That argument becomes meaningless with the flag gone; drop it like the others.No call site anywhere passes an argument after the
authslot — verified across the SDK and the consuming app. Soignore,optionsandapiVersionare never bound positionally past it, and this sweep is a deletion of arguments, not a renumbering. Any site missed becomes a compile error, not a silent misbinding.Two commented-out lines in
lib/api/api.tsalso show the old signature. Leave them or delete them; they are not live code.Consumer impact
None. The React Native app's own
get/postwrappers take(endpoint, params)only and forward two arguments, and no app call site passes a third positional argument.Steps to reproduce
From a user's perspective, against an app built on
mobileHEAD:Current login.Deterministic reproduction, which is what the fix should be verified against:
Apiwith noLogin, socurrentLoginisnull.poston anyEndpoint, e.g.users.forgotPassword.requires a login, and theREST clientis never called.Tests
Add a spec in the API suite asserting the corrected behaviour:
currentLogin === null,poston a pre-loginEndpointreaches theREST clientwith theEndpointand payload intact, instead of throwingrequires a login.Current loginset, a call still carries the auth headers — confirming the removal did not disturb credential attachment.Use a stubbed
IClientto observe the call. Do not edit existing specs to accommodate the change.