Skip to content

Commit

Permalink
Remove unused userinfo and online token validation (distributed-syste…
Browse files Browse the repository at this point in the history 
…m-analysis#3239)

* Remove unused userinfo and online token validation

- Removes the get_userinfo functionality from the auth module
- Removes the online token validation capability from the server
- OIDC token validation will only happen locally

PBENCH-1074
  • Loading branch information
@npalaska
npalaska committed Mar 27, 2023
1 parent 8101add commit 6a39237
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 271 deletions.
94 changes: 1 addition & 93 deletions lib/pbench/server/auth/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -362,78 +362,14 @@ def __init__(
pem_public_key += "-----END PUBLIC KEY-----\n"
self._pem_public_key = pem_public_key

well_known_endpoint = ".well-known/openid-configuration"
well_known_uri = f"realms/{self._realm_name}/{well_known_endpoint}"
endpoints_json = self._connection.get(well_known_uri).json()

try:
self._userinfo_endpoint = endpoints_json["userinfo_endpoint"]
self._tokeninfo_endpoint = endpoints_json["introspection_endpoint"]
except KeyError as e:
raise OpenIDClientError(
HTTPStatus.BAD_GATEWAY,
f"Missing endpoint {e!r} at {well_known_uri}; Endpoints found:"
f" {endpoints_json}",
)

def __repr__(self):
return (
f"OpenIDClient(server_url={self._connection.server_url}, "
f"client_id={self.client_id}, realm_name={self._realm_name}, "
f"headers={self._connection.headers})"
)

def token_introspect_online(self, token: str) -> Optional[JSON]:
"""The introspection endpoint is used to retrieve the active state of a
token.
It can only be invoked by confidential clients.
The introspected JWT token contains the claims specified in
https://tools.ietf.org/html/rfc7662.
Note: this is not supposed to be used in production, instead rely on
offline token validation because of security concerns mentioned in
https://www.rfc-editor.org/rfc/rfc7662.html#section-4.
Args:
token : token value to introspect
Returns:
Extracted token information
{
"aud": <targeted_audience_id>,
"email_verified": <boolean>,
"expires_in": <number_of_seconds>,
"access_type": "offline",
"exp": <unix_timestamp>,
"azp": <client_id>,
"scope": <scope_string>, # "openid email profile"
"email": <user_email>,
"sub": <user_id>
}
"""
if not self._tokeninfo_endpoint:
return None

payload = {
"client_id": self.client_id,
"client_secret": self._client_secret_key,
"token": token,
}
token_payload = self._connection.post(
self._tokeninfo_endpoint, data=payload
).json()

audience = token_payload.get("aud")
if not audience or self.client_id not in audience:
# If our client is not an intended audience for the token,
# we will not verify the token.
token_payload = None

return token_payload

def token_introspect_offline(self, token: str) -> JSON:
def token_introspect(self, token: str) -> JSON:
"""Utility method to decode access/Id tokens using the public key
provided by the identity provider.
Expand Down Expand Up @@ -481,31 +417,3 @@ def token_introspect_offline(self, token: str) -> JSON:
jwt.InvalidAudienceError,
) as exc:
raise OpenIDTokenInvalid() from exc

def get_userinfo(self, token: str = None) -> JSON:
"""The userinfo endpoint returns standard claims about the authenticated
user, and is protected by a bearer token.
FIXME - This method appears unused in the rest of the code.
http://openid.net/specs/openid-connect-core-1_0.html#UserInfo
Args:
token : Valid token to extract the userinfo
Returns:
Userinfo payload
{
"family_name": <surname>,
"sub": <user_id>,
"picture": <URL>,
"locale": <locale_name>,
"email_verified": <boolean>,
"given_name": <given_name>,
"email": <email_address>,
"hd": <domain_name>,
"name": <full_name>
}
"""
headers = {"Authorization": f"Bearer {token}"} if token else None
return self._connection.get(self._userinfo_endpoint, headers=headers).json()
24 changes: 3 additions & 21 deletions lib/pbench/server/auth/auth.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,7 @@
import jwt

from pbench.server import PbenchServerConfig
from pbench.server.auth import (
InternalUser,
OpenIDClient,
OpenIDClientError,
OpenIDTokenInvalid,
)
from pbench.server.auth import InternalUser, OpenIDClient, OpenIDTokenInvalid
from pbench.server.database.models.auth_tokens import AuthToken
from pbench.server.database.models.users import User

Expand Down Expand Up @@ -225,28 +220,15 @@ def verify_auth_oidc(auth_token: str) -> Optional[InternalUser]:
InternalUser object if the verification succeeds, None on failure.
"""
try:
token_payload = oidc_client.token_introspect_offline(token=auth_token)
token_payload = oidc_client.token_introspect(token=auth_token)
except OpenIDTokenInvalid:
token_payload = None
except Exception:
current_app.logger.exception(
"Unexpected exception occurred while verifying the auth token {}",
auth_token,
)

# Offline token verification resulted in some unexpected error,
# perform the online token verification.

# Note: Online verification should NOT be performed frequently, and it
# is only allowed for non-public clients.
try:
token_payload = oidc_client.token_introspect_online(token=auth_token)
except OpenIDClientError as exc:
current_app.logger.debug(
"Can not perform OIDC online token verification, '{}'", exc
)
token_payload = None

token_payload = None
return (
None
if token_payload is None
Expand Down
166 changes: 9 additions & 157 deletions lib/pbench/test/unit/server/auth/test_auth.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,35 +352,10 @@ def __init__(
def get(self, path: str, **kwargs) -> MockResponse:
if path.endswith(f"realms/{realm_name}"):
json_d = {"public_key": public_key}
elif path.endswith(f"realms/{realm_name}/.well-known/openid-configuration"):
json_d = {
"userinfo_endpoint": f"{self.server_url}/userinfo",
"introspection_endpoint": f"{self.server_url}/introspection",
}
if client_id == "us-missing-userinfo-ep":
del json_d["userinfo_endpoint"]
elif client_id == "us-missing-introspection-ep":
del json_d["introspection_endpoint"]
elif client_id == "us-empty-tokeninfo-ep":
json_d["introspection_endpoint"] = ""
elif path.endswith("userinfo"):
json_d = {"path": path, "kwargs": kwargs}
else:
raise Exception(f"MockConnection: unrecognized .get(path={path})")
return MockResponse(json_d)

def post(self, path: str, data: Dict, **kwargs) -> MockResponse:
if path.endswith("/introspection"):
if client_id == "us-raise-exc":
raise OpenIDClientError(400, "Introspection failed")
cid = "other-client" if client_id == "us-other-aud" else client_id
json_d = {"aud": [cid], "data": data}
if client_id == "us-token-payload":
json_d["sub"] = "67890"
else:
raise Exception(f"MockConnection: unrecognized .post(path={path})")
return MockResponse(json_d)

monkeypatch.setattr(pbench.server.auth, "Connection", MockConnection)
return config

Expand Down Expand Up @@ -436,61 +411,8 @@ def test_construct_oidc_client_succ(self, monkeypatch):
oidc_client._pem_public_key
== f"-----BEGIN PUBLIC KEY-----\n{public_key}\n-----END PUBLIC KEY-----\n"
)
assert oidc_client._userinfo_endpoint.endswith("/userinfo")
assert oidc_client._tokeninfo_endpoint.endswith("/introspection")

def test_construct_oidc_client_fail_ep(self, monkeypatch):
"""Verify .construct_oidc_client() failure mode where OpenIDClientError
is raised
"""
# First failure should be caused by missing "userinfo" endpoint.
client_id = "us-missing-userinfo-ep"
config = mock_connection(monkeypatch, client_id)
with pytest.raises(OpenIDClientError):
OpenIDClient.construct_oidc_client(config)

# Second failure should be caused by missing "introspection" endpoint.
client_id = "us-missing-introspection-ep"
config = mock_connection(monkeypatch, client_id)
with pytest.raises(OpenIDClientError):
OpenIDClient.construct_oidc_client(config)

def test_token_introspect_online_no_ep(self, monkeypatch):
"""Verify .token_introspect_online() with empty token information end
point
"""
client_id = "us-empty-tokeninfo-ep"
public_key = "opqrstu"
config = mock_connection(monkeypatch, client_id, public_key)
oidc_client = OpenIDClient.construct_oidc_client(config)
json_d = oidc_client.token_introspect_online("my-token")
assert json_d is None

def test_token_introspect_online_not_aud(self, monkeypatch):
"""Verify .token_introspect_online() with different audience."""
client_id = "us-other-aud"
public_key = "vwxyz01"
config = mock_connection(monkeypatch, client_id, public_key)
oidc_client = OpenIDClient.construct_oidc_client(config)
json_d = oidc_client.token_introspect_online("my-token")
assert json_d is None, f"{json_d!r}"

def test_token_introspect_online_succ(self, monkeypatch):
"""Verify .token_introspect_online() with different audience."""
client_id = "us"
public_key = "2345678"
config = mock_connection(monkeypatch, client_id, public_key)
secret = config["openid-connect"]["secret"]
oidc_client = OpenIDClient.construct_oidc_client(config)
json_d = oidc_client.token_introspect_online("my-token")
assert json_d["aud"] == [client_id]
assert json_d["data"] == {
"client_id": client_id,
"client_secret": secret,
"token": "my-token",
}, f"{json_d!r}"

def test_token_introspect_offline_succ(self, monkeypatch, rsa_keys):
def test_token_introspect_succ(self, monkeypatch, rsa_keys):
"""Verify .token_introspect_offline() success path"""
client_id = "us"
token, expected_payload = gen_rsa_token(client_id, rsa_keys["private_key"])
Expand All @@ -514,10 +436,10 @@ def test_token_introspect_offline_succ(self, monkeypatch, rsa_keys):
)

# This is the target test case.
response = oidc_client.token_introspect_offline(token)
response = oidc_client.token_introspect(token)
assert response == expected_payload

def test_token_introspect_offline_exp(self, monkeypatch, rsa_keys):
def test_token_introspect_exp(self, monkeypatch, rsa_keys):
"""Verify .token_introspect_offline() failure via expiration"""
client_id = "us"
token, expected_payload = gen_rsa_token(
Expand All @@ -531,12 +453,12 @@ def test_token_introspect_offline_exp(self, monkeypatch, rsa_keys):
oidc_client = OpenIDClient.construct_oidc_client(config)

with pytest.raises(OpenIDTokenInvalid) as exc:
oidc_client.token_introspect_offline(token)
oidc_client.token_introspect(token)
assert (
str(exc.value.__cause__) == "Signature has expired"
), f"{exc.value.__cause__}"

def test_token_introspect_offline_aud(self, monkeypatch, rsa_keys):
def test_token_introspect_aud(self, monkeypatch, rsa_keys):
"""Verify .token_introspect_offline() failure via audience error"""
client_id = "us"
token, expected_payload = gen_rsa_token(client_id, rsa_keys["private_key"])
Expand All @@ -547,10 +469,10 @@ def test_token_introspect_offline_aud(self, monkeypatch, rsa_keys):
oidc_client = OpenIDClient.construct_oidc_client(config)

with pytest.raises(OpenIDTokenInvalid) as exc:
oidc_client.token_introspect_offline(token)
oidc_client.token_introspect(token)
assert str(exc.value.__cause__) == "Invalid audience", f"{exc.value.__cause__}"

def test_token_introspect_offline_sig(self, monkeypatch, rsa_keys):
def test_token_introspect_sig(self, monkeypatch, rsa_keys):
"""Verify .token_introspect_offline() failure via signature error"""
client_id = "us"
token, expected_payload = gen_rsa_token(client_id, rsa_keys["private_key"])
Expand All @@ -564,27 +486,11 @@ def test_token_introspect_offline_sig(self, monkeypatch, rsa_keys):

with pytest.raises(OpenIDTokenInvalid) as exc:
# Make the signature invalid.
oidc_client.token_introspect_offline(token + "1")
oidc_client.token_introspect(token + "1")
assert (
str(exc.value.__cause__) == "Signature verification failed"
), f"{exc.value.__cause__}"

def test_get_userinfo(self, monkeypatch):
"""Verify .get_userinfo() properly invokes Connection.get()"""
# Mock the Connection object and generate an OpenIDClient object.
client_id = "us"
config = mock_connection(monkeypatch, client_id)
oidc_client = OpenIDClient.construct_oidc_client(config)

# Ensure .get_userinfo() invokes Connection.get() with the correct
# parameters.
token = "the-token"
json_d = oidc_client.get_userinfo(token)
assert json_d == {
"kwargs": {"headers": {"Authorization": "Bearer the-token"}},
"path": "https://example.com/userinfo",
}


@dataclass
class MockRequest:
Expand Down Expand Up @@ -776,61 +682,7 @@ def tio_exc(token: str) -> JSON:
app = Flask("test-verify-auth-oidc-offline-invalid")
app.logger = make_logger
with app.app_context():
monkeypatch.setattr(oidc_client, "token_introspect_offline", tio_exc)
user = Auth.verify_auth(token)

assert user is None

def test_verify_auth_oidc_online(self, monkeypatch, rsa_keys, make_logger):
"""Verify OIDC token online verification works via Auth.verify_auth()"""
# Use the client ID to direct the MockConnection class to return a
# token payload.
client_id = "us-token-payload"
token, expected_payload = gen_rsa_token(client_id, rsa_keys["private_key"])

# Mock the Connection object and generate an OpenIDClient object,
# installing it as Auth module's OIDC client.
config = mock_connection(
monkeypatch, client_id, public_key=rsa_keys["public_key"]
)
oidc_client = OpenIDClient.construct_oidc_client(config)
monkeypatch.setattr(Auth, "oidc_client", oidc_client)

def tio_exc(token: str) -> JSON:
raise Exception("Token introspection offline failed for some reason")

app = Flask("test-verify-auth-oidc-online")
app.logger = make_logger
with app.app_context():
monkeypatch.setattr(oidc_client, "token_introspect_offline", tio_exc)
user = Auth.verify_auth(token)

assert user.id == "67890"

def test_verify_auth_oidc_online_fail(self, monkeypatch, rsa_keys, make_logger):
"""Verify OIDC token online verification via Auth.verify_auth() fails
returning None
"""
# Use the client ID to direct the online token introspection to raise
# an OpenIDClientError.
client_id = "us-raise-exc"
token, expected_payload = gen_rsa_token(client_id, rsa_keys["private_key"])

# Mock the Connection object and generate an OpenIDClient object,
# installing it as Auth module's OIDC client.
config = mock_connection(
monkeypatch, client_id, public_key=rsa_keys["public_key"]
)
oidc_client = OpenIDClient.construct_oidc_client(config)
monkeypatch.setattr(Auth, "oidc_client", oidc_client)

def tio_exc(token: str) -> JSON:
raise Exception("Token introspection offline failed for some reason")

app = Flask("test-verify-auth-oidc-online-fail")
app.logger = make_logger
with app.app_context():
monkeypatch.setattr(oidc_client, "token_introspect_offline", tio_exc)
monkeypatch.setattr(oidc_client, "token_introspect", tio_exc)
user = Auth.verify_auth(token)

assert user is None

0 comments on commit 6a39237

Please sign in to comment.