Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix JWT OIDC decode yet again #3466

Merged
merged 1 commit into from
Jun 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
webbnh marked this conversation as resolved.
Show resolved Hide resolved
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