From 11ca7d46bcaacc8f7ed2434ae8de98bbfc8f7f3b Mon Sep 17 00:00:00 2001 From: Mad Price Ball Date: Fri, 1 Nov 2019 15:15:03 -0700 Subject: [PATCH] Improve auto OAuth2 for permission request changes --- open_humans/views.py | 26 +++----------------------- private_sharing/models.py | 33 +++++++++++++++++++++++++++++++++ private_sharing/views.py | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 23 deletions(-) diff --git a/open_humans/views.py b/open_humans/views.py index f02ea7b24..9ccc51a6f 100644 --- a/open_humans/views.py +++ b/open_humans/views.py @@ -370,30 +370,10 @@ def get_context_data(self, **kwargs): permissions_changed = False if "project_id" in self.activity: - project_permissions = { - "share_username": project.request_username_access, - "share_sources": requested_activities, - "all_sources": project.all_sources_access, - "returned_data_description": project.returned_data_description, - } if self.activity["is_connected"]: project_member = project.active_user(self.request.user) - granted_sources = project_member.granted_sources.all() - granted_permissions = { - "share_username": project_member.username_shared, - "share_sources": project_member.granted_sources.all(), - "all_sources": project_member.all_sources_shared, - "returned_data_description": project.returned_data_description, - } - permissions_changed = not all( - [ - granted_permissions[x] == project_permissions[x] - for x in ["share_username", "all_sources"] - ] - ) - gs = set(granted_sources.values_list("id", flat=True)) - ra = set(requested_activities.values_list("id", flat=True)) - permissions_changed = permissions_changed or gs.symmetric_difference(ra) + granted_permissions = project_member.granted_permissions + permissions_changed = project_member.permissions_changed if project.no_public_data: public_files = [] @@ -412,7 +392,7 @@ def get_context_data(self, **kwargs): "source": self.activity["source_name"], "project": project, "project_member": project_member, - "project_permissions": project_permissions, + "project_permissions": project.permissions, "granted_permissions": granted_permissions, "permissions_changed": permissions_changed, "public_files": public_files, diff --git a/private_sharing/models.py b/private_sharing/models.py index e933431d7..88e995f26 100644 --- a/private_sharing/models.py +++ b/private_sharing/models.py @@ -298,6 +298,16 @@ def type(self): def authorized_members(self): return self.project_members.filter_active().count() + @property + def permissions(self): + permissions = { + "share_username": self.request_username_access, + "share_sources": self.requested_sources.all(), + "all_sources": self.all_sources_access, + "returned_data_description": self.returned_data_description, + } + return permissions + def active_user(self, user): try: return DataRequestProjectMember.objects.get( @@ -522,6 +532,29 @@ def joined_date(self): return None return dateutil.parser.parse(self.last_joined[-1][1]) + @property + def granted_permissions(self): + granted_permissions = { + "share_username": self.username_shared, + "share_sources": self.granted_sources.all(), + "all_sources": self.all_sources_shared, + "returned_data_description": self.project.returned_data_description, + } + return granted_permissions + + @property + def permissions_changed(self): + permissions_changed = not all( + [ + self.granted_permissions[x] == self.project.permissions[x] + for x in ["share_username", "all_sources"] + ] + ) + gs = set(self.granted_sources.values_list("id", flat=True)) + ra = set(self.project.requested_sources.values_list("id", flat=True)) + permissions_changed = permissions_changed or gs.symmetric_difference(ra) + return permissions_changed + @staticmethod def random_project_member_id(): """ diff --git a/private_sharing/views.py b/private_sharing/views.py index 3587ed91b..9533ef1f4 100644 --- a/private_sharing/views.py +++ b/private_sharing/views.py @@ -1,8 +1,12 @@ +import urllib + from django.conf import settings from django.contrib import messages as django_messages from django.core.exceptions import ObjectDoesNotExist from django.http import Http404, HttpResponseRedirect +from django.shortcuts import redirect from django.urls import reverse, reverse_lazy +from django.utils.encoding import escape_uri_path from django.views.generic import ( CreateView, DetailView, @@ -341,6 +345,35 @@ def form_valid(self, form): return super().form_valid(form) + def get(self, request, *args, **kwargs): + """ + Override to reload with an updated parameter forcing reauthorization. + + Without this, an existing project member won't be prompted to + reauthorize a project that has updated permissions. + """ + project = self.get_object() + try: + project_member = DataRequestProjectMember.objects.get( + project=project, + member=request.user.member, + joined=True, + authorized=True, + revoked=False, + ) + if project_member.permissions_changed: + if request.GET.get("approval_prompt", "auto") == "auto": + params = request.GET.copy() + params["approval_prompt"] = "force" + reconstructed = "{0}{1}".format( + escape_uri_path(request.path), + "?" + urllib.parse.urlencode(params), + ) + return redirect(reconstructed) + except DataRequestProjectMember.DoesNotExist: + pass + return super().get(request, *args, **kwargs) + def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs)