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

Permissions for groups can not be revoked w/o complete reset #267

Closed
temparus opened this issue May 4, 2018 · 6 comments
Closed

Permissions for groups can not be revoked w/o complete reset #267

temparus opened this issue May 4, 2018 · 6 comments
Labels
Milestone

Comments

@temparus
Copy link
Member

temparus commented May 4, 2018

Original permission of a given group

"permissions": {
        "users": "readwrite",
        "sessions": "readwrite",
        "events": "readwrite",
        "eventsignups": "readwrite"
}

The PATCH request to change a group's permissions with

"permissions": {
        "users": "readwrite",
        "sessions": "read",
        "studydocuments": "readwrite"
}

results in

"permissions": {
        "users": "readwrite",
        "sessions": "read",
        "events": "readwrite",
        "eventsignups": "readwrite",
        "studydocuments": "readwrite"
}

In order to revoke any permissions completely for a given group, first we have to patch the permissions field with null before we can patch the resource with the desired permissions.

@temparus temparus added the bug label May 4, 2018
@temparus temparus added this to the Version 1.0 milestone May 4, 2018
@temparus temparus changed the title Permissions for groups can not be revoked Permissions for groups can not be revoked w/o complete reset May 4, 2018
@hermannsblum
Copy link
Contributor

hermannsblum commented May 4, 2018

As we found out, this is the intended behaviour in eve

A possible solution would be to change the type for permissions from dict to list

This would then look like this:

'permissions': {
            'type': 'list',
            'schema': {
                 'type': 'dict',
                 'schema': {
                       'resource':  {'type': 'string', 'api_resources': True},
                        'permission': {'type': 'string',
                                          'allowed': ['read', 'readwrite']},
                 },
             },
            'nullable': True,
}

@NotSpecial
Copy link
Contributor

NotSpecial commented May 4, 2018

I think the behavior of eve is really weird in this case, it's a bit inconsistent. Maybe we should submit a pull request to allow disabling this behavior in the config.

@hermannsblum
Copy link
Contributor

If you look at the response from nicolai to the issue, I don't see much hope for this to be patched in eve, his philosophy seems to be that overwrites should use PUT.

@NotSpecial
Copy link
Contributor

I have created a pull request, lets see what the response will be!

pyeve/eve#1140

@NotSpecial
Copy link
Contributor

My pull request has been accepted. In the next release of Eve (0.8), we'll be able to disable this behavior in Eve.

@cburchert
Copy link
Member

cburchert commented May 12, 2018 via email

NotSpecial added a commit that referenced this issue Oct 7, 2018
In it's default configuration, Eve merges nested documents on PATCH,
and therefore using PATCH, it is impossible to delete keys from nested
documents, e.g. it is not possible to remove single permissions from groups.

We patched Eve to allow turning this feature off.

With the switch to Eve 0.8, this feature is included in Eve and
so this patch can turn off merging of nested documents.

To make sure it's working, a test is added.

Resolves #267.
NotSpecial added a commit that referenced this issue Oct 7, 2018
In it's default configuration, Eve merges nested documents on PATCH,
and therefore using PATCH, it is impossible to delete keys from nested
documents, e.g. it is not possible to remove single permissions from groups.

We patched Eve to allow turning this feature off.

With the switch to Eve 0.8, this feature is included in Eve and
so this patch can turn off merging of nested documents.

To make sure it's working, a test is added.

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

No branches or pull requests

4 participants