From 06fa2d1482ddcd37d8d082eaa64d43ef5d99cb92 Mon Sep 17 00:00:00 2001 From: Sushma Sreedharan Date: Tue, 20 Oct 2026 10:09:19 -0700 Subject: [PATCH 01/10] @W-24145098 - Add Workload Identity Federation (WIF) admin REST API Expose the Tableau Server WIF admin surface (REST API 3.30+ / Tableau Server release 2026.3) on the TSC server object as `server.auth_federation`. Site admins can now manage the whole WIF lifecycle from the client, complementing the per-connection WIF attributes delivered in PR #1887. New endpoint methods on `server.auth_federation`: Admin config (`/sites/{siteLuid}/auth-federation/admin`) - get() -> AuthFederationItem - enable(config) -> AuthFederationItem (idempotent PUT; requires enabled=True) - update(config) -> AuthFederationItem (merge-patch; only `token_lifetime_seconds` is mutable - `issuer_url`, `active_key_id`, and `connectors` are read-only and never sent on the wire) - delete() (deletes config + keys + connectors) Connectors (`.../admin/connectors`) - create_connector(connector) -> AuthFederationConnectorItem - list_connectors() -> list[AuthFederationConnectorItem] - get_connector(id) (accepts luid string or item) - delete_connector(id) (accepts luid string or item; idempotent) Keys (`.../admin/keys`) - create_key() -> AuthFederationKeyItem (inactive by default; private key is never returned) - activate_key(id) (atomic; deactivates whatever key was active) - deactivate_key(id) - delete_key(id) (400 if the key is still active) Three new public models exported from both `tableauserverclient.models` and the TSC top-level: `AuthFederationItem`, `AuthFederationConnectorItem`, `AuthFederationKeyItem`. All methods version-gated by @api(version="3.30"). Includes 22 unit tests under `test/test_auth_federation.py` covering the happy path for every method, request-body shape (via defusedxml parsing of `request_history[0].text`) for PUT/PATCH/POST verbs, the read-only-field hygiene of `update()`, the enabled=True guard on `enable()`, the required-field guards on `create_connector`, the list-connectors empty case, both the "pass a str" and "pass an item" forms of every id-taking method, and the EndpointUnavailableError raised when running against a server older than 3.30. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 27 ++ tableauserverclient/__init__.py | 6 + tableauserverclient/models/__init__.py | 8 + .../models/auth_federation_item.py | 121 +++++++ .../server/endpoint/__init__.py | 2 + .../endpoint/auth_federation_endpoint.py | 263 +++++++++++++++ tableauserverclient/server/request_factory.py | 31 ++ tableauserverclient/server/server.py | 2 + .../auth_federation_connector_create.xml | 10 + test/assets/auth_federation_connector_get.xml | 10 + .../auth_federation_connectors_list.xml | 16 + .../auth_federation_connectors_list_empty.xml | 7 + test/assets/auth_federation_enable.xml | 11 + test/assets/auth_federation_get.xml | 22 ++ test/assets/auth_federation_key_create.xml | 13 + test/assets/auth_federation_update.xml | 11 + test/test_auth_federation.py | 299 ++++++++++++++++++ 17 files changed, 859 insertions(+) create mode 100644 tableauserverclient/models/auth_federation_item.py create mode 100644 tableauserverclient/server/endpoint/auth_federation_endpoint.py create mode 100644 test/assets/auth_federation_connector_create.xml create mode 100644 test/assets/auth_federation_connector_get.xml create mode 100644 test/assets/auth_federation_connectors_list.xml create mode 100644 test/assets/auth_federation_connectors_list_empty.xml create mode 100644 test/assets/auth_federation_enable.xml create mode 100644 test/assets/auth_federation_get.xml create mode 100644 test/assets/auth_federation_key_create.xml create mode 100644 test/assets/auth_federation_update.xml create mode 100644 test/test_auth_federation.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f13b9605..17d15ebdf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,33 @@ ## Unreleased +* Added Workload Identity Federation (WIF) admin REST API support + (`server.auth_federation`). Site admins can now manage the WIF config and + its signing keys and connectors from TSC. New endpoints: + - `server.auth_federation.get()` -> `AuthFederationItem` — read the + site's WIF admin config (enabled flag, issuer URL, active key id, + JWT lifetime, connectors) + - `server.auth_federation.enable(config)` -> `AuthFederationItem` — + idempotent PUT; `AuthFederationItem.enabled` must be `True` + - `server.auth_federation.update(config)` -> `AuthFederationItem` — + merge-patch; only `token_lifetime_seconds` is mutable + - `server.auth_federation.delete()` — deletes the WIF config along + with every signing key and connector on the site + - `server.auth_federation.create_connector(connector)` -> `AuthFederationConnectorItem` + - `server.auth_federation.list_connectors()` -> `list[AuthFederationConnectorItem]` + - `server.auth_federation.get_connector(id)` and + `server.auth_federation.delete_connector(id)` accept either a connector + LUID string or an `AuthFederationConnectorItem` with `luid` set + - `server.auth_federation.create_key()` -> `AuthFederationKeyItem` — + generates a new (inactive) signing key; private key is never returned + - `server.auth_federation.activate_key(id)` / + `server.auth_federation.deactivate_key(id)` / + `server.auth_federation.delete_key(id)` accept either a `kid` string + or an `AuthFederationKeyItem` with `kid` set; `activate_key` is atomic + and deactivates any previously active key + Three new exports: `AuthFederationItem`, `AuthFederationConnectorItem`, + `AuthFederationKeyItem`. Requires Tableau Server REST API 3.30+ + (Tableau Server release 26.3) and site-admin credentials. * Added Workload Identity Federation (WIF) support to the connection update endpoints. `ConnectionItem` has two new string properties, `workgroup_auth_mode` and `auth_federation_service_account`, that are diff --git a/tableauserverclient/__init__.py b/tableauserverclient/__init__.py index cb197d47c..819cfcfa6 100644 --- a/tableauserverclient/__init__.py +++ b/tableauserverclient/__init__.py @@ -1,6 +1,9 @@ from tableauserverclient.bin._version import get_versions from tableauserverclient.namespace import NEW_NAMESPACE as DEFAULT_NAMESPACE from tableauserverclient.models import ( + AuthFederationConnectorItem, + AuthFederationItem, + AuthFederationKeyItem, BackgroundJobItem, CollectionItem, ColumnItem, @@ -80,6 +83,9 @@ ) __all__ = [ + "AuthFederationConnectorItem", + "AuthFederationItem", + "AuthFederationKeyItem", "BackgroundJobItem", "CollectionItem", "ColumnItem", diff --git a/tableauserverclient/models/__init__.py b/tableauserverclient/models/__init__.py index aa28e0dbf..5eb79f8e8 100644 --- a/tableauserverclient/models/__init__.py +++ b/tableauserverclient/models/__init__.py @@ -1,3 +1,8 @@ +from tableauserverclient.models.auth_federation_item import ( + AuthFederationConnectorItem, + AuthFederationItem, + AuthFederationKeyItem, +) from tableauserverclient.models.collection_item import CollectionItem from tableauserverclient.models.column_item import ColumnItem from tableauserverclient.models.connection_credentials import ConnectionCredentials @@ -55,6 +60,9 @@ from tableauserverclient.models.extract_item import ExtractItem __all__ = [ + "AuthFederationConnectorItem", + "AuthFederationItem", + "AuthFederationKeyItem", "CollectionItem", "ColumnItem", "ConnectionCredentials", diff --git a/tableauserverclient/models/auth_federation_item.py b/tableauserverclient/models/auth_federation_item.py new file mode 100644 index 000000000..b369385c1 --- /dev/null +++ b/tableauserverclient/models/auth_federation_item.py @@ -0,0 +1,121 @@ +from defusedxml.ElementTree import fromstring + + +class AuthFederationConnectorItem: + def __init__(self) -> None: + self.luid: str | None = None + self.connector_class: str | None = None + self.audience: str | None = None + + def __str__(self) -> str: + return ( + f"{self.__class__.__qualname__}(luid={self.luid}, " + f"connector_class={self.connector_class}, audience={self.audience})" + ) + + def __repr__(self) -> str: + return f"<{str(self)}>" + + @classmethod + def from_element(cls, elem) -> "AuthFederationConnectorItem": + connector = cls() + connector.luid = elem.get("luid") + connector.connector_class = elem.get("connectorClass") + connector.audience = elem.get("audience") + return connector + + @classmethod + def from_response(cls, raw_xml: bytes, ns) -> "AuthFederationConnectorItem": + root = fromstring(raw_xml) + elem = root.find("t:authFederationConnector", namespaces=ns) + if elem is None: + raise ValueError("No authFederationConnector element found in the XML.") + return cls.from_element(elem) + + @classmethod + def list_from_response(cls, raw_xml: bytes, ns) -> list["AuthFederationConnectorItem"]: + root = fromstring(raw_xml) + list_elem = root.find("t:authFederationConnectors", namespaces=ns) + if list_elem is None: + return [] + return [cls.from_element(c) for c in list_elem.findall("t:connector", namespaces=ns)] + + +class AuthFederationKeyItem: + def __init__(self) -> None: + self.kid: str | None = None + self.kty: str | None = None + self.alg: str | None = None + self.use: str | None = None + self.n: str | None = None + self.e: str | None = None + + def __str__(self) -> str: + return f"{self.__class__.__qualname__}(kid={self.kid}, kty={self.kty}, " f"alg={self.alg}, use={self.use})" + + def __repr__(self) -> str: + return f"<{str(self)}>" + + @classmethod + def from_element(cls, elem) -> "AuthFederationKeyItem": + key = cls() + key.kid = elem.get("kid") + key.kty = elem.get("kty") + key.alg = elem.get("alg") + key.use = elem.get("use") + key.n = elem.get("n") + key.e = elem.get("e") + return key + + @classmethod + def from_response(cls, raw_xml: bytes, ns) -> "AuthFederationKeyItem": + root = fromstring(raw_xml) + elem = root.find("t:authFederationKey", namespaces=ns) + if elem is None: + raise ValueError("No authFederationKey element found in the XML.") + return cls.from_element(elem) + + +class AuthFederationItem: + def __init__(self) -> None: + self.enabled: bool = False + self.issuer_url: str | None = None + self.active_key_id: str | None = None + self.token_lifetime_seconds: int | None = None + self.connectors: list[AuthFederationConnectorItem] = [] + + def __str__(self) -> str: + return ( + f"{self.__class__.__qualname__}(enabled={self.enabled}, " + f"issuer_url={self.issuer_url}, active_key_id={self.active_key_id}, " + f"token_lifetime_seconds={self.token_lifetime_seconds}, " + f"connectors={len(self.connectors)})" + ) + + def __repr__(self) -> str: + return f"<{str(self)}>" + + @classmethod + def from_response(cls, raw_xml: bytes, ns) -> "AuthFederationItem": + root = fromstring(raw_xml) + elem = root.find("t:authFederationAdmin", namespaces=ns) + if elem is None: + raise ValueError("No authFederationAdmin element found in the XML.") + config = cls() + config.enabled = _str_to_bool(elem.get("enabled", "false")) + config.issuer_url = elem.get("issuerUrl") + config.active_key_id = elem.get("activeKeyId") + token_lifetime = elem.get("tokenLifetimeSeconds") + if token_lifetime is not None: + config.token_lifetime_seconds = int(token_lifetime) + connectors_elem = elem.find("t:connectors", namespaces=ns) + if connectors_elem is not None: + config.connectors = [ + AuthFederationConnectorItem.from_element(c) + for c in connectors_elem.findall("t:connector", namespaces=ns) + ] + return config + + +def _str_to_bool(s: str) -> bool: + return s == "true" diff --git a/tableauserverclient/server/endpoint/__init__.py b/tableauserverclient/server/endpoint/__init__.py index 0a6eb8640..de2f7e8a1 100644 --- a/tableauserverclient/server/endpoint/__init__.py +++ b/tableauserverclient/server/endpoint/__init__.py @@ -1,4 +1,5 @@ from tableauserverclient.server.endpoint.auth_endpoint import Auth +from tableauserverclient.server.endpoint.auth_federation_endpoint import AuthFederation from tableauserverclient.server.endpoint.custom_views_endpoint import CustomViews from tableauserverclient.server.endpoint.data_acceleration_report_endpoint import DataAccelerationReport from tableauserverclient.server.endpoint.data_alert_endpoint import DataAlerts @@ -35,6 +36,7 @@ __all__ = [ "Auth", + "AuthFederation", "CustomViews", "DataAccelerationReport", "DataAlerts", diff --git a/tableauserverclient/server/endpoint/auth_federation_endpoint.py b/tableauserverclient/server/endpoint/auth_federation_endpoint.py new file mode 100644 index 000000000..159d00243 --- /dev/null +++ b/tableauserverclient/server/endpoint/auth_federation_endpoint.py @@ -0,0 +1,263 @@ +from typing import Protocol, TYPE_CHECKING, Union + +from tableauserverclient.models.auth_federation_item import ( + AuthFederationConnectorItem, + AuthFederationItem, + AuthFederationKeyItem, +) +from tableauserverclient.server.endpoint.endpoint import Endpoint, api +from tableauserverclient.server.request_factory import RequestFactory + +if TYPE_CHECKING: + from tableauserverclient.server.server import Server + + +class HasLuid(Protocol): + @property + def luid(self) -> str | None: ... + + +class HasKid(Protocol): + @property + def kid(self) -> str | None: ... + + +ConnectorIdentifier = Union[str, HasLuid] +KeyIdentifier = Union[str, HasKid] + + +class AuthFederation(Endpoint): + """ + Workload Identity Federation (WIF) admin configuration for a Tableau Cloud + site. Lets a site admin enable WIF, manage the signing keys that mint JWTs + for cloud-provider federation, and register the connectors (BigQuery, + Snowflake, ...) that each JWT is scoped to. Requires REST API version 3.30 + (Tableau Server 2026.3) or later. + + REST API: https://help.tableau.com/current/api/rest_api/en-us/REST/rest_api_ref_auth_federation.htm + """ + + def __init__(self, server: "Server") -> None: + self.parent_srv = server + + @property + def baseurl(self) -> str: + return f"{self.parent_srv.baseurl}/sites/{self.parent_srv.site_id}/auth-federation/admin" + + # -- Admin config -------------------------------------------------------- + + @api(version="3.30") + def get(self) -> AuthFederationItem: + """ + Get the WIF admin config for the currently authenticated Tableau Cloud + site. A site that has never enabled WIF returns a default item with + enabled=False rather than raising a 404. + + Returns + ------- + AuthFederationItem + """ + response = self.get_request(self.baseurl) + return AuthFederationItem.from_response(response.content, self.parent_srv.namespace) + + @api(version="3.30") + def enable(self, config: AuthFederationItem) -> AuthFederationItem: + """ + Enable WIF for the currently authenticated Tableau Cloud site. The + operation is idempotent: calling enable on an already-enabled site + returns the current config. The request body must set enabled=True; + use delete() to turn WIF off. + + Parameters + ---------- + config : AuthFederationItem + Must have enabled=True. ``token_lifetime_seconds`` is optional and + applied if provided. All other fields on the item are ignored on + enable (issuer_url and active_key_id are server-assigned). + + Returns + ------- + AuthFederationItem + The current WIF admin config. + """ + if not config.enabled: + raise ValueError("AuthFederation.enable requires config.enabled=True; use delete() to disable WIF.") + req = RequestFactory.AuthFederation.enable_req(config) + response = self.put_request(self.baseurl, req) + return AuthFederationItem.from_response(response.content, self.parent_srv.namespace) + + @api(version="3.30") + def update(self, config: AuthFederationItem) -> AuthFederationItem: + """ + Merge-patch the mutable fields of the WIF admin config. Only + ``token_lifetime_seconds`` is mutable; any other fields set on the + item (``issuer_url``, ``active_key_id``, ``connectors``) are silently + ignored by the server. + + Parameters + ---------- + config : AuthFederationItem + Patch document. Fields set to None are not sent. + + Returns + ------- + AuthFederationItem + The updated WIF admin config. + """ + req = RequestFactory.AuthFederation.update_req(config) + response = self.patch_request(self.baseurl, req) + return AuthFederationItem.from_response(response.content, self.parent_srv.namespace) + + @api(version="3.30") + def delete(self) -> None: + """ + Delete the WIF admin config, all signing keys, and all connectors for + the currently authenticated site. Any datasource or workbook connections + on the site that reference a WIF connector will no longer be able to + authenticate until WIF is re-enabled. + """ + self.delete_request(self.baseurl) + + # -- Connectors ---------------------------------------------------------- + + @api(version="3.30") + def create_connector(self, connector: AuthFederationConnectorItem) -> AuthFederationConnectorItem: + """ + Register a new WIF connector on the site. The server assigns the + connector's LUID. Fails with 409 if a connector with the same + connector_class already exists on the site. + + Parameters + ---------- + connector : AuthFederationConnectorItem + ``connector_class`` and ``audience`` are required. + + Returns + ------- + AuthFederationConnectorItem + The created connector, with the server-assigned ``luid`` populated. + """ + url = f"{self.baseurl}/connectors" + req = RequestFactory.AuthFederation.create_connector_req(connector) + response = self.post_request(url, req) + return AuthFederationConnectorItem.from_response(response.content, self.parent_srv.namespace) + + @api(version="3.30") + def list_connectors(self) -> list[AuthFederationConnectorItem]: + """ + List every WIF connector registered on the site. Returns an empty list + if WIF has no connectors (or is not enabled). + + Returns + ------- + list[AuthFederationConnectorItem] + """ + url = f"{self.baseurl}/connectors" + response = self.get_request(url) + return AuthFederationConnectorItem.list_from_response(response.content, self.parent_srv.namespace) + + @api(version="3.30") + def get_connector(self, connector: ConnectorIdentifier) -> AuthFederationConnectorItem: + """ + Get a single WIF connector by LUID. + + Parameters + ---------- + connector : str | HasLuid + The connector LUID, or an ``AuthFederationConnectorItem`` whose + ``luid`` is set. + + Returns + ------- + AuthFederationConnectorItem + """ + target = getattr(connector, "luid", connector) + url = f"{self.baseurl}/connectors/{target}" + response = self.get_request(url) + return AuthFederationConnectorItem.from_response(response.content, self.parent_srv.namespace) + + @api(version="3.30") + def delete_connector(self, connector: ConnectorIdentifier) -> None: + """ + Delete a WIF connector by LUID. Idempotent: returns normally even if + the connector does not exist. Existing site connections that reference + the removed connector are orphaned, not cascade-deleted. + + Parameters + ---------- + connector : str | HasLuid + The connector LUID, or an ``AuthFederationConnectorItem`` whose + ``luid`` is set. + """ + target = getattr(connector, "luid", connector) + url = f"{self.baseurl}/connectors/{target}" + self.delete_request(url) + + # -- Keys ---------------------------------------------------------------- + + @api(version="3.30") + def create_key(self) -> AuthFederationKeyItem: + """ + Generate a new WIF signing key. Keys are created in the inactive state; + use ``activate_key`` to promote one to the site's active signing key. + Fails with 409 if the site is already at the maximum number of keys. + + Returns + ------- + AuthFederationKeyItem + The created key's JWK metadata (``kid``, ``kty``, ``alg``, ``use``, + ``n``, ``e``). The private key is never exposed by the REST API. + """ + url = f"{self.baseurl}/keys" + req = RequestFactory.Empty.empty_req() + response = self.post_request(url, req) + return AuthFederationKeyItem.from_response(response.content, self.parent_srv.namespace) + + @api(version="3.30") + def activate_key(self, key: KeyIdentifier) -> None: + """ + Promote the given key to the site's active signing key. Atomic: + deactivates whichever key was previously active in the same operation. + + Parameters + ---------- + key : str | HasKid + The key ID (``kid``), or an ``AuthFederationKeyItem`` whose ``kid`` + is set. + """ + target = getattr(key, "kid", key) + url = f"{self.baseurl}/keys/{target}/activate" + self.put_request(url) + + @api(version="3.30") + def deactivate_key(self, key: KeyIdentifier) -> None: + """ + Deactivate the given key. A deactivated key stays on the site (and in + the public JWKS) so that already-issued JWTs can still be verified + during their validity window, but no new JWTs will be signed with it. + + Parameters + ---------- + key : str | HasKid + The key ID (``kid``), or an ``AuthFederationKeyItem`` whose ``kid`` + is set. + """ + target = getattr(key, "kid", key) + url = f"{self.baseurl}/keys/{target}/deactivate" + self.put_request(url) + + @api(version="3.30") + def delete_key(self, key: KeyIdentifier) -> None: + """ + Delete a WIF signing key. Fails with 400 if the key is active; + deactivate it first. + + Parameters + ---------- + key : str | HasKid + The key ID (``kid``), or an ``AuthFederationKeyItem`` whose ``kid`` + is set. + """ + target = getattr(key, "kid", key) + url = f"{self.baseurl}/keys/{target}" + self.delete_request(url) diff --git a/tableauserverclient/server/request_factory.py b/tableauserverclient/server/request_factory.py index ca097fbc9..aa35199d8 100644 --- a/tableauserverclient/server/request_factory.py +++ b/tableauserverclient/server/request_factory.py @@ -104,6 +104,36 @@ def switch_req(self, site_content_url): return ET.tostring(xml_request) +class AuthFederationRequest: + @_tsrequest_wrapped + def enable_req(self, xml_request: ET.Element, config: "AuthFederationItem") -> bytes: + elem = ET.SubElement(xml_request, "authFederationAdmin") + elem.attrib["enabled"] = str(config.enabled).lower() + if config.token_lifetime_seconds is not None: + elem.attrib["tokenLifetimeSeconds"] = str(config.token_lifetime_seconds) + return ET.tostring(xml_request) + + @_tsrequest_wrapped + def update_req(self, xml_request: ET.Element, config: "AuthFederationItem") -> bytes: + elem = ET.SubElement(xml_request, "authFederationAdmin") + # Only token_lifetime_seconds is mutable on PATCH; issuerUrl, activeKeyId, + # and connectors are read-only and silently ignored by the server. + if config.token_lifetime_seconds is not None: + elem.attrib["tokenLifetimeSeconds"] = str(config.token_lifetime_seconds) + return ET.tostring(xml_request) + + @_tsrequest_wrapped + def create_connector_req(self, xml_request: ET.Element, connector: "AuthFederationConnectorItem") -> bytes: + elem = ET.SubElement(xml_request, "authFederationConnector") + if connector.connector_class is None: + raise ValueError(f"AuthFederationConnectorItem missing connector_class: {connector}") + if connector.audience is None: + raise ValueError(f"AuthFederationConnectorItem missing audience: {connector}") + elem.attrib["connectorClass"] = connector.connector_class + elem.attrib["audience"] = connector.audience + return ET.tostring(xml_request) + + class ColumnRequest: def update_req(self, column_item): xml_request = ET.Element("tsRequest") @@ -1754,6 +1784,7 @@ def update_site_extensions(self, xml_request: ET.Element, extensions_site_settin class RequestFactory: Auth = AuthRequest() + AuthFederation = AuthFederationRequest() Connection = Connection() Column = ColumnRequest() CustomView = CustomViewRequest() diff --git a/tableauserverclient/server/server.py b/tableauserverclient/server/server.py index 9e5797dc4..853bdea68 100644 --- a/tableauserverclient/server/server.py +++ b/tableauserverclient/server/server.py @@ -19,6 +19,7 @@ Datasources, Projects, Auth, + AuthFederation, Schedules, ServerInfo, Tasks, @@ -232,6 +233,7 @@ def __init__(self, server_address, use_server_version=False, http_options=None, self.tags = Tags(self) self.virtual_connections = VirtualConnections(self) self.oidc = OIDC(self) + self.auth_federation = AuthFederation(self) self.extensions = Extensions(self) self._http_options = dict() # must set this before making a server call diff --git a/test/assets/auth_federation_connector_create.xml b/test/assets/auth_federation_connector_create.xml new file mode 100644 index 000000000..7c7236952 --- /dev/null +++ b/test/assets/auth_federation_connector_create.xml @@ -0,0 +1,10 @@ + + + + diff --git a/test/assets/auth_federation_connector_get.xml b/test/assets/auth_federation_connector_get.xml new file mode 100644 index 000000000..7c7236952 --- /dev/null +++ b/test/assets/auth_federation_connector_get.xml @@ -0,0 +1,10 @@ + + + + diff --git a/test/assets/auth_federation_connectors_list.xml b/test/assets/auth_federation_connectors_list.xml new file mode 100644 index 000000000..fa3eb4f5e --- /dev/null +++ b/test/assets/auth_federation_connectors_list.xml @@ -0,0 +1,16 @@ + + + + + + + diff --git a/test/assets/auth_federation_connectors_list_empty.xml b/test/assets/auth_federation_connectors_list_empty.xml new file mode 100644 index 000000000..bf1ce9545 --- /dev/null +++ b/test/assets/auth_federation_connectors_list_empty.xml @@ -0,0 +1,7 @@ + + + + diff --git a/test/assets/auth_federation_enable.xml b/test/assets/auth_federation_enable.xml new file mode 100644 index 000000000..9c9e0e8ae --- /dev/null +++ b/test/assets/auth_federation_enable.xml @@ -0,0 +1,11 @@ + + + + diff --git a/test/assets/auth_federation_get.xml b/test/assets/auth_federation_get.xml new file mode 100644 index 000000000..c9714e6d8 --- /dev/null +++ b/test/assets/auth_federation_get.xml @@ -0,0 +1,22 @@ + + + + + + + + + diff --git a/test/assets/auth_federation_key_create.xml b/test/assets/auth_federation_key_create.xml new file mode 100644 index 000000000..c16f79c6f --- /dev/null +++ b/test/assets/auth_federation_key_create.xml @@ -0,0 +1,13 @@ + + + + diff --git a/test/assets/auth_federation_update.xml b/test/assets/auth_federation_update.xml new file mode 100644 index 000000000..5efbb868a --- /dev/null +++ b/test/assets/auth_federation_update.xml @@ -0,0 +1,11 @@ + + + + diff --git a/test/test_auth_federation.py b/test/test_auth_federation.py new file mode 100644 index 000000000..bd3d38629 --- /dev/null +++ b/test/test_auth_federation.py @@ -0,0 +1,299 @@ +from pathlib import Path + +import pytest +import requests_mock +from defusedxml.ElementTree import fromstring + +import tableauserverclient as TSC +from tableauserverclient.server.exceptions import EndpointUnavailableError + +assets = Path(__file__).parent / "assets" +GET = assets / "auth_federation_get.xml" +ENABLE = assets / "auth_federation_enable.xml" +UPDATE = assets / "auth_federation_update.xml" +CONNECTOR_CREATE = assets / "auth_federation_connector_create.xml" +CONNECTOR_GET = assets / "auth_federation_connector_get.xml" +CONNECTORS_LIST = assets / "auth_federation_connectors_list.xml" +CONNECTORS_LIST_EMPTY = assets / "auth_federation_connectors_list_empty.xml" +KEY_CREATE = assets / "auth_federation_key_create.xml" + +NS = {"t": "http://tableau.com/api"} +CONNECTOR_LUID = "d3a8e5b2-7c01-4e13-9c82-6f45a9b4e8d1" +KEY_KID = "a4b9c2d7-ff81-45e8-9c3e-6b19d2a5c0ee" + + +@pytest.fixture(scope="function") +def server() -> TSC.Server: + server = TSC.Server("http://test", False) + server._site_id = "dad65087-b08b-4603-af4e-2887b8aafc67" + server._auth_token = "j80k54ll2lfMZ0tv97mlPvvSCRyD0DOM" + server.version = "3.30" + return server + + +def test_auth_federation_get(server: TSC.Server) -> None: + with requests_mock.mock() as m: + m.get(server.auth_federation.baseurl, text=GET.read_text()) + config = server.auth_federation.get() + + assert config.enabled is True + assert config.issuer_url == "https://10ax.online.tableau.com/sites/mySite/auth-federation" + assert config.active_key_id == "a4b9c2d7-ff81-45e8-9c3e-6b19d2a5c0ee" + assert config.token_lifetime_seconds == 3600 + assert len(config.connectors) == 2 + assert {c.connector_class for c in config.connectors} == {"bigquery", "snowflake"} + bigquery = next(c for c in config.connectors if c.connector_class == "bigquery") + assert bigquery.luid == CONNECTOR_LUID + assert bigquery.audience.startswith("//iam.googleapis.com/") + + +def test_auth_federation_get_defaults_when_never_enabled(server: TSC.Server) -> None: + default_body = ( + '' '' "" + ) + with requests_mock.mock() as m: + m.get(server.auth_federation.baseurl, text=default_body) + config = server.auth_federation.get() + + assert config.enabled is False + assert config.issuer_url is None + assert config.active_key_id is None + assert config.token_lifetime_seconds is None + assert config.connectors == [] + + +def test_auth_federation_enable(server: TSC.Server) -> None: + config_in = TSC.AuthFederationItem() + config_in.enabled = True + config_in.token_lifetime_seconds = 3600 + + with requests_mock.mock() as m: + m.put(server.auth_federation.baseurl, text=ENABLE.read_text()) + config_out = server.auth_federation.enable(config_in) + body = m.request_history[0].text + + sent = fromstring(body) + elem = sent.find("authFederationAdmin") + assert elem is not None + assert elem.get("enabled") == "true" + assert elem.get("tokenLifetimeSeconds") == "3600" + assert elem.find("connectors") is None + + assert config_out.enabled is True + assert config_out.active_key_id == "a4b9c2d7-ff81-45e8-9c3e-6b19d2a5c0ee" + assert config_out.token_lifetime_seconds == 3600 + + +def test_auth_federation_enable_without_token_lifetime(server: TSC.Server) -> None: + config_in = TSC.AuthFederationItem() + config_in.enabled = True + + with requests_mock.mock() as m: + m.put(server.auth_federation.baseurl, text=ENABLE.read_text()) + server.auth_federation.enable(config_in) + body = m.request_history[0].text + + elem = fromstring(body).find("authFederationAdmin") + assert elem is not None + assert elem.get("enabled") == "true" + assert "tokenLifetimeSeconds" not in elem.attrib + + +def test_auth_federation_enable_rejects_disabled(server: TSC.Server) -> None: + config_in = TSC.AuthFederationItem() + config_in.enabled = False + + with pytest.raises(ValueError, match="enabled=True"): + server.auth_federation.enable(config_in) + + +def test_auth_federation_update(server: TSC.Server) -> None: + config_in = TSC.AuthFederationItem() + config_in.token_lifetime_seconds = 7200 + config_in.issuer_url = "ignored-by-server" + config_in.active_key_id = "ignored-by-server" + + with requests_mock.mock() as m: + m.patch(server.auth_federation.baseurl, text=UPDATE.read_text()) + config_out = server.auth_federation.update(config_in) + body = m.request_history[0].text + + elem = fromstring(body).find("authFederationAdmin") + assert elem is not None + assert elem.get("tokenLifetimeSeconds") == "7200" + # Read-only fields must not be sent; the server would ignore them anyway, + # but we don't want the client to pretend they're mutable. + assert "issuerUrl" not in elem.attrib + assert "activeKeyId" not in elem.attrib + + assert config_out.token_lifetime_seconds == 7200 + + +def test_auth_federation_delete(server: TSC.Server) -> None: + with requests_mock.mock() as m: + m.delete(server.auth_federation.baseurl, status_code=204) + server.auth_federation.delete() + + assert m.called_once + + +def test_auth_federation_create_connector(server: TSC.Server) -> None: + connector_in = TSC.AuthFederationConnectorItem() + connector_in.connector_class = "bigquery" + connector_in.audience = ( + "//iam.googleapis.com/projects/123456789/locations/global/" + "workloadIdentityPools/tableau-pool/providers/tableau-provider" + ) + + with requests_mock.mock() as m: + m.post(f"{server.auth_federation.baseurl}/connectors", text=CONNECTOR_CREATE.read_text()) + connector_out = server.auth_federation.create_connector(connector_in) + body = m.request_history[0].text + + elem = fromstring(body).find("authFederationConnector") + assert elem is not None + assert elem.get("connectorClass") == "bigquery" + assert elem.get("audience") == connector_in.audience + assert "luid" not in elem.attrib + + assert connector_out.luid == CONNECTOR_LUID + assert connector_out.connector_class == "bigquery" + + +def test_auth_federation_create_connector_requires_class(server: TSC.Server) -> None: + connector_in = TSC.AuthFederationConnectorItem() + connector_in.audience = "snowflakecomputing.com" + + with pytest.raises(ValueError, match="connector_class"): + server.auth_federation.create_connector(connector_in) + + +def test_auth_federation_create_connector_requires_audience(server: TSC.Server) -> None: + connector_in = TSC.AuthFederationConnectorItem() + connector_in.connector_class = "snowflake" + + with pytest.raises(ValueError, match="audience"): + server.auth_federation.create_connector(connector_in) + + +def test_auth_federation_list_connectors(server: TSC.Server) -> None: + with requests_mock.mock() as m: + m.get(f"{server.auth_federation.baseurl}/connectors", text=CONNECTORS_LIST.read_text()) + connectors = server.auth_federation.list_connectors() + + assert len(connectors) == 2 + classes = {c.connector_class for c in connectors} + assert classes == {"bigquery", "snowflake"} + + +def test_auth_federation_list_connectors_empty(server: TSC.Server) -> None: + with requests_mock.mock() as m: + m.get(f"{server.auth_federation.baseurl}/connectors", text=CONNECTORS_LIST_EMPTY.read_text()) + connectors = server.auth_federation.list_connectors() + + assert connectors == [] + + +def test_auth_federation_get_connector_by_luid(server: TSC.Server) -> None: + with requests_mock.mock() as m: + m.get( + f"{server.auth_federation.baseurl}/connectors/{CONNECTOR_LUID}", + text=CONNECTOR_GET.read_text(), + ) + connector = server.auth_federation.get_connector(CONNECTOR_LUID) + + assert connector.luid == CONNECTOR_LUID + assert connector.connector_class == "bigquery" + + +def test_auth_federation_get_connector_by_item(server: TSC.Server) -> None: + stub = TSC.AuthFederationConnectorItem() + stub.luid = CONNECTOR_LUID + + with requests_mock.mock() as m: + m.get( + f"{server.auth_federation.baseurl}/connectors/{CONNECTOR_LUID}", + text=CONNECTOR_GET.read_text(), + ) + connector = server.auth_federation.get_connector(stub) + + assert connector.luid == CONNECTOR_LUID + + +def test_auth_federation_delete_connector_by_luid(server: TSC.Server) -> None: + with requests_mock.mock() as m: + m.delete(f"{server.auth_federation.baseurl}/connectors/{CONNECTOR_LUID}", status_code=204) + server.auth_federation.delete_connector(CONNECTOR_LUID) + + assert m.called_once + + +def test_auth_federation_delete_connector_by_item(server: TSC.Server) -> None: + stub = TSC.AuthFederationConnectorItem() + stub.luid = CONNECTOR_LUID + + with requests_mock.mock() as m: + m.delete(f"{server.auth_federation.baseurl}/connectors/{CONNECTOR_LUID}", status_code=204) + server.auth_federation.delete_connector(stub) + + assert m.called_once + + +def test_auth_federation_create_key(server: TSC.Server) -> None: + with requests_mock.mock() as m: + m.post(f"{server.auth_federation.baseurl}/keys", text=KEY_CREATE.read_text()) + key = server.auth_federation.create_key() + body = m.request_history[0].text + + # POST /keys takes no body besides the empty tsRequest envelope. + envelope = fromstring(body) + assert envelope.tag == "tsRequest" + assert len(envelope) == 0 + + assert key.kid == KEY_KID + assert key.kty == "RSA" + assert key.alg == "RS256" + assert key.use == "sig" + assert key.e == "AQAB" + assert key.n is not None and len(key.n) > 0 + + +def test_auth_federation_activate_key(server: TSC.Server) -> None: + with requests_mock.mock() as m: + m.put(f"{server.auth_federation.baseurl}/keys/{KEY_KID}/activate", status_code=204) + server.auth_federation.activate_key(KEY_KID) + + assert m.called_once + + +def test_auth_federation_activate_key_by_item(server: TSC.Server) -> None: + stub = TSC.AuthFederationKeyItem() + stub.kid = KEY_KID + + with requests_mock.mock() as m: + m.put(f"{server.auth_federation.baseurl}/keys/{KEY_KID}/activate", status_code=204) + server.auth_federation.activate_key(stub) + + assert m.called_once + + +def test_auth_federation_deactivate_key(server: TSC.Server) -> None: + with requests_mock.mock() as m: + m.put(f"{server.auth_federation.baseurl}/keys/{KEY_KID}/deactivate", status_code=204) + server.auth_federation.deactivate_key(KEY_KID) + + assert m.called_once + + +def test_auth_federation_delete_key(server: TSC.Server) -> None: + with requests_mock.mock() as m: + m.delete(f"{server.auth_federation.baseurl}/keys/{KEY_KID}", status_code=204) + server.auth_federation.delete_key(KEY_KID) + + assert m.called_once + + +def test_auth_federation_requires_server_version_3_30(server: TSC.Server) -> None: + server.version = "3.29" + with pytest.raises(EndpointUnavailableError): + server.auth_federation.get() From 6d683aab344a85f008b8aec697f1ccb5a307d684 Mon Sep 17 00:00:00 2001 From: Sushma Sreedharan Date: Tue, 20 Oct 2026 10:27:06 -0700 Subject: [PATCH 02/10] Document that AuthFederation.update() drops `enabled` Server-side AuthFederationAdminAppService.updateConfig rejects any non-null `enabled` on PATCH with HTTP 400 and tells callers to use enable() / delete() to change state. The client's update_req already only sends `tokenLifetimeSeconds`, but the docstring listed only issuer_url, active_key_id, and connectors as "silently ignored", leaving `enabled` as a hidden footgun. Add it to the list and point callers at the correct methods. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../server/endpoint/auth_federation_endpoint.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tableauserverclient/server/endpoint/auth_federation_endpoint.py b/tableauserverclient/server/endpoint/auth_federation_endpoint.py index 159d00243..25662eb55 100644 --- a/tableauserverclient/server/endpoint/auth_federation_endpoint.py +++ b/tableauserverclient/server/endpoint/auth_federation_endpoint.py @@ -92,7 +92,8 @@ def update(self, config: AuthFederationItem) -> AuthFederationItem: Merge-patch the mutable fields of the WIF admin config. Only ``token_lifetime_seconds`` is mutable; any other fields set on the item (``issuer_url``, ``active_key_id``, ``connectors``) are silently - ignored by the server. + ignored by the server. ``enabled`` is also silently dropped on the + wire — use ``enable()`` / ``delete()`` to change WIF state. Parameters ---------- From 15c975b4661c10f784950d73b9b9a9a298eaffec Mon Sep 17 00:00:00 2001 From: Sushma Sreedharan Date: Tue, 20 Oct 2026 10:36:38 -0700 Subject: [PATCH 03/10] Align AuthFederation docstrings with monolith server behaviour MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three docstring claims didn't match what the server actually does per AuthFederationService.java and RestApiAuthFederationKeysAppService.java: * enable(): did not mention that AuthFederationService.enableWif auto-generates one active signing key on first enable (AuthFederationService.java:160-164). A caller doesn't need to immediately create_key + activate_key — the returned item's active_key_id is already populated. * list_connectors(): claimed empty-list on "WIF not enabled". The server's requireWifEnabled (AuthFederationService.java:226-233) throws WIF_NOT_ENABLED_FOR_SITE / FAILED_PRECONDITION, which the client surfaces as ServerResponseError. Only an empty configured site returns []. * create_key(): claimed 409 for the max-keys-exceeded case. The server throws BadRequestRestException (HTTP 400), not 409 (RestApiAuthFederationKeysAppService.java:44-48). Docstring-only; no code or test changes. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../server/endpoint/auth_federation_endpoint.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tableauserverclient/server/endpoint/auth_federation_endpoint.py b/tableauserverclient/server/endpoint/auth_federation_endpoint.py index 25662eb55..97963a388 100644 --- a/tableauserverclient/server/endpoint/auth_federation_endpoint.py +++ b/tableauserverclient/server/endpoint/auth_federation_endpoint.py @@ -68,6 +68,11 @@ def enable(self, config: AuthFederationItem) -> AuthFederationItem: returns the current config. The request body must set enabled=True; use delete() to turn WIF off. + On first enable (not on subsequent idempotent calls), the server + auto-generates one active signing key and populates + ``active_key_id`` on the returned item, so a caller does not need + to immediately ``create_key`` + ``activate_key``. + Parameters ---------- config : AuthFederationItem @@ -147,7 +152,9 @@ def create_connector(self, connector: AuthFederationConnectorItem) -> AuthFedera def list_connectors(self) -> list[AuthFederationConnectorItem]: """ List every WIF connector registered on the site. Returns an empty list - if WIF has no connectors (or is not enabled). + if WIF is enabled but has no connectors. If WIF is not enabled on the + site, the server raises a precondition error (surfaced as + ``ServerResponseError``) — call ``enable()`` first. Returns ------- @@ -201,7 +208,8 @@ def create_key(self) -> AuthFederationKeyItem: """ Generate a new WIF signing key. Keys are created in the inactive state; use ``activate_key`` to promote one to the site's active signing key. - Fails with 409 if the site is already at the maximum number of keys. + Fails with 400 (``ServerResponseError``) if the site is already at the + maximum number of keys. Returns ------- From c48c0845c63a7978003dca938b0797221a736fd5 Mon Sep 17 00:00:00 2001 From: Sushma Sreedharan Date: Tue, 20 Oct 2026 10:45:44 -0700 Subject: [PATCH 04/10] Correct list_connectors() docstring: server returns HTTP 404, not 412 RestApiAuthFederationAdminAppService.listConnectors intercepts TableauStatusCode.FAILED_PRECONDITION from the service-layer requireWifEnabled check and re-throws configNotFound (HTTP 404) with AUTH_FEDERATION_NOT_FOUND -- see RestApiAuthFederationAdminAppService.java:157-162. The service-layer behavior cited in the previous commit does not reach the wire; callers see 404. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../server/endpoint/auth_federation_endpoint.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tableauserverclient/server/endpoint/auth_federation_endpoint.py b/tableauserverclient/server/endpoint/auth_federation_endpoint.py index 97963a388..0d62a9f99 100644 --- a/tableauserverclient/server/endpoint/auth_federation_endpoint.py +++ b/tableauserverclient/server/endpoint/auth_federation_endpoint.py @@ -153,8 +153,8 @@ def list_connectors(self) -> list[AuthFederationConnectorItem]: """ List every WIF connector registered on the site. Returns an empty list if WIF is enabled but has no connectors. If WIF is not enabled on the - site, the server raises a precondition error (surfaced as - ``ServerResponseError``) — call ``enable()`` first. + site, the server returns HTTP 404 (surfaced as ``ServerResponseError``) + — call ``enable()`` first. Returns ------- From ab763dcb5662f5376c36760a4f33e734d8115520 Mon Sep 17 00:00:00 2001 From: Sushma Sreedharan Date: Tue, 20 Oct 2026 13:07:18 -0700 Subject: [PATCH 05/10] Address review feedback on AuthFederation docs and tests - Reword PAT/OAuth-scope guidance: the four key methods are bound to OAuthScopes.NO_ACCESS server-side, so every OAuth-scoped session (including every PAT) is rejected unconditionally. Authenticate with a username/password session instead. Reflected in CHANGELOG and the class docstring. - Drop the misleading "Snowflake" case-sensitivity counter-example in create_connector; just state that class names are case-sensitive and must be the exact lowercase name. - Note in CHANGELOG that delete_key fails with 400 on an active key; deactivate first. - Replace vague "server rejects the request" in the class docstring with the specific HTTP 404 the server returns when WIF has never been enabled. - Make the _coerce_uuid wording in CHANGELOG explicit: None, empty strings, and non-UUID-shaped targets are rejected client-side. - Add a comment in test_auth_federation_update_empty_patch noting that server-side binding of an empty must be covered by an integration test, not this unit test. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 45 ++++-- .../models/auth_federation_item.py | 16 +- .../endpoint/auth_federation_endpoint.py | 136 ++++++++++------ tableauserverclient/server/request_factory.py | 44 ++--- test/test_auth_federation.py | 150 +++++++++++++----- 5 files changed, 260 insertions(+), 131 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 17d15ebdf..6fcf193ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,28 +6,47 @@ its signing keys and connectors from TSC. New endpoints: - `server.auth_federation.get()` -> `AuthFederationItem` — read the site's WIF admin config (enabled flag, issuer URL, active key id, - JWT lifetime, connectors) - - `server.auth_federation.enable(config)` -> `AuthFederationItem` — - idempotent PUT; `AuthFederationItem.enabled` must be `True` - - `server.auth_federation.update(config)` -> `AuthFederationItem` — - merge-patch; only `token_lifetime_seconds` is mutable - - `server.auth_federation.delete()` — deletes the WIF config along - with every signing key and connector on the site - - `server.auth_federation.create_connector(connector)` -> `AuthFederationConnectorItem` + JWT lifetime, connectors). A never-enabled site returns a shell with + `enabled=False` rather than 404. + - `server.auth_federation.enable(token_lifetime_seconds=None)` -> + `AuthFederationItem` — idempotent PUT. Server auto-generates one active + signing key on first enable. + - `server.auth_federation.update(token_lifetime_seconds=None)` -> + `AuthFederationItem` — merge-patch; only `token_lifetime_seconds` is + mutable. `enabled` is never sent on PATCH; use `enable()` / `delete()` + to change WIF state. + - `server.auth_federation.delete()` — permanently deletes the WIF config + along with every signing key and connector on the site. A subsequent + `enable()` starts from a clean slate; prior connectors and keys are + not restored. + - `server.auth_federation.create_connector(connector)` -> + `AuthFederationConnectorItem`. `connector_class` must match a Tableau + connection type exactly (case-sensitive, e.g. `"bigquery"`, + `"snowflake"`). Fails with 409 if a connector with the same class + already exists on the site. - `server.auth_federation.list_connectors()` -> `list[AuthFederationConnectorItem]` - `server.auth_federation.get_connector(id)` and `server.auth_federation.delete_connector(id)` accept either a connector - LUID string or an `AuthFederationConnectorItem` with `luid` set + LUID string or an `AuthFederationConnectorItem` with `luid` set. All + id-taking methods reject `None`, empty strings, and non-UUID-shaped + targets client-side (raising `ValueError`) to prevent path injection. - `server.auth_federation.create_key()` -> `AuthFederationKeyItem` — - generates a new (inactive) signing key; private key is never returned + generates a new (inactive) signing key; private key is never returned. + Fails with 400 if the site is already at the maximum number of keys. - `server.auth_federation.activate_key(id)` / `server.auth_federation.deactivate_key(id)` / `server.auth_federation.delete_key(id)` accept either a `kid` string or an `AuthFederationKeyItem` with `kid` set; `activate_key` is atomic - and deactivates any previously active key + and deactivates any previously active key. `delete_key` fails with 400 + if the key is currently active — call `deactivate_key` first. The four + key-management methods are bound to `NO_ACCESS` server-side, which + rejects every OAuth-scoped session unconditionally (including every PAT + session, regardless of granted scopes); authenticate with a + username/password session. Three new exports: `AuthFederationItem`, `AuthFederationConnectorItem`, `AuthFederationKeyItem`. Requires Tableau Server REST API 3.30+ - (Tableau Server release 26.3) and site-admin credentials. + (Tableau Server 2026.3) and the Site Administrator role on the current + site; non-admin callers receive HTTP 403. * Added Workload Identity Federation (WIF) support to the connection update endpoints. `ConnectionItem` has two new string properties, `workgroup_auth_mode` and `auth_federation_service_account`, that are @@ -56,7 +75,7 @@ `ConnectionItem.workgroup_auth_mode` and `ConnectionItem.auth_federation_service_account` parsed from a response will typically be `None` even when the connection is WIF-configured. - Requires Tableau Server REST API 3.30+ (Tableau Server release 26.3) with + Requires Tableau Server REST API 3.30+ (Tableau Server 2026.3) with WIF configured and enabled on the site. Older servers silently ignore unknown connection attributes, so these fields are safe to leave unset on pre-3.30 targets. diff --git a/tableauserverclient/models/auth_federation_item.py b/tableauserverclient/models/auth_federation_item.py index b369385c1..9fc898ef5 100644 --- a/tableauserverclient/models/auth_federation_item.py +++ b/tableauserverclient/models/auth_federation_item.py @@ -1,5 +1,7 @@ from defusedxml.ElementTree import fromstring +from tableauserverclient.helpers.strings import nullable_str_to_bool, nullable_str_to_int + class AuthFederationConnectorItem: def __init__(self) -> None: @@ -37,7 +39,7 @@ def list_from_response(cls, raw_xml: bytes, ns) -> list["AuthFederationConnector root = fromstring(raw_xml) list_elem = root.find("t:authFederationConnectors", namespaces=ns) if list_elem is None: - return [] + raise ValueError("No authFederationConnectors element found in the XML.") return [cls.from_element(c) for c in list_elem.findall("t:connector", namespaces=ns)] @@ -51,7 +53,7 @@ def __init__(self) -> None: self.e: str | None = None def __str__(self) -> str: - return f"{self.__class__.__qualname__}(kid={self.kid}, kty={self.kty}, " f"alg={self.alg}, use={self.use})" + return f"{self.__class__.__qualname__}(kid={self.kid}, kty={self.kty}, alg={self.alg}, use={self.use})" def __repr__(self) -> str: return f"<{str(self)}>" @@ -102,12 +104,10 @@ def from_response(cls, raw_xml: bytes, ns) -> "AuthFederationItem": if elem is None: raise ValueError("No authFederationAdmin element found in the XML.") config = cls() - config.enabled = _str_to_bool(elem.get("enabled", "false")) + config.enabled = nullable_str_to_bool(elem.get("enabled")) or False config.issuer_url = elem.get("issuerUrl") config.active_key_id = elem.get("activeKeyId") - token_lifetime = elem.get("tokenLifetimeSeconds") - if token_lifetime is not None: - config.token_lifetime_seconds = int(token_lifetime) + config.token_lifetime_seconds = nullable_str_to_int(elem.get("tokenLifetimeSeconds")) connectors_elem = elem.find("t:connectors", namespaces=ns) if connectors_elem is not None: config.connectors = [ @@ -115,7 +115,3 @@ def from_response(cls, raw_xml: bytes, ns) -> "AuthFederationItem": for c in connectors_elem.findall("t:connector", namespaces=ns) ] return config - - -def _str_to_bool(s: str) -> bool: - return s == "true" diff --git a/tableauserverclient/server/endpoint/auth_federation_endpoint.py b/tableauserverclient/server/endpoint/auth_federation_endpoint.py index 0d62a9f99..6f0954d8e 100644 --- a/tableauserverclient/server/endpoint/auth_federation_endpoint.py +++ b/tableauserverclient/server/endpoint/auth_federation_endpoint.py @@ -1,4 +1,5 @@ -from typing import Protocol, TYPE_CHECKING, Union +import uuid +from typing import Protocol, TYPE_CHECKING from tableauserverclient.models.auth_federation_item import ( AuthFederationConnectorItem, @@ -22,19 +23,50 @@ class HasKid(Protocol): def kid(self) -> str | None: ... -ConnectorIdentifier = Union[str, HasLuid] -KeyIdentifier = Union[str, HasKid] +ConnectorIdentifier = str | HasLuid +KeyIdentifier = str | HasKid + + +def _coerce_uuid(target: object) -> str: + """Validate ``target`` is a UUID and return its canonical string form. + + Guards every id-taking method against path injection: without this, a + caller could pass ``"../../admin"`` and delete the entire WIF config via + the key-delete route. + """ + try: + return str(uuid.UUID(str(target))) + except (ValueError, AttributeError, TypeError) as e: + raise ValueError(f"Invalid UUID: {target!r}") from e class AuthFederation(Endpoint): """ Workload Identity Federation (WIF) admin configuration for a Tableau Cloud - site. Lets a site admin enable WIF, manage the signing keys that mint JWTs - for cloud-provider federation, and register the connectors (BigQuery, - Snowflake, ...) that each JWT is scoped to. Requires REST API version 3.30 - (Tableau Server 2026.3) or later. - - REST API: https://help.tableau.com/current/api/rest_api/en-us/REST/rest_api_ref_auth_federation.htm + / Tableau Server site. Lets a site admin enable WIF, manage the signing + keys that mint JWTs for cloud-provider federation, and register the + connectors (BigQuery, Snowflake, ...) that each JWT is scoped to. Requires + REST API version 3.30 (Tableau Server 2026.3) or later. + + All methods require the caller to be a Site Administrator on the current + site; the server returns HTTP 403 (``ServerResponseError``) for non-admin + callers. Beyond ``get()`` and ``enable()``, every method also requires + WIF to be enabled on the site — call ``enable()`` first, otherwise the + server returns HTTP 404. + + The four key-management methods (``create_key``, ``activate_key``, + ``deactivate_key``, ``delete_key``) are bound to ``OAuthScopes.NO_ACCESS`` + server-side, which means every OAuth-scoped session is rejected + unconditionally — including every PAT session, regardless of granted + scopes. Authenticate with a username/password session to call them. The + config and connector methods accept the + ``tableau:connectivity_auth_federation:*`` OAuth scopes. + + ``delete()`` permanently removes the config, every signing key, and every + connector on the site; re-enabling with ``enable()`` starts from a clean + slate — prior connectors and keys are not restored, and any existing + datasource / workbook connections that reference WIF stay broken until + each connector is re-created. """ def __init__(self, server: "Server") -> None: @@ -49,9 +81,9 @@ def baseurl(self) -> str: @api(version="3.30") def get(self) -> AuthFederationItem: """ - Get the WIF admin config for the currently authenticated Tableau Cloud - site. A site that has never enabled WIF returns a default item with - enabled=False rather than raising a 404. + Get the WIF admin config for the currently authenticated site. A site + that has never enabled WIF returns a default item with + ``enabled=False`` rather than raising a 404. Returns ------- @@ -61,66 +93,64 @@ def get(self) -> AuthFederationItem: return AuthFederationItem.from_response(response.content, self.parent_srv.namespace) @api(version="3.30") - def enable(self, config: AuthFederationItem) -> AuthFederationItem: + def enable(self, token_lifetime_seconds: int | None = None) -> AuthFederationItem: """ - Enable WIF for the currently authenticated Tableau Cloud site. The - operation is idempotent: calling enable on an already-enabled site - returns the current config. The request body must set enabled=True; - use delete() to turn WIF off. + Enable WIF for the currently authenticated site. Idempotent: calling + enable on an already-enabled site returns the current config. On first enable (not on subsequent idempotent calls), the server - auto-generates one active signing key and populates - ``active_key_id`` on the returned item, so a caller does not need - to immediately ``create_key`` + ``activate_key``. + auto-generates one active signing key and populates ``active_key_id`` + on the returned item, so a caller does not need to immediately + ``create_key`` + ``activate_key``. Parameters ---------- - config : AuthFederationItem - Must have enabled=True. ``token_lifetime_seconds`` is optional and - applied if provided. All other fields on the item are ignored on - enable (issuer_url and active_key_id are server-assigned). + token_lifetime_seconds : int | None + Optional JWT lifetime in seconds. ``None`` leaves the server + default in place (or, on an already-enabled site, keeps whatever + lifetime is currently configured). Returns ------- AuthFederationItem The current WIF admin config. """ - if not config.enabled: - raise ValueError("AuthFederation.enable requires config.enabled=True; use delete() to disable WIF.") - req = RequestFactory.AuthFederation.enable_req(config) + req = RequestFactory.AuthFederation.enable_req(token_lifetime_seconds=token_lifetime_seconds) response = self.put_request(self.baseurl, req) return AuthFederationItem.from_response(response.content, self.parent_srv.namespace) @api(version="3.30") - def update(self, config: AuthFederationItem) -> AuthFederationItem: + def update(self, token_lifetime_seconds: int | None = None) -> AuthFederationItem: """ Merge-patch the mutable fields of the WIF admin config. Only - ``token_lifetime_seconds`` is mutable; any other fields set on the - item (``issuer_url``, ``active_key_id``, ``connectors``) are silently - ignored by the server. ``enabled`` is also silently dropped on the - wire — use ``enable()`` / ``delete()`` to change WIF state. + ``token_lifetime_seconds`` is mutable; ``enabled``, ``issuer_url``, + ``active_key_id``, and ``connectors`` are not — use ``enable()`` / + ``delete()`` to change WIF state and the connector / key endpoints + to manage those resources. Returns HTTP 404 if WIF has never been + enabled on the site. Parameters ---------- - config : AuthFederationItem - Patch document. Fields set to None are not sent. + token_lifetime_seconds : int | None + New JWT lifetime in seconds. ``None`` sends an empty patch (no + fields changed) — rarely useful, but accepted by the server. Returns ------- AuthFederationItem The updated WIF admin config. """ - req = RequestFactory.AuthFederation.update_req(config) + req = RequestFactory.AuthFederation.update_req(token_lifetime_seconds=token_lifetime_seconds) response = self.patch_request(self.baseurl, req) return AuthFederationItem.from_response(response.content, self.parent_srv.namespace) @api(version="3.30") def delete(self) -> None: """ - Delete the WIF admin config, all signing keys, and all connectors for - the currently authenticated site. Any datasource or workbook connections - on the site that reference a WIF connector will no longer be able to - authenticate until WIF is re-enabled. + Delete the WIF admin config, every signing key, and every connector + on the site. Any datasource or workbook connections that reference a + WIF connector will no longer be able to authenticate until WIF is + re-enabled; the removed connectors and keys are not restored. """ self.delete_request(self.baseurl) @@ -131,12 +161,17 @@ def create_connector(self, connector: AuthFederationConnectorItem) -> AuthFedera """ Register a new WIF connector on the site. The server assigns the connector's LUID. Fails with 409 if a connector with the same - connector_class already exists on the site. + ``connector_class`` already exists on the site. Parameters ---------- connector : AuthFederationConnectorItem ``connector_class`` and ``audience`` are required. + ``connector_class`` must equal the Tableau connection class + exactly — what ``ConnectionItem.connection_type`` reports for the + connector (e.g. ``"bigquery"``, ``"snowflake"``). Class names + are case-sensitive; use the exact lowercase class name of the + connector. Returns ------- @@ -179,7 +214,7 @@ def get_connector(self, connector: ConnectorIdentifier) -> AuthFederationConnect ------- AuthFederationConnectorItem """ - target = getattr(connector, "luid", connector) + target = _coerce_uuid(getattr(connector, "luid", connector)) url = f"{self.baseurl}/connectors/{target}" response = self.get_request(url) return AuthFederationConnectorItem.from_response(response.content, self.parent_srv.namespace) @@ -187,9 +222,14 @@ def get_connector(self, connector: ConnectorIdentifier) -> AuthFederationConnect @api(version="3.30") def delete_connector(self, connector: ConnectorIdentifier) -> None: """ - Delete a WIF connector by LUID. Idempotent: returns normally even if - the connector does not exist. Existing site connections that reference - the removed connector are orphaned, not cascade-deleted. + Delete a WIF connector by LUID. Idempotent on an unknown but + well-formed LUID (the server returns 204 even if no such connector + exists) — but only when WIF is enabled on the site; otherwise the + server returns a WIF-not-enabled error. Client-side UUID validation + rejects a malformed id before any HTTP call. + + Existing site connections that reference the removed connector are + orphaned, not cascade-deleted. Parameters ---------- @@ -197,7 +237,7 @@ def delete_connector(self, connector: ConnectorIdentifier) -> None: The connector LUID, or an ``AuthFederationConnectorItem`` whose ``luid`` is set. """ - target = getattr(connector, "luid", connector) + target = _coerce_uuid(getattr(connector, "luid", connector)) url = f"{self.baseurl}/connectors/{target}" self.delete_request(url) @@ -234,7 +274,7 @@ def activate_key(self, key: KeyIdentifier) -> None: The key ID (``kid``), or an ``AuthFederationKeyItem`` whose ``kid`` is set. """ - target = getattr(key, "kid", key) + target = _coerce_uuid(getattr(key, "kid", key)) url = f"{self.baseurl}/keys/{target}/activate" self.put_request(url) @@ -251,7 +291,7 @@ def deactivate_key(self, key: KeyIdentifier) -> None: The key ID (``kid``), or an ``AuthFederationKeyItem`` whose ``kid`` is set. """ - target = getattr(key, "kid", key) + target = _coerce_uuid(getattr(key, "kid", key)) url = f"{self.baseurl}/keys/{target}/deactivate" self.put_request(url) @@ -267,6 +307,6 @@ def delete_key(self, key: KeyIdentifier) -> None: The key ID (``kid``), or an ``AuthFederationKeyItem`` whose ``kid`` is set. """ - target = getattr(key, "kid", key) + target = _coerce_uuid(getattr(key, "kid", key)) url = f"{self.baseurl}/keys/{target}" self.delete_request(url) diff --git a/tableauserverclient/server/request_factory.py b/tableauserverclient/server/request_factory.py index aa35199d8..2c781a572 100644 --- a/tableauserverclient/server/request_factory.py +++ b/tableauserverclient/server/request_factory.py @@ -105,33 +105,35 @@ def switch_req(self, site_content_url): class AuthFederationRequest: - @_tsrequest_wrapped - def enable_req(self, xml_request: ET.Element, config: "AuthFederationItem") -> bytes: - elem = ET.SubElement(xml_request, "authFederationAdmin") - elem.attrib["enabled"] = str(config.enabled).lower() - if config.token_lifetime_seconds is not None: - elem.attrib["tokenLifetimeSeconds"] = str(config.token_lifetime_seconds) - return ET.tostring(xml_request) - - @_tsrequest_wrapped - def update_req(self, xml_request: ET.Element, config: "AuthFederationItem") -> bytes: - elem = ET.SubElement(xml_request, "authFederationAdmin") - # Only token_lifetime_seconds is mutable on PATCH; issuerUrl, activeKeyId, - # and connectors are read-only and silently ignored by the server. - if config.token_lifetime_seconds is not None: - elem.attrib["tokenLifetimeSeconds"] = str(config.token_lifetime_seconds) - return ET.tostring(xml_request) - - @_tsrequest_wrapped - def create_connector_req(self, xml_request: ET.Element, connector: "AuthFederationConnectorItem") -> bytes: - elem = ET.SubElement(xml_request, "authFederationConnector") + # The WIF admin controller binds the request body directly as + # ``AuthFederationConfigType`` / ``AuthFederationConnectorType`` — a bare + # element, not the usual ```` envelope. These methods therefore + # skip ``_tsrequest_wrapped`` and emit the bare root themselves. + + def enable_req(self, token_lifetime_seconds: int | None = None) -> bytes: + elem = ET.Element("authFederationAdmin") + elem.attrib["enabled"] = "true" + if token_lifetime_seconds is not None: + elem.attrib["tokenLifetimeSeconds"] = str(token_lifetime_seconds) + return ET.tostring(elem) + + def update_req(self, token_lifetime_seconds: int | None = None) -> bytes: + # ``enabled`` must never be sent on PATCH — the server rejects a merge + # patch with ``enabled`` present. Enable/disable goes through PUT/DELETE. + elem = ET.Element("authFederationAdmin") + if token_lifetime_seconds is not None: + elem.attrib["tokenLifetimeSeconds"] = str(token_lifetime_seconds) + return ET.tostring(elem) + + def create_connector_req(self, connector: "AuthFederationConnectorItem") -> bytes: if connector.connector_class is None: raise ValueError(f"AuthFederationConnectorItem missing connector_class: {connector}") if connector.audience is None: raise ValueError(f"AuthFederationConnectorItem missing audience: {connector}") + elem = ET.Element("authFederationConnector") elem.attrib["connectorClass"] = connector.connector_class elem.attrib["audience"] = connector.audience - return ET.tostring(xml_request) + return ET.tostring(elem) class ColumnRequest: diff --git a/test/test_auth_federation.py b/test/test_auth_federation.py index bd3d38629..ac2b770de 100644 --- a/test/test_auth_federation.py +++ b/test/test_auth_federation.py @@ -48,33 +48,37 @@ def test_auth_federation_get(server: TSC.Server) -> None: def test_auth_federation_get_defaults_when_never_enabled(server: TSC.Server) -> None: + # Server GET on a never-enabled site still returns ``issuerUrl`` and the + # default ``tokenLifetimeSeconds``; only ``activeKeyId`` and the + # ```` wrapper are absent. default_body = ( - '' '' "" + '' + '' + "" ) with requests_mock.mock() as m: m.get(server.auth_federation.baseurl, text=default_body) config = server.auth_federation.get() assert config.enabled is False - assert config.issuer_url is None + assert config.issuer_url == "https://10ax.online.tableau.com/sites/mySite/auth-federation" assert config.active_key_id is None - assert config.token_lifetime_seconds is None + assert config.token_lifetime_seconds == 3600 assert config.connectors == [] def test_auth_federation_enable(server: TSC.Server) -> None: - config_in = TSC.AuthFederationItem() - config_in.enabled = True - config_in.token_lifetime_seconds = 3600 - with requests_mock.mock() as m: m.put(server.auth_federation.baseurl, text=ENABLE.read_text()) - config_out = server.auth_federation.enable(config_in) + config_out = server.auth_federation.enable(token_lifetime_seconds=3600) body = m.request_history[0].text - sent = fromstring(body) - elem = sent.find("authFederationAdmin") - assert elem is not None + # WIF admin controller binds the body as a bare AuthFederationConfigType; + # the request MUST NOT be wrapped in . + elem = fromstring(body) + assert elem.tag == "authFederationAdmin" assert elem.get("enabled") == "true" assert elem.get("tokenLifetimeSeconds") == "3600" assert elem.find("connectors") is None @@ -85,50 +89,52 @@ def test_auth_federation_enable(server: TSC.Server) -> None: def test_auth_federation_enable_without_token_lifetime(server: TSC.Server) -> None: - config_in = TSC.AuthFederationItem() - config_in.enabled = True - with requests_mock.mock() as m: m.put(server.auth_federation.baseurl, text=ENABLE.read_text()) - server.auth_federation.enable(config_in) + server.auth_federation.enable() body = m.request_history[0].text - elem = fromstring(body).find("authFederationAdmin") - assert elem is not None + elem = fromstring(body) + assert elem.tag == "authFederationAdmin" assert elem.get("enabled") == "true" assert "tokenLifetimeSeconds" not in elem.attrib -def test_auth_federation_enable_rejects_disabled(server: TSC.Server) -> None: - config_in = TSC.AuthFederationItem() - config_in.enabled = False - - with pytest.raises(ValueError, match="enabled=True"): - server.auth_federation.enable(config_in) - - def test_auth_federation_update(server: TSC.Server) -> None: - config_in = TSC.AuthFederationItem() - config_in.token_lifetime_seconds = 7200 - config_in.issuer_url = "ignored-by-server" - config_in.active_key_id = "ignored-by-server" - with requests_mock.mock() as m: m.patch(server.auth_federation.baseurl, text=UPDATE.read_text()) - config_out = server.auth_federation.update(config_in) + config_out = server.auth_federation.update(token_lifetime_seconds=7200) body = m.request_history[0].text - elem = fromstring(body).find("authFederationAdmin") - assert elem is not None + elem = fromstring(body) + assert elem.tag == "authFederationAdmin" assert elem.get("tokenLifetimeSeconds") == "7200" - # Read-only fields must not be sent; the server would ignore them anyway, - # but we don't want the client to pretend they're mutable. + # ``enabled`` must never be sent on PATCH — the server rejects a merge + # patch that includes it. + assert "enabled" not in elem.attrib + # Read-only fields must not be sent either. assert "issuerUrl" not in elem.attrib assert "activeKeyId" not in elem.attrib assert config_out.token_lifetime_seconds == 7200 +def test_auth_federation_update_empty_patch(server: TSC.Server) -> None: + # This exercises the client wire shape only. Whether the server binds an + # empty ```` as a non-null ``AuthFederationConfigType`` + # with every field null (a no-op merge-patch) or as a null body (400) must + # be covered by a server-side integration test — not here. + with requests_mock.mock() as m: + m.patch(server.auth_federation.baseurl, text=UPDATE.read_text()) + server.auth_federation.update() + body = m.request_history[0].text + + elem = fromstring(body) + assert elem.tag == "authFederationAdmin" + assert "enabled" not in elem.attrib + assert "tokenLifetimeSeconds" not in elem.attrib + + def test_auth_federation_delete(server: TSC.Server) -> None: with requests_mock.mock() as m: m.delete(server.auth_federation.baseurl, status_code=204) @@ -150,8 +156,8 @@ def test_auth_federation_create_connector(server: TSC.Server) -> None: connector_out = server.auth_federation.create_connector(connector_in) body = m.request_history[0].text - elem = fromstring(body).find("authFederationConnector") - assert elem is not None + elem = fromstring(body) + assert elem.tag == "authFederationConnector" assert elem.get("connectorClass") == "bigquery" assert elem.get("audience") == connector_in.audience assert "luid" not in elem.attrib @@ -285,6 +291,17 @@ def test_auth_federation_deactivate_key(server: TSC.Server) -> None: assert m.called_once +def test_auth_federation_deactivate_key_by_item(server: TSC.Server) -> None: + stub = TSC.AuthFederationKeyItem() + stub.kid = KEY_KID + + with requests_mock.mock() as m: + m.put(f"{server.auth_federation.baseurl}/keys/{KEY_KID}/deactivate", status_code=204) + server.auth_federation.deactivate_key(stub) + + assert m.called_once + + def test_auth_federation_delete_key(server: TSC.Server) -> None: with requests_mock.mock() as m: m.delete(f"{server.auth_federation.baseurl}/keys/{KEY_KID}", status_code=204) @@ -293,7 +310,62 @@ def test_auth_federation_delete_key(server: TSC.Server) -> None: assert m.called_once -def test_auth_federation_requires_server_version_3_30(server: TSC.Server) -> None: +def test_auth_federation_delete_key_by_item(server: TSC.Server) -> None: + stub = TSC.AuthFederationKeyItem() + stub.kid = KEY_KID + + with requests_mock.mock() as m: + m.delete(f"{server.auth_federation.baseurl}/keys/{KEY_KID}", status_code=204) + server.auth_federation.delete_key(stub) + + assert m.called_once + + +# -- UUID path-injection guard ------------------------------------------------- + +_UUID_METHODS = [ + "get_connector", + "delete_connector", + "activate_key", + "deactivate_key", + "delete_key", +] + + +@pytest.mark.parametrize("method_name", _UUID_METHODS) +@pytest.mark.parametrize("bad_target", ["../../admin", "not-a-uuid", "a4b9c2d7", "", "d3a8e5b2-7c01-4e13-9c82"]) +def test_auth_federation_rejects_non_uuid_target(server: TSC.Server, method_name: str, bad_target: str) -> None: + method = getattr(server.auth_federation, method_name) + with requests_mock.mock() as m: + m.register_uri(requests_mock.ANY, requests_mock.ANY) + with pytest.raises(ValueError, match="Invalid UUID"): + method(bad_target) + assert not m.called # must reject before any HTTP call + + +# -- Version gate -------------------------------------------------------------- + +_V3_30_METHODS = [ + ("get", (), {}), + ("enable", (), {"token_lifetime_seconds": 3600}), + ("update", (), {"token_lifetime_seconds": 7200}), + ("delete", (), {}), + ("create_connector", (TSC.AuthFederationConnectorItem(),), {}), + ("list_connectors", (), {}), + ("get_connector", (CONNECTOR_LUID,), {}), + ("delete_connector", (CONNECTOR_LUID,), {}), + ("create_key", (), {}), + ("activate_key", (KEY_KID,), {}), + ("deactivate_key", (KEY_KID,), {}), + ("delete_key", (KEY_KID,), {}), +] + + +@pytest.mark.parametrize("method_name,args,kwargs", _V3_30_METHODS, ids=[m[0] for m in _V3_30_METHODS]) +def test_auth_federation_requires_server_version_3_30( + server: TSC.Server, method_name: str, args: tuple, kwargs: dict +) -> None: server.version = "3.29" + method = getattr(server.auth_federation, method_name) with pytest.raises(EndpointUnavailableError): - server.auth_federation.get() + method(*args, **kwargs) From 4778874d4d2c6f981dc98640b33dfa88c7aefab0 Mon Sep 17 00:00:00 2001 From: Sushma Sreedharan Date: Wed, 21 Oct 2026 10:35:38 -0700 Subject: [PATCH 06/10] Correct 403/404 and PAT semantics; add e2e round-trip test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up on PR #1894. - 403/404 docstring and CHANGELOG rewritten per server behavior: authorizationCheck() runs requireWifFeatureEnabled first, so every method (incl. get()) returns 403 when the per-site WIF feature flag is off — even for a site admin. 404 holds only for update() and the four connector methods; delete() on a never-enabled site is a no-op; create_key() has no enable() gate and leaves an orphan key against the per-site key limit; activate/deactivate/delete_key 404 only on unknown kid. - PAT wording corrected: NO_ACCESS rejects only OAuth-scope-restricted sessions (e.g. connected-app JWT) with 401. PAT and password sessions do not carry scopes and are accepted — this is what the repo's own e2e harness relies on. - delete_key rotation guidance: deactivating the active key with no replacement leaves the site unable to mint tokens. Safe order is create_key() -> activate_key(new) -> delete_key(old). - New test_e2e/test_auth_federation.py: PAT-signed-in round trip across every method. Primarily a wire-shape regression test for the three bare-body routes (PUT/PATCH /admin, POST /admin/connectors) that mocked unit tests cannot catch. Guards against clobbering a live WIF config and tears down to a clean site on exit. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 17 ++- .../endpoint/auth_federation_endpoint.py | 38 +++-- test_e2e/test_auth_federation.py | 132 ++++++++++++++++++ 3 files changed, 169 insertions(+), 18 deletions(-) create mode 100644 test_e2e/test_auth_federation.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 6fcf193ad..8a2cad09b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,15 +38,18 @@ `server.auth_federation.delete_key(id)` accept either a `kid` string or an `AuthFederationKeyItem` with `kid` set; `activate_key` is atomic and deactivates any previously active key. `delete_key` fails with 400 - if the key is currently active — call `deactivate_key` first. The four - key-management methods are bound to `NO_ACCESS` server-side, which - rejects every OAuth-scoped session unconditionally (including every PAT - session, regardless of granted scopes); authenticate with a - username/password session. + if the key is currently active. Safe rotation is `create_key()` → + `activate_key(new_kid)` → `delete_key(old_kid)`; `deactivate_key` on + the active key with no replacement leaves the site unable to mint WIF + tokens. The four key-management methods are bound to `NO_ACCESS` + server-side, which only rejects OAuth-scope-restricted sessions + (e.g. connected-app JWT) with 401; password and personal-access-token + (PAT) sessions do not carry scopes and are accepted. Three new exports: `AuthFederationItem`, `AuthFederationConnectorItem`, `AuthFederationKeyItem`. Requires Tableau Server REST API 3.30+ - (Tableau Server 2026.3) and the Site Administrator role on the current - site; non-admin callers receive HTTP 403. + (Tableau Server 2026.3), the WIF feature enabled on the site, and the + Site Administrator role on the current site; the server returns 403 if + any of those is missing (including on `get()`). * Added Workload Identity Federation (WIF) support to the connection update endpoints. `ConnectionItem` has two new string properties, `workgroup_auth_mode` and `auth_federation_service_account`, that are diff --git a/tableauserverclient/server/endpoint/auth_federation_endpoint.py b/tableauserverclient/server/endpoint/auth_federation_endpoint.py index 6f0954d8e..394d951ef 100644 --- a/tableauserverclient/server/endpoint/auth_federation_endpoint.py +++ b/tableauserverclient/server/endpoint/auth_federation_endpoint.py @@ -48,18 +48,28 @@ class AuthFederation(Endpoint): connectors (BigQuery, Snowflake, ...) that each JWT is scoped to. Requires REST API version 3.30 (Tableau Server 2026.3) or later. - All methods require the caller to be a Site Administrator on the current - site; the server returns HTTP 403 (``ServerResponseError``) for non-admin - callers. Beyond ``get()`` and ``enable()``, every method also requires - WIF to be enabled on the site — call ``enable()`` first, otherwise the - server returns HTTP 404. + All methods require the WIF feature to be enabled on the site AND the + caller to be a Site Administrator; the server returns HTTP 403 + (``ServerResponseError``) when either is missing — including on + ``get()``, so a site admin on a site where WIF has not been turned on + will see 403. + + Once WIF is available on the site, ``get()`` works immediately (returns + a shell with ``enabled=False`` until the admin runs ``enable()``). + ``update()`` and the four connector methods return HTTP 404 until + ``enable()`` has run; ``delete()`` on a never-enabled site is a no-op. + ``create_key()`` is not gated by ``enable()`` and will create an orphan + key against the per-site key limit if called first; + ``activate_key``/``deactivate_key``/``delete_key`` return HTTP 404 only + if the ``kid`` does not exist on the site. The four key-management methods (``create_key``, ``activate_key``, ``deactivate_key``, ``delete_key``) are bound to ``OAuthScopes.NO_ACCESS`` - server-side, which means every OAuth-scoped session is rejected - unconditionally — including every PAT session, regardless of granted - scopes. Authenticate with a username/password session to call them. The - config and connector methods accept the + server-side, which only rejects OAuth-scope-restricted sessions + (e.g. a connected-app JWT session) — the server returns HTTP 401, + surfaced by TSC as ``FailedSignInError``. Password sessions and + personal-access-token (PAT) sessions do not carry scopes and are + accepted. The config and connector methods additionally accept the ``tableau:connectivity_auth_federation:*`` OAuth scopes. ``delete()`` permanently removes the config, every signing key, and every @@ -298,8 +308,14 @@ def deactivate_key(self, key: KeyIdentifier) -> None: @api(version="3.30") def delete_key(self, key: KeyIdentifier) -> None: """ - Delete a WIF signing key. Fails with 400 if the key is active; - deactivate it first. + Delete a WIF signing key. Fails with 400 if the key is active. + + For rotation, call ``create_key()`` → ``activate_key(new_kid)`` → + ``delete_key(old_kid)``: ``activate_key`` atomically deactivates the + previously active key, so the site is never without an active signing + key. Calling ``deactivate_key`` on the active key before another is + activated leaves the site with no active key, and no new WIF tokens + can be minted until a replacement is activated. Parameters ---------- diff --git a/test_e2e/test_auth_federation.py b/test_e2e/test_auth_federation.py new file mode 100644 index 000000000..67431b866 --- /dev/null +++ b/test_e2e/test_auth_federation.py @@ -0,0 +1,132 @@ +""" +E2E tests for Workload Identity Federation (WIF) admin REST API. + +Covers the bare-body wire shape that unit tests cannot: `enable()`, +`update()`, and `create_connector()` send bare XML roots (no `` +envelope) because the admin controller binds `@RequestBody ` +rather than `@RequestBody TsRequest`. If the server is later aligned to its +published 3.30 schema (which nests these elements under `tsRequest`), +`enable()` / `create_connector()` would start returning HTTP 400 and +`update()` would silently become a no-op — mocked unit tests cannot catch +either regression. + +Run with: + TABLEAU_SERVER=https://... TABLEAU_SITE=mysite \\ + TABLEAU_TOKEN_NAME=... TABLEAU_TOKEN=... \\ + pytest test_e2e/test_auth_federation.py -v + +Requires: + * Tableau Server 2026.3+ (REST API 3.30+). + * The `WorkloadIdentityFederation` feature flag enabled for the site. + * The signed-in user to have the Site Administrator role. + * WIF currently NOT enabled on the site — the test refuses to run if + WIF is already enabled, to avoid destroying an existing configuration. +""" + +import pytest + +import tableauserverclient as TSC +from tableauserverclient import ServerResponseError + +pytestmark = pytest.mark.e2e + +TEST_CONNECTOR_CLASS = "bigquery" +TEST_AUDIENCE = ( + "//iam.googleapis.com/projects/0/locations/global/workloadIdentityPools/" "tsc-e2e-pool/providers/tsc-e2e-provider" +) + + +@pytest.fixture(scope="module") +def wif_site(server): + """Guard against clobbering a live WIF config, then yield the server. + + Teardown deletes the entire WIF config (keys + connectors) so the site + is left in the pre-test state. Fails loudly if the feature flag is off + (403) or if WIF is already enabled, rather than silently destroying + production state. + """ + try: + config = server.auth_federation.get() + except ServerResponseError as exc: + if getattr(exc, "code", None) == "403" or "403" in str(exc): + pytest.skip("WIF feature flag not enabled for this site") + raise + + if config.enabled: + pytest.skip( + "WIF is already enabled on this site — refusing to run destructive E2E. " + "Point TABLEAU_SITE at a site where WIF has not been enabled." + ) + + yield server + + try: + server.auth_federation.delete() + except ServerResponseError: + pass + + +def test_wif_round_trip_bare_body_wire_contract(wif_site): + """End-to-end round-trip exercising every method. + + Primarily a wire-shape regression test for the three bare-body routes: + PUT /auth-federation/admin, PATCH /auth-federation/admin, and + POST /auth-federation/admin/connectors. Each would surface as HTTP 400 + (enable, create_connector) or a silent no-op (update) if the server + tightened its binding to the nested-under- shape. + """ + server = wif_site + + # --- enable: PUT with bare + config = server.auth_federation.enable(token_lifetime_seconds=3600) + assert config.enabled is True + assert config.token_lifetime_seconds == 3600 + assert config.issuer_url is not None + # First enable mints exactly one active key. + assert config.active_key_id is not None + initial_active_kid = config.active_key_id + + # --- update: PATCH with bare + # (no `enabled` attribute — the server 400s if PATCH carries `enabled`). + updated = server.auth_federation.update(token_lifetime_seconds=7200) + assert updated.token_lifetime_seconds == 7200 + assert updated.enabled is True + + # --- create_connector: POST with bare + new_connector = TSC.AuthFederationConnectorItem() + new_connector.connector_class = TEST_CONNECTOR_CLASS + new_connector.audience = TEST_AUDIENCE + created = server.auth_federation.create_connector(new_connector) + assert created.luid is not None + assert created.connector_class == TEST_CONNECTOR_CLASS + assert created.audience == TEST_AUDIENCE + + # --- list + get connector + connectors = server.auth_federation.list_connectors() + assert any(c.luid == created.luid for c in connectors) + + fetched = server.auth_federation.get_connector(created.luid) + assert fetched.luid == created.luid + assert fetched.connector_class == TEST_CONNECTOR_CLASS + + # --- key lifecycle: create → activate (swaps active) → deactivate → delete. + # Per-site key limit is 3; one slot is held by the auto-minted key. + new_key = server.auth_federation.create_key() + assert new_key.kid is not None + assert new_key.kid != initial_active_kid + + server.auth_federation.activate_key(new_key.kid) + after_activate = server.auth_federation.get() + assert after_activate.active_key_id == new_key.kid + + # The previously active key is now inactive and safe to delete. + server.auth_federation.delete_key(initial_active_kid) + + # Deactivate the current active key, then delete it. + server.auth_federation.deactivate_key(new_key.kid) + server.auth_federation.delete_key(new_key.kid) + + # --- delete connector (idempotent on well-formed-but-unknown LUIDs) + server.auth_federation.delete_connector(created.luid) + remaining = server.auth_federation.list_connectors() + assert all(c.luid != created.luid for c in remaining) From b9db79979cdd54b60e5cdec9ace2841618352d3c Mon Sep 17 00:00:00 2001 From: Sushma Sreedharan Date: Wed, 21 Oct 2026 10:54:28 -0700 Subject: [PATCH 07/10] Tighten AuthFederation tests + require update() lifetime arg MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review nits from Wayne Xie's second pass on PR #1894: - Realism in the "never-enabled" GET fixture: use the real server defaults — token lifetime 7200 (AuthFederationService.java:86) and the actual issuer URL shape %s/api/%s/sites//auth-federation (:748-766), not the fabricated /sites/mySite/... URL and 3600 lifetime. - UUID-guard matrix: add None to the bad-target list and parametrize over {raw, item} so the item-wrapped None path (connector.luid=None / key.kid=None) is also covered. Widens bad_target to object. - update() token_lifetime_seconds is now required: there is no other mutable field on the config, so a no-arg call is not a meaningful operation. Tightens the matching request_factory signature and drops the test that exercised the empty-patch wire shape. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 6 +- .../endpoint/auth_federation_endpoint.py | 9 +-- tableauserverclient/server/request_factory.py | 5 +- test/test_auth_federation.py | 57 +++++++++++-------- 4 files changed, 43 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a2cad09b..b0cf1bda4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,10 +11,10 @@ - `server.auth_federation.enable(token_lifetime_seconds=None)` -> `AuthFederationItem` — idempotent PUT. Server auto-generates one active signing key on first enable. - - `server.auth_federation.update(token_lifetime_seconds=None)` -> + - `server.auth_federation.update(token_lifetime_seconds)` -> `AuthFederationItem` — merge-patch; only `token_lifetime_seconds` is - mutable. `enabled` is never sent on PATCH; use `enable()` / `delete()` - to change WIF state. + mutable (required), so `update()` always changes it. `enabled` is never + sent on PATCH; use `enable()` / `delete()` to change WIF state. - `server.auth_federation.delete()` — permanently deletes the WIF config along with every signing key and connector on the site. A subsequent `enable()` starts from a clean slate; prior connectors and keys are diff --git a/tableauserverclient/server/endpoint/auth_federation_endpoint.py b/tableauserverclient/server/endpoint/auth_federation_endpoint.py index 394d951ef..e5b980e1d 100644 --- a/tableauserverclient/server/endpoint/auth_federation_endpoint.py +++ b/tableauserverclient/server/endpoint/auth_federation_endpoint.py @@ -130,7 +130,7 @@ def enable(self, token_lifetime_seconds: int | None = None) -> AuthFederationIte return AuthFederationItem.from_response(response.content, self.parent_srv.namespace) @api(version="3.30") - def update(self, token_lifetime_seconds: int | None = None) -> AuthFederationItem: + def update(self, token_lifetime_seconds: int) -> AuthFederationItem: """ Merge-patch the mutable fields of the WIF admin config. Only ``token_lifetime_seconds`` is mutable; ``enabled``, ``issuer_url``, @@ -141,9 +141,10 @@ def update(self, token_lifetime_seconds: int | None = None) -> AuthFederationIte Parameters ---------- - token_lifetime_seconds : int | None - New JWT lifetime in seconds. ``None`` sends an empty patch (no - fields changed) — rarely useful, but accepted by the server. + token_lifetime_seconds : int + New JWT lifetime in seconds. Required — there is nothing else to + patch, so a call with no fields to change is not a meaningful + operation. Returns ------- diff --git a/tableauserverclient/server/request_factory.py b/tableauserverclient/server/request_factory.py index 2c781a572..517a26956 100644 --- a/tableauserverclient/server/request_factory.py +++ b/tableauserverclient/server/request_factory.py @@ -117,12 +117,11 @@ def enable_req(self, token_lifetime_seconds: int | None = None) -> bytes: elem.attrib["tokenLifetimeSeconds"] = str(token_lifetime_seconds) return ET.tostring(elem) - def update_req(self, token_lifetime_seconds: int | None = None) -> bytes: + def update_req(self, token_lifetime_seconds: int) -> bytes: # ``enabled`` must never be sent on PATCH — the server rejects a merge # patch with ``enabled`` present. Enable/disable goes through PUT/DELETE. elem = ET.Element("authFederationAdmin") - if token_lifetime_seconds is not None: - elem.attrib["tokenLifetimeSeconds"] = str(token_lifetime_seconds) + elem.attrib["tokenLifetimeSeconds"] = str(token_lifetime_seconds) return ET.tostring(elem) def create_connector_req(self, connector: "AuthFederationConnectorItem") -> bytes: diff --git a/test/test_auth_federation.py b/test/test_auth_federation.py index ac2b770de..da282ff7c 100644 --- a/test/test_auth_federation.py +++ b/test/test_auth_federation.py @@ -50,12 +50,17 @@ def test_auth_federation_get(server: TSC.Server) -> None: def test_auth_federation_get_defaults_when_never_enabled(server: TSC.Server) -> None: # Server GET on a never-enabled site still returns ``issuerUrl`` and the # default ``tokenLifetimeSeconds``; only ``activeKeyId`` and the - # ```` wrapper are absent. + # ```` wrapper are absent. Values mirror the real server: + # DEFAULT_TOKEN_LIFETIME_SECONDS=7200 (AuthFederationService.java:86) and + # ``/api//sites//auth-federation`` (:748-766). + default_issuer_url = ( + "https://10ax.online.tableau.com/api/3.30/sites/" "dad65087-b08b-4603-af4e-2887b8aafc67/auth-federation" + ) default_body = ( '' '' + f'issuerUrl="{default_issuer_url}" ' + 'tokenLifetimeSeconds="7200"/>' "" ) with requests_mock.mock() as m: @@ -63,9 +68,9 @@ def test_auth_federation_get_defaults_when_never_enabled(server: TSC.Server) -> config = server.auth_federation.get() assert config.enabled is False - assert config.issuer_url == "https://10ax.online.tableau.com/sites/mySite/auth-federation" + assert config.issuer_url == default_issuer_url assert config.active_key_id is None - assert config.token_lifetime_seconds == 3600 + assert config.token_lifetime_seconds == 7200 assert config.connectors == [] @@ -119,22 +124,6 @@ def test_auth_federation_update(server: TSC.Server) -> None: assert config_out.token_lifetime_seconds == 7200 -def test_auth_federation_update_empty_patch(server: TSC.Server) -> None: - # This exercises the client wire shape only. Whether the server binds an - # empty ```` as a non-null ``AuthFederationConfigType`` - # with every field null (a no-op merge-patch) or as a null body (400) must - # be covered by a server-side integration test — not here. - with requests_mock.mock() as m: - m.patch(server.auth_federation.baseurl, text=UPDATE.read_text()) - server.auth_federation.update() - body = m.request_history[0].text - - elem = fromstring(body) - assert elem.tag == "authFederationAdmin" - assert "enabled" not in elem.attrib - assert "tokenLifetimeSeconds" not in elem.attrib - - def test_auth_federation_delete(server: TSC.Server) -> None: with requests_mock.mock() as m: m.delete(server.auth_federation.baseurl, status_code=204) @@ -330,16 +319,36 @@ def test_auth_federation_delete_key_by_item(server: TSC.Server) -> None: "deactivate_key", "delete_key", ] +_KEY_METHODS = {"activate_key", "deactivate_key", "delete_key"} +@pytest.mark.parametrize("wrap_as_item", [False, True], ids=["raw", "item"]) @pytest.mark.parametrize("method_name", _UUID_METHODS) -@pytest.mark.parametrize("bad_target", ["../../admin", "not-a-uuid", "a4b9c2d7", "", "d3a8e5b2-7c01-4e13-9c82"]) -def test_auth_federation_rejects_non_uuid_target(server: TSC.Server, method_name: str, bad_target: str) -> None: +@pytest.mark.parametrize( + "bad_target", + [None, "../../admin", "not-a-uuid", "a4b9c2d7", "", "d3a8e5b2-7c01-4e13-9c82"], + ids=["None", "path", "noise", "short", "empty", "truncated"], +) +def test_auth_federation_rejects_non_uuid_target( + server: TSC.Server, method_name: str, bad_target: object, wrap_as_item: bool +) -> None: + if wrap_as_item: + if method_name in _KEY_METHODS: + item = TSC.AuthFederationKeyItem() + item.kid = bad_target # type: ignore[assignment] + target: object = item + else: + connector = TSC.AuthFederationConnectorItem() + connector.luid = bad_target # type: ignore[assignment] + target = connector + else: + target = bad_target + method = getattr(server.auth_federation, method_name) with requests_mock.mock() as m: m.register_uri(requests_mock.ANY, requests_mock.ANY) with pytest.raises(ValueError, match="Invalid UUID"): - method(bad_target) + method(target) assert not m.called # must reject before any HTTP call From f71cf38be912a34732aaddabe9249909cc1922b6 Mon Sep 17 00:00:00 2001 From: Sushma Sreedharan Date: Wed, 21 Oct 2026 15:41:44 -0700 Subject: [PATCH 08/10] Address Copilot review comments on WIF admin PR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - E2E teardown: let `ServerResponseError` from `auth_federation.delete()` propagate instead of swallowing it. A failed teardown left WIF enabled on the site, so the next run would hit the fixture's `enabled=True` guard and silently skip — the destructive paths stopped being exercised without anyone noticing. - Key-rotation guidance (endpoint docstring, CHANGELOG, and docs): the previous `create_key() -> activate_key(new) -> delete_key(old)` sequence omitted that the old key must stay in the JWKS until every JWT it signed has expired. Deleting it immediately removes it from the key set, and verifiers that fetch JWKS afterwards reject the still-valid tokens signed with it. New guidance keeps the old key inactive (but published) for up to `token_lifetime_seconds` before deleting. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 10 +++++++--- .../endpoint/auth_federation_endpoint.py | 19 +++++++++++++------ test_e2e/test_auth_federation.py | 9 +++++---- 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b0cf1bda4..5f87359ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,9 +39,13 @@ or an `AuthFederationKeyItem` with `kid` set; `activate_key` is atomic and deactivates any previously active key. `delete_key` fails with 400 if the key is currently active. Safe rotation is `create_key()` → - `activate_key(new_kid)` → `delete_key(old_kid)`; `deactivate_key` on - the active key with no replacement leaves the site unable to mint WIF - tokens. The four key-management methods are bound to `NO_ACCESS` + `activate_key(new_kid)`, then retain the old (now-inactive) key in the + site's JWKS until every JWT it signed has expired (upper bound: the + site's current `token_lifetime_seconds`), and only then + `delete_key(old_kid)` — deleting it earlier removes it from the JWKS + and any verifier fetching the key set afterwards will reject still- + unexpired tokens signed with it. `deactivate_key` on the active key + with no replacement leaves the site unable to mint WIF tokens. The four key-management methods are bound to `NO_ACCESS` server-side, which only rejects OAuth-scope-restricted sessions (e.g. connected-app JWT) with 401; password and personal-access-token (PAT) sessions do not carry scopes and are accepted. diff --git a/tableauserverclient/server/endpoint/auth_federation_endpoint.py b/tableauserverclient/server/endpoint/auth_federation_endpoint.py index e5b980e1d..9074ecfa2 100644 --- a/tableauserverclient/server/endpoint/auth_federation_endpoint.py +++ b/tableauserverclient/server/endpoint/auth_federation_endpoint.py @@ -311,12 +311,19 @@ def delete_key(self, key: KeyIdentifier) -> None: """ Delete a WIF signing key. Fails with 400 if the key is active. - For rotation, call ``create_key()`` → ``activate_key(new_kid)`` → - ``delete_key(old_kid)``: ``activate_key`` atomically deactivates the - previously active key, so the site is never without an active signing - key. Calling ``deactivate_key`` on the active key before another is - activated leaves the site with no active key, and no new WIF tokens - can be minted until a replacement is activated. + For rotation, call ``create_key()`` → ``activate_key(new_kid)``, then + **retain** the old (now-inactive) key in the site's JWKS until every + JWT it signed has expired (upper bound: the site's current + ``token_lifetime_seconds``, default 7200 s / 2 h), and only then + call ``delete_key(old_kid)``. ``activate_key`` atomically deactivates + the previously active key, so the site is never without an active + signing key — but the inactive key stays published in the JWKS so + verifiers can keep validating tokens it already signed. Deleting it + before those tokens have expired removes it from the JWKS, and any + verifier that fetches the key set afterwards will reject the still- + unexpired tokens. Calling ``deactivate_key`` on the active key + before another is activated leaves the site with no active key, and + no new WIF tokens can be minted until a replacement is activated. Parameters ---------- diff --git a/test_e2e/test_auth_federation.py b/test_e2e/test_auth_federation.py index 67431b866..e5035e4e2 100644 --- a/test_e2e/test_auth_federation.py +++ b/test_e2e/test_auth_federation.py @@ -60,10 +60,11 @@ def wif_site(server): yield server - try: - server.auth_federation.delete() - except ServerResponseError: - pass + # Let any ServerResponseError propagate: a failed teardown delete must + # surface as a pytest teardown error. Swallowing it would leave WIF + # enabled on the site, so the next run would skip (fixture guard above) + # and silently stop exercising the destructive paths. + server.auth_federation.delete() def test_wif_round_trip_bare_body_wire_contract(wif_site): From e487e0cd976335ad70f0e1b95ba544f2bdb0179a Mon Sep 17 00:00:00 2001 From: Sushma Sreedharan Date: Wed, 21 Oct 2026 15:42:39 -0700 Subject: [PATCH 09/10] Remove rationale comment from WIF e2e teardown The behavior is self-evident from a bare `delete()` with no try/except; the rationale belongs in the PR / commit history, not the test body. Co-Authored-By: Claude Opus 4.7 (1M context) --- test_e2e/test_auth_federation.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test_e2e/test_auth_federation.py b/test_e2e/test_auth_federation.py index e5035e4e2..bfe078601 100644 --- a/test_e2e/test_auth_federation.py +++ b/test_e2e/test_auth_federation.py @@ -60,10 +60,6 @@ def wif_site(server): yield server - # Let any ServerResponseError propagate: a failed teardown delete must - # surface as a pytest teardown error. Swallowing it would leave WIF - # enabled on the site, so the next run would skip (fixture guard above) - # and silently stop exercising the destructive paths. server.auth_federation.delete() From b7fce309f31b42067e805cd2ecacbc686bba262b Mon Sep 17 00:00:00 2001 From: Sushma Sreedharan Date: Wed, 21 Oct 2026 16:14:06 -0700 Subject: [PATCH 10/10] Rework key-lifecycle docstrings per PR #1894 review - deactivate_key: absorb the "deactivate on the active key with no replacement leaves the site unable to mint" warning, which belongs on deactivate_key itself (not delete_key). - delete_key: drop the create_key / activate_key chain from the rotation guidance. Reframe the docstring around what distinguishes delete from deactivate: both stop new signing with the key, but delete also pulls it from the JWKS, so still-unexpired tokens signed with it stop verifying. Rotation guidance now reduces to "keep the old key deactivated-but-published until its tokens expire, then delete". Co-Authored-By: Claude Opus 4.7 (1M context) --- .../endpoint/auth_federation_endpoint.py | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/tableauserverclient/server/endpoint/auth_federation_endpoint.py b/tableauserverclient/server/endpoint/auth_federation_endpoint.py index 9074ecfa2..3fa96f703 100644 --- a/tableauserverclient/server/endpoint/auth_federation_endpoint.py +++ b/tableauserverclient/server/endpoint/auth_federation_endpoint.py @@ -295,6 +295,9 @@ def deactivate_key(self, key: KeyIdentifier) -> None: Deactivate the given key. A deactivated key stays on the site (and in the public JWKS) so that already-issued JWTs can still be verified during their validity window, but no new JWTs will be signed with it. + Deactivating the active key before another is activated leaves the + site with no active key, and no new WIF tokens can be minted until + a replacement is activated. Parameters ---------- @@ -311,19 +314,14 @@ def delete_key(self, key: KeyIdentifier) -> None: """ Delete a WIF signing key. Fails with 400 if the key is active. - For rotation, call ``create_key()`` → ``activate_key(new_kid)``, then - **retain** the old (now-inactive) key in the site's JWKS until every - JWT it signed has expired (upper bound: the site's current - ``token_lifetime_seconds``, default 7200 s / 2 h), and only then - call ``delete_key(old_kid)``. ``activate_key`` atomically deactivates - the previously active key, so the site is never without an active - signing key — but the inactive key stays published in the JWKS so - verifiers can keep validating tokens it already signed. Deleting it - before those tokens have expired removes it from the JWKS, and any - verifier that fetches the key set afterwards will reject the still- - unexpired tokens. Calling ``deactivate_key`` on the active key - before another is activated leaves the site with no active key, and - no new WIF tokens can be minted until a replacement is activated. + Unlike ``deactivate_key``, which keeps the (now-inactive) key + published in the site's JWKS so that already-issued JWTs can still + be verified, ``delete_key`` removes the key from the JWKS entirely — + any verifier that fetches the key set afterwards will reject still- + unexpired tokens signed with it. For a safe rotation, keep the old + key deactivated-but-published until every JWT it signed has expired + (upper bound: the site's current ``token_lifetime_seconds``, default + 7200 s / 2 h), and only then delete it. Parameters ----------