Feat/id token projects (#684)

* feat(id_token): don't include user projects in id_tokens for implicit flow * feat(userinfo): add projects to match whats in tokens * fix(implicit): correctly determine whether or not to include access token for implicit flow * feat(implicit): don't include project access in any tokens returned by implicit flow * Update fence/jwt/token.py Co-Authored-By: Pauline Ribeyre <ribeyre@uchicago.edu> * Update fence/jwt/token.py Co-Authored-By: Pauline Ribeyre <ribeyre@uchicago.edu> * chore(userinfo): don't duplicate project access, just leave existing name * fix(tokens): add missing paren from merge
uc-cdis · Sep 4, 2019 · 3b57b74 · 3b57b74
1 parent a95c91d
commit 3b57b74
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 25 deletions.
diff --git a/fence/jwt/token.py b/fence/jwt/token.py
@@ -190,6 +190,7 @@ def generate_signed_id_token(
     auth_time=None,
     max_age=None,
     nonce=None,
+    include_project_access=True,
     **kwargs
 ):
     """
@@ -218,6 +219,7 @@ def generate_signed_id_token(
         user,
         expires_in,
         client_id,
+        include_project_access=include_project_access,
         audiences=audiences,
         auth_time=auth_time,
         max_age=max_age,
@@ -324,6 +326,7 @@ def generate_signed_access_token(
     forced_exp_time=None,
     client_id=None,
     linked_google_email=None,
+    include_project_access=True,
 ):
     """
     Generate a JWT access token and output a UTF-8
@@ -372,29 +375,30 @@ def generate_signed_access_token(
         "azp": client_id or "",
     }
 
-    # NOTE: "THIS IS A TERRIBLE STOP-GAP SOLUTION SO THAT USERS WITH
-    #       MINIMAL ACCESS CAN STILL USE LATEST VERSION OF FENCE
-    #       WITH VERSIONS OF PEREGRINE/SHEEPDOG THAT DO NOT CURENTLY
-    #       SUPPORT AUTHORIZATION CHECKS AGAINST ARBORIST (AND INSTEAD
-    #       RELY ON THE PROJECTS IN THE TOKEN). If the token is too large
-    #       everything breaks. I'm sorry" --See PXP-3717
-    if len(dict(user.project_access)) < config["TOKEN_PROJECTS_CUTOFF"]:
-        claims["context"]["user"]["projects"] = dict(user.project_access)
-    else:
-        # truncate to configured number of projects in token
-        projects = dict(user.project_access)
-        for key in list(projects)[config["TOKEN_PROJECTS_CUTOFF"]:]:
-            del projects[key]
-        claims["context"]["user"]["projects"] = projects
-        logger.warning(
-            "NOT including project_access = {} in claims for user {} because there are too many projects for the token\n".format(
-                {
-                    k: dict(user.project_access)[k]
-                    for k in set(dict(user.project_access)) - set(projects)
-                },
-                user.username,
-            )
-        )
+    if include_project_access:
+      # NOTE: "THIS IS A TERRIBLE STOP-GAP SOLUTION SO THAT USERS WITH
+      #       MINIMAL ACCESS CAN STILL USE LATEST VERSION OF FENCE
+      #       WITH VERSIONS OF PEREGRINE/SHEEPDOG THAT DO NOT CURENTLY
+      #       SUPPORT AUTHORIZATION CHECKS AGAINST ARBORIST (AND INSTEAD
+      #       RELY ON THE PROJECTS IN THE TOKEN). If the token is too large
+      #       everything breaks. I'm sorry" --See PXP-3717
+      if len(dict(user.project_access)) < config["TOKEN_PROJECTS_CUTOFF"]:
+          claims["context"]["user"]["projects"] = dict(user.project_access)
+      else:
+          # truncate to configured number of projects in token
+          projects = dict(user.project_access)
+          for key in list(projects)[config["TOKEN_PROJECTS_CUTOFF"]:]:
+              del projects[key]
+          claims["context"]["user"]["projects"] = projects
+          logger.warning(
+              "NOT including project_access = {} in claims for user {} because there are too many projects for the token\n".format(
+                  {
+                      k: dict(user.project_access)[k]
+                      for k in set(dict(user.project_access)) - set(projects)
+                  },
+                  user.username,
+              )
+          )
 
     # only add google linkage information if provided
     if linked_google_email:
@@ -423,6 +427,7 @@ def generate_id_token(
     auth_time=None,
     max_age=None,
     nonce=None,
+    include_project_access=True,
     **kwargs
 ):
     """
@@ -476,7 +481,6 @@ def generate_id_token(
             "user": {
                 "name": user.username,
                 "is_admin": user.is_admin,
-                "projects": dict(user.project_access),
                 "email": user.email,
                 "display_name": user.display_name,
                 "phone_number": user.phone_number,
@@ -501,6 +505,9 @@ def generate_id_token(
     if nonce:
         claims["nonce"] = nonce
 
+    if include_project_access:
+        claims["context"]["user"]["projects"] = dict(user.project_access)
+
     logger.info("issuing JWT ID token\n" + json.dumps(claims, indent=4))
 
     token_options = {

diff --git a/fence/oidc/grants/implicit_grant.py b/fence/oidc/grants/implicit_grant.py
@@ -27,7 +27,7 @@ def create_authorization_response(self, grant_user):
         if grant_user:
             self.request.user = grant_user
             client = self.request.client
-            include_access_token = self.request.response_type == "id_token token"
+            include_access_token = self.request.response_type != "id_token"
             nonce = self.request.data.get("nonce")
             token_response = self.generate_token(
                 client,

diff --git a/fence/oidc/jwt_generator.py b/fence/oidc/jwt_generator.py
@@ -80,6 +80,9 @@ def generate_implicit_response(
     if not "user" in scope:
         scope.append("user")
 
+    # don't provide user projects access in id_tokens for implicit flow
+    # due to issues with "Location" header size during redirect (and b/c
+    # of general deprecation of user access information in tokens)
     id_token = generate_signed_id_token(
         kid=keypair.kid,
         private_key=keypair.private_key,
@@ -90,6 +93,7 @@ def generate_implicit_response(
         nonce=nonce,
         linked_google_email=linked_google_email,
         linked_google_account_exp=linked_google_account_exp,
+        include_project_access=False,
     ).token
 
     # ``expires_in`` is just the token expiration time.
@@ -102,6 +106,9 @@ def generate_implicit_response(
         # "state" handled in authlib
     }
 
+    # don't provide user projects access in access_tokens for implicit flow
+    # due to issues with "Location" header size during redirect (and b/c
+    # of general deprecation of user access information in tokens)
     if include_access_token:
         access_token = generate_signed_access_token(
             kid=keypair.kid,
@@ -111,6 +118,7 @@ def generate_implicit_response(
             scopes=scope,
             client_id=client.client_id,
             linked_google_email=linked_google_email,
+            include_project_access=False,
         ).token
         response["access_token"] = access_token