Improved error messages and exception handling

lib/galaxy/authnz/custos_authnz.py

@@ -512,27 +512,36 @@ def decode_user_access_token(self, sa_session, access_token):
         :type  access_token: string
         :param access_token: An OIDC access token
 
-        :return: A tuple containing the user and decoded jwt data
+        :return: A tuple containing the user and decoded jwt data or [None, None]
+                 if the access token does not belong to this provider.
         :rtype: Tuple[User, dict]
         """
         if not self.jwks_client:
             return None
-        signing_key = self.jwks_client.get_signing_key_from_jwt(access_token)
-        decoded_jwt = jwt.decode(
-            access_token,
-            signing_key.key,
-            algorithms=["RS256"],
-            issuer=self.config.issuer,
-            audience=self.config.accepted_audiences,
-            options={
-                "verify_signature": True,
-                "verify_exp": True,
-                "verify_nbf": True,
-                "verify_iat": True,
-                "verify_aud": bool(self.config.accepted_audiences),
-                "verify_iss": True,
-            },
-        )
+        try:
+            signing_key = self.jwks_client.get_signing_key_from_jwt(access_token)
+            decoded_jwt = jwt.decode(
+                access_token,
+                signing_key.key,
+                algorithms=["RS256"],
+                issuer=self.config.issuer,
+                audience=self.config.accepted_audiences,
+                options={
+                    "verify_signature": True,
+                    "verify_exp": True,
+                    "verify_nbf": True,
+                    "verify_iat": True,
+                    "verify_aud": bool(self.config.accepted_audiences),
+                    "verify_iss": True,
+                },
+            )
+        except jwt.exceptions.PyJWKClientError:
+            log.debug(f"Could not get signing keys for access token with provider: {self.config.provider}. Ignoring...")
+            return None, None
+        except jwt.exceptions.InvalidIssuerError:
+            # An Invalid issuer means that the access token is not relevant to this provider.
+            # All other exceptions are bubbled up
+            return None, None
         # jwt verified, we can now fetch the user
         user_id = decoded_jwt["sub"]
         custos_authnz_token = self._get_custos_authnz_token(sa_session, user_id, self.config.provider)

lib/galaxy/authnz/managers.py

@@ -430,18 +430,24 @@ def _match_access_token_to_user_in_provider(self, sa_session, provider, access_t
                 msg = f"An error occurred when obtaining user by token with provider `{provider}`: {message}"
                 log.error(msg)
                 return None
-            user, jwt = backend.decode_user_access_token(sa_session, access_token)
-            if user:
-                log.debug(f"Found user: {user} via `{provider}` identity provider")
+            user, jwt = None, None
+            try:
+                user, jwt = backend.decode_user_access_token(sa_session, access_token)
+            except Exception:
+                log.exception("Could not decode access token")
+                raise exceptions.AuthenticationFailed(err_msg="Invalid access token or an unexpected error occurred.")
+            if user and jwt:
                 self._validate_permissions(user, jwt)
                 return user
+            elif not user and jwt:
+                # jwt was decoded, but no user could be matched
+                raise exceptions.AuthenticationFailed(
+                    err_msg="Cannot locate user by access token. The user should log into Galaxy at least once with this OIDC provider."
+                )
+            # Both jwt and user are empty, which means that this provider can't process this access token
             return None
         except NotImplementedError:
             return None
-        except Exception as e:
-            msg = f"An error occurred with provider: {provider} when finding user by token: {e}"
-            log.error(msg)
-            return None
 
     def match_access_token_to_user(self, sa_session, access_token):
         for provider in self.oidc_backends_config:

test/integration/oidc/test_auth_oidc.py

@@ -299,15 +299,16 @@ def test_auth_by_access_token_logged_in_once(self):
         # login at least once
         self._login_via_keycloak("gxyuser_logged_in_once", KEYCLOAK_TEST_PASSWORD)
         access_token = self._get_keycloak_access_token(username="gxyuser_logged_in_once")
-        response = self._get("users/current", headers={"Authorization": f"Bearer {access_token}"})
+        response = self._get("whoami", headers={"Authorization": f"Bearer {access_token}"})
         self._assert_status_code_is(response, 200)
         assert response.json()["email"] == "[email protected]"
 
     def test_auth_by_access_token_never_logged_in(self):
         # If the user has not previously logged in via OIDC at least once, OIDC API calls are not allowed
         access_token = self._get_keycloak_access_token(username="gxyuser_never_logged_in")
         response = self._get("users/current", headers={"Authorization": f"Bearer {access_token}"})
-        self._assert_status_code_is(response, 400)
+        self._assert_status_code_is(response, 401)
+        assert "The user should log into Galaxy at least once" in response.json()["err_msg"]
 
     def test_auth_with_expired_token(self):
         _, response = self._login_via_keycloak(KEYCLOAK_TEST_USERNAME, KEYCLOAK_TEST_PASSWORD)
@@ -321,7 +322,7 @@ def test_auth_with_expired_token(self):
         # token should have expired in 7 seconds, so the call should fail
         time.sleep(7)
         response = self._get("users/current", headers={"Authorization": f"Bearer {access_token}"})
-        self._assert_status_code_is(response, 400)
+        self._assert_status_code_is(response, 401)
 
     def test_auth_with_another_authorized_client(self):
         _, response = self._login_via_keycloak(KEYCLOAK_TEST_USERNAME, KEYCLOAK_TEST_PASSWORD)
@@ -336,15 +337,18 @@ def test_auth_with_authorized_client_but_unauthorized_audience(self):
         _, response = self._login_via_keycloak("bpaonlyuser", KEYCLOAK_TEST_PASSWORD)
         access_token = self._get_keycloak_access_token(client_id="bpaclient", username="bpaonlyuser")
         response = self._get("users/current", headers={"Authorization": f"Bearer {access_token}"})
-        self._assert_status_code_is(response, 400)
+        self._assert_status_code_is(response, 401)
+        assert "Invalid access token" in response.json()["err_msg"]
 
     def test_auth_with_unauthorized_client(self):
         _, response = self._login_via_keycloak(KEYCLOAK_TEST_USERNAME, KEYCLOAK_TEST_PASSWORD)
         access_token = self._get_keycloak_access_token(client_id="unauthorizedclient")
         response = self._get("users/current", headers={"Authorization": f"Bearer {access_token}"})
-        self._assert_status_code_is(response, 400)
+        self._assert_status_code_is(response, 401)
+        assert "Invalid access token" in response.json()["err_msg"]
 
     def test_auth_without_required_scopes(self):
         access_token = self._get_keycloak_access_token(client_id="bpaclient")
         response = self._get("users/current", headers={"Authorization": f"Bearer {access_token}"})
-        self._assert_status_code_is(response, 400)
+        self._assert_status_code_is(response, 401)
+        assert "Invalid access token" in response.json()["err_msg"]