element-hq / synapse

Synapse: Matrix homeserver written in Python/Twisted.
https://element-hq.github.io/synapse
GNU Affero General Public License v3.0
1.52k stars 186 forks source link

Int vs Bool confusion for the fields returned by the users admin API #16733

Open matrixbot opened 10 months ago

matrixbot commented 10 months ago

This issue has been migrated from #16733.


Description

The List Accounts User Admin API (GET /_synapse/admin/v2/users) used to be returning certain boolean-like fields as integers (e.g. "is_guest": 0).

The v1.97.0 documentation page shows examples that use integers (e.g. "is_guest": 0) while actually saying something like:

The following fields are returned in the JSON response body: ... is_guest - bool - Status if that user is a guest account.

This is confusing - is it an integer or a boolean that we're receiving?

However, going back to the documentation for an ancient version (like v1.35.0), one can read the same exact thing.

On that old version, I believe the API was definitely returning integers, not booleans.

However, on v1.97.0 (at least.. possibly even earlier), the API seems to have started returning booleans. It may be an accidental change. It may have been announced in some release notes and I may have missed it.

Nevertheless, I believe the documentation page is confusing (is it a bool or an integer?) and potentially out of date now - it needs fixes.

Related to: https://github.com/devture/matrix-corporal/issues/30

Steps to reproduce

  1. Run Synapse v1.97.0
  2. Use the GET /_synapse/admin/v2/users API with an admin token
  3. Observe that various fields (is_guest, admin, deactivated, shadow_banned, approved, erased, locked) all a boolean type of value (false or true). On previous versions, integers (0, 1) were used.

Homeserver

another homeserver

Synapse Version

1.97.0

Installation Method

Docker (matrixdotorg/synapse)

Database

PostgreSQL (irrelevant)

Workers

Multiple workers

Platform

VM + containers

Configuration

No response

Relevant log output

None

Anything else that would be useful to know?

No response

erikjohnston commented 9 months ago

We should update the documentation to use bool

reivilibre commented 3 months ago

Database

PostgreSQL (irrelevant)

funnily I suspect this is the worst time to write irrelevant; the ints are probably returned by SQLite because SQLite doesn't (or didn't, at least) support a native bool type...

Would be worth confirming if ints are still returned in SQLite mode

reivilibre commented 3 months ago

Looks like even SQLite returns proper bools these days, so this is just doc change :)

{
  "users": [
    {
      "name": "@attacher:syn1",
      "user_type": null,
      "is_guest": false,
      "admin": false,
      "deactivated": false,
      "shadow_banned": false,
      "displayname": "attacher",
      "avatar_url": null,
      "creation_ts": 1686316400000,
      "approved": true,
      "erased": false,
      "last_seen_ts": null,
      "locked": false
    }
...