diff --git a/.gitignore b/.gitignore index ad24a4a4..566eb6e7 100644 --- a/.gitignore +++ b/.gitignore @@ -85,7 +85,7 @@ celerybeat-schedule # Environments .env -.venv +.venv* env/ venv/ ENV/ diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md index b4612a7b..26da0945 100644 --- a/CODE_OF_CONDUCT.md +++ b/CODE_OF_CONDUCT.md @@ -6,11 +6,11 @@ Equality is a core value at Salesforce. We believe a diverse and inclusive community fosters innovation and creativity, and are committed to building a culture where everyone feels included. -Salesforce open-source projects are committed to providing a friendly, safe, and -welcoming environment for all, regardless of gender identity and expression, -sexual orientation, disability, physical appearance, body size, ethnicity, nationality, -race, age, religion, level of experience, education, socioeconomic status, or -other similar personal characteristics. +Salesforce open-source projects are committed to providing a friendly, safe, +and welcoming environment for all, regardless of gender identity and +expression, sexual orientation, disability, physical appearance, body size, +ethnicity, nationality, race, age, religion, level of experience, education, +socioeconomic status, or other similar personal characteristics. The goal of this code of conduct is to specify a baseline standard of behavior so that people with different social values and communication styles can work @@ -19,16 +19,16 @@ It also establishes a mechanism for reporting issues and resolving conflicts. All questions and reports of abusive, harassing, or otherwise unacceptable behavior in a Salesforce open-source project may be reported by contacting the Salesforce -Open Source Conduct Committee at ossconduct@salesforce.com. +Open Source Conduct Committee at . ## Our Pledge In the interest of fostering an open and welcoming environment, we as contributors and maintainers pledge to making participation in our project and -our community a harassment-free experience for everyone, regardless of gender -identity and expression, sexual orientation, disability, physical appearance, -body size, ethnicity, nationality, race, age, religion, level of experience, education, -socioeconomic status, or other similar personal characteristics. +our community a harassment-free experience for everyone, regardless of gender +identity and expression, sexual orientation, disability, physical appearance, +body size, ethnicity, nationality, race, age, religion, level of experience, +education, socioeconomic status, or other similar personal characteristics. ## Our Standards @@ -77,23 +77,24 @@ further defined and clarified by project maintainers. ## Enforcement Instances of abusive, harassing, or otherwise unacceptable behavior may be -reported by contacting the Salesforce Open Source Conduct Committee -at ossconduct@salesforce.com. All complaints will be reviewed and investigated -and will result in a response that is deemed necessary and appropriate to the -circumstances. The committee is obligated to maintain confidentiality with -regard to the reporter of an incident. Further details of specific enforcement +reported by contacting the Salesforce Open Source Conduct Committee +at . All complaints will be reviewed and investigated +and will result in a response that is deemed necessary and appropriate to the +circumstances. The committee is obligated to maintain confidentiality with +regard to the reporter of an incident. Further details of specific enforcement policies may be posted separately. Project maintainers who do not follow or enforce the Code of Conduct in good faith may face temporary or permanent repercussions as determined by other -members of the project's leadership and the Salesforce Open Source Conduct +members of the project's leadership and the Salesforce Open Source Conduct Committee. ## Attribution -This Code of Conduct is adapted from the [Contributor Covenant][contributor-covenant-home], -version 1.4, available at https://www.contributor-covenant.org/version/1/4/code-of-conduct.html. -It includes adaptions and additions from [Go Community Code of Conduct][golang-coc], +This Code of Conduct is adapted from the +[Contributor Covenant][contributor-covenant-home], version 1.4, available at +[https://www.contributor-covenant.org/version/1/4/code-of-conduct.html](https://www.contributor-covenant.org/version/1/4/code-of-conduct.html). +It includes adaptions and additions from [Go Community Code of Conduct][golang-coc], [CNCF Code of Conduct][cncf-coc], and [Microsoft Open Source Code of Conduct][microsoft-coc]. This Code of Conduct is licensed under the [Creative Commons Attribution 3.0 License][cc-by-3-us]. @@ -102,4 +103,4 @@ This Code of Conduct is licensed under the [Creative Commons Attribution 3.0 Lic [golang-coc]: https://golang.org/conduct [cncf-coc]: https://github.com/cncf/foundation/blob/master/code-of-conduct.md [microsoft-coc]: https://opensource.microsoft.com/codeofconduct/ -[cc-by-3-us]: https://creativecommons.org/licenses/by/3.0/us/ \ No newline at end of file +[cc-by-3-us]: https://creativecommons.org/licenses/by/3.0/us/ diff --git a/SECURITY.md b/SECURITY.md index b69c021e..ca9d1034 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -1,4 +1,4 @@ -## Security +# Security Please report any security issue to [https://www.sfdc.co/SubmitVuln](https://www.sfdc.co/SubmitVuln) as soon as it is discovered. This library limits its runtime dependencies in diff --git a/docs/server-config.md b/docs/server-config.md index c57219e5..9e3cc5bd 100755 --- a/docs/server-config.md +++ b/docs/server-config.md @@ -209,9 +209,10 @@ connection scenario. ## Authentication -TabPy supports basic access authentication (see +TabPy supports two authentication methods, which can be enabled independently +or at the same time: basic access authentication (see [https://en.wikipedia.org/wiki/Basic_access_authentication](https://en.wikipedia.org/wiki/Basic_access_authentication) -for more details). +for more details) and OAuth/JWT Bearer token authentication. ### Enabling Authentication @@ -269,6 +270,63 @@ will be generated and displayed in the command line. To delete an account open password file in any text editor and delete the line with the user name. +### OAuth / JWT Bearer Token Authentication + +TabPy can validate OAuth 2.0 JWT Bearer tokens on incoming requests. This +allows per-user credentials issued by an external identity provider (IdP) +to be used instead of, or alongside, a single shared password-file +credential. + +To enable it, specify the following parameters in the TabPy configuration +file: + +```sh +TABPY_OAUTH_ENABLED = true +TABPY_OAUTH_ISSUER = https://idp.example.com/ +TABPY_OAUTH_JWKS_URI = https://idp.example.com/.well-known/jwks.json +TABPY_OAUTH_AUDIENCE = tabpy +``` + +`TABPY_OAUTH_ISSUER`, `TABPY_OAUTH_JWKS_URI`, and `TABPY_OAUTH_AUDIENCE` are +all required when `TABPY_OAUTH_ENABLED` is `true`; TabPy will fail to start +if any are missing. `TABPY_OAUTH_ISSUER` and `TABPY_OAUTH_JWKS_URI` must use +`https://` -- the JWKS response is the trust anchor for verifying JWT +signatures, so fetching it over plain HTTP would let anyone on the network +path substitute their own keys. + +- `TABPY_OAUTH_ISSUER` is the expected `iss` claim on incoming JWTs. +- `TABPY_OAUTH_JWKS_URI` is the IdP's JWKS endpoint, used to fetch and cache + the signing keys used to verify JWT signatures. +- `TABPY_OAUTH_AUDIENCE` is the expected `aud` claim on incoming JWTs. + +Two additional parameters are optional: + +```sh +TABPY_OAUTH_REQUIRED_SCOPES = tabpy:query,tabpy:evaluate +TABPY_OAUTH_LOG_USER = true +``` + +- `TABPY_OAUTH_REQUIRED_SCOPES` is a comma-separated list of scopes that + must all be present in the JWT's `scope` claim for the request to be + accepted. If unset, no scope check is performed. +- `TABPY_OAUTH_LOG_USER` (default `true`) sets the JWT's `sub` claim as the + authenticated user for logging purposes. + +To authenticate a request, send the JWT as a Bearer token: + +```sh +curl -H "Authorization: Bearer " http://localhost:9004/info +``` + +When both basic access authentication and OAuth are enabled, TabPy picks the +method based on the scheme of the `Authorization` header sent by the client +(`Basic` or `Bearer`), so both can be used against the same server. + +JWKS lookups are cached, but because TabPy serves requests on a single +thread, a slow or unresponsive IdP during a cache-cold fetch (startup, or a +key rotation) will briefly stall all concurrent requests, not just the one +that triggered the fetch. + ### Endpoint Security All endpoints require authentication if it is enabled for the server. diff --git a/setup.py b/setup.py index f4f9b987..e87fb96e 100755 --- a/setup.py +++ b/setup.py @@ -72,6 +72,7 @@ def read(fname): "configparser", "coverage", "coveralls", + "cryptography", "docopt", "future", "genson", @@ -81,6 +82,7 @@ def read(fname): "nltk", "numpy", "pandas", + "pyjwt", "pyopenssl", "pytest", "pytest-cov", diff --git a/tabpy/tabpy_server/app/app.py b/tabpy/tabpy_server/app/app.py index 421464b0..10e8acfc 100644 --- a/tabpy/tabpy_server/app/app.py +++ b/tabpy/tabpy_server/app/app.py @@ -357,8 +357,15 @@ def _parse_config(self, config_file): 100, None), (SettingsParameters.GzipEnabled, ConfigParameters.TABPY_GZIP_ENABLE, True, parser.getboolean), - (SettingsParameters.ArrowEnabled, ConfigParameters.TABPY_ARROW_ENABLE, False, parser.getboolean), + (SettingsParameters.ArrowEnabled, ConfigParameters.TABPY_ARROW_ENABLE, False, parser.getboolean), (SettingsParameters.ArrowFlightPort, ConfigParameters.TABPY_ARROWFLIGHT_PORT, 13622, parser.getint), + (SettingsParameters.OAuthEnabled, ConfigParameters.TABPY_OAUTH_ENABLED, False, parser.getboolean), + (SettingsParameters.OAuthIssuer, ConfigParameters.TABPY_OAUTH_ISSUER, None, None), + (SettingsParameters.OAuthJwksUri, ConfigParameters.TABPY_OAUTH_JWKS_URI, None, None), + (SettingsParameters.OAuthAudience, ConfigParameters.TABPY_OAUTH_AUDIENCE, None, None), + (SettingsParameters.OAuthRequiredScopes, ConfigParameters.TABPY_OAUTH_REQUIRED_SCOPES, + None, None), + (SettingsParameters.OAuthLogUser, ConfigParameters.TABPY_OAUTH_LOG_USER, True, parser.getboolean), ] for setting, parameter, default_val, parse_function in settings_parameters: @@ -403,6 +410,10 @@ def _parse_config(self, config_file): if state_config.has_option("Service Info", "Subdirectory"): self.subdirectory = "/" + state_config.get("Service Info", "Subdirectory") + # Validate OAuth config if enabled + if self.settings[SettingsParameters.OAuthEnabled]: + self._validate_oauth_settings() + # If passwords file specified load credentials if ConfigParameters.TABPY_PWD_FILE in self.settings: if not self._parse_pwd_file(): @@ -412,7 +423,7 @@ def _parse_config(self, config_file): ) logger.critical(msg) raise RuntimeError(msg) - else: + elif not self.settings[SettingsParameters.OAuthEnabled]: self._handle_configuration_without_authentication() features = self._get_features() @@ -513,14 +524,58 @@ def _handle_configuration_without_authentication(self): "https://github.com/tableau/TabPy/blob/master/docs/server-config.md#authentication.") exit() + def _validate_oauth_settings(self): + required = [ + (SettingsParameters.OAuthIssuer, ConfigParameters.TABPY_OAUTH_ISSUER), + (SettingsParameters.OAuthJwksUri, ConfigParameters.TABPY_OAUTH_JWKS_URI), + (SettingsParameters.OAuthAudience, ConfigParameters.TABPY_OAUTH_AUDIENCE), + ] + missing = [ + config_key for setting, config_key in required + if not self.settings.get(setting) + ] + if missing: + msg = ( + f"{ConfigParameters.TABPY_OAUTH_ENABLED} is true but missing required " + f"setting(s): {', '.join(missing)}" + ) + logger.critical(msg) + raise RuntimeError(msg) + + # JWKS/issuer are the trust anchor for JWT verification, so both must + # be fetched over https to prevent an on-path attacker from substituting + # their own keys/issuer. + insecure = [ + config_key for setting, config_key in ( + (SettingsParameters.OAuthIssuer, ConfigParameters.TABPY_OAUTH_ISSUER), + (SettingsParameters.OAuthJwksUri, ConfigParameters.TABPY_OAUTH_JWKS_URI), + ) + if not self.settings[setting].lower().startswith("https://") + ] + if insecure: + msg = ( + f"{', '.join(insecure)} must use https: JWKS/issuer are the trust " + "anchor for JWT verification and must not be fetched over plain HTTP" + ) + logger.critical(msg) + raise RuntimeError(msg) + def _get_features(self): features = {} # Check for auth - if ConfigParameters.TABPY_PWD_FILE in self.settings: + if ( + ConfigParameters.TABPY_PWD_FILE in self.settings + or self.settings[SettingsParameters.OAuthEnabled] + ): + methods = {} + if ConfigParameters.TABPY_PWD_FILE in self.settings: + methods["basic-auth"] = {} + if self.settings[SettingsParameters.OAuthEnabled]: + methods["oauth-jwt"] = {} features["authentication"] = { "required": True, - "methods": {"basic-auth": {}}, + "methods": methods, } features["evaluate_enabled"] = self.settings[SettingsParameters.EvaluateEnabled] diff --git a/tabpy/tabpy_server/app/app_parameters.py b/tabpy/tabpy_server/app/app_parameters.py index 3dab6c46..2c31c62b 100644 --- a/tabpy/tabpy_server/app/app_parameters.py +++ b/tabpy/tabpy_server/app/app_parameters.py @@ -22,6 +22,14 @@ class ConfigParameters: TABPY_ARROW_ENABLE = "TABPY_ARROW_ENABLE" TABPY_ARROWFLIGHT_PORT = "TABPY_ARROWFLIGHT_PORT" + # OAuth/JWT specific settings + TABPY_OAUTH_ENABLED = "TABPY_OAUTH_ENABLED" + TABPY_OAUTH_ISSUER = "TABPY_OAUTH_ISSUER" + TABPY_OAUTH_JWKS_URI = "TABPY_OAUTH_JWKS_URI" + TABPY_OAUTH_AUDIENCE = "TABPY_OAUTH_AUDIENCE" + TABPY_OAUTH_REQUIRED_SCOPES = "TABPY_OAUTH_REQUIRED_SCOPES" + TABPY_OAUTH_LOG_USER = "TABPY_OAUTH_LOG_USER" + class SettingsParameters: """ @@ -47,3 +55,11 @@ class SettingsParameters: # Arrow specific settings ArrowEnabled = "arrow_enabled" ArrowFlightPort = "arrowflight_port" + + # OAuth/JWT specific settings + OAuthEnabled = "oauth_enabled" + OAuthIssuer = "oauth_issuer" + OAuthJwksUri = "oauth_jwks_uri" + OAuthAudience = "oauth_audience" + OAuthRequiredScopes = "oauth_required_scopes" + OAuthLogUser = "oauth_log_user" diff --git a/tabpy/tabpy_server/common/default.conf b/tabpy/tabpy_server/common/default.conf index 17ee5e3b..e01c732e 100644 --- a/tabpy/tabpy_server/common/default.conf +++ b/tabpy/tabpy_server/common/default.conf @@ -38,6 +38,27 @@ # Enable Gzip compression for requests and responses. # TABPY_GZIP_ENABLE = true +# Enable OAuth/JWT Bearer-token authentication. When enabled, +# TABPY_OAUTH_ISSUER, TABPY_OAUTH_JWKS_URI, and TABPY_OAUTH_AUDIENCE are +# all required. +# TABPY_OAUTH_ENABLED = true + +# Expected `iss` claim on incoming JWTs. +# TABPY_OAUTH_ISSUER = https://idp.example.com/ + +# JWKS endpoint used to fetch and cache the IdP's signing keys. +# TABPY_OAUTH_JWKS_URI = https://idp.example.com/.well-known/jwks.json + +# Expected `aud` claim on incoming JWTs. +# TABPY_OAUTH_AUDIENCE = tabpy + +# Comma-separated list of scopes that must all be present in the JWT's +# `scope` claim. Leave unset to skip scope enforcement. +# TABPY_OAUTH_REQUIRED_SCOPES = tabpy:query,tabpy:evaluate + +# Log the JWT subject as the authenticated user for OAuth requests. +# TABPY_OAUTH_LOG_USER = true + [loggers] keys=root diff --git a/tabpy/tabpy_server/handlers/base_handler.py b/tabpy/tabpy_server/handlers/base_handler.py index 3b5ae0cc..12731325 100644 --- a/tabpy/tabpy_server/handlers/base_handler.py +++ b/tabpy/tabpy_server/handlers/base_handler.py @@ -5,6 +5,7 @@ import logging import tornado.web from tabpy.tabpy_server.app.app_parameters import SettingsParameters +from tabpy.tabpy_server.handlers.jwt_auth import JwtValidationError, validate_jwt from tabpy.tabpy_server.handlers.util import hash_password from tabpy.tabpy_server.handlers.util import AuthErrorStates import uuid @@ -58,10 +59,21 @@ def enable_context_logging(self, enable: bool): """ self.log_request_context = enable + def log_context_now(self): + """ + Logs the request-context line, if not already logged for this + request. Call only after auth has resolved, since the line + includes self.tabpy_username. + """ + self._log_context_info() + def _log_context_info(self): if not self.log_request_context: return + if self.request_context_logged: + return + context = f"Call ID: {self.call_id}" if self.remote_ip is not None: @@ -91,6 +103,9 @@ def log(self, level: int, msg: str): call ID added to any log entry is specified by if context logging is enabled (see CallContext.enable_context_logging for more details). + Does not trigger the request-context line itself; see + log_context_now. + Parameters ---------- level: int @@ -107,9 +122,6 @@ def log(self, level: int, msg: str): """ extended_msg = msg if self.log_request_context: - if not self.request_context_logged: - self._log_context_info() - extended_msg += f", <>" logging.getLogger(__name__).log(level, extended_msg) @@ -126,6 +138,8 @@ def initialize(self, app): self.credentials = app.credentials self.username = None self.password = None + self.jwt_token = None + self.auth_method = None self.eval_timeout = self.settings[SettingsParameters.EvaluateTimeout] self.max_request_size = app.max_request_size @@ -135,6 +149,7 @@ def initialize(self, app): ) self.logger.log(logging.DEBUG, "Checking if need to handle authentication") self.auth_error = self.handle_authentication("v1") + self.logger.log_context_now() def error_out(self, code, log_message, info=None): self.set_status(code) @@ -224,8 +239,21 @@ def _get_auth_method(self, api_version) -> (bool, str): ) methods = auth_feature["methods"] - if "basic-auth" in auth_feature["methods"]: + + # If more than one method is configured, pick by the scheme the + # client sent so Basic Auth and OAuth can coexist on the same + # server. Case-insensitive per RFC 7235. + auth_header = self.request.headers.get("Authorization", "") + scheme = auth_header.split(" ")[0].lower() if auth_header else "" + if scheme == "bearer" and "oauth-jwt" in methods: + return True, "oauth-jwt" + if scheme == "basic" and "basic-auth" in methods: return True, "basic-auth" + + if "basic-auth" in methods: + return True, "basic-auth" + if "oauth-jwt" in methods: + return True, "oauth-jwt" # Add new methods here... # No known methods were found @@ -236,34 +264,60 @@ def _get_auth_method(self, api_version) -> (bool, str): ) return False, "" - def _get_basic_auth_credentials(self) -> bool: + def _get_auth_header_value(self, expected_scheme: str) -> str: """ - Find credentials for basic access authentication method. Credentials if - found stored in Credentials.username and Credentials.password. + Finds and returns the credential portion of the Authorization + header if it was sent with the given scheme (case-insensitive per + RFC 7235). + + Parameters + ---------- + expected_scheme: str + Auth scheme to match, e.g. "Basic" or "Bearer". Returns ------- - bool - True if valid credentials were found. - False otherwise. + str + The credential portion of the header if found and the scheme + matches. None otherwise. """ self.logger.log( logging.DEBUG, "Checking request headers for authentication data" ) if "Authorization" not in self.request.headers: self.logger.log(logging.INFO, "Authorization header not found") - return False + return None auth_header = self.request.headers["Authorization"] auth_header_list = auth_header.split(" ") - if len(auth_header_list) != 2 or auth_header_list[0] != "Basic": + if ( + len(auth_header_list) != 2 + or auth_header_list[0].lower() != expected_scheme.lower() + ): self.logger.log( logging.ERROR, f'Unknown authentication method "{auth_header}"' ) + return None + + return auth_header_list[1] + + def _get_basic_auth_credentials(self) -> bool: + """ + Find credentials for basic access authentication method. Credentials if + found stored in Credentials.username and Credentials.password. + + Returns + ------- + bool + True if valid credentials were found. + False otherwise. + """ + encoded_credentials = self._get_auth_header_value("Basic") + if encoded_credentials is None: return False try: - cred = base64.b64decode(auth_header_list[1]).decode("utf-8") + cred = base64.b64decode(encoded_credentials).decode("utf-8") except (binascii.Error, UnicodeDecodeError) as ex: self.logger.log(logging.CRITICAL, f"Cannot decode credentials: {str(ex)}") return False @@ -278,6 +332,25 @@ def _get_basic_auth_credentials(self) -> bool: self.password = login_pwd[1] return True + def _get_bearer_token(self) -> bool: + """ + Extracts the Bearer token for the oauth-jwt authentication method. + The raw token is stored in self.jwt_token for validation by + _validate_credentials. + + Returns + ------- + bool + True if a Bearer token was found. + False otherwise. + """ + token = self._get_auth_header_value("Bearer") + if token is None: + return False + + self.jwt_token = token + return True + def _get_credentials(self, method) -> bool: """ Find credentials for specified authentication method. Credentials if @@ -296,6 +369,8 @@ def _get_credentials(self, method) -> bool: """ if method == "basic-auth": return self._get_basic_auth_credentials() + if method == "oauth-jwt": + return self._get_bearer_token() # Add new methods here... # No known methods were found @@ -334,6 +409,38 @@ def _validate_basic_auth_credentials(self) -> bool: return True + def _validate_jwt_credentials(self) -> bool: + """ + Validates the Bearer token found by _get_bearer_token against the + configured IdP: signature (via JWKS), issuer, audience, expiry, + nbf, and optionally required scopes. + + Returns + ------- + bool + True if the JWT is valid. + False otherwise. + """ + try: + claims = validate_jwt( + self.jwt_token, + issuer=self.settings[SettingsParameters.OAuthIssuer], + jwks_uri=self.settings[SettingsParameters.OAuthJwksUri], + audience=self.settings[SettingsParameters.OAuthAudience], + required_scopes=self.settings.get(SettingsParameters.OAuthRequiredScopes), + ) + except JwtValidationError as ex: + self.logger.log(logging.ERROR, str(ex)) + return False + + if self.settings.get(SettingsParameters.OAuthLogUser, True): + subject = claims.get("sub") + if subject: + self.username = subject + self.logger.set_tabpy_username(self.username) + + return True + def _validate_credentials(self, method) -> bool: """ Validates credentials according to specified methods if they @@ -352,6 +459,8 @@ def _validate_credentials(self, method) -> bool: """ if method == "basic-auth": return self._validate_basic_auth_credentials() + if method == "oauth-jwt": + return self._validate_jwt_credentials() # Add new methods here... # No known methods were found @@ -381,6 +490,7 @@ def handle_authentication(self, api_version): """ self.logger.log(logging.DEBUG, "Handling authentication") found, method = self._get_auth_method(api_version) + self.auth_method = method if not found: return AuthErrorStates.NotAuthorized @@ -420,15 +530,28 @@ def should_fail_with_auth_error(self): """ return self.auth_error + def _www_authenticate_scheme(self) -> str: + """ + Returns the auth-scheme to challenge with in a WWW-Authenticate + header, matching whichever method was actually attempted so + OAuth clients aren't told to retry with Basic Auth (RFC 7235). + """ + if self.auth_method == "oauth-jwt": + return "Bearer" + return "Basic" + def fail_with_auth_error(self): """ Prepares server 401 response and server 406 response depending on the value of the self.auth_error flag """ + scheme = self._www_authenticate_scheme() if self.auth_error == AuthErrorStates.NotAuthorized: self.logger.log(logging.ERROR, "Failing with 401 for unauthorized request") self.set_status(401) - self.set_header("WWW-Authenticate", f'Basic realm="{self.tabpy_state.name}"') + self.set_header( + "WWW-Authenticate", f'{scheme} realm="{self.tabpy_state.name}"' + ) self.error_out( 401, info="Unauthorized request.", @@ -437,7 +560,9 @@ def fail_with_auth_error(self): else: self.logger.log(logging.ERROR, "Failing with 406 for Not Acceptable") self.set_status(406) - self.set_header("WWW-Authenticate", f'Basic realm="{self.tabpy_state.name}"') + self.set_header( + "WWW-Authenticate", f'{scheme} realm="{self.tabpy_state.name}"' + ) self.error_out( 406, info="Not Acceptable", diff --git a/tabpy/tabpy_server/handlers/jwt_auth.py b/tabpy/tabpy_server/handlers/jwt_auth.py new file mode 100644 index 00000000..3b7cc9ee --- /dev/null +++ b/tabpy/tabpy_server/handlers/jwt_auth.py @@ -0,0 +1,168 @@ +import logging +import time + +import jwt +from jwt import PyJWKClient + +logger = logging.getLogger(__name__) + +# PyJWKClient fetches JWKS synchronously (via requests), and that call runs +# directly on TabPy's single Tornado IO-loop thread -- a slow/unresponsive +# IdP therefore stalls every concurrent request on the server, not just the +# one that triggered the fetch, for up to this many seconds. Caching (see +# _jwks_clients) and the refresh rate limit below bound how often this can +# happen, but a cold start or a legitimate key rotation still pays this +# cost. Moving the fetch to a thread pool (e.g. via IOLoop.run_in_executor) +# would remove the stall entirely, at the cost of making the auth path +# asynchronous; not done here. +JWKS_FETCH_TIMEOUT_SECONDS = 10 + +# An unauthenticated caller can force a fresh JWKS fetch just by sending a +# made-up `kid` (read from the token header pre-signature-check), and that +# fetch blocks the single IO-loop thread. This bounds how often a *failed* +# forced refresh (kid still not found) can refetch again, regardless of +# how many distinct bogus `kid`s are tried. Only failures set the cooldown +# -- a successful refresh (e.g. a genuine key rotation) must not be +# penalized, or a bogus `kid` sent right before a real rotation could lock +# out legitimate holders of the new key for the rest of the window. +JWKS_MIN_REFRESH_INTERVAL_SECONDS = 30 + +# One PyJWKClient per JWKS URI, reused so its JWK Set cache actually avoids +# per-request fetches. Process-global and not lock-protected: safe only +# because TabPy runs a single app instance per process on a single IO-loop +# thread. Would need a lock (or per-app scoping) if that ever changes. +_jwks_clients = {} + +# jwks_uri -> monotonic timestamp of the last failed forced refresh, used +# to rate-limit that path. Same single-threaded caveat as _jwks_clients. +_jwks_last_failed_refresh = {} + + +def _get_jwks_client(jwks_uri: str) -> PyJWKClient: + client = _jwks_clients.get(jwks_uri) + if client is None: + client = PyJWKClient( + jwks_uri, cache_jwk_set=True, timeout=JWKS_FETCH_TIMEOUT_SECONDS + ) + _jwks_clients[jwks_uri] = client + return client + + +def _get_signing_key(jwks_client: PyJWKClient, jwks_uri: str, token: str): + """ + Resolves the signing key for `token`'s `kid`, same as + PyJWKClient.get_signing_key_from_jwt(), except the cache-miss retry is + rate-limited per jwks_uri (see JWKS_MIN_REFRESH_INTERVAL_SECONDS) + instead of firing unconditionally. + """ + header = jwt.get_unverified_header(token) + kid = header.get("kid") + + signing_keys = jwks_client.get_signing_keys() + signing_key = PyJWKClient.match_kid(signing_keys, kid) + if signing_key is not None: + return signing_key + + now = time.monotonic() + last_failure = _jwks_last_failed_refresh.get(jwks_uri, 0) + if now - last_failure < JWKS_MIN_REFRESH_INTERVAL_SECONDS: + raise jwt.exceptions.PyJWKClientError( + f'Unable to find a signing key that matches: "{kid}"' + ) + + signing_keys = jwks_client.get_signing_keys(refresh=True) + signing_key = PyJWKClient.match_kid(signing_keys, kid) + if signing_key is None: + _jwks_last_failed_refresh[jwks_uri] = now + raise jwt.exceptions.PyJWKClientError( + f'Unable to find a signing key that matches: "{kid}"' + ) + return signing_key + + +class JwtValidationError(Exception): + """Raised when a JWT Bearer token fails validation.""" + + +def validate_jwt( + token: str, + issuer: str, + jwks_uri: str, + audience: str, + required_scopes: str = None, +) -> dict: + """ + Validates a JWT Bearer token's signature, issuer, audience, expiry, + and not-before claims, plus optional required scopes. + + Parameters + ---------- + token : str + The raw JWT (without the "Bearer " prefix). + issuer : str + Expected `iss` claim. + jwks_uri : str + JWKS endpoint used to resolve the token's signing key. + audience : str + Expected `aud` claim. + required_scopes : str, optional + Comma-separated scopes that must all be present in the token's + `scope` claim. Skipped if None or empty. + + Returns + ------- + dict + The decoded JWT claims. + + Raises + ------ + JwtValidationError + If the token is missing, malformed, expired, or fails any of + the signature/issuer/audience/scope checks. + """ + if not token: + raise JwtValidationError("Missing JWT") + + try: + jwks_client = _get_jwks_client(jwks_uri) + signing_key = _get_signing_key(jwks_client, jwks_uri, token) + except (jwt.exceptions.PyJWKClientError, jwt.exceptions.InvalidTokenError) as ex: + logger.log(logging.ERROR, f"Unable to resolve JWT signing key: {str(ex)}") + raise JwtValidationError("Unable to resolve JWT signing key") from ex + except Exception as ex: + # Must still surface as a 401, not a 500 (e.g. malformed JWKS response). + logger.log(logging.ERROR, f"Unexpected error resolving JWT signing key: {str(ex)}") + raise JwtValidationError("Unable to resolve JWT signing key") from ex + + try: + claims = jwt.decode( + token, + signing_key.key, + algorithms=[signing_key.algorithm_name], + issuer=issuer, + audience=audience, + options={"require": ["exp", "iat"]}, + ) + except jwt.exceptions.InvalidTokenError as ex: + logger.log(logging.ERROR, f"JWT validation failed: {str(ex)}") + raise JwtValidationError(f"JWT validation failed: {str(ex)}") from ex + + if required_scopes: + try: + _check_scopes(claims, required_scopes) + except JwtValidationError: + raise + except Exception as ex: + # Must still fail closed as a 401 (e.g. a `scope` claim that + # isn't a space-separated string), not an uncaught 500. + logger.log(logging.ERROR, f"Unable to evaluate JWT scopes: {str(ex)}") + raise JwtValidationError("Unable to evaluate JWT scopes") from ex + + return claims + + +def _check_scopes(claims: dict, required_scopes: str) -> None: + granted = set(claims.get("scope", "").split()) + missing = [s for s in (s.strip() for s in required_scopes.split(",")) if s and s not in granted] + if missing: + raise JwtValidationError(f"JWT missing required scope(s): {', '.join(missing)}") diff --git a/tests/unit/server_tests/jwt_test_helpers.py b/tests/unit/server_tests/jwt_test_helpers.py new file mode 100644 index 00000000..b3420159 --- /dev/null +++ b/tests/unit/server_tests/jwt_test_helpers.py @@ -0,0 +1,40 @@ +""" +Shared JWT test fixtures for test_jwt_auth.py and test_oauth_handler.py, +so the two suites can't silently drift apart on how tokens/signing keys +are faked. +""" +import datetime +from unittest.mock import patch + +import jwt + +ISSUER = "https://idp.example.com/" +AUDIENCE = "tabpy" +JWKS_URI = "https://idp.example.com/.well-known/jwks.json" + + +def make_token(private_key, claims_override=None, headers=None): + now = datetime.datetime.now(datetime.timezone.utc) + claims = { + "iss": ISSUER, + "aud": AUDIENCE, + "sub": "user1", + "iat": now, + "exp": now + datetime.timedelta(minutes=5), + } + if claims_override: + claims.update(claims_override) + return jwt.encode(claims, private_key, algorithm="RS256", headers=headers) + + +def patched_jwks_client(private_key, kid=None): + """Patches PyJWKClient.get_signing_keys to return private_key's public + key instead of making a network call.""" + signing_key = type("SigningKey", (), {})() + signing_key.key = private_key.public_key() + signing_key.algorithm_name = "RS256" + signing_key.key_id = kid + return patch( + "tabpy.tabpy_server.handlers.jwt_auth.PyJWKClient.get_signing_keys", + return_value=[signing_key], + ) diff --git a/tests/unit/server_tests/test_config.py b/tests/unit/server_tests/test_config.py index d36e7e99..af87bec8 100644 --- a/tests/unit/server_tests/test_config.py +++ b/tests/unit/server_tests/test_config.py @@ -397,3 +397,140 @@ def test_future_cert(self): def test_valid_cert(self): path = os.path.join(self.resources_path, "valid.crt") validate_cert(path) + + +class TestOAuthConfigValidation(unittest.TestCase): + def __init__(self, *args, **kwargs): + super(TestOAuthConfigValidation, self).__init__(*args, **kwargs) + self.fp = None + + def setUp(self): + self.fp = NamedTemporaryFile(mode="w+t", delete=False) + + def tearDown(self): + os.remove(self.fp.name) + self.fp = None + + def test_oauth_disabled_by_default(self): + self.fp.write("[TabPy]\n") + self.fp.close() + + app = TabPyApp(self.fp.name) + self.assertFalse(app.settings["oauth_enabled"]) + self.assertNotIn("authentication", app._get_features()) + + def test_oauth_enabled_with_all_required_settings_succeeds(self): + self.fp.write( + "[TabPy]\n" + "TABPY_OAUTH_ENABLED = true\n" + "TABPY_OAUTH_ISSUER = https://idp.example.com/\n" + "TABPY_OAUTH_JWKS_URI = https://idp.example.com/.well-known/jwks.json\n" + "TABPY_OAUTH_AUDIENCE = tabpy\n" + ) + self.fp.close() + + app = TabPyApp(self.fp.name) + self.assertTrue(app.settings["oauth_enabled"]) + methods = app._get_features()["authentication"]["methods"] + self.assertIn("oauth-jwt", methods) + + def test_oauth_enabled_missing_issuer_raises(self): + self.fp.write( + "[TabPy]\n" + "TABPY_OAUTH_ENABLED = true\n" + "TABPY_OAUTH_JWKS_URI = https://idp.example.com/.well-known/jwks.json\n" + "TABPY_OAUTH_AUDIENCE = tabpy\n" + ) + self.fp.close() + + with self.assertRaises(RuntimeError) as err: + TabPyApp(self.fp.name) + self.assertIn("TABPY_OAUTH_ISSUER", err.exception.args[0]) + + def test_oauth_enabled_missing_jwks_uri_raises(self): + self.fp.write( + "[TabPy]\n" + "TABPY_OAUTH_ENABLED = true\n" + "TABPY_OAUTH_ISSUER = https://idp.example.com/\n" + "TABPY_OAUTH_AUDIENCE = tabpy\n" + ) + self.fp.close() + + with self.assertRaises(RuntimeError) as err: + TabPyApp(self.fp.name) + self.assertIn("TABPY_OAUTH_JWKS_URI", err.exception.args[0]) + + def test_oauth_enabled_missing_audience_raises(self): + self.fp.write( + "[TabPy]\n" + "TABPY_OAUTH_ENABLED = true\n" + "TABPY_OAUTH_ISSUER = https://idp.example.com/\n" + "TABPY_OAUTH_JWKS_URI = https://idp.example.com/.well-known/jwks.json\n" + ) + self.fp.close() + + with self.assertRaises(RuntimeError) as err: + TabPyApp(self.fp.name) + self.assertIn("TABPY_OAUTH_AUDIENCE", err.exception.args[0]) + + def test_oauth_enabled_with_http_jwks_uri_raises(self): + self.fp.write( + "[TabPy]\n" + "TABPY_OAUTH_ENABLED = true\n" + "TABPY_OAUTH_ISSUER = https://idp.example.com/\n" + "TABPY_OAUTH_JWKS_URI = http://idp.example.com/.well-known/jwks.json\n" + "TABPY_OAUTH_AUDIENCE = tabpy\n" + ) + self.fp.close() + + with self.assertRaises(RuntimeError) as err: + TabPyApp(self.fp.name) + self.assertIn("TABPY_OAUTH_JWKS_URI", err.exception.args[0]) + + def test_oauth_enabled_with_http_issuer_raises(self): + self.fp.write( + "[TabPy]\n" + "TABPY_OAUTH_ENABLED = true\n" + "TABPY_OAUTH_ISSUER = http://idp.example.com/\n" + "TABPY_OAUTH_JWKS_URI = https://idp.example.com/.well-known/jwks.json\n" + "TABPY_OAUTH_AUDIENCE = tabpy\n" + ) + self.fp.close() + + with self.assertRaises(RuntimeError) as err: + TabPyApp(self.fp.name) + self.assertIn("TABPY_OAUTH_ISSUER", err.exception.args[0]) + + @patch('builtins.input') + def test_oauth_only_does_not_trigger_no_auth_warning(self, mock_input): + self.fp.write( + "[TabPy]\n" + "TABPY_OAUTH_ENABLED = true\n" + "TABPY_OAUTH_ISSUER = https://idp.example.com/\n" + "TABPY_OAUTH_JWKS_URI = https://idp.example.com/.well-known/jwks.json\n" + "TABPY_OAUTH_AUDIENCE = tabpy\n" + ) + self.fp.close() + + TabPyApp(self.fp.name) + mock_input.assert_not_called() + + def test_basic_auth_and_oauth_both_enabled_advertises_both_methods(self): + pwd_file = os.path.join( + os.path.dirname(os.path.dirname(os.path.dirname(__file__))), + "integration", "resources", "pwdfile.txt", + ) + self.fp.write( + "[TabPy]\n" + f"TABPY_PWD_FILE = {pwd_file}\n" + "TABPY_OAUTH_ENABLED = true\n" + "TABPY_OAUTH_ISSUER = https://idp.example.com/\n" + "TABPY_OAUTH_JWKS_URI = https://idp.example.com/.well-known/jwks.json\n" + "TABPY_OAUTH_AUDIENCE = tabpy\n" + ) + self.fp.close() + + app = TabPyApp(self.fp.name) + methods = app._get_features()["authentication"]["methods"] + self.assertIn("basic-auth", methods) + self.assertIn("oauth-jwt", methods) diff --git a/tests/unit/server_tests/test_jwt_auth.py b/tests/unit/server_tests/test_jwt_auth.py new file mode 100644 index 00000000..6d90a7d2 --- /dev/null +++ b/tests/unit/server_tests/test_jwt_auth.py @@ -0,0 +1,363 @@ +import base64 +import datetime +import hmac +import hashlib +import json +import unittest +from unittest.mock import patch + +import jwt +from cryptography.hazmat.primitives import serialization +from cryptography.hazmat.primitives.asymmetric import rsa + +from tabpy.tabpy_server.handlers.jwt_auth import JwtValidationError, validate_jwt +from tests.unit.server_tests.jwt_test_helpers import ( + AUDIENCE, + ISSUER, + JWKS_URI, + make_token, + patched_jwks_client, +) + + +def b64url(data: bytes) -> str: + return base64.urlsafe_b64encode(data).rstrip(b"=").decode() + + +class TestJwtAuth(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.private_key = rsa.generate_private_key(public_exponent=65537, key_size=2048) + + def _make_token(self, claims_override=None, headers=None): + return make_token( + self.private_key, claims_override=claims_override, headers=headers + ) + + def _patched_jwks_client(self, kid=None): + return patched_jwks_client(self.private_key, kid=kid) + + def test_valid_jwt_is_accepted(self): + token = self._make_token() + with self._patched_jwks_client(): + claims = validate_jwt(token, issuer=ISSUER, jwks_uri=JWKS_URI, audience=AUDIENCE) + self.assertEqual(claims["sub"], "user1") + + def test_missing_token_raises(self): + with self.assertRaises(JwtValidationError): + validate_jwt("", issuer=ISSUER, jwks_uri=JWKS_URI, audience=AUDIENCE) + + def test_expired_jwt_is_rejected(self): + past = datetime.datetime.now(datetime.timezone.utc) - datetime.timedelta(hours=1) + token = self._make_token({"iat": past - datetime.timedelta(minutes=5), "exp": past}) + with self._patched_jwks_client(): + with self.assertRaises(JwtValidationError): + validate_jwt(token, issuer=ISSUER, jwks_uri=JWKS_URI, audience=AUDIENCE) + + def test_wrong_issuer_is_rejected(self): + token = self._make_token({"iss": "https://wrong-idp.example.com/"}) + with self._patched_jwks_client(): + with self.assertRaises(JwtValidationError): + validate_jwt(token, issuer=ISSUER, jwks_uri=JWKS_URI, audience=AUDIENCE) + + def test_wrong_audience_is_rejected(self): + token = self._make_token({"aud": "wrong-audience"}) + with self._patched_jwks_client(): + with self.assertRaises(JwtValidationError): + validate_jwt(token, issuer=ISSUER, jwks_uri=JWKS_URI, audience=AUDIENCE) + + def test_missing_required_scope_is_rejected(self): + token = self._make_token({"scope": "tabpy:query"}) + with self._patched_jwks_client(): + with self.assertRaises(JwtValidationError): + validate_jwt( + token, + issuer=ISSUER, + jwks_uri=JWKS_URI, + audience=AUDIENCE, + required_scopes="tabpy:query,tabpy:evaluate", + ) + + def test_non_string_scope_claim_is_rejected_not_raised(self): + """ + Some IdPs emit `scope`/permissions as a JSON list rather than a + space-separated string. That must still fail closed as a + JwtValidationError (401), not escape as an uncaught exception (500). + """ + token = self._make_token({"scope": ["tabpy:query", "tabpy:evaluate"]}) + with self._patched_jwks_client(): + with self.assertRaises(JwtValidationError): + validate_jwt( + token, + issuer=ISSUER, + jwks_uri=JWKS_URI, + audience=AUDIENCE, + required_scopes="tabpy:query", + ) + + def test_present_required_scopes_are_accepted(self): + token = self._make_token({"scope": "tabpy:query tabpy:evaluate"}) + with self._patched_jwks_client(): + claims = validate_jwt( + token, + issuer=ISSUER, + jwks_uri=JWKS_URI, + audience=AUDIENCE, + required_scopes="tabpy:query,tabpy:evaluate", + ) + self.assertEqual(claims["sub"], "user1") + + def test_malformed_token_is_rejected(self): + with self.assertRaises(JwtValidationError): + validate_jwt( + "not-a-real-jwt", issuer=ISSUER, jwks_uri=JWKS_URI, audience=AUDIENCE + ) + + def test_wrong_signing_key_is_rejected(self): + token = self._make_token() + other_key = rsa.generate_private_key(public_exponent=65537, key_size=2048) + signing_key = type("SigningKey", (), {})() + signing_key.key = other_key.public_key() + signing_key.algorithm_name = "RS256" + signing_key.key_id = None + with patch( + "tabpy.tabpy_server.handlers.jwt_auth.PyJWKClient.get_signing_keys", + return_value=[signing_key], + ): + with self.assertRaises(JwtValidationError): + validate_jwt(token, issuer=ISSUER, jwks_uri=JWKS_URI, audience=AUDIENCE) + + def test_not_yet_valid_token_is_rejected(self): + future = datetime.datetime.now(datetime.timezone.utc) + datetime.timedelta(minutes=10) + token = self._make_token({"nbf": future}) + with self._patched_jwks_client(): + with self.assertRaises(JwtValidationError): + validate_jwt(token, issuer=ISSUER, jwks_uri=JWKS_URI, audience=AUDIENCE) + + def test_unresolvable_signing_key_is_rejected(self): + token = self._make_token() + with patch( + "tabpy.tabpy_server.handlers.jwt_auth.PyJWKClient.get_signing_keys", + side_effect=jwt.exceptions.PyJWKClientError("Unable to find a signing key"), + ): + with self.assertRaises(JwtValidationError): + validate_jwt(token, issuer=ISSUER, jwks_uri=JWKS_URI, audience=AUDIENCE) + + def test_unexpected_jwks_error_is_rejected_not_raised(self): + """ + A malformed JWKS response or other network-layer failure that PyJWT + doesn't wrap as PyJWKClientError/InvalidTokenError must still come + out as JwtValidationError (401), not an uncaught exception (500). + """ + token = self._make_token() + with patch( + "tabpy.tabpy_server.handlers.jwt_auth.PyJWKClient.get_signing_keys", + side_effect=ValueError("Expecting value: line 1 column 1 (char 0)"), + ): + with self.assertRaises(JwtValidationError): + validate_jwt(token, issuer=ISSUER, jwks_uri=JWKS_URI, audience=AUDIENCE) + + def test_jwks_client_is_created_with_bounded_timeout(self): + """ + TabPy serves requests on a single IO-loop thread, so the JWKS HTTP + client must not be allowed to hang indefinitely on a slow/unreachable + IdP -- that would stall the entire server, not just one request. + """ + import tabpy.tabpy_server.handlers.jwt_auth as jwt_auth_module + + jwt_auth_module._jwks_clients.clear() + client = jwt_auth_module._get_jwks_client(JWKS_URI) + self.assertEqual(client.timeout, jwt_auth_module.JWKS_FETCH_TIMEOUT_SECONDS) + self.assertLess(jwt_auth_module.JWKS_FETCH_TIMEOUT_SECONDS, 30) + + def test_algorithm_is_pinned_to_jwks_key_not_token_header(self): + """ + Guards against algorithm-confusion attacks: even if a forged token + claims alg=none in its header, validate_jwt only ever decodes using + the algorithm associated with the resolved JWKS signing key. + """ + now = datetime.datetime.now(datetime.timezone.utc) + header = b64url(json.dumps({"alg": "none", "typ": "JWT"}).encode()) + payload = b64url( + json.dumps( + { + "iss": ISSUER, + "aud": AUDIENCE, + "sub": "attacker", + "iat": int(now.timestamp()), + "exp": int((now + datetime.timedelta(minutes=5)).timestamp()), + } + ).encode() + ) + forged_token = f"{header}.{payload}." + + with self._patched_jwks_client(): + with self.assertRaises(JwtValidationError): + validate_jwt(forged_token, issuer=ISSUER, jwks_uri=JWKS_URI, audience=AUDIENCE) + + def test_matching_kid_is_selected_from_multi_key_jwks(self): + """ + Every other test in this suite mocks a single signing key with + key_id=None matching a header-less token, which trivially matches + via `None == None` -- unrepresentative of a real JWKS, where every + entry has a non-empty kid (PyJWT filters out keys without one). + This exercises the actual kid-comparison logic against a JWKS with + multiple keys, matching the token's real `kid` header. + """ + token = self._make_token(headers={"kid": "key-2"}) + + other_key = rsa.generate_private_key(public_exponent=65537, key_size=2048) + wrong_key = type("SigningKey", (), {})() + wrong_key.key = other_key.public_key() + wrong_key.algorithm_name = "RS256" + wrong_key.key_id = "key-1" + + right_key = type("SigningKey", (), {})() + right_key.key = self.private_key.public_key() + right_key.algorithm_name = "RS256" + right_key.key_id = "key-2" + + with patch( + "tabpy.tabpy_server.handlers.jwt_auth.PyJWKClient.get_signing_keys", + return_value=[wrong_key, right_key], + ): + claims = validate_jwt(token, issuer=ISSUER, jwks_uri=JWKS_URI, audience=AUDIENCE) + self.assertEqual(claims["sub"], "user1") + + def test_missing_kid_does_not_force_unbounded_jwks_refresh(self): + """ + An unauthenticated caller can send a token with a made-up `kid` + (read from the unverified header, before any signature check). + Repeated unknown `kid`s must not each force a fresh JWKS fetch -- + that fetch is a blocking network call on TabPy's single IO-loop + thread, so unbounded refetching is a pre-auth DoS vector. Only one + forced refresh per jwks_uri is allowed within + JWKS_MIN_REFRESH_INTERVAL_SECONDS. + """ + import tabpy.tabpy_server.handlers.jwt_auth as jwt_auth_module + + jwt_auth_module._jwks_clients.clear() + jwt_auth_module._jwks_last_failed_refresh.clear() + + token1 = self._make_token(headers={"kid": "unknown-kid-1"}) + token2 = self._make_token(headers={"kid": "unknown-kid-2"}) + + signing_key = type("SigningKey", (), {})() + signing_key.key = self.private_key.public_key() + signing_key.algorithm_name = "RS256" + signing_key.key_id = "the-real-kid" + + with patch( + "tabpy.tabpy_server.handlers.jwt_auth.PyJWKClient.get_signing_keys", + return_value=[signing_key], + ) as mock_get_signing_keys: + with self.assertRaises(JwtValidationError): + validate_jwt(token1, issuer=ISSUER, jwks_uri=JWKS_URI, audience=AUDIENCE) + with self.assertRaises(JwtValidationError): + validate_jwt(token2, issuer=ISSUER, jwks_uri=JWKS_URI, audience=AUDIENCE) + + # get_signing_keys(refresh=True) is only allowed once per token + # (the first cache-miss lookup), plus at most one forced refresh + # shared across both tokens -- not one forced refresh per token. + refresh_calls = [ + call for call in mock_get_signing_keys.call_args_list + if call.kwargs.get("refresh") or (call.args and call.args[0]) + ] + self.assertLessEqual(len(refresh_calls), 1) + + def test_successful_refresh_is_not_rate_limited_after_a_bogus_kid(self): + """ + Only a *failed* forced refresh should arm the cooldown. If a bogus + `kid` is sent right before an IdP genuinely rotates its signing + keys, a legitimate token using the newly-rotated key must still be + accepted -- otherwise an attacker could "prime" a denial window + against real users just by sending one throwaway bad-kid request. + """ + import tabpy.tabpy_server.handlers.jwt_auth as jwt_auth_module + + jwt_auth_module._jwks_clients.clear() + jwt_auth_module._jwks_last_failed_refresh.clear() + + bogus_token = self._make_token(headers={"kid": "unknown-kid"}) + rotated_signing_key = type("SigningKey", (), {})() + rotated_signing_key.key = self.private_key.public_key() + rotated_signing_key.algorithm_name = "RS256" + rotated_signing_key.key_id = "rotated-kid" + + with patch( + "tabpy.tabpy_server.handlers.jwt_auth.PyJWKClient.get_signing_keys", + return_value=[], + ): + with self.assertRaises(JwtValidationError): + validate_jwt(bogus_token, issuer=ISSUER, jwks_uri=JWKS_URI, audience=AUDIENCE) + + rotated_token = self._make_token(headers={"kid": "rotated-kid"}) + with patch( + "tabpy.tabpy_server.handlers.jwt_auth.PyJWKClient.get_signing_keys", + return_value=[rotated_signing_key], + ): + claims = validate_jwt( + rotated_token, issuer=ISSUER, jwks_uri=JWKS_URI, audience=AUDIENCE + ) + self.assertEqual(claims["sub"], "user1") + + def test_hs256_substitution_using_public_key_is_rejected(self): + """ + Guards against the classic RS256->HS256 confusion attack: an + attacker who knows the RSA public key forges an HS256 token using + that public key as the HMAC secret. This must be rejected because + the algorithm is pinned to the JWKS-resolved key's own + algorithm_name (RS256), never trusted from the token header. + """ + public_pem = self.private_key.public_key().public_bytes( + serialization.Encoding.PEM, + serialization.PublicFormat.SubjectPublicKeyInfo, + ) + now = datetime.datetime.now(datetime.timezone.utc) + header = b64url(json.dumps({"alg": "HS256", "typ": "JWT"}).encode()) + payload = b64url( + json.dumps( + { + "iss": ISSUER, + "aud": AUDIENCE, + "sub": "attacker", + "iat": int(now.timestamp()), + "exp": int((now + datetime.timedelta(minutes=5)).timestamp()), + } + ).encode() + ) + signing_input = f"{header}.{payload}".encode() + signature = hmac.new(public_pem, signing_input, hashlib.sha256).digest() + forged_token = f"{header}.{payload}.{b64url(signature)}" + + with self._patched_jwks_client(): + with self.assertRaises(JwtValidationError): + validate_jwt(forged_token, issuer=ISSUER, jwks_uri=JWKS_URI, audience=AUDIENCE) + + def test_jwks_client_is_reused_for_same_uri(self): + """ + _get_jwks_client must return the same PyJWKClient instance for + repeat calls with the same jwks_uri, so PyJWKClient's own JWK Set + cache (avoiding per-request JWKS fetches) is actually effective. + """ + import tabpy.tabpy_server.handlers.jwt_auth as jwt_auth_module + + jwt_auth_module._jwks_clients.clear() + first = jwt_auth_module._get_jwks_client(JWKS_URI) + second = jwt_auth_module._get_jwks_client(JWKS_URI) + self.assertIs(first, second) + + def test_validation_failure_does_not_log_raw_token(self): + token = self._make_token({"iss": "https://wrong-idp.example.com/"}) + with self._patched_jwks_client(): + with self.assertLogs( + "tabpy.tabpy_server.handlers.jwt_auth", level="ERROR" + ) as log_ctx: + with self.assertRaises(JwtValidationError): + validate_jwt(token, issuer=ISSUER, jwks_uri=JWKS_URI, audience=AUDIENCE) + logged_text = " ".join(log_ctx.output) + self.assertNotIn(token, logged_text) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/server_tests/test_oauth_handler.py b/tests/unit/server_tests/test_oauth_handler.py new file mode 100644 index 00000000..78788d22 --- /dev/null +++ b/tests/unit/server_tests/test_oauth_handler.py @@ -0,0 +1,210 @@ +import base64 +import datetime +import json +import os +import tempfile +import unittest + +from cryptography.hazmat.primitives.asymmetric import rsa +from tornado.testing import AsyncHTTPTestCase + +from tabpy.tabpy_server.app.app import TabPyApp +from tests.unit.server_tests.jwt_test_helpers import ( + AUDIENCE, + ISSUER, + JWKS_URI, + make_token, + patched_jwks_client, +) + + +class BaseTestOAuthHandler(AsyncHTTPTestCase): + def get_app(self): + self.app = TabPyApp(self.config_file.name) + return self.app._create_tornado_web_app() + + @classmethod + def tearDownClass(cls): + os.remove(cls.state_file.name) + os.remove(cls.config_file.name) + os.rmdir(cls.state_dir) + + @classmethod + def setUpClass(cls): + cls.private_key = rsa.generate_private_key(public_exponent=65537, key_size=2048) + + cls.state_dir = tempfile.mkdtemp(prefix=cls.prefix) + with open(os.path.join(cls.state_dir, "state.ini"), "w+") as cls.state_file: + cls.state_file.write( + "[Service Info]\n" + "Name = TabPy Serve\n" + "Description = \n" + "Creation Time = 0\n" + "Access-Control-Allow-Origin = \n" + "Access-Control-Allow-Headers = \n" + "Access-Control-Allow-Methods = \n" + "\n" + "[Query Objects Service Versions]\n" + "\n" + "[Query Objects Docstrings]\n" + "\n" + "[Meta]\n" + "Revision Number = 1\n" + ) + cls.state_file.close() + + cls.config_file = tempfile.NamedTemporaryFile( + prefix=cls.prefix, suffix=".conf", delete=False, mode="w" + ) + cls.config_file.write("[TabPy]\n") + for line in cls.tabpy_config: + cls.config_file.write(line) + cls.config_file.close() + + def _make_token(self, claims_override=None): + return make_token(self.private_key, claims_override=claims_override) + + def _patched_jwks_client(self): + return patched_jwks_client(self.private_key) + + +class TestOAuthOnlyHandler(BaseTestOAuthHandler): + @classmethod + def setUpClass(cls): + cls.prefix = "__TestOAuthOnlyHandler_" + cls.tabpy_config = [ + "TABPY_OAUTH_ENABLED = true\n", + f"TABPY_OAUTH_ISSUER = {ISSUER}\n", + f"TABPY_OAUTH_JWKS_URI = {JWKS_URI}\n", + f"TABPY_OAUTH_AUDIENCE = {AUDIENCE}\n", + ] + super().setUpClass() + + def test_missing_token_returns_401(self): + response = self.fetch("/info") + self.assertEqual(response.code, 401) + + def test_valid_jwt_is_accepted(self): + token = self._make_token() + headers = {"Authorization": f"Bearer {token}"} + with self._patched_jwks_client(): + response = self.fetch("/info", headers=headers) + self.assertEqual(response.code, 200) + + def test_expired_jwt_returns_401(self): + past = datetime.datetime.now(datetime.timezone.utc) - datetime.timedelta(hours=1) + token = self._make_token({"iat": past - datetime.timedelta(minutes=5), "exp": past}) + headers = {"Authorization": f"Bearer {token}"} + with self._patched_jwks_client(): + response = self.fetch("/info", headers=headers) + self.assertEqual(response.code, 401) + + def test_info_advertises_oauth_jwt_method(self): + token = self._make_token() + headers = {"Authorization": f"Bearer {token}"} + with self._patched_jwks_client(): + response = self.fetch("/info", headers=headers) + body = json.loads(response.body) + features = body["versions"]["v1"]["features"] + self.assertIn("oauth-jwt", features["authentication"]["methods"]) + + +class TestOAuthAndBasicAuthCoexist(BaseTestOAuthHandler): + @classmethod + def setUpClass(cls): + cls.prefix = "__TestOAuthAndBasicAuthCoexist_" + cls.tabpy_config = [ + "TABPY_PWD_FILE = ./tests/integration/resources/pwdfile.txt\n", + "TABPY_OAUTH_ENABLED = true\n", + f"TABPY_OAUTH_ISSUER = {ISSUER}\n", + f"TABPY_OAUTH_JWKS_URI = {JWKS_URI}\n", + f"TABPY_OAUTH_AUDIENCE = {AUDIENCE}\n", + ] + super().setUpClass() + + def test_basic_auth_still_works_when_oauth_also_enabled(self): + headers = { + "Authorization": "Basic " + + base64.b64encode(b"user1:P@ssw0rd").decode("utf-8"), + } + response = self.fetch("/info", headers=headers) + self.assertEqual(response.code, 200) + + def test_bearer_token_also_works_when_basic_auth_also_enabled(self): + token = self._make_token() + headers = {"Authorization": f"Bearer {token}"} + with self._patched_jwks_client(): + response = self.fetch("/info", headers=headers) + self.assertEqual(response.code, 200) + + def test_invalid_basic_auth_credentials_still_rejected(self): + headers = { + "Authorization": "Basic " + + base64.b64encode(b"user1:wrong_password").decode("utf-8"), + } + response = self.fetch("/info", headers=headers) + self.assertEqual(response.code, 401) + + +class TestOAuthLogUserEnabled(BaseTestOAuthHandler): + @classmethod + def setUpClass(cls): + cls.prefix = "__TestOAuthLogUserEnabled_" + cls.tabpy_config = [ + "TABPY_OAUTH_ENABLED = true\n", + f"TABPY_OAUTH_ISSUER = {ISSUER}\n", + f"TABPY_OAUTH_JWKS_URI = {JWKS_URI}\n", + f"TABPY_OAUTH_AUDIENCE = {AUDIENCE}\n", + "TABPY_OAUTH_LOG_USER = true\n", + "TABPY_LOG_DETAILS = true\n", + ] + super().setUpClass() + + def test_subject_is_logged_in_request_context_line(self): + """ + Regression test: the request-context line is built once per + request the first time something is logged, which (before a fix) + happened before auth had resolved -- so the subject was always + missing from it despite TABPY_OAUTH_LOG_USER being enabled. + """ + token = self._make_token({"sub": "user1"}) + headers = {"Authorization": f"Bearer {token}"} + with self._patched_jwks_client(): + with self.assertLogs( + "tabpy.tabpy_server.handlers.base_handler", level="INFO" + ) as log_ctx: + response = self.fetch("/info", headers=headers) + self.assertEqual(response.code, 200) + logged_text = " ".join(log_ctx.output) + self.assertIn("TabPy user: user1", logged_text) + + +class TestOAuthLogUserDisabled(BaseTestOAuthHandler): + @classmethod + def setUpClass(cls): + cls.prefix = "__TestOAuthLogUserDisabled_" + cls.tabpy_config = [ + "TABPY_OAUTH_ENABLED = true\n", + f"TABPY_OAUTH_ISSUER = {ISSUER}\n", + f"TABPY_OAUTH_JWKS_URI = {JWKS_URI}\n", + f"TABPY_OAUTH_AUDIENCE = {AUDIENCE}\n", + "TABPY_OAUTH_LOG_USER = false\n", + "TABPY_LOG_DETAILS = true\n", + ] + super().setUpClass() + + def test_subject_is_not_logged_when_oauth_log_user_disabled(self): + token = self._make_token({"sub": "should-not-be-logged"}) + headers = {"Authorization": f"Bearer {token}"} + with self._patched_jwks_client(): + with self.assertLogs( + "tabpy.tabpy_server.handlers.base_handler", level="INFO" + ) as log_ctx: + response = self.fetch("/info", headers=headers) + self.assertEqual(response.code, 200) + logged_text = " ".join(log_ctx.output) + self.assertNotIn("should-not-be-logged", logged_text) + + +if __name__ == "__main__": + unittest.main()