PXP-6617 Remove scopes from aud claim in tokens

.pre-commit-config.yaml

@@ -1,18 +1,18 @@
 repos:
 -   repo: git@github.com:Yelp/detect-secrets
-    rev: v0.13.1
+    rev: v1.1.0
     hooks:
     -   id: detect-secrets
         args: ['--baseline', '.secrets.baseline']
         exclude: poetry.lock
 -   repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v2.5.0
+    rev: v4.0.1
     hooks:
     -   id: trailing-whitespace
     -   id: end-of-file-fixer
     -   id: no-commit-to-branch
         args: [--branch, develop, --branch, master, --pattern, release/.*]
 -   repo: https://github.com/psf/black
-    rev: stable
+    rev: 21.5b1
     hooks:
     -   id: black

.secrets.baseline

@@ -1,9 +1,5 @@
 {
-  "exclude": {
-    "files": "poetry.lock",
-    "lines": null
-  },
-  "generated_at": "2021-05-25T21:22:19Z",
+  "version": "1.1.0",
   "plugins_used": [
     {
       "name": "ArtifactoryDetector"
@@ -80,6 +76,12 @@
     {
       "path": "detect_secrets.filters.heuristic.is_likely_id_string"
     },
+    {
+      "path": "detect_secrets.filters.heuristic.is_lock_file"
+    },
+    {
+      "path": "detect_secrets.filters.heuristic.is_not_alphanumeric_string"
+    },
     {
       "path": "detect_secrets.filters.heuristic.is_potential_uuid"
     },
@@ -89,6 +91,9 @@
     {
       "path": "detect_secrets.filters.heuristic.is_sequential_string"
     },
+    {
+      "path": "detect_secrets.filters.heuristic.is_swagger_file"
+    },
     {
       "path": "detect_secrets.filters.heuristic.is_templated_secret"
     }
@@ -110,6 +115,13 @@
         "hashed_secret": "98c144f5ecbb4dbe575147a39698b6be1a5649dd",
         "is_verified": false,
         "line_number": 66
+      },
+      {
+        "type": "Secret Keyword",
+        "filename": "fence/blueprints/storage_creds/other.py",
+        "hashed_secret": "98c144f5ecbb4dbe575147a39698b6be1a5649dd",
+        "is_verified": false,
+        "line_number": 66
       }
     ],
     "fence/config-default.yaml": [
@@ -119,13 +131,6 @@
         "hashed_secret": "a94a8fe5ccb19ba61c4c0873d391e987982fbbd3",
         "is_verified": false,
         "line_number": 31
-      },
-      {
-        "type": "Secret Keyword",
-        "filename": "fence/config-default.yaml",
-        "hashed_secret": "dd29ecf524b030a65261e3059c48ab9e1ecb2585",
-        "is_verified": false,
-        "line_number": 101
       }
     ],
     "fence/local_settings.example.py": [
@@ -167,91 +172,52 @@
         "hashed_secret": "5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8",
         "is_verified": false,
         "line_number": 248
-      }
-    ],
-    "openapis/swagger.yaml": [
-      {
-        "type": "Private Key",
-        "filename": "openapis/swagger.yaml",
-        "hashed_secret": "1348b145fa1a555461c1b790a2f66614781091e9",
-        "is_verified": false,
-        "line_number": 1927
       },
       {
         "type": "Secret Keyword",
-        "filename": "openapis/swagger.yaml",
-        "hashed_secret": "8cb81a55dff48721f04fe341f33ee5b623dd9144",
-        "is_verified": false,
-        "line_number": 1927
-      },
-      {
-        "type": "Secret Keyword",
-        "filename": "openapis/swagger.yaml",
-        "hashed_secret": "41c39979fd01095376b3e9456f1058c33483dbbe",
-        "is_verified": false,
-        "line_number": 1994
-      },
-      {
-        "type": "JSON Web Token",
-        "filename": "openapis/swagger.yaml",
-        "hashed_secret": "d6b66ddd9ea7dbe760114bfe9a97352a5e139134",
+        "filename": "fence/utils.py",
+        "hashed_secret": "8954f53c9dc3f57137230a016d65bfaee24f8bc5",
         "is_verified": false,
-        "line_number": 1994
-      },
+        "line_number": 249
+      }
+    ],
+    "tests/conftest.py": [
       {
-        "type": "Base64 High Entropy String",
-        "filename": "openapis/swagger.yaml",
-        "hashed_secret": "98c144f5ecbb4dbe575147a39698b6be1a5649dd",
+        "type": "Private Key",
+        "filename": "tests/conftest.py",
+        "hashed_secret": "1348b145fa1a555461c1b790a2f66614781091e9",
         "is_verified": false,
-        "line_number": 2041
+        "line_number": 1176
       },
       {
         "type": "Base64 High Entropy String",
-        "filename": "openapis/swagger.yaml",
-        "hashed_secret": "2f58edc671a89190115ecebddf4c70bdd87e3267",
+        "filename": "tests/conftest.py",
+        "hashed_secret": "227dea087477346785aefd575f91dd13ab86c108",
         "is_verified": false,
-        "line_number": 2084
+        "line_number": 1199
       }
     ],
-    "poetry.lock": [
+    "tests/credentials/google/test_credentials.py": [
       {
-        "type": "Hex High Entropy String",
-        "filename": "poetry.lock",
-        "hashed_secret": "640e60795f08744221f6816fe9dc949c58465256",
-        "is_verified": false,
-        "line_number": 1177,
-        "type": "Private Key"
-      },
-      {
-        "type": "Hex High Entropy String",
-        "filename": "poetry.lock",
-        "hashed_secret": "6642e431aaa417100a91214385af6657acb3fab7",
+        "type": "Secret Keyword",
+        "filename": "tests/credentials/google/test_credentials.py",
+        "hashed_secret": "a06bdb09c0106ab559bd6acab2f1935e19f7e939",
         "is_verified": false,
-        "line_number": 1368
+        "line_number": 381
       },
       {
-        "type": "Hex High Entropy String",
-        "filename": "poetry.lock",
-        "hashed_secret": "205b95ce89ff252c6045d78ca9d007e73b45dc00",
-        "is_verified": false,
-        "line_number": 1200,
-        "type": "Base64 High Entropy String"
-      }
-    ],
-    "tests/conftest.py": [
-      {
-        "type": "Private Key",
-        "filename": "tests/conftest.py",
-        "hashed_secret": "1348b145fa1a555461c1b790a2f66614781091e9",
+        "type": "Secret Keyword",
+        "filename": "tests/credentials/google/test_credentials.py",
+        "hashed_secret": "93aa43c580f5347782e17fba5091f944767b15f0",
         "is_verified": false,
-        "line_number": 1151
+        "line_number": 474
       },
       {
-        "type": "Base64 High Entropy String",
-        "filename": "tests/conftest.py",
-        "hashed_secret": "227dea087477346785aefd575f91dd13ab86c108",
+        "type": "Secret Keyword",
+        "filename": "tests/credentials/google/test_credentials.py",
+        "hashed_secret": "768b7fe00de4fd233c0c72375d12f87ce9670144",
         "is_verified": false,
-        "line_number": 1174
+        "line_number": 476
       }
     ],
     "tests/keys/2018-05-01T21:29:02Z/jwt_private_key.pem": [
@@ -287,7 +253,7 @@
         "filename": "tests/scripting/test_fence-create.py",
         "hashed_secret": "e5e9fa1ba31ecd1ae84f75caaa474f3a663f05f4",
         "is_verified": false,
-        "line_number": 1117
+        "line_number": 1120
       }
     ],
     "tests/test-fence-config.yaml": [
@@ -298,13 +264,6 @@
         "is_verified": false,
         "line_number": 31
       },
-      {
-        "type": "Secret Keyword",
-        "filename": "tests/test-fence-config.yaml",
-        "hashed_secret": "dd29ecf524b030a65261e3059c48ab9e1ecb2585",
-        "is_verified": false,
-        "line_number": 85
-      },
       {
         "type": "Secret Keyword",
         "filename": "tests/test-fence-config.yaml",
@@ -314,5 +273,5 @@
       }
     ]
   },
-  "generated_at": "2021-05-26T14:24:12Z"
+  "generated_at": "2021-05-27T15:18:42Z"
 }

TECHDEBT.md

@@ -1,22 +1 @@
 #  Tech debt
-
-### Using 'aud' claim for scopes
-- Observed: July 2020
-- Impact: (If this tech debt affected your work somehow, add a +1 here with a date and note)
-  - +1 Zoe 2020 July 15 This is an example of a +1
-  - +1 Vahid Oct 2020
-
-##### Problem:
-Fence puts OAuth2 scopes into the 'aud' claim of access tokens.
-##### Why it was done this way:
-We don't know.
-##### Why this way is problematic:
-Per RFC7519 the aud claim [is not meant for scopes](https://tools.ietf.org/html/rfc7519#section-4.1.3).
-##### What the solution might be:
-GA4GH AAI [already requires](https://github.com/ga4gh/data-security/blob/master/AAI/AAIConnectProfile.md#access_token-issued-by-broker) that a 'scope' claim be included in access tokens issued by Passport Brokers. So as of July 2020 we will put scopes in the 'scope' claim. However, this is in addition to keeping them in the 'aud' claim. Ideally we would only have the scopes in the 'scope' claim.
-##### Why we aren't already doing the above:
-Fence presently guards several endpoints (e.g. /data, signed urls, SA registration) by checking the scopes in the 'aud' claim of the JWT. This code would need to be changed.
-##### Next steps:
-Address above.
-##### Other notes:
-n/a

fence/auth.py

@@ -209,7 +209,10 @@ def has_oauth(scope=None):
     scope = scope or set()
     scope.update({"openid"})
     try:
-        access_token_claims = validate_jwt(aud=scope, purpose="access")
+        access_token_claims = validate_jwt(
+            scope=scope,
+            purpose="access",
+        )
     except JWTError as e:
         raise Unauthorized("failed to validate token: {}".format(e))
     user_id = access_token_claims["sub"]

fence/blueprints/data/blueprint.py

@@ -19,7 +19,7 @@
 
 
 @blueprint.route("/<path:file_id>", methods=["DELETE"])
-@require_auth_header(aud={"data"})
+@require_auth_header(scope={"data"})
 @login_required({"data"})
 def delete_data_file(file_id):
     """
@@ -106,7 +106,7 @@ def delete_data_file(file_id):
 
 
 @blueprint.route("/upload", methods=["POST"])
-@require_auth_header(aud={"data"})
+@require_auth_header(scope={"data"})
 @login_required({"data"})
 def upload_data_file():
     """
@@ -181,7 +181,7 @@ def upload_data_file():
 
 
 @blueprint.route("/multipart/init", methods=["POST"])
-@require_auth_header(aud={"data"})
+@require_auth_header(scope={"data"})
 @login_required({"data"})
 @check_arborist_auth(resource="/data_file", method="file_upload")
 def init_multipart_upload():
@@ -212,7 +212,7 @@ def init_multipart_upload():
 
 
 @blueprint.route("/multipart/upload", methods=["POST"])
-@require_auth_header(aud={"data"})
+@require_auth_header(scope={"data"})
 @login_required({"data"})
 @check_arborist_auth(resource="/data_file", method="file_upload")
 def generate_multipart_upload_presigned_url():
@@ -246,7 +246,7 @@ def generate_multipart_upload_presigned_url():
 
 
 @blueprint.route("/multipart/complete", methods=["POST"])
-@require_auth_header(aud={"data"})
+@require_auth_header(scope={"data"})
 @login_required({"data"})
 @check_arborist_auth(resource="/data_file", method="file_upload")
 def complete_multipart_upload():

fence/blueprints/data/indexd.py

@@ -14,7 +14,6 @@
 
 from fence.auth import (
     get_jwt,
-    has_oauth,
     current_token,
     login_required,
     set_current_token,
@@ -1148,7 +1147,9 @@ def _get_user_info(sub_to_string=True):
     populated information about an anonymous user.
     """
     try:
-        set_current_token(validate_request(aud={"user"}))
+        set_current_token(
+            validate_request(scope={"user"}, audience=config.get("BASE_URL"))
+        )
         user_id = current_token["sub"]
         if sub_to_string:
             user_id = str(user_id)

fence/blueprints/link.py

@@ -274,7 +274,9 @@ def get(self):
             # if we're mocking google auth, mock response to include the email
             # from the provided access token
             try:
-                token = validate_request({"user"})
+                token = validate_request(
+                    scope={"user"}, audience=config.get("BASE_URL")
+                )
                 email = get_user_from_claims(token).username
             except Exception as exc:
                 logger.info(

fence/blueprints/login/fence_login.py

@@ -92,7 +92,7 @@ def get(self):
             redirect_uri, **flask.request.args.to_dict()
         )
         id_token_claims = validate_jwt(
-            tokens["id_token"], aud={"openid"}, purpose="id", attempt_refresh=True
+            tokens["id_token"], scope="openid", purpose="id", attempt_refresh=True
         )
         username = id_token_claims["context"]["user"]["name"]
         login_user(

fence/blueprints/well_known.py

@@ -78,6 +78,7 @@ def openid_configuration():
         "jti",
         "auth_time",
         "azp",
+        "scope",
         "nonce",
         "context",
     ]

fence/jwt/blacklist.py

@@ -89,7 +89,10 @@ def blacklist_encoded_token(encoded_token, public_key=None):
     public_key = public_key or keys.default_public_key()
     try:
         claims = jwt.decode(
-            encoded_token, public_key, algorithm="RS256", audience="openid"
+            encoded_token,
+            public_key,
+            algorithm="RS256",
+            options={"verify_aud": False},
         )
     except jwt.InvalidTokenError as e:
         raise BlacklistingError("failed to decode token: {}".format(e))
@@ -138,7 +141,10 @@ def is_token_blacklisted(encoded_token, public_key=None):
     public_key = public_key or keys.default_public_key()
     try:
         token = jwt.decode(
-            encoded_token, public_key, algorithm="RS256", audience="openid"
+            encoded_token,
+            public_key,
+            algorithm="RS256",
+            options={"verify_aud": False},
         )
     except jwt.exceptions.InvalidTokenError as e:
         raise JWTError("could not decode token to check blacklisting: {}".format(e))