[BB2-1105] Generate New App Creds on each Creds Request fetch/download (#1012)

* changed fetch/download logic so that related application's client_id and client_secret re-generated.

* fix CI alert.

* generate only client_secret at creds request record creation time, keep client_id unchanged - per PR review.

* fix edge case: deliver creds for app owned by 'root'

Author: James Fuqian

apps/creds/admin.py

@@ -1,5 +1,6 @@
 from django.contrib import admin
 from django.contrib.auth.models import User
+from oauth2_provider.generators import generate_client_secret
 from oauth2_provider.models import get_application_model
 
 from apps.accounts.models import UserProfile
@@ -18,14 +19,30 @@ class Meta:
 
 @admin.register(MyCredentialingRequest)
 class MyCredentialingRequestAdmin(admin.ModelAdmin):
-    readonly_fields = ('id', 'get_user', 'get_organization', 'updated_at', 'last_visit', 'visits_count', 'get_creds_req_url',)
-    list_display = ("application", "id",
-                    "get_user", "get_organization", "get_creds_req_url",
-                    "created_at", "updated_at",
-                    "last_visit", "visits_count")
-    list_filter = ('application__user__username',)
+    readonly_fields = (
+        "id",
+        "get_user",
+        "get_organization",
+        "updated_at",
+        "last_visit",
+        "visits_count",
+        "get_creds_req_url",
+    )
+    list_display = (
+        "application",
+        "id",
+        "get_user",
+        "get_organization",
+        "get_creds_req_url",
+        "created_at",
+        "updated_at",
+        "last_visit",
+        "visits_count",
+    )
 
-    search_fields = ('application__name', 'application__user__username', '=id')
+    list_filter = ("application__user__username",)
+
+    search_fields = ("application__name", "application__user__username", "=id")
 
     raw_id_fields = ("application",)
 
@@ -59,3 +76,9 @@ def get_creds_req_url(self, obj):
 
     get_creds_req_url.admin_order_field = "get_creds_req_url"
     get_creds_req_url.short_description = "URL for credentials request"
+
+    def save_model(self, request, obj, form, change):
+        app = Application.objects.get(id=obj.application.id)
+        app.client_secret = generate_client_secret()
+        app.save()
+        super().save_model(request, obj, form, change)

apps/creds/utils.py

@@ -1,9 +1,11 @@
+import string
+
 from django.conf import settings
 from django.contrib.auth.models import User
 from oauth2_provider.models import get_application_model
+from rest_framework import exceptions, status
 
 from apps.accounts.models import UserProfile
-
 from .models import CredentialingReqest
 
 
@@ -14,39 +16,58 @@ def get_url(creds_request_id):
     return "{}/creds/{}".format(settings.HOSTNAME_URL, creds_request_id)
 
 
-def get_creds_by_id(creds_request_id: str):
-    return get_creds_by_obj(CredentialingReqest.objects.get(id=creds_request_id))
+def get_app_creds(creds_request_id: string):
+    creds_req = CredentialingReqest.objects.get(id=creds_request_id)
+
+    creds_dict = get_app_usr_info(creds_req)
+
+    if creds_req.updated_at is not None:
+        raise exceptions.PermissionDenied(
+            "Credentials already fetched (download), doing it again not allowed.",
+            code=status.HTTP_403_FORBIDDEN,
+        )
+
+    app = Application.objects.get(pk=creds_req.application_id)
 
+    creds_dict.update(
+        {
+            "client_id": app.client_id,
+            "client_secret": app.client_secret,
+        }
+    )
 
-def get_creds_by_obj(creds_req: CredentialingReqest):
-    creds_dict = {
+    return creds_dict
+
+
+def get_app_usr_info(creds_req: CredentialingReqest):
+
+    app_usr_info = {
         "user_name": None,
         "org_name": None,
         "app_id": None,
         "app_name": None,
-        "client_id": None,
-        "client_secret": None,
+        "creds_req_id": creds_req.id,
     }
-    if creds_req:
 
-        app = Application.objects.get(pk=creds_req.application_id)
+    app = Application.objects.get(pk=creds_req.application_id)
 
-        if app:
-            creds_dict.update(
-                {
-                    "app_id": app.id,
-                    "app_name": app.name,
-                    "client_id": app.client_id,
-                    "client_secret": app.client_secret,
-                }
-            )
+    if app:
+        app_usr_info.update(
+            {
+                "app_id": app.id,
+                "app_name": app.name,
+            }
+        )
 
-            user = User.objects.get(pk=app.user_id)
+        user = User.objects.get(pk=app.user_id)
 
-            if user:
-                creds_dict.update({"user_name": user.username})
+        if user:
+            app_usr_info.update({"user_name": user.username})
+            try:
                 usrprofile = UserProfile.objects.get(user=user)
                 if usrprofile:
-                    creds_dict.update({"org_name": usrprofile.organization_name})
+                    app_usr_info.update({"org_name": usrprofile.organization_name})
+            except UserProfile.DoesNotExist:
+                pass
 
-    return creds_dict
+    return app_usr_info

apps/creds/views.py

@@ -8,7 +8,7 @@
 from rest_framework import exceptions, status
 from rest_framework.views import APIView
 
-from apps.creds.utils import get_creds_by_obj
+from apps.creds.utils import get_app_usr_info, get_app_creds
 from .models import CredentialingReqest
 
 Application = get_application_model()
@@ -20,69 +20,62 @@ def get(self, request, *args, **kwargs):
 
         creds_req_id = kwargs.get("prod_cred_req_id")
         creds_req = self._get_creds_req(creds_req_id)
+        action = request.query_params.get("action", "")
 
-        # check if expired
-        if self._is_expired(creds_req):
-            raise exceptions.PermissionDenied(
-                "Generated credentialing request expired.",
-                code=status.HTTP_403_FORBIDDEN,
-            )
-
-        creds_dict = get_creds_by_obj(creds_req)
-        # fetch creds request and update visits count and relevant timestamps
-        creds_req.visits_count = creds_req.visits_count + 1
-        creds_req.last_visit = datetime.datetime.now(datetime.timezone.utc)
-
-        ctx = creds_dict
-        ctx.update({"creds_req_id": creds_req_id})
+        self._validate_expiration(creds_req)
 
         log_dict = {
             "type": "credentials request",
             "id": creds_req_id,
-            "app_id": ctx.get("app_id", ""),
-            "app_name": ctx.get("app_name", ""),
         }
 
-        action = request.query_params.get("action", "")
+        response = None
+        creds_dict = {}
+        updated = False
 
         if action == "fetch" or action == "download":
-            if creds_req.updated_at is None:
-                creds_req.updated_at = datetime.datetime.now(datetime.timezone.utc)
-                ctx.update(fetch=action)
-                log_dict.update(action=action)
-            else:
-                # already fetched, fetch again forbidden
-                raise exceptions.PermissionDenied(
-                    "Credentials already fetched (download), doing it again not allowed.",
-                    code=status.HTTP_403_FORBIDDEN,
-                )
-        else:
-            # do not give out creds yet if not a fetch request
-            if "client_id" in ctx:
-                ctx.pop("client_id")
-            if "client_secret" in ctx:
-                ctx.pop("client_secret")
-        # update visits and fetch status etc.
-        creds_req.save()
-
-        logger.info(log_dict)
+            # generate new client_id, client_secret
+            creds_dict = get_app_creds(creds_req_id)
+            updated = True
+            log_dict.update(
+                {
+                    "action": action,
+                    "user_name": creds_dict["user_name"],
+                    "org_name": creds_dict["org_name"],
+                    "app_id": creds_dict["app_id"],
+                    "app_name": creds_dict["app_name"],
+                }
+            )
 
         if action == "download":
             response = JsonResponse(creds_dict)
             response["Content-Disposition"] = 'attachment; filename="{}.json"'.format(
                 creds_req_id
             )
-            return response
         else:
-            return render(request, "get_creds.html", ctx)
+            # response creds request page and update visits count and relevant timestamps
+            app_usr_info = get_app_usr_info(creds_req)
+            log_dict.update(app_usr_info)
+            ctx = {"fetch": action}
+            ctx.update(creds_dict)
+            ctx.update(app_usr_info)
+            response = render(request, "get_creds.html", ctx)
+
+        self._update_creds_req_stats(creds_req, updated)
+
+        logger.info(log_dict)
 
-    def _is_expired(self, creds_req):
+        return response
+
+    def _validate_expiration(self, creds_req):
         t_elapsed_since_created = (
             datetime.datetime.now(datetime.timezone.utc) - creds_req.created_at
         )
-        return (
-            t_elapsed_since_created.seconds > settings.CREDENTIALS_REQUEST_URL_TTL * 60
-        )
+        if t_elapsed_since_created.seconds > settings.CREDENTIALS_REQUEST_URL_TTL * 60:
+            raise exceptions.PermissionDenied(
+                "Generated credentialing request expired.",
+                code=status.HTTP_403_FORBIDDEN,
+            )
 
     def _get_creds_req(self, id):
 
@@ -101,3 +94,10 @@ def _get_creds_req(self, id):
             raise exceptions.NotFound("Credentialing request not found.")
 
         return creds_req
+
+    def _update_creds_req_stats(self, creds_req: CredentialingReqest, updated: False):
+        creds_req.visits_count = creds_req.visits_count + 1
+        creds_req.last_visit = datetime.datetime.now(datetime.timezone.utc)
+        if updated:
+            creds_req.updated_at = datetime.datetime.now(datetime.timezone.utc)
+        creds_req.save()