Merge pull request #610 from kids-first/rm-ego-fields

🔥 Remove lingering ego auth fields

creator/groups.py

@@ -84,6 +84,8 @@
         "change_datareview",
         "view_datareview",
         "list_all_datareview",
+        "subscribe_study",
+        "unsubscribe_study",
     ],
     "Developers": [
         "view_study",
@@ -110,6 +112,8 @@
         "change_my_version_meta",
         "view_my_event",
         "view_my_study_project",
+        "subscribe_my_study",
+        "unsubscribe_my_study",
     ],
     "Bioinformatics": [
         "view_study",

creator/middleware.py

@@ -12,8 +12,7 @@
 from django.contrib.auth.models import update_last_login
 
 
-logger = logging.getLogger()
-logger.setLevel(logging.INFO)
+logger = logging.getLogger(__name__)
 
 
 class SentryMiddleware:
@@ -68,10 +67,6 @@ def get_jwt_user(request):
             User = get_user_model()
             # Pretend the user is admin when running in development
             user = User.objects.get(username="testuser")
-            # Assign specific auth permissions if they are specified in env
-            if settings.USER_ROLES or settings.USER_GROUPS:
-                user.ego_roles = settings.USER_ROLES
-                user.ego_groups = settings.USER_GROUPS
             return user
 
         user = request.user
@@ -100,10 +95,12 @@ def get_jwt_user(request):
             return AnonymousUser()
 
         sub = token.get("sub")
-        groups = token.get("https://kidsfirstdrc.org/groups")
         roles = token.get("https://kidsfirstdrc.org/roles")
-        # Currently unused
-        permissions = token.get("https://kidsfirstdrc.org/permissions")
+
+        # We need a sub to try to reconcile a user, if it doesn't exist,
+        # bail and auth as an anonymous user
+        if sub is None:
+            return AnonymousUser()
 
         User = get_user_model()
 
@@ -114,48 +111,35 @@ def get_jwt_user(request):
             and settings.CLIENT_ADMIN_SCOPE in token.get("scope", "").split()
         ):
             user = User(username=None)
-            user.ego_roles = ["ADMIN"]
-            user.ego_groups = []
             # We will return the service user here without trying to save it
             # to the database.
             return user
 
-        # Don't allow if it looks like the token doesn't have correct fields
-        if groups is None or roles is None or sub is None:
-            return AnonymousUser()
-
         # Now we know that the token is valid so we will try to see if the user
         # is in our database yet, if so, we will return that user, if not, we
         # will create a new user by fetching more information from auth0 and
         # saving it in the database
         try:
-            user = User.objects.get(sub=token["sub"])
+            user = User.objects.get(sub=sub)
             # The user is already in the database, update their last login
             update_last_login(None, user)
         except User.DoesNotExist:
             profile = Auth0AuthenticationMiddleware._get_profile(encoded)
-            # Problem getting the profile, don't try to create the user now
+            # We don't have enough info to create the user
             if profile is None:
-                user = User(ego_groups=groups, ego_roles=roles)
-                return user
+                return AnonymousUser()
             user = User(
                 username=profile.get("nickname", ""),
                 email=profile.get("email", ""),
                 first_name=profile.get("given_name", ""),
                 last_name=profile.get("family_name", ""),
                 picture=profile.get("picture", ""),
-                ego_groups=[],
-                ego_roles=[],
-                sub=token.get("sub"),
+                sub=sub,
             )
             user.save()
             update_last_login(None, user)
 
-        # NB: We ALWAYS use the JWT as the source of truth for authorization
-        # fields. They will always be stored as empty arrays in the database
-        # and populated on every request from the token after validation.
-        user.ego_groups = groups
-        user.ego_roles = roles
+        # Elevate the user to admin if they have the right role
         if "ADMIN" in roles:
             admins = Group.objects.filter(name="Administrators").first()
             user.groups.add(admins)

creator/migrations/0013_drop_ego_fields.py

@@ -0,0 +1,21 @@
+# Generated by Django 2.2.13 on 2021-03-30 17:35
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('creator', '0012_move_jobs'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='user',
+            name='ego_groups',
+        ),
+        migrations.RemoveField(
+            model_name='user',
+            name='ego_roles',
+        ),
+    ]

creator/models.py

@@ -13,12 +13,7 @@ def create_user(self, username, email, password=None):
         Creates and saves a User with the given email, date of
         birth and password.
         """
-        user = self.model(
-            username=username,
-            ego_groups=[],
-            ego_roles=[],
-            email=self.normalize_email(email),
-        )
+        user = self.model(username=username, email=self.normalize_email(email))
 
         user.set_password(password)
         user.save(using=self._db)
@@ -34,8 +29,6 @@ def create_superuser(self, username, email, password):
             email=self.normalize_email(email),
         )
         user.is_superuser = True
-        user.ego_groups = []
-        user.ego_roles = ['ADMIN']
         user.set_password(password)
         user.save(using=self._db)
         return user
@@ -60,8 +53,6 @@ class Meta:
         db_index=True,
         help_text="The subject of the JWT and primary user identifier",
     )
-    ego_groups = ArrayField(models.CharField(max_length=100, blank=True))
-    ego_roles = ArrayField(models.CharField(max_length=100, blank=True))
     picture = models.CharField(max_length=500, blank=True)
     slack_notify = models.BooleanField(
         default=False,
@@ -85,7 +76,7 @@ class Meta:
 
     @property
     def is_admin(self):
-        return 'ADMIN' in self.ego_roles
+        return self.groups.filter(name="Administrators").exists()
 
     @property
     def display_name(self):

creator/settings/development.py

@@ -313,12 +313,6 @@
 
 CLIENT_ADMIN_SCOPE = "role:admin"
 
-# User roles and groups overrides applied during auth
-USER_ROLES = os.environ.get("USER_ROLES", "")
-USER_ROLES = None if USER_ROLES == "" else USER_ROLES.split(",")
-USER_GROUPS = os.environ.get("USER_GROUPS", "")
-USER_GROUPS = None if USER_GROUPS == "" else USER_GROUPS.split(",")
-
 # Number of seconds after which to timeout any outgoing requests
 REQUESTS_TIMEOUT = os.environ.get("REQUESTS_TIMEOUT", 30)
 REQUESTS_HEADERS = {"User-Agent": "StudyCreator/development (python-requests)"}

creator/settings/production.py

@@ -350,11 +350,6 @@ def traces_sampler(sampling_context):
 
 CLIENT_ADMIN_SCOPE = "role:admin"
 
-# User roles and groups overrides applied during auth
-# These should never be set in prod and should be left up to the auth provider
-USER_ROLES = None
-USER_GROUPS = None
-
 # Number of seconds after which to timeout any outgoing requests
 REQUESTS_TIMEOUT = os.environ.get("REQUESTS_TIMEOUT", 30)
 REQUESTS_HEADERS = {"User-Agent": "StudyCreator/production (python-requests)"}

creator/settings/testing.py

@@ -314,12 +314,6 @@
 
 CLIENT_ADMIN_SCOPE = "role:admin"
 
-# User roles and groups overrides applied during auth
-USER_ROLES = os.environ.get("USER_ROLES", "")
-USER_ROLES = None if USER_ROLES == "" else USER_ROLES.split(",")
-USER_GROUPS = os.environ.get("USER_GROUPS", "")
-USER_GROUPS = None if USER_GROUPS == "" else USER_GROUPS.split(",")
-
 # Number of seconds after which to timeout any outgoing requests
 REQUESTS_TIMEOUT = os.environ.get("REQUESTS_TIMEOUT", 30)
 REQUESTS_HEADERS = {"User-Agent": "StudyCreator/testing (python-requests)"}

creator/studies/migrations/0023_add_subscription_permissions.py

@@ -0,0 +1,17 @@
+# Generated by Django 2.2.13 on 2021-03-30 19:35
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('studies', '0022_add_investigator_name'),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name='study',
+            options={'permissions': [('view_my_study', 'Can list studies that the user belongs to'), ('add_collaborator', 'Can add a collaborator to the study'), ('remove_collaborator', 'Can remove a collaborator to the study'), ('change_sequencing_status', 'Can update the sequencing status of a study'), ('change_ingestion_status', 'Can update the ingestion status of a study'), ('change_phenotype_status', 'Can update the phenotype status of a study'), ('subscribe_study', 'Can subscribe to a study for notifications'), ('subscribe_my_study', 'Can subscribe to a study that the user belongs to.'), ('unsubscribe_study', 'Can unsubscribe to a study for notifications'), ('unsubscribe_my_study', 'Can unsubscribe to a study that the user belongs to.')]},
+        ),
+    ]

creator/studies/models.py

@@ -60,6 +60,22 @@ class Meta:
                 "change_phenotype_status",
                 "Can update the phenotype status of a study",
             ),
+            (
+                "subscribe_study",
+                "Can subscribe to a study for notifications",
+            ),
+            (
+                "subscribe_my_study",
+                "Can subscribe to a study that the user belongs to.",
+            ),
+            (
+                "unsubscribe_study",
+                "Can unsubscribe to a study for notifications",
+            ),
+            (
+                "unsubscribe_my_study",
+                "Can unsubscribe to a study that the user belongs to.",
+            ),
         ]
 
     kf_id = models.CharField(

creator/users/factories.py

@@ -7,12 +7,10 @@
 class UserFactory(factory.DjangoModelFactory):
     class Meta:
         model = User
-        django_get_or_create = ('sub',)
+        django_get_or_create = ("sub",)
 
     sub = factory.Faker("uuid4")
     username = factory.Faker("name")
     first_name = factory.Faker("first_name")
     last_name = factory.Faker("last_name")
     email = factory.Faker("email")
-    ego_groups = []
-    ego_roles = []

creator/users/mutations.py

@@ -103,15 +103,21 @@ def mutate(self, info, study_id, **kwargs):
         """
         user = info.context.user
         if user is None or not user.is_authenticated:
-            raise GraphQLError("Not authenticated to subscribe")
+            raise GraphQLError("Not allowed")
 
         try:
             study = Study.objects.get(kf_id=study_id)
         except Study.DoesNotExist:
             raise GraphQLError("Study does not exist.")
 
-        if study_id not in user.ego_groups and "ADMIN" not in user.ego_roles:
-            raise GraphQLError("Not authenticated to subscribe")
+        if not (
+            user.has_perm("studies.subscribe_study")
+            or (
+                user.has_perm("studies.subscribe_my_study")
+                and user.studies.filter(kf_id=study.kf_id).exists()
+            )
+        ):
+            raise GraphQLError("Not allowed")
 
         # Add the study to the users subscriptions
         user.study_subscriptions.add(study)
@@ -136,13 +142,22 @@ def mutate(self, info, study_id, **kwargs):
         """
         user = info.context.user
         if user is None or not user.is_authenticated:
-            raise GraphQLError("Not authenticated to unsubscribe")
+            raise GraphQLError("Not allowed")
 
         try:
             study = Study.objects.get(kf_id=study_id)
         except Study.DoesNotExist:
             raise GraphQLError("Study does not exist.")
 
+        if not (
+            user.has_perm("studies.unsubscribe_study")
+            or (
+                user.has_perm("studies.unsubscribe_my_study")
+                and user.studies.filter(kf_id=study.kf_id).exists()
+            )
+        ):
+            raise GraphQLError("Not allowed")
+
         # Remove the study to the users subscriptions
         user.study_subscriptions.remove(study)
 

creator/users/schema.py

@@ -92,12 +92,6 @@ def _get_study_id(self, info):
 
 class UserNode(DjangoObjectType):
     display_name = graphene.String(source="display_name")
-    roles = graphene.List(
-        graphene.String, description="Roles that the user has"
-    )
-
-    def resolve_roles(self, info):
-        return self.ego_roles
 
     class Meta:
         model = User