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

[NEW] REST API endpoints permissions.list and permissions.update. Deprecated endpoint permissions #10975

Merged
merged 14 commits into from
Jun 21, 2018

Conversation

vynmera
Copy link
Contributor

@vynmera vynmera commented Jun 1, 2018

Closes #7961

This PR deprecates /api/v1/permissions in favor of a new /api/v1/permissions.list and adds /api/v1/permissions.update.

permissions.list

Practically the same as the old permissions, but the permissions are now in a permissions field and a success field is included.

Example response:

{
  "permissions": [
    {
      "_id": "access-permissions",
      "roles": [
        "admin"
      ],
      "_updatedAt": "2018-01-25T13:03:20.879Z",
      "meta": {
        "revision": 0,
        "created": 1516980515815,
        "version": 0
      },
      "$loki": 1
    },{
      ...
    }
  ],
  "success": true
}

permissions.update

Takes a single field permissions containing an array of objects, which contain the field _id and roles. These new permissions are then merged onto the previous.

Example request:

{
  "permissions": [
    {
      "_id": "access-permissions",
      "roles": ["admin"]
    }, {
      "_id": "clean-channel-history",
      "roles": ["admin", "owner"]
    }, {
      ...
    }
  ]
}

In this example, the access-permissions permission is changed to be granted to admins, and the clean-channel-history permission is changed to be granted to admins and channel owners.

The returned value is the same as that of permissions.list.

permissions deprecation

Since permissions directly returns an array, I have no clue how to implement a deprecation message properly. I've left it out for now, please do tell me if there is any way I could've handled it better.
Furthermore, the deprecation warnings I did add are targeted for 0.69.0, assuming my PR will be included in 0.66.0. If this is not the case, please change it or request me to change it.

@graywolf336
Copy link
Contributor

@vynmera you don't have to keep merging develop into your branch, once we get ready to merge then we'll do it. :)

As far as deprecation notice, we have a helper method for that on the rest api: https://github.com/RocketChat/Rocket.Chat/blob/develop/packages/rocketchat-api/server/helpers/deprecationWarning.js

@vynmera
Copy link
Contributor Author

vynmera commented Jun 7, 2018

@graywolf336 Aight :) If you need I can remake the branch.

I tried to use the deprecation notice, but there's a problem with the implementation of permissions.
permissions directly returns an Array (not an object), whereas deprecationWarning's parameter and return data is an object (it adds fields to the object you give it).
To implement that warning, we'd need to make a breaking change to the existing permissions, just to deprecate it.

@graywolf336
Copy link
Contributor

graywolf336 commented Jun 7, 2018

@vynmera ahhhhhh okay, that makes sense. Then just duplicate that method's logging notice and update the docs.

As far as the branch, you can keep this one. :) Since we'll squash merge this pull request so there is only one commit for it.

@vynmera
Copy link
Contributor Author

vynmera commented Jun 8, 2018

@graywolf336 Done! The docs update should already be in this PR: https://github.com/RocketChat/docs/pull/777

@RocketChat RocketChat deleted a comment Jun 8, 2018
@RocketChat RocketChat deleted a comment Jun 8, 2018
@RocketChat RocketChat deleted a comment Jun 8, 2018
@MarcosSpessatto MarcosSpessatto added this to the 0.66.0 milestone Jun 12, 2018
@rodrigok rodrigok changed the title [NEW] [BREAK] REST API: permissions.list and permissions.update [NEW] REST API endpoints permissions.list and permissions.update. Deprecated endpoint permissions Jun 21, 2018
@rodrigok rodrigok merged commit daa53bf into RocketChat:develop Jun 21, 2018
@rodrigok rodrigok mentioned this pull request Jun 28, 2018
@vynmera vynmera deleted the rest-permissions branch July 3, 2018 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants