Skip to content

fix: pre-login REST calls throw locally because request() gates on loggedIn() #385

Description

@diegolmello

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.

  1. In lib/api/api.ts, delete the three guard lines from request, and drop the auth parameter from its signature and its doc comment.
  2. Drop the auth slot from the four verb wrappers (post, get, put, del) and their forwarding calls to request.
  3. Drop auth?: boolean from IAPIRequest in interfaces/index.ts, and the matching @param auth line above it.
  4. 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.tspost('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:

  1. Install the app fresh, so there is no stored Current login.
  2. Add a server.
  3. Tap Register (or Forgot password).
  4. Fill the form and submit.
  5. The request fails immediately. No network request is made.

Deterministic reproduction, which is what the fix should be verified against:

  1. Construct an Api with no Login, so currentLogin is null.
  2. Call post on any Endpoint, e.g. users.forgotPassword.
  3. 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingready-for-agentFully specified, ready for an AFK agent

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions