Skip to content

Conversation

@zhalloran
Copy link
Contributor

@zhalloran zhalloran commented Jun 26, 2016

This pull request makes the following changes:

  • Adds immutable "protected" field to the Role entity
  • Modifies Role Authorizer to remove "delete" from allowed_privileges for protected roles
  • Adds behat tests for protected roles

Test these changes by:

  • Making authenticated DELETE requests against protected & unprotected roles

Fixes #1017, #1018, #1206 .

Ping @ushahidi/platform


This change is Reviewable

- Adds immutable protected field to the Role entity
- Modifies Role Authorizer to remove "delete" from allowed_privileges for protected roles
- Adds tests for protected roles
@zhalloran
Copy link
Contributor Author

@jasonmule When you get a chance, can you please do a code review of this pull request? Thanks!

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 62.784% when pulling 5a93b7f on feature/add-protected-status-to-roles into 5d3d8a1 on master.

SET protected = 1
WHERE
name = 'user' OR name = 'admin'
");
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this migration reinstate the user and admin roles if they don't exist too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. I'll get that in today.

@rjmackay
Copy link
Contributor

This looks really good. Happy to land.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 62.772% when pulling cc18896 on feature/add-protected-status-to-roles into 5d3d8a1 on master.

@rjmackay rjmackay merged commit 09916fc into master Jun 29, 2016
@rjmackay rjmackay deleted the feature/add-protected-status-to-roles branch June 29, 2016 01:23
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.

4 participants