Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve documentation around /account/whoami #1152

Merged
merged 5 commits into from Mar 6, 2018
Merged

Improve documentation around /account/whoami #1152

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2e4e5e2
Improve documentation around /account/whoami
turt2live Mar 6, 2018
5285dbc
Add clarifications of /account/whoami to changelog
turt2live Mar 6, 2018
6ba5d7c
Reword appservice requirements for /account/whoami
turt2live Mar 6, 2018
b7f8f20
Reword the appservice portion of /account/whoami
turt2live Mar 6, 2018
159ab73
Fix typos in whoami.yaml
turt2live Mar 6, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 33 additions & 3 deletions api/client-server/whoami.yaml
View file
Open in desktop
Expand Up @@ -29,7 +29,13 @@ paths:
get:
summary: Gets information about the owner of an access token.
description: |-
Gets information about the owner of a given access token.
Gets information about the owner of a given access token.

If the owner of the access token is an application service,
the server should return the user ID making the request. The
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure "the user ID making the request" is clear.

Presumably this refers to the user_id specified as a query param? Could you say that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, although that parameter is optional (which therefore makes the request by the bridge bot)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, sorry, but I'm afraid this isn't any clearer to me at all :/.

To me, in the AS model, "the user ID making the request" is the AS user; the AS user can masquerade as a user within its namespace by providing a user_id parameter.

So the server should not return the user ID making the request.

Rather than all this blurb, which is common to most of the C-S API, I would say something like:

Note that, as with the rest of the Client-Server API, Application Services may masquerade as users within their namespace by giving a user_id query parameter. In this situation, the server should verify that the given user_id is registered, and return it in the response body.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yours is a lot clearer and what I was trying to say. I'm just bad with words this time around it seems.

Taken nearly verbatim (minor edits).

server should respect the application service client/server API
extensions during this request. If the request is made for a
virtual user, the server should verify that it is registered.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should I know what a virtual user is?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Readers should be, or can at least become aware of what this means by reading the appservice spec.

operationId: getTokenOwner
security:
- accessToken: []
Expand All @@ -40,14 +46,38 @@ paths:
The token belongs to a known user.
examples:
application/json: {
"user_id": "@joe:example.org"
}
"user_id": "@joe:example.org"
}
schema:
type: object
required: ["user_id"]
properties:
user_id:
type: string
description: The user id that owns the access token.
401:
description:
The token is not recongized
examples:
application/json: {
"errcode": "M_UNKNOWN_TOKEN",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: you want to use "Unrecognized"

"error": "Unrecongised access token."
}
schema:
"$ref": "definitions/error.yaml"
403:
description:
The appservice cannot masquerade the user or has not registered them.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

masquerade as

examples:
application/json: {
"errcode": "M_FORBIDDEN",
"error": "Application service has not registered this user."
}
schema:
"$ref": "definitions/error.yaml"
429:
description: This request was rate-limited.
schema:
"$ref": "definitions/error.yaml"
tags:
- User data
2 changes: 2 additions & 0 deletions changelogs/client_server.rst
View file
Open in desktop
Expand Up @@ -23,6 +23,8 @@ Unreleased changes
(`#1137 <https://github.com/matrix-org/matrix-doc/pull/1137>`_).
- Clarify that ``m.tag`` ordering is done with numbers, not strings
(`#1139 <https://github.com/matrix-org/matrix-doc/pull/1139>`_).
- Clarify that ``/account/whoami`` should consider application services
(`#1152 <https://github.com/matrix-org/matrix-doc/pull/1152>`_).

- Changes to the API which will be backwards-compatible for clients:

Expand Down