Skip to content

Commit

Permalink
Fix JWT OIDC decode yet again (#3466)
Browse files Browse the repository at this point in the history
PBENCH-1182

JWT added yet another `DecodeError` in our API Key validation path in some
recent update. Instead of continuing to add specific errors to be converted
into a specific internal exception, just handle all exceptions in the primary
OIDC decode by attempting the token as an API key. If that fails, report the
API key validation error, and re-raise the original OIDC error.

(Note that `verify_auth` will in turn just report this error and act as if no
authentication token was given.)
  • Loading branch information
dbutenhof authored Jun 17, 2023
1 parent fb64f6f commit 8561ce6
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 57 deletions.
35 changes: 11 additions & 24 deletions lib/pbench/server/auth/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@ def __str__(self) -> str:
return self.message


class OpenIDTokenInvalid(Exception):
pass


class Connection:
"""Helper connection class for use by an OpenIDClient instance."""

Expand Down Expand Up @@ -359,23 +355,14 @@ def token_introspect(self, token: str) -> JSON:
"sub": <user_id>
}
"""
try:
return jwt.decode(
token,
self._pem_public_key,
algorithms=[self._TOKEN_ALG],
audience=[self.client_id],
options={
"verify_signature": True,
"verify_aud": True,
"verify_exp": True,
},
)
except (
jwt.ExpiredSignatureError,
jwt.InvalidSignatureError,
jwt.InvalidAudienceError,
jwt.InvalidAlgorithmError,
ValueError,
) as exc:
raise OpenIDTokenInvalid() from exc
return jwt.decode(
token,
self._pem_public_key,
algorithms=[self._TOKEN_ALG],
audience=[self.client_id],
options={
"verify_signature": True,
"verify_aud": True,
"verify_exp": True,
},
)
25 changes: 9 additions & 16 deletions lib/pbench/server/auth/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from flask_restful import abort

from pbench.server import PbenchServerConfig
from pbench.server.auth import OpenIDClient, OpenIDTokenInvalid
from pbench.server.auth import OpenIDClient
from pbench.server.database.models.api_keys import APIKey
from pbench.server.database.models.users import User

Expand Down Expand Up @@ -143,14 +143,15 @@ def verify_auth_oidc(auth_token: str) -> Optional[User]:
"""
try:
token_payload = oidc_client.token_introspect(token=auth_token)
except OpenIDTokenInvalid:
# The token is not a valid access token, fall through.
pass
except Exception:
current_app.logger.exception(
"Unexpected exception occurred while verifying the auth token {}",
auth_token,
)
try:
return verify_auth_api_key(auth_token)
except Exception:
current_app.logger.exception(
"Unexpected exception occurred while verifying the API key {}",
auth_token,
)
raise
else:
# Extract what we want to cache from the access token
user_id = token_payload["sub"]
Expand All @@ -168,12 +169,4 @@ def verify_auth_oidc(auth_token: str) -> Optional[User]:
user.update(username=username, roles=roles)
return user

try:
return verify_auth_api_key(auth_token)
except Exception:
current_app.logger.exception(
"Unexpected exception occurred while verifying the API key {}",
auth_token,
)

return None
25 changes: 8 additions & 17 deletions lib/pbench/test/unit/server/auth/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,7 @@

from pbench.server import JSON
import pbench.server.auth
from pbench.server.auth import (
Connection,
OpenIDClient,
OpenIDClientError,
OpenIDTokenInvalid,
)
from pbench.server.auth import Connection, OpenIDClient, OpenIDClientError
import pbench.server.auth.auth as Auth
from pbench.test.unit.server import DRB_USER_ID
from pbench.test.unit.server.conftest import jwt_secret
Expand Down Expand Up @@ -435,9 +430,8 @@ def test_token_introspect_exp(self, monkeypatch, rsa_keys):
)
oidc_client = OpenIDClient.construct_oidc_client(config)

with pytest.raises(OpenIDTokenInvalid) as exc:
with pytest.raises(jwt.exceptions.ExpiredSignatureError):
oidc_client.token_introspect(token)
assert isinstance(exc.value.__cause__, jwt.exceptions.ExpiredSignatureError)

def test_token_introspect_aud(self, monkeypatch, rsa_keys):
"""Verify .token_introspect() failure via audience error"""
Expand All @@ -449,9 +443,8 @@ def test_token_introspect_aud(self, monkeypatch, rsa_keys):
config = mock_connection(monkeypatch, "them", public_key=rsa_keys["public_key"])
oidc_client = OpenIDClient.construct_oidc_client(config)

with pytest.raises(OpenIDTokenInvalid) as exc:
with pytest.raises(jwt.exceptions.InvalidAudienceError):
oidc_client.token_introspect(token)
assert isinstance(exc.value.__cause__, jwt.exceptions.InvalidAudienceError)

def test_token_introspect_sig(self, monkeypatch, rsa_keys):
"""Verify .token_introspect() failure via signature error"""
Expand All @@ -465,10 +458,9 @@ def test_token_introspect_sig(self, monkeypatch, rsa_keys):
)
oidc_client = OpenIDClient.construct_oidc_client(config)

with pytest.raises(OpenIDTokenInvalid) as exc:
with pytest.raises(jwt.exceptions.InvalidSignatureError):
# Make the signature invalid.
oidc_client.token_introspect(token + "1")
assert isinstance(exc.value.__cause__, jwt.exceptions.InvalidSignatureError)

def test_token_introspect_alg(self, monkeypatch, rsa_keys):
"""Verify .token_introspect() failure via algorithm error"""
Expand All @@ -483,9 +475,8 @@ def test_token_introspect_alg(self, monkeypatch, rsa_keys):
)
oidc_client = OpenIDClient.construct_oidc_client(config)

with pytest.raises(OpenIDTokenInvalid) as exc:
with pytest.raises(jwt.exceptions.InvalidAlgorithmError):
oidc_client.token_introspect(generated_api_key)
assert isinstance(exc.value.__cause__, jwt.exceptions.InvalidAlgorithmError)


@dataclass
Expand Down Expand Up @@ -671,7 +662,7 @@ def test_verify_auth_oidc_invalid(self, monkeypatch, rsa_keys, make_logger):
monkeypatch.setattr(Auth, "oidc_client", oidc_client)

def tio_exc(token: str) -> JSON:
raise OpenIDTokenInvalid()
raise Exception("OIDC validation is disabled")

app = Flask("test-verify-auth-oidc-invalid")
app.logger = make_logger
Expand All @@ -693,7 +684,7 @@ def test_verify_auth_api_key(
monkeypatch.setattr(Auth, "oidc_client", oidc_client)

def tio_exc(token: str) -> JSON:
raise OpenIDTokenInvalid()
raise Exception("OIDC validation is disabled")

app = Flask("test_verify_auth_api_key")
app.logger = make_logger
Expand All @@ -716,7 +707,7 @@ def test_verify_auth_api_key_invalid(
monkeypatch.setattr(Auth, "oidc_client", oidc_client)

def tio_exc(token: str) -> JSON:
raise OpenIDTokenInvalid()
raise Exception("OIDC validation is disabled")

app = Flask("test_verify_auth_api_key_invalid")
app.logger = make_logger
Expand Down

0 comments on commit 8561ce6

Please sign in to comment.