darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2026-05-05 00:20:43 -04:00
Code Issues Packages Projects Releases Wiki Activity
Files
a38baa0bd8a9f4164c42c9eda1d54bcd31d31b7e
notifications-admin/app/utils/user_permissions.py

53 lines
2.0 KiB
Python
Raw Normal View History

Make user API client return JSON, not a model The data flow of other bits of our application looks like this: ``` API (returns JSON) ⬇ API client (returns a built in type, usually `dict`) ⬇ Model (returns an instance, eg of type `Service`) ⬇ View (returns HTML) ``` The user API client was architected weirdly, in that it returned a model directly, like this: ``` API (returns JSON) ⬇ API client (returns a model, of type `User`, `InvitedUser`, etc) ⬇ View (returns HTML) ``` This mixing of different layers of the application is bad because it makes it hard to write model code that doesn’t have circular dependencies. As our application gets more complicated we will be relying more on models to manage this complexity, so we should make it easy, not hard to write them. It also means that most of our mocking was of the User model, not just the underlying JSON. So it would have been easy to introduce subtle bugs to the user model, because it wasn’t being comprehensively tested. A lot of the changed lines of code in this commit mean changing the tests to mock only the JSON, which means that the model layer gets implicitly tested. For those reasons this commit changes the user API client to return JSON, not an instance of `User` or other models.
2019-05-23 15:27:35 +01:00
from itertools import chain
roles = {
'send_messages': ['send_texts', 'send_emails', 'send_letters'],
'manage_templates': ['manage_templates'],
'manage_service': ['manage_users', 'manage_settings'],
'manage_api_keys': ['manage_api_keys'],
'view_activity': ['view_activity'],
Split the templates permission out again It will likely be the same people who have permission to create alerts and edit templates (maybe someone in a comms role). But combining the two permissions makes the options presented in the form feel clunky because ‘alerts’ and ‘templates’ are conceptually quite different. So I think it makes sense to keep the templates permission the same as it is for regular Notify services.
2021-07-22 14:15:02 +01:00
'create_broadcasts': ['create_broadcasts', 'reject_broadcasts', 'cancel_broadcasts'],
Allow users with new broadcast permissions access to routes Added two new permissions - `create_broadcasts` and `approve_broadcasts`. These new permissions get added to the `has_permissions` decorator of the broadcast routes to allow the routes to be accessed with either the old permissions on the new ones while we switch over. We were using the `send_messages` permission for the broadcast routes. By having two new permissions we can allow a more granular control of these routes.
2021-06-17 18:03:58 +01:00
'approve_broadcasts': ['approve_broadcasts', 'reject_broadcasts', 'cancel_broadcasts'],
Make user API client return JSON, not a model The data flow of other bits of our application looks like this: ``` API (returns JSON) ⬇ API client (returns a built in type, usually `dict`) ⬇ Model (returns an instance, eg of type `Service`) ⬇ View (returns HTML) ``` The user API client was architected weirdly, in that it returned a model directly, like this: ``` API (returns JSON) ⬇ API client (returns a model, of type `User`, `InvitedUser`, etc) ⬇ View (returns HTML) ``` This mixing of different layers of the application is bad because it makes it hard to write model code that doesn’t have circular dependencies. As our application gets more complicated we will be relying more on models to manage this complexity, so we should make it easy, not hard to write them. It also means that most of our mocking was of the User model, not just the underlying JSON. So it would have been easy to introduce subtle bugs to the user model, because it wasn’t being comprehensively tested. A lot of the changed lines of code in this commit mean changing the tests to mock only the JSON, which means that the model layer gets implicitly tested. For those reasons this commit changes the user API client to return JSON, not an instance of `User` or other models.
2019-05-23 15:27:35 +01:00
}
Update the broadcast roles We've added new broadcast roles in the database (`create_broadcasts` and `approve_broadcasts`). Adding these has meant we've needed to do a bit of a rewrite of the roles and permissions code since this had been based on the assumption that each database permission only belongs to one admin role - this is no longer true. This means that flipping the roles dict round to create a dict which contains database permissions as the keys is no longer possible. We can't necessarily tell which admin role someone has given a database permission. To check if a user has an admin role given a list of database permissions, the user must now have ALL the database permissions mapped to that role (instead of just one). This works because no one has the `manage_users` permission without also having the `manage_settings` (and similar for the other admin roles which map to multiple database permissions). Some test data was changed because it was using admin roles where database permissions are actually used when the app is running. I've kept the functionality of the `translate_permissions_from_db_to_admin_roles` function passing through any unknown roles it is passed as an argument. This is not necessary, so can be changed later if we decide it will not ever be used. However, removing it would require updating a lot of tests since the tests rely on this behaviour.
2021-06-18 15:09:11 +01:00
all_permissions = set(roles.keys())
all_database_permissions = set(chain(*roles.values()))
Make user API client return JSON, not a model The data flow of other bits of our application looks like this: ``` API (returns JSON) ⬇ API client (returns a built in type, usually `dict`) ⬇ Model (returns an instance, eg of type `Service`) ⬇ View (returns HTML) ``` The user API client was architected weirdly, in that it returned a model directly, like this: ``` API (returns JSON) ⬇ API client (returns a model, of type `User`, `InvitedUser`, etc) ⬇ View (returns HTML) ``` This mixing of different layers of the application is bad because it makes it hard to write model code that doesn’t have circular dependencies. As our application gets more complicated we will be relying more on models to manage this complexity, so we should make it easy, not hard to write them. It also means that most of our mocking was of the User model, not just the underlying JSON. So it would have been easy to introduce subtle bugs to the user model, because it wasn’t being comprehensively tested. A lot of the changed lines of code in this commit mean changing the tests to mock only the JSON, which means that the model layer gets implicitly tested. For those reasons this commit changes the user API client to return JSON, not an instance of `User` or other models.
2019-05-23 15:27:35 +01:00
Rename unclear "permissions" attributes These are more than a list of permissions: each item includes the label to use when displaying it as an option on a form. Switching to a name that reflects how the attributes are used will help to avoid confusion when we rename some of the other attributes in the same file in later commits.
2021-07-22 14:15:24 +01:00
permission_options = (
Make user API client return JSON, not a model The data flow of other bits of our application looks like this: ``` API (returns JSON) ⬇ API client (returns a built in type, usually `dict`) ⬇ Model (returns an instance, eg of type `Service`) ⬇ View (returns HTML) ``` The user API client was architected weirdly, in that it returned a model directly, like this: ``` API (returns JSON) ⬇ API client (returns a model, of type `User`, `InvitedUser`, etc) ⬇ View (returns HTML) ``` This mixing of different layers of the application is bad because it makes it hard to write model code that doesn’t have circular dependencies. As our application gets more complicated we will be relying more on models to manage this complexity, so we should make it easy, not hard to write them. It also means that most of our mocking was of the User model, not just the underlying JSON. So it would have been easy to introduce subtle bugs to the user model, because it wasn’t being comprehensively tested. A lot of the changed lines of code in this commit mean changing the tests to mock only the JSON, which means that the model layer gets implicitly tested. For those reasons this commit changes the user API client to return JSON, not an instance of `User` or other models.
2019-05-23 15:27:35 +01:00
('view_activity', 'See dashboard'),
('send_messages', 'Send messages'),
('manage_templates', 'Add and edit templates'),
('manage_service', 'Manage settings, team and usage'),
('manage_api_keys', 'Manage API integration'),
)
Rename unclear "permissions" attributes These are more than a list of permissions: each item includes the label to use when displaying it as an option on a form. Switching to a name that reflects how the attributes are used will help to avoid confusion when we rename some of the other attributes in the same file in later commits.
2021-07-22 14:15:24 +01:00
broadcast_permission_options = (
Split the templates permission out again It will likely be the same people who have permission to create alerts and edit templates (maybe someone in a comms role). But combining the two permissions makes the options presented in the form feel clunky because ‘alerts’ and ‘templates’ are conceptually quite different. So I think it makes sense to keep the templates permission the same as it is for regular Notify services.
2021-07-22 14:15:02 +01:00
('manage_templates', 'Add and edit templates'),
Disambiguate sent and created At the moment we say that you either ‘add’ an alert or ‘send’ it. This is confusing because: - an alert isn’t received on people’s phones until it’s approved, so this is really when it is ‘sent’ conceptually - an alert can be rejected before anyone receives it, so the UI can say an alert that no-one ever received was sent This commit re-labels things so that the the first part of the process is ‘creating’ the alert. This makes all the permissions nice and distinct from each other. Adding templates and adding alerts feel conceptually quite different things (what are you adding the alert to?).
2021-07-22 14:27:22 +01:00
('create_broadcasts', 'Create new alerts'),
Update the broadcast roles We've added new broadcast roles in the database (`create_broadcasts` and `approve_broadcasts`). Adding these has meant we've needed to do a bit of a rewrite of the roles and permissions code since this had been based on the assumption that each database permission only belongs to one admin role - this is no longer true. This means that flipping the roles dict round to create a dict which contains database permissions as the keys is no longer possible. We can't necessarily tell which admin role someone has given a database permission. To check if a user has an admin role given a list of database permissions, the user must now have ALL the database permissions mapped to that role (instead of just one). This works because no one has the `manage_users` permission without also having the `manage_settings` (and similar for the other admin roles which map to multiple database permissions). Some test data was changed because it was using admin roles where database permissions are actually used when the app is running. I've kept the functionality of the `translate_permissions_from_db_to_admin_roles` function passing through any unknown roles it is passed as an argument. This is not necessary, so can be changed later if we decide it will not ever be used. However, removing it would require updating a lot of tests since the tests rely on this behaviour.
2021-06-18 15:09:11 +01:00
('approve_broadcasts', 'Approve alerts'),
Only show relevant user permissions for broadcast services For services with the broadcast permission this hides: - the ‘View dashboard’ permission (and defaults it to _checked_) because all users of broadcast services will need to see the dashboard - the ‘Manage API keys’ permission (and defaults it to _not checked_) because we don’t offer an API integration for broadcast services yet – if we do we won’t want existing users to automatically get the permission It relabels: - the ‘Send’ permission to ‘Prepare and approve’ to match the current, slightly clunky language on the templates page - the ‘Manage settings’ label to not refer to ‘usage’ because broadcast services won’t incur cost
2020-07-13 11:10:34 +01:00
)
Make user API client return JSON, not a model The data flow of other bits of our application looks like this: ``` API (returns JSON) ⬇ API client (returns a built in type, usually `dict`) ⬇ Model (returns an instance, eg of type `Service`) ⬇ View (returns HTML) ``` The user API client was architected weirdly, in that it returned a model directly, like this: ``` API (returns JSON) ⬇ API client (returns a model, of type `User`, `InvitedUser`, etc) ⬇ View (returns HTML) ``` This mixing of different layers of the application is bad because it makes it hard to write model code that doesn’t have circular dependencies. As our application gets more complicated we will be relying more on models to manage this complexity, so we should make it easy, not hard to write them. It also means that most of our mocking was of the User model, not just the underlying JSON. So it would have been easy to introduce subtle bugs to the user model, because it wasn’t being comprehensively tested. A lot of the changed lines of code in this commit mean changing the tests to mock only the JSON, which means that the model layer gets implicitly tested. For those reasons this commit changes the user API client to return JSON, not an instance of `User` or other models.
2019-05-23 15:27:35 +01:00
def translate_permissions_from_db_to_admin_roles(permissions):
"""
Given a list of database permissions, return a set of roles
Update the broadcast roles We've added new broadcast roles in the database (`create_broadcasts` and `approve_broadcasts`). Adding these has meant we've needed to do a bit of a rewrite of the roles and permissions code since this had been based on the assumption that each database permission only belongs to one admin role - this is no longer true. This means that flipping the roles dict round to create a dict which contains database permissions as the keys is no longer possible. We can't necessarily tell which admin role someone has given a database permission. To check if a user has an admin role given a list of database permissions, the user must now have ALL the database permissions mapped to that role (instead of just one). This works because no one has the `manage_users` permission without also having the `manage_settings` (and similar for the other admin roles which map to multiple database permissions). Some test data was changed because it was using admin roles where database permissions are actually used when the app is running. I've kept the functionality of the `translate_permissions_from_db_to_admin_roles` function passing through any unknown roles it is passed as an argument. This is not necessary, so can be changed later if we decide it will not ever be used. However, removing it would require updating a lot of tests since the tests rely on this behaviour.
2021-06-18 15:09:11 +01:00
A role is returned if all of its database permissions are in the permission list that is passed in.
Any permissions in the list that are not database permissions are also returned.
Make user API client return JSON, not a model The data flow of other bits of our application looks like this: ``` API (returns JSON) ⬇ API client (returns a built in type, usually `dict`) ⬇ Model (returns an instance, eg of type `Service`) ⬇ View (returns HTML) ``` The user API client was architected weirdly, in that it returned a model directly, like this: ``` API (returns JSON) ⬇ API client (returns a model, of type `User`, `InvitedUser`, etc) ⬇ View (returns HTML) ``` This mixing of different layers of the application is bad because it makes it hard to write model code that doesn’t have circular dependencies. As our application gets more complicated we will be relying more on models to manage this complexity, so we should make it easy, not hard to write them. It also means that most of our mocking was of the User model, not just the underlying JSON. So it would have been easy to introduce subtle bugs to the user model, because it wasn’t being comprehensively tested. A lot of the changed lines of code in this commit mean changing the tests to mock only the JSON, which means that the model layer gets implicitly tested. For those reasons this commit changes the user API client to return JSON, not an instance of `User` or other models.
2019-05-23 15:27:35 +01:00
"""
Update the broadcast roles We've added new broadcast roles in the database (`create_broadcasts` and `approve_broadcasts`). Adding these has meant we've needed to do a bit of a rewrite of the roles and permissions code since this had been based on the assumption that each database permission only belongs to one admin role - this is no longer true. This means that flipping the roles dict round to create a dict which contains database permissions as the keys is no longer possible. We can't necessarily tell which admin role someone has given a database permission. To check if a user has an admin role given a list of database permissions, the user must now have ALL the database permissions mapped to that role (instead of just one). This works because no one has the `manage_users` permission without also having the `manage_settings` (and similar for the other admin roles which map to multiple database permissions). Some test data was changed because it was using admin roles where database permissions are actually used when the app is running. I've kept the functionality of the `translate_permissions_from_db_to_admin_roles` function passing through any unknown roles it is passed as an argument. This is not necessary, so can be changed later if we decide it will not ever be used. However, removing it would require updating a lot of tests since the tests rely on this behaviour.
2021-06-18 15:09:11 +01:00
unknown_database_permissions = {p for p in permissions if p not in all_database_permissions}
return {
admin_role for admin_role, db_role_list in roles.items()
if set(db_role_list) <= set(permissions)
} | unknown_database_permissions
Make user API client return JSON, not a model The data flow of other bits of our application looks like this: ``` API (returns JSON) ⬇ API client (returns a built in type, usually `dict`) ⬇ Model (returns an instance, eg of type `Service`) ⬇ View (returns HTML) ``` The user API client was architected weirdly, in that it returned a model directly, like this: ``` API (returns JSON) ⬇ API client (returns a model, of type `User`, `InvitedUser`, etc) ⬇ View (returns HTML) ``` This mixing of different layers of the application is bad because it makes it hard to write model code that doesn’t have circular dependencies. As our application gets more complicated we will be relying more on models to manage this complexity, so we should make it easy, not hard to write them. It also means that most of our mocking was of the User model, not just the underlying JSON. So it would have been easy to introduce subtle bugs to the user model, because it wasn’t being comprehensively tested. A lot of the changed lines of code in this commit mean changing the tests to mock only the JSON, which means that the model layer gets implicitly tested. For those reasons this commit changes the user API client to return JSON, not an instance of `User` or other models.
2019-05-23 15:27:35 +01:00
def translate_permissions_from_admin_roles_to_db(permissions):
"""
Given a list of admin roles (ie: checkboxes on a permissions edit page for example), return a set of db permissions
Looks them up in the roles dict, falling back to just passing through if they're not recognised.
"""
return set(chain.from_iterable(roles.get(permission, [permission]) for permission in permissions))
Reference in New Issue Copy Permalink