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.

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

Add routes for user management #1

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

602p
Copy link
Member

@602p 602p commented Sep 28, 2018

Correct-er wording for the capabilities requirement probably depend on wether or not we are going to have expressions in the jwt.

@602p 602p requested a review from remexre September 28, 2018 02:31
doc/api.md Show resolved Hide resolved
doc/api.md Show resolved Hide resolved
doc/api.md Outdated Show resolved Hide resolved
doc/api.md Show resolved Hide resolved
doc/api.md Show resolved Hide resolved
doc/api.md Outdated
Returns the canonical user object for the user with database id `<id>`.

### POST `/api/thetis/users/user/<id>`

**UNIMPLEMENTED**

`id` is a base-10 integer corresponding to the `db_id` of the user you wish to access.
Requires an authentication token granting membership privileges to access any user that is not the user whose token was used to make the request.
Requires an authentication token granting the requisite `user.modify.<field>(db_id)` privileged.
Copy link
Contributor

Choose a reason for hiding this comment

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

typo (s in privileges)

@@ -162,22 +162,22 @@ Returns the canonical user object for the user whose token was used to make the
**UNIMPLEMENTED**

`id` is a base-10 integer corresponding to the `db_id` of the user you wish to access.
Requires an authentication token granting membership privileges to access any user that is not the user whose token was used to make the request.
Requires an authentication token granting view permissions for the user.
Copy link
Contributor

Choose a reason for hiding this comment

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

uh which caps?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, yeah, so we don't have a users.view cap that is separate from users.list. users.list presumably allows you to see the details for all listed users, so we need something more fine-grained. I'd propose

cap(Id, users.view(Id)).
cap(Id, users.view(_)) :- cap(Id, users.list).

Copy link
Contributor

Choose a reason for hiding this comment

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

Ugh, really we should have users.view(Id, Property); e.g. the discord bot shouldn't have access to people's card numbers etc.

And really that should apply to searches too, but I can't think of a way to do that sanely...

If we aren't doing fine-grained permissions for search, it'd ideally be separate from view so that could do fine-grained permissions.

If that seems OTT though, I'm fine with yours.

Takes a parameter `update` containing a user object with one or more fields set. Updates those fields on the user with database id `<id>` and returns a canonical user object for that user.

### POST `/api/thetis/users/user/new`

**UNIMPLEMENTED**

Requires an authentication token granting membership privileges.
Requires an authentication token granting `user.add`.
Copy link
Contributor

Choose a reason for hiding this comment

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

acmw semantics?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants