Skip to content

Passing a role name to wp user remove-cap should probably return an error #98

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

Open
johnbillion opened this issue Oct 6, 2017 · 3 comments · May be fixed by #530
Open

Passing a role name to wp user remove-cap should probably return an error #98

johnbillion opened this issue Oct 6, 2017 · 3 comments · May be fixed by #530

Comments

@johnbillion
Copy link
Contributor

At the user level, WordPress internally treats roles and capabilities the same. They're all stored in the wp_capabilities user meta field. This is unfortunate.

This means that wp user remove-cap <user> <role> works, which can have unexpected consequences if, for example, a script blindly loops over capabilities in order to remove them from a user and one of the capabilities is a role name.

I think if an attempt is made to remove a capability from a user that matches a role name, it should return an error along with recommending that wp user remove-role is used instead.

@johnbillion
Copy link
Contributor Author

It's worth noting that WP_User::remove_cap( <role> ) also suffers from the same problem. I'm not sure on WP-CLI's policy of improving upon core's APIs in this way.

@danielbachhuber
Copy link
Member

It's worth noting that WP_User::remove_cap( <role> ) also suffers from the same problem. I'm not sure on WP-CLI's policy of improving upon core's APIs in this way.

I'm in support of WP-CLI improving upon core's APIs, as long as it's not in opposition to, or diverge too much from, core behavior.

@schlessera
Copy link
Member

I think it might make sense to skip mismatches with a warning, with a --force flag to just execute indiscriminately.

However, that would be a BC break...

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

Successfully merging a pull request may close this issue.

3 participants