PXP-6406 Feat/ga4gh scope (#799)

* feat(ga4gh-scope): Add ga4gh_passport_v1 to user/sess scopes

* feat(ga4gh-scope): Add new scopes claim to access tokens

* feat(ga4gh-scope): Check access tkn scopes in userinfo

* docs(debt-scopes): Add tech debt note on aud/scope claims

* test(scope): Fix test so it expects new scope claim in tkn

* docs(debt-scopes): Reformat TECHDEBT.md

TECHDEBT.md

@@ -0,0 +1,20 @@
+#  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
+##### 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/config-default.yaml

@@ -210,6 +210,7 @@ USER_ALLOWED_SCOPES:
   - "google_credentials"
   - "google_service_account"
   - "google_link"
+  - "ga4gh_passport_v1"
 
 # these are the scopes that a browser session can create for a user (very
 # similar to USER_ALLOWED_SCOPES, as the session will actually create access_tokens
@@ -223,6 +224,7 @@ SESSION_ALLOWED_SCOPES:
   - "google_credentials"
   - "google_service_account"
   - "google_link"
+  - "ga4gh_passport_v1"
 
 # //////////////////////////////////////////////////////////////////////////////////////
 # LOGIN

fence/jwt/token.py

@@ -385,6 +385,7 @@ def generate_signed_access_token(
         "iat": iat,
         "exp": exp,
         "jti": jti,
+        "scope": scopes,
         "context": {
             "user": {
                 "name": user.username,

fence/resources/user/__init__.py

@@ -130,9 +130,15 @@ def get_user_info(current_session, username):
         info.update(optional_info)
 
     # Include ga4gh passport visas
-    # TODO: Respect access token claims (only include if ga4gh_passport_v1 scope present)
-    encoded_visas = [row.ga4gh_visa for row in user.ga4gh_visas_v1]
-    info["ga4gh_passport_v1"] = encoded_visas
+    if not flask.g.access_token:
+        logger.warning(
+            "Session token present but no access token found. Unable to check scopes in userinfo; returning."
+        )
+    else:
+        at_scopes = jwt.decode(flask.g.access_token, verify=False).get("scope", "")
+        if "ga4gh_passport_v1" in at_scopes:
+            encoded_visas = [row.ga4gh_visa for row in user.ga4gh_visas_v1]
+            info["ga4gh_passport_v1"] = encoded_visas
 
     return info
 

tests/oidc/core/token/test_token_response.py

@@ -72,6 +72,7 @@ def test_access_token_correct_fields(token_response):
         "exp",
         "iat",
         "jti",
+        "scope",
         "context",
         "azp",
     }