From 112bc602fef099e386401e192304fae3dd69dd4a Mon Sep 17 00:00:00 2001 From: npalaska Date: Wed, 8 Feb 2023 17:50:36 -0500 Subject: [PATCH 1/2] 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 --- lib/pbench/server/auth/__init__.py | 94 +--------- lib/pbench/server/auth/auth.py | 18 +- lib/pbench/test/unit/server/auth/test_auth.py | 166 +----------------- 3 files changed, 12 insertions(+), 266 deletions(-) diff --git a/lib/pbench/server/auth/__init__.py b/lib/pbench/server/auth/__init__.py index bb434e085f..aae89fd003 100644 --- a/lib/pbench/server/auth/__init__.py +++ b/lib/pbench/server/auth/__init__.py @@ -362,20 +362,6 @@ 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}, " @@ -383,57 +369,7 @@ def __repr__(self): 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": , - "email_verified": , - "expires_in": , - "access_type": "offline", - "exp": , - "azp": , - "scope": , # "openid email profile" - "email": , - "sub": - } - """ - 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. @@ -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": , - "sub": , - "picture": , - "locale": , - "email_verified": , - "given_name": , - "email": , - "hd": , - "name": - } - """ - headers = {"Authorization": f"Bearer {token}"} if token else None - return self._connection.get(self._userinfo_endpoint, headers=headers).json() diff --git a/lib/pbench/server/auth/auth.py b/lib/pbench/server/auth/auth.py index 1f8bdacfc2..7dc9d190db 100644 --- a/lib/pbench/server/auth/auth.py +++ b/lib/pbench/server/auth/auth.py @@ -11,7 +11,6 @@ from pbench.server.auth import ( InternalUser, OpenIDClient, - OpenIDClientError, OpenIDTokenInvalid, ) from pbench.server.database.models.active_tokens import ActiveTokens @@ -194,7 +193,7 @@ 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: @@ -202,20 +201,7 @@ def verify_auth_oidc(auth_token: str) -> Optional[InternalUser]: "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 diff --git a/lib/pbench/test/unit/server/auth/test_auth.py b/lib/pbench/test/unit/server/auth/test_auth.py index 93d9ce26f9..241217e8cc 100644 --- a/lib/pbench/test/unit/server/auth/test_auth.py +++ b/lib/pbench/test/unit/server/auth/test_auth.py @@ -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 @@ -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"]) @@ -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( @@ -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"]) @@ -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"]) @@ -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: @@ -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 From b9a99924205b4fcce4d3bb08e1c01f3c591e4c8b Mon Sep 17 00:00:00 2001 From: npalaska Date: Wed, 8 Feb 2023 19:31:18 -0500 Subject: [PATCH 2/2] fix auth.py imports --- lib/pbench/server/auth/auth.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/pbench/server/auth/auth.py b/lib/pbench/server/auth/auth.py index 7dc9d190db..90af96854a 100644 --- a/lib/pbench/server/auth/auth.py +++ b/lib/pbench/server/auth/auth.py @@ -8,11 +8,7 @@ import jwt from pbench.server import PbenchServerConfig -from pbench.server.auth import ( - InternalUser, - OpenIDClient, - OpenIDTokenInvalid, -) +from pbench.server.auth import InternalUser, OpenIDClient, OpenIDTokenInvalid from pbench.server.database.models.active_tokens import ActiveTokens from pbench.server.database.models.users import User