Merge pull request #11322 from dannon/custos-fixes

Custos `aud` handling fix

lib/galaxy/authnz/custos_authnz.py

@@ -42,6 +42,9 @@ class CustosAuthnz(IdentityProvider):
         elif provider == 'keycloak':
             self._load_config_for_keycloak()
 
+    def _decode_token_no_signature(self, token):
+        return jwt.decode(token, audience=self.config['client_id'], options={"verify_signature": False})
+
     def authenticate(self, trans, idphint=None):
         base_authorize_url = self.config['authorization_endpoint']
         scopes = ['openid', 'email', 'profile']
@@ -77,7 +80,7 @@ class CustosAuthnz(IdentityProvider):
         # Get nonce from token['id_token'] and validate. 'nonce' in the
         # id_token is a hash of the nonce stored in the NONCE_COOKIE_NAME
         # cookie.
-        id_token_decoded = jwt.decode(id_token, options={"verify_signature": False})
+        id_token_decoded = self._decode_token_no_signature(id_token)
         nonce_hash = id_token_decoded['nonce']
         self._validate_nonce(trans, nonce_hash)
 
@@ -144,7 +147,7 @@ class CustosAuthnz(IdentityProvider):
         # Get nonce from token['id_token'] and validate. 'nonce' in the
         # id_token is a hash of the nonce stored in the NONCE_COOKIE_NAME
         # cookie.
-        userinfo = jwt.decode(id_token, options={"verify_signature": False})
+        userinfo = self._decode_token_no_signature(id_token)
 
         # Get userinfo and create Galaxy user record
         email = userinfo['email']
@@ -180,7 +183,7 @@ class CustosAuthnz(IdentityProvider):
                 raise Exception("User is not associated with provider {}".format(self.config["provider"]))
             if len(provider_tokens) > 1:
                 for idx, token in enumerate(provider_tokens):
-                    id_token_decoded = jwt.decode(token.id_token, options={"verify_signature": False})
+                    id_token_decoded = self._decode_token_no_signature(token.id_token)
                     if (id_token_decoded['email'] == email):
                         index = idx
             trans.sa_session.delete(provider_tokens[index])

lib/galaxy/webapps/galaxy/controllers/authnz.py

@@ -3,6 +3,7 @@ OAuth 2.0 and OpenID Connect Authentication and Authorization Controller.
 """
 
 
+import datetime
 import json
 import logging
 
@@ -42,8 +43,23 @@ class OIDC(JSAppLauncher):
             rtv.append({'id': trans.app.security.encode_id(authnz.id), 'provider': authnz.provider, 'email': authnz.uid})
         # Add cilogon and custos identities
         for token in trans.user.custos_auth:
-            userinfo = jwt.decode(token.id_token, options={"verify_signature": False})
-            rtv.append({'id': trans.app.security.encode_id(token.id), 'provider': token.provider, 'email': userinfo['email']})
+            # for purely displaying the info to user, we bypass verification of
+            # signature, audience, and expiration as that's potentially useful
+            # information to share with the end user
+            try:
+                userinfo = jwt.decode(token.id_token, options={'verify_signature': False, 'verify_aud': False, 'verify_exp': False})
+                rtv.append({
+                    'id': trans.app.security.encode_id(token.id),
+                    'provider': token.provider,
+                    'email': userinfo['email'],
+                    'expiration': str(datetime.datetime.utcfromtimestamp(userinfo['exp']))
+                })
+            except Exception:
+                rtv.append({
+                    'id': trans.app.security.encode_id(token.id),
+                    'provider': token.provider,
+                    'error': "Unable to decode token"
+                })
         return rtv
 
     @web.json

test/unit/authnz/test_custos_authnz.py

@@ -39,26 +39,28 @@ class CustosAuthnzTestCase(unittest.TestCase):
 
     def setUp(self):
         self.orig_requests_get = requests.get
-        requests.get = self.mockRequest({
-            self._get_well_known_url(): {
-                "authorization_endpoint": "https://test-auth-endpoint",
-                "token_endpoint": "https://test-token-endpoint",
-                "userinfo_endpoint": "https://test-userinfo-endpoint",
-                "end_session_endpoint": "https://test-end-session-endpoint"
-            },
-            self._get_credential_url(): {
-                "iam_client_secret": "TESTSECRET"
+        requests.get = self.mockRequest(
+            {
+                self._get_well_known_url(): {
+                    "authorization_endpoint": "https://test-auth-endpoint",
+                    "token_endpoint": "https://test-token-endpoint",
+                    "userinfo_endpoint": "https://test-userinfo-endpoint",
+                    "end_session_endpoint": "https://test-end-session-endpoint",
+                },
+                self._get_credential_url(): {"iam_client_secret": "TESTSECRET"},
             }
-        })
-        self.custos_authnz = custos_authnz.CustosAuthnz('Custos', {
-            'VERIFY_SSL': True
-        }, {
-            'url': self._get_idp_url(),
-            'client_id': 'test-client-id',
-            'client_secret': 'test-client-secret',
-            'redirect_uri': 'https://test-redirect-uri',
-            'realm': 'test-realm'
-        })
+        )
+        self.custos_authnz = custos_authnz.CustosAuthnz(
+            "Custos",
+            {"VERIFY_SSL": True},
+            {
+                "url": self._get_idp_url(),
+                "client_id": "test-client-id",
+                "client_secret": "test-client-secret",
+                "redirect_uri": "https://test-redirect-uri",
+                "realm": "test-realm",
+            },
+        )
         self.setupMocks()
         self.test_state = "abc123"
         self.test_nonce = b"4662892146306485421546981092"
@@ -84,7 +86,13 @@ class CustosAuthnzTestCase(unittest.TestCase):
 
     @property
     def test_id_token(self):
-        return unicodify(jwt.encode({'nonce': self.test_nonce_hash}, key=None, algorithm=None))
+        return unicodify(
+            jwt.encode(
+                {"nonce": self.test_nonce_hash, "aud": "test-client-id"},
+                key=None,
+                algorithm=None,
+            )
+        )
 
     def mock_create_oauth2_session(self, custos_authnz):
         orig_create_oauth2_session = custos_authnz._create_oauth2_session
@@ -361,12 +369,19 @@ class CustosAuthnzTestCase(unittest.TestCase):
         )
         self.assertEqual(0, len(self.trans.sa_session.items))
 
-        test_id_token = unicodify(jwt.encode({
-            'nonce': self.test_nonce_hash,
-            'email': self.test_email,
-            'preferred_username': self.test_username,
-            'sub': self.test_sub
-        }, key=None, algorithm=None))
+        test_id_token = unicodify(
+            jwt.encode(
+                {
+                    "nonce": self.test_nonce_hash,
+                    "email": self.test_email,
+                    "preferred_username": self.test_username,
+                    "sub": self.test_sub,
+                    "aud": "test-client-id",
+                },
+                key=None,
+                algorithm=None,
+            )
+        )
 
         self._raw_token = {
             "access_token": self.test_access_token,