From 50ae4115752b62c7e94765a2b940ecf8ea8239e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Thu, 15 Jun 2017 12:50:28 +0200 Subject: [PATCH] Only users with attract-user role can use Attract Subscribers without that role still have read-only access to Attract, assuming they have access to the project at all. NOTE: this only handles the web interface. API calls are still governed by the nodes permission system, which doesn't currently allow these kinds of role-based user checks. --- attract/__init__.py | 2 + attract/auth.py | 107 ++++++++++++++++++ attract/routes.py | 8 ++ attract/shots_and_assets/routes_assets.py | 11 +- attract/shots_and_assets/routes_shots.py | 13 ++- attract/tasks/routes.py | 12 +- src/templates/attract/assets/for_project.jade | 5 +- .../attract/assets/view_asset_embed.jade | 6 +- src/templates/attract/shots/for_project.jade | 3 +- src/templates/attract/tasks/for_project.jade | 3 +- 10 files changed, 155 insertions(+), 15 deletions(-) create mode 100644 attract/auth.py diff --git a/attract/__init__.py b/attract/__init__.py index 773ae0f..7211e9e 100644 --- a/attract/__init__.py +++ b/attract/__init__.py @@ -9,6 +9,7 @@ from pillar.web.nodes.routes import url_for_node import pillarsdk +import attract.auth import attract.tasks import attract.shots_and_assets @@ -27,6 +28,7 @@ class AttractExtension(PillarExtension): self._log = logging.getLogger('%s.AttractExtension' % __name__) self.task_manager = attract.tasks.TaskManager() self.shot_manager = attract.shots_and_assets.ShotAssetManager() + self.auth = attract.auth.Auth() @property def name(self): diff --git a/attract/auth.py b/attract/auth.py new file mode 100644 index 0000000..aea38db --- /dev/null +++ b/attract/auth.py @@ -0,0 +1,107 @@ +import enum +import flask + +import attr +import bson + +from pillar import attrs_extra + +# Having either of these roles is minimum requirement for using Attract. +ROLES_REQUIRED_TO_USE_ATTRACT = {'attract-user', 'admin'} +ROLES_REQUIRED_TO_VIEW_ATTRACT = {'admin', 'subscriber', 'demo'} + +# Having any of these methods on a project means you can use Attract. +# Prerequisite: the project is set up for Attract and has a Manager assigned to it. +PROJECT_METHODS_TO_USE_ATTRACT = {'PUT'} + + +class Actions(enum.Enum): + VIEW = 'view' + USE = 'use' + + +# Required roles for a given action. +req_roles = { + Actions.VIEW: ROLES_REQUIRED_TO_VIEW_ATTRACT, + Actions.USE: ROLES_REQUIRED_TO_USE_ATTRACT, +} + + +@attr.s +class Auth(object): + """Handles authorization for Attract.""" + + _log = attrs_extra.log('%s.Auth' % __name__) + Actions = Actions # this allows using current_attract.auth.Actions + + def current_user_is_attract_user(self) -> bool: + """Returns True iff the current user has Attract User role.""" + + from pillar.api.utils.authentication import current_user_id + + return self.user_is_attract_user(current_user_id()) + + def user_is_attract_user(self, user_id: bson.ObjectId) -> bool: + """Returns True iff the user has Attract User role.""" + + from pillar import current_app + + assert isinstance(user_id, bson.ObjectId) + + # TODO: move role checking code to Pillar. + users_coll = current_app.db('users') + user = users_coll.find_one({'_id': user_id}, {'roles': 1}) + if not user: + self._log.debug('user_is_attract_user: User %s not found', user_id) + return False + + user_roles = set(user.get('roles', [])) + require_roles = set(ROLES_REQUIRED_TO_USE_ATTRACT) + + intersection = require_roles.intersection(user_roles) + return bool(intersection) + + def current_user_may(self, action: Actions) -> bool: + """Returns True iff the user is authorised to use/view Attract on the current project. + + Requires that determine_user_rights() was called before. + """ + + try: + attract_rights = flask.g.attract_rights + except AttributeError: + self._log.error('current_user_may() called without previous call ' + 'to current_user_rights()') + return False + + return action in attract_rights + + def determine_user_rights(self, project_id: bson.ObjectId): + """Updates g.attract_rights to reflect the current user's usage rights. + + g.attract_rights is a frozenset that contains zero or more Actions. + """ + + from pillar.api.utils.authorization import user_matches_roles + from pillar.api.utils.authentication import current_user_id + from pillar.api.projects.utils import user_rights_in_project + + user_id = current_user_id() + if not user_id: + self._log.debug('Anonymous user never has access to Attract.') + flask.g.attract_rights = frozenset() + return + + rights = set() + for action in Actions: + roles = req_roles[action] + if user_matches_roles(roles): + rights.add(action) + + # TODO Sybren: possibly split this up into a manager-fetching func + authorisation func. + # TODO: possibly store the user rights on the current project in the current_user object? + allowed_on_proj = user_rights_in_project(project_id) + if not allowed_on_proj.intersection(PROJECT_METHODS_TO_USE_ATTRACT): + rights.discard(Actions.USE) + + flask.g.attract_rights = frozenset(rights) diff --git a/attract/routes.py b/attract/routes.py index 58c43f6..346fc94 100644 --- a/attract/routes.py +++ b/attract/routes.py @@ -6,6 +6,7 @@ import flask_login import werkzeug.exceptions as wz_exceptions from pillar.auth import current_web_user as current_user +from pillar.api.utils import str2id from pillar.web.utils import attach_project_pictures import pillar.web.subquery from pillar.web.system_util import pillar_api @@ -130,6 +131,13 @@ def attract_project_view(extra_project_projections: dict=None, extension_props=F if not is_attract: return error_project_not_setup_for_attract() + # Check user access. + auth = current_attract.auth + auth.determine_user_rights(str2id(project['_id'])) + if not auth.current_user_may(auth.Actions.VIEW): + log.info('User %s not allowed to use Attract', current_user) + raise wz_exceptions.Forbidden() + if extension_props: pprops = project.extension_props.attract return wrapped(project, pprops, *args, **kwargs) diff --git a/attract/shots_and_assets/routes_assets.py b/attract/shots_and_assets/routes_assets.py index 8c887bc..53be8e6 100644 --- a/attract/shots_and_assets/routes_assets.py +++ b/attract/shots_and_assets/routes_assets.py @@ -32,6 +32,8 @@ def for_project(project, attract_props, task_id=None, asset_id=None): attract_props['task_types'][node_type_name], project, attract_props, task_id, asset_id) + can_create = current_attract.auth.current_user_may(current_attract.auth.Actions.USE) + return render_template('attract/assets/for_project.html', assets=assets, tasks_for_assets=tasks_for_assets, @@ -39,7 +41,9 @@ def for_project(project, attract_props, task_id=None, asset_id=None): open_task_id=task_id, open_asset_id=asset_id, project=project, - attract_props=attract_props) + attract_props=attract_props, + can_create_task=can_create, + can_create_asset=can_create) @perproject_blueprint.route('/') @@ -50,12 +54,15 @@ def view_asset(project, attract_props, asset_id): asset, node_type = routes_common.view_node(project, asset_id, node_type_asset['name']) + auth = current_attract.auth + can_edit = auth.current_user_may(auth.Actions.USE) and 'PUT' in asset.allowed_methods + return render_template('attract/assets/view_asset_embed.html', asset=asset, project=project, asset_node_type=node_type, attract_props=attract_props, - can_edit='PUT' in asset.allowed_methods) + can_edit=can_edit) @perproject_blueprint.route('/', methods=['POST']) diff --git a/attract/shots_and_assets/routes_shots.py b/attract/shots_and_assets/routes_shots.py index bef0d5e..4655d0b 100644 --- a/attract/shots_and_assets/routes_shots.py +++ b/attract/shots_and_assets/routes_shots.py @@ -39,6 +39,7 @@ def for_project(project, attract_props, task_id=None, shot_id=None): for shot in shots if shot.properties.used_in_edit), } + can_create_task = current_attract.auth.current_user_may(current_attract.auth.Actions.USE) return render_template('attract/shots/for_project.html', shots=shots, @@ -48,7 +49,8 @@ def for_project(project, attract_props, task_id=None, shot_id=None): open_shot_id=shot_id, project=project, attract_props=attract_props, - stats=stats) + stats=stats, + can_create_task=can_create_task) @perproject_blueprint.route('/') @@ -58,13 +60,14 @@ def view_shot(project, attract_props, shot_id): return for_project(project, attract_props, shot_id=shot_id) shot, node_type = routes_common.view_node(project, shot_id, node_type_shot['name']) + can_edit = current_attract.auth.current_user_may(current_attract.auth.Actions.USE) return render_template('attract/shots/view_shot_embed.html', shot=shot, project=project, shot_node_type=node_type, attract_props=attract_props, - can_edit='PUT' in shot.allowed_methods) + can_edit=can_edit and 'PUT' in shot.allowed_methods) @perproject_blueprint.route('/', methods=['POST']) @@ -73,6 +76,9 @@ def save(project, shot_id): log.info('Saving shot %s', shot_id) log.debug('Form data: %s', request.form) + if not current_attract.auth.current_user_may(current_attract.auth.Actions.USE): + raise wz_exceptions.Forbidden() + shot_dict = request.form.to_dict() current_attract.shot_manager.edit_shot(shot_id, **shot_dict) @@ -86,6 +92,9 @@ def save(project, shot_id): @perproject_blueprint.route('/create', methods=['POST', 'GET']) @attract_project_view() def create_shot(project): + if not current_attract.auth.current_user_may(current_attract.auth.Actions.USE): + raise wz_exceptions.Forbidden() + shot = current_attract.shot_manager.create_shot(project) resp = flask.make_response() diff --git a/attract/tasks/routes.py b/attract/tasks/routes.py index e04b2c4..a138b97 100644 --- a/attract/tasks/routes.py +++ b/attract/tasks/routes.py @@ -36,6 +36,9 @@ def index(): @blueprint.route('/', methods=['DELETE']) def delete(task_id): + if not current_attract.auth.current_user_may(current_attract.auth.Actions.USE): + raise wz_exceptions.Forbidden() + log.info('Deleting task %s', task_id) etag = request.form['etag'] @@ -48,10 +51,13 @@ def delete(task_id): @attract_project_view() def for_project(project, task_id=None): tasks = current_attract.task_manager.tasks_for_project(project['_id']) + can_create_task = current_attract.auth.current_user_may(current_attract.auth.Actions.USE) + return render_template('attract/tasks/for_project.html', tasks=tasks['_items'], open_task_id=task_id, - project=project) + project=project, + can_create_task=can_create_task) @perproject_blueprint.route('/') @@ -76,7 +82,9 @@ def view_task(project, attract_props, task_id): task.properties.due_date = parser.parse('%s' % task.properties.due_date) # Fetch project users so that we can assign them tasks - can_edit = 'PUT' in task.allowed_methods + auth = current_attract.auth + can_edit = 'PUT' in task.allowed_methods and auth.current_user_may(auth.Actions.USE) + if can_edit: users = project.get_users(api=api) project.users = users['_items'] diff --git a/src/templates/attract/assets/for_project.jade b/src/templates/attract/assets/for_project.jade index 758e5c4..540fad7 100644 --- a/src/templates/attract/assets/for_project.jade +++ b/src/templates/attract/assets/for_project.jade @@ -6,7 +6,9 @@ .col_header.item-list-header a.item-project(href="{{url_for('projects.view', project_url=project.url)}}") {{ project.name }} span.item-extra Assets ({{ assets | count }}) + | {% if can_create_asset %} a#item-add(href="javascript:asset_create('{{ project.url }}');") + Create Asset + | {% endif %} .item-list.asset.col-scrollable .table @@ -61,8 +63,7 @@ span {{ task.properties.status[0] }} | #} | {% endfor %} - //- Dirty hack, assume a user can create a task for a asset if they can edit the asset. - | {% if 'PUT' in asset.allowed_methods %} + | {% if can_create_task %} button.task-add( title="Add a new '{{ task_type }}' task", class="task-add-link {% if tasks_for_assets[asset._id][task_type] %}hidden{% endif %}", diff --git a/src/templates/attract/assets/view_asset_embed.jade b/src/templates/attract/assets/view_asset_embed.jade index fd7f577..5224872 100644 --- a/src/templates/attract/assets/view_asset_embed.jade +++ b/src/templates/attract/assets/view_asset_embed.jade @@ -2,7 +2,7 @@ form#item_form(onsubmit="return asset_save('{{asset._id}}', '{{ url_for('attract.assets.perproject.save', project_url=project['url'], asset_id=asset._id) }}')") input(type='hidden',name='_etag',value='{{ asset._etag }}') .input-group - | {% if 'PUT' in asset.allowed_methods %} + | {% if can_edit %} input.item-name( name="name", type="text", @@ -19,7 +19,7 @@ title="Copy ID to clipboard") | ID - | {% if 'PUT' in asset.allowed_methods %} + | {% if can_edit %} .input-group textarea#item-description.input-transparent( name="description", @@ -108,7 +108,7 @@ script. $('.js-help').openModalUrl('Help', "{{ url_for('attract.help', project_url=project.url) }}"); - {% if 'PUT' in asset.allowed_methods %} + {% if can_edit %} /* Resize textareas */ var textAreaFields = $('#item-description, #item-notes'); diff --git a/src/templates/attract/shots/for_project.jade b/src/templates/attract/shots/for_project.jade index 8bd8c63..f0bde26 100644 --- a/src/templates/attract/shots/for_project.jade +++ b/src/templates/attract/shots/for_project.jade @@ -60,8 +60,7 @@ span {{ task.properties.status[0] }} | #} | {% endfor %} - //- Dirty hack, assume a user can create a task for a shot if they can edit the shot. - | {% if 'PUT' in shot.allowed_methods %} + | {% if can_create_task %} button.task-add( title="Add a new '{{ task_type }}' task", class="task-add-link {% if tasks_for_shots[shot._id][task_type] %}hidden{% endif %}" diff --git a/src/templates/attract/tasks/for_project.jade b/src/templates/attract/tasks/for_project.jade index a927435..389a919 100644 --- a/src/templates/attract/tasks/for_project.jade +++ b/src/templates/attract/tasks/for_project.jade @@ -6,8 +6,7 @@ .col_header.item-list-header a.item-project(href="{{url_for('projects.view', project_url=project.url)}}") {{ project.name }} span.item-extra Tasks ({{ tasks | count }}) - //- Dirty hack, assume a user can create a task if they can edit the project. - | {% if 'PUT' in project.allowed_methods %} + | {% if can_create_task %} a#item-add(href="javascript:task_create(undefined, 'generic');") + Create Task | {% endif %} .item-list.task.col-list.col-scrollable