From 03878ee643c575c6df9ebc5fb49da6f7a204a81e Mon Sep 17 00:00:00 2001
From: Matthieu Huin <mhuin@redhat.com>
Date: Thu, 12 May 2022 14:49:06 +0200
Subject: [PATCH] Handle jwt decoding error, fix exception default messages

Using a badly formatted token resulted in an error 500 from zuul-web.
Return a more precise error message and an error 401 in zuul-web when
this occurs.

Also fix a typo in default messages for some auth-related exceptions.

Change-Id: I4abe013e76ac51c3dad7ccd969ffe79f5cb459e3
---
 tests/unit/test_web.py | 27 +++++++++++++++++++++++++++
 zuul/exceptions.py     |  4 ++--
 zuul/lib/auth.py       | 14 +++++++++-----
 3 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/tests/unit/test_web.py b/tests/unit/test_web.py
index 4b696534bc..81861c9165 100644
--- a/tests/unit/test_web.py
+++ b/tests/unit/test_web.py
@@ -1740,6 +1740,33 @@ class TestTenantScopedWebApi(BaseTestWeb):
                   'pipeline': 'check'})
         self.assertEqual(401, resp.status_code)
 
+    def test_bad_format_JWT_token(self):
+        token = 'thisisnotwhatatokenshouldbelike'
+        resp = self.post_url(
+            "api/tenant/tenant-one/project/org/project/autohold",
+            headers={'Authorization': 'Bearer %s' % token},
+            json={'job': 'project-test1',
+                  'count': 1,
+                  'reason': 'because',
+                  'node_hold_expiration': 36000})
+        self.assertEqual(401, resp.status_code)
+        resp = self.post_url(
+            "api/tenant/tenant-one/project/org/project/enqueue",
+            headers={'Authorization': 'Bearer %s' % token},
+            json={'trigger': 'gerrit',
+                  'change': '2,1',
+                  'pipeline': 'check'})
+        self.assertEqual(401, resp.status_code)
+        resp = self.post_url(
+            "api/tenant/tenant-one/project/org/project/enqueue",
+            headers={'Authorization': 'Bearer %s' % token},
+            json={'trigger': 'gerrit',
+                  'ref': 'abcd',
+                  'newrev': 'aaaa',
+                  'oldrev': 'bbbb',
+                  'pipeline': 'check'})
+        self.assertEqual(401, resp.status_code)
+
     def test_expired_JWT_token(self):
         authz = {'iss': 'zuul_operator',
                  'sub': 'testuser',
diff --git a/zuul/exceptions.py b/zuul/exceptions.py
index ec1e5cdaf4..a332ba1afa 100644
--- a/zuul/exceptions.py
+++ b/zuul/exceptions.py
@@ -82,11 +82,11 @@ class AuthTokenUnauthorizedException(AuthTokenException):
 
 
 class AuthTokenUndecodedException(AuthTokenUnauthorizedException):
-    default_msg = 'Auth Token could not be decoded'
+    defaultMsg = 'Auth Token could not be decoded'
 
 
 class AuthTokenInvalidSignatureException(AuthTokenUnauthorizedException):
-    default_msg = 'Invalid signature'
+    defaultMsg = 'Invalid signature'
 
 
 class BearerTokenRequiredError(AuthTokenUnauthorizedException):
diff --git a/zuul/lib/auth.py b/zuul/lib/auth.py
index a5866e908d..db37d56fac 100644
--- a/zuul/lib/auth.py
+++ b/zuul/lib/auth.py
@@ -74,10 +74,14 @@ class AuthenticatorRegistry(object):
         cpb.capabilities_registry.register_capabilities('auth', capabilities)
 
     def authenticate(self, rawToken):
-        unverified = jwt.decode(rawToken, options={'verify_signature': False})
-        for auth_name in self.authenticators:
-            authenticator = self.authenticators[auth_name]
-            if authenticator.issuer_id == unverified.get('iss', ''):
-                return authenticator.authenticate(rawToken)
+        try:
+            unverified = jwt.decode(rawToken,
+                                    options={'verify_signature': False})
+            for auth_name in self.authenticators:
+                authenticator = self.authenticators[auth_name]
+                if authenticator.issuer_id == unverified.get('iss', ''):
+                    return authenticator.authenticate(rawToken)
+        except jwt.exceptions.DecodeError:
+            raise exceptions.AuthTokenUndecodedException(self.default_realm)
         # No known issuer found, use default realm
         raise exceptions.IssuerUnknownError(self.default_realm)