diff --git a/CHANGELOG.md b/CHANGELOG.md
index 943436b27..1a9173680 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,14 @@
hierarchy path (e.g. `"Marketing/Q1 Reports"`). The walk is performed level by
level using the REST API name filter, so a path with *n* components issues *n*
requests. Returns the matching `ProjectItem` or `None` if no project is found.
+* Preserve HTTP method and body across 3xx redirects. Previously `requests`
+ followed 301/302/303 by converting POST to GET and dropping the body, so
+ endpoints like `users.add`, `workbooks.publish`, and any write hitting a
+ server behind a redirect would 405. TSC now disables `requests`'s
+ auto-redirect and walks the chain manually, up to `session.max_redirects`
+ hops (default 30). Refuses HTTPS -> HTTP scheme downgrades and raises
+ `RedirectError` with a clear message on missing `Location` headers or hop
+ overflow. Fixes #1127 and #1828.
## 0.18.0 (6 April 2022)
* Switched to using defused_xml for xml attack protection
diff --git a/tableauserverclient/server/endpoint/endpoint.py b/tableauserverclient/server/endpoint/endpoint.py
index 31a0806dc..58529dde0 100644
--- a/tableauserverclient/server/endpoint/endpoint.py
+++ b/tableauserverclient/server/endpoint/endpoint.py
@@ -3,6 +3,7 @@
import os
from contextlib import closing
from typing_extensions import Concatenate, ParamSpec
+from urllib.parse import urljoin, urlparse
from tableauserverclient import datetime_helpers as datetime
import abc
@@ -30,6 +31,7 @@
InternalServerError,
NonXMLResponseError,
NotSignedInError,
+ RedirectError,
)
from tableauserverclient.server.exceptions import EndpointUnavailableError
@@ -45,6 +47,13 @@
Success_codes = [200, 201, 202, 204]
+# 301/302/303/307/308 all indicate the caller should re-request at a new URL.
+# `requests`' default handler converts POST -> GET on 301/302/303, which drops
+# the POST body and breaks sign-in / addusers / publish / any write endpoint
+# whose target sits behind a redirect. We disable that and walk the chain
+# manually, keeping the original method and body across every hop.
+Redirect_codes = [301, 302, 303, 307, 308]
+
XML_CONTENT_TYPE = "text/xml"
JSON_CONTENT_TYPE = "application/json"
@@ -120,6 +129,11 @@ def _make_request(
parameters = Endpoint.set_parameters(
self.parent_srv.http_options, auth_token, content, content_type, parameters
)
+ # Manual redirect handling: see Redirect_codes comment. `requests`
+ # follows 301/302/303 by converting POST to GET (RFC-conforming but
+ # loses the body). We disable it here and re-issue the same method
+ # ourselves in _follow_redirect_if_any.
+ parameters["allow_redirects"] = False
logger.debug(f"request method {method.__name__}, url: {url}")
if content:
@@ -144,6 +158,7 @@ def _make_request(
raise RuntimeError
if isinstance(server_response, Exception):
raise server_response
+ server_response, url = self._follow_redirect_if_any(method, url, parameters, server_response)
self._check_status(server_response, url)
loggable_response = self.log_response_safely(server_response)
@@ -157,6 +172,54 @@ def _make_request(
return server_response
+ def _follow_redirect_if_any(
+ self,
+ method: Callable[..., "Response"],
+ url: str,
+ parameters: dict[str, Any],
+ server_response: "Response",
+ ) -> tuple["Response", str]:
+ # Walk a 301/302/303/307/308 chain up to session.max_redirects hops,
+ # preserving method and body. Rejects HTTPS -> HTTP scheme downgrades
+ # (silent security regression). Raises RedirectError on a missing
+ # Location header instead of the KeyError requests emits deep in its
+ # internals, and on exceeding the session hop limit.
+ try:
+ max_hops = int(self.parent_srv.session.max_redirects)
+ except (AttributeError, TypeError):
+ max_hops = 30 # requests' library default
+ current_url = url
+ response = server_response
+ for hop in range(max_hops):
+ if response.status_code not in Redirect_codes:
+ return response, current_url
+ location = response.headers.get("Location")
+ if not location:
+ raise RedirectError(
+ f"{method.__name__.upper()} {current_url} returned HTTP {response.status_code} "
+ f"without a Location header; can't follow the redirect."
+ )
+ # Support relative Locations per RFC 7231.
+ next_url = urljoin(current_url, location)
+ if urlparse(current_url).scheme == "https" and urlparse(next_url).scheme == "http":
+ raise RedirectError(
+ f"Refusing to follow redirect from {current_url} to {next_url}: "
+ f"HTTPS -> HTTP scheme downgrade would send request data over plaintext."
+ )
+ logger.debug(f"Following {response.status_code} redirect: {current_url} -> {next_url}")
+ current_url = next_url
+ next_response = self._blocking_request(method, current_url, parameters)
+ if next_response is None:
+ raise RuntimeError(f"No response after redirect to {current_url}")
+ if isinstance(next_response, Exception):
+ raise next_response
+ response = next_response
+ # Still a redirect after max_hops hops -> loop / misconfiguration.
+ raise RedirectError(
+ f"Exceeded {max_hops} redirect hops starting from {url}; last Location was {current_url}. "
+ f"Increase session.max_redirects if this is legitimate."
+ )
+
def _check_status(self, server_response: "Response", url: str | None = None):
logger.debug(f"Response status: {server_response}")
if not hasattr(server_response, "status_code"):
diff --git a/tableauserverclient/server/endpoint/exceptions.py b/tableauserverclient/server/endpoint/exceptions.py
index 49e065ed3..2a94a2969 100644
--- a/tableauserverclient/server/endpoint/exceptions.py
+++ b/tableauserverclient/server/endpoint/exceptions.py
@@ -130,3 +130,10 @@ class FlowRunCancelledException(FlowRunFailedException):
class UnsupportedAttributeError(TableauError):
pass
+
+
+class RedirectError(TableauError):
+ # Raised when a manual redirect can't be followed safely or at all.
+ # Cases: missing Location header, HTTPS -> HTTP downgrade, redirect loop
+ # exceeding session.max_redirects. See Endpoint._follow_redirect_if_any.
+ pass
diff --git a/test/test_redirect_handling.py b/test/test_redirect_handling.py
new file mode 100644
index 000000000..877682ae9
--- /dev/null
+++ b/test/test_redirect_handling.py
@@ -0,0 +1,192 @@
+"""Tests for manual redirect handling in Endpoint._make_request.
+
+`requests` follows 301/302/303 by converting POST to GET (dropping the body).
+We disable auto-redirect and re-issue the same method ourselves in
+Endpoint._follow_redirect_if_any. These tests cover the resulting behavior:
+
+- POST body preserved across a redirect
+- multi-hop chains
+- HTTPS -> HTTP scheme downgrade refused
+- missing Location header raises RedirectError
+- exceeding session.max_redirects raises RedirectError
+- GET redirects still work
+"""
+
+from pathlib import Path
+
+import pytest
+import requests_mock
+
+import tableauserverclient as TSC
+from tableauserverclient.server.endpoint.exceptions import RedirectError
+
+TEST_ASSET_DIR = Path(__file__).parent / "assets"
+SIGN_IN_XML = TEST_ASSET_DIR / "auth_sign_in.xml"
+
+
+@pytest.fixture
+def server() -> TSC.Server:
+ return TSC.Server("http://test", False)
+
+
+@pytest.fixture
+def signed_in_server() -> TSC.Server:
+ s = TSC.Server("http://test", False)
+ s._set_auth("site-id", "user-id", "auth-token", "")
+ return s
+
+
+def _sign_in_xml() -> str:
+ with open(SIGN_IN_XML, "rb") as f:
+ return f.read().decode("utf-8")
+
+
+def test_post_body_preserved_across_redirect(signed_in_server: TSC.Server) -> None:
+ # Regression for tableau/tabcmd#309: POST -> 302 previously turned into GET
+ # and dropped the request body. Verify the body reaches the final URL intact.
+ seen_bodies: list[bytes | None] = []
+
+ def record(request, context):
+ seen_bodies.append(request.body)
+ context.status_code = 200
+ return b""
+
+ with requests_mock.mock() as m:
+ m.post("http://test/redirect-from", status_code=302, headers={"Location": "http://test/redirect-to"})
+ m.post("http://test/redirect-to", content=record)
+
+ resp = signed_in_server.session.post(
+ "http://test/redirect-from",
+ data=b"payload=1",
+ allow_redirects=False,
+ )
+ # The Endpoint layer, not the raw session, is what re-issues. Route
+ # through _make_request so we exercise the code under test.
+ from tableauserverclient.server.endpoint.endpoint import Endpoint
+
+ endpoint = Endpoint(signed_in_server)
+ final, url = endpoint._follow_redirect_if_any(
+ signed_in_server.session.post,
+ "http://test/redirect-from",
+ {"data": b"payload=1", "allow_redirects": False},
+ resp,
+ )
+
+ assert final.status_code == 200
+ assert url == "http://test/redirect-to"
+ assert seen_bodies == [b"payload=1"], seen_bodies
+
+
+def test_multi_hop_redirect_chain(signed_in_server: TSC.Server) -> None:
+ with requests_mock.mock() as m:
+ m.post("http://test/a", status_code=301, headers={"Location": "http://test/b"})
+ m.post("http://test/b", status_code=302, headers={"Location": "http://test/c"})
+ m.post("http://test/c", status_code=200, text="")
+
+ resp = signed_in_server.session.post("http://test/a", allow_redirects=False)
+ from tableauserverclient.server.endpoint.endpoint import Endpoint
+
+ endpoint = Endpoint(signed_in_server)
+ final, url = endpoint._follow_redirect_if_any(
+ signed_in_server.session.post, "http://test/a", {"allow_redirects": False}, resp
+ )
+
+ assert final.status_code == 200
+ assert url == "http://test/c"
+
+
+def test_relative_location_header(signed_in_server: TSC.Server) -> None:
+ # RFC 7231 allows relative Location values; join them against the request URL.
+ with requests_mock.mock() as m:
+ m.post("http://test/api/v1/thing", status_code=302, headers={"Location": "/api/v2/thing"})
+ m.post("http://test/api/v2/thing", status_code=200, text="")
+
+ resp = signed_in_server.session.post("http://test/api/v1/thing", allow_redirects=False)
+ from tableauserverclient.server.endpoint.endpoint import Endpoint
+
+ endpoint = Endpoint(signed_in_server)
+ final, url = endpoint._follow_redirect_if_any(
+ signed_in_server.session.post, "http://test/api/v1/thing", {"allow_redirects": False}, resp
+ )
+
+ assert final.status_code == 200
+ assert url == "http://test/api/v2/thing"
+
+
+def test_https_to_http_downgrade_rejected() -> None:
+ # HTTPS -> HTTP redirect is never legitimate: quietly following it would
+ # send auth material over plaintext. Refuse and surface a clear error.
+ s = TSC.Server("https://secure.test", False)
+ s._set_auth("site-id", "user-id", "auth-token", "")
+
+ with requests_mock.mock() as m:
+ m.post("https://secure.test/signin", status_code=301, headers={"Location": "http://insecure.test/signin"})
+ resp = s.session.post("https://secure.test/signin", allow_redirects=False)
+ from tableauserverclient.server.endpoint.endpoint import Endpoint
+
+ endpoint = Endpoint(s)
+ with pytest.raises(RedirectError, match="HTTPS -> HTTP"):
+ endpoint._follow_redirect_if_any(
+ s.session.post, "https://secure.test/signin", {"allow_redirects": False}, resp
+ )
+
+
+def test_missing_location_header_raises_redirecterror(signed_in_server: TSC.Server) -> None:
+ # `requests`' internal resolve_redirects raises KeyError('location') with no
+ # context. We raise RedirectError with the URL, method, and status code.
+ with requests_mock.mock() as m:
+ m.post("http://test/broken", status_code=302) # no Location header
+ resp = signed_in_server.session.post("http://test/broken", allow_redirects=False)
+ from tableauserverclient.server.endpoint.endpoint import Endpoint
+
+ endpoint = Endpoint(signed_in_server)
+ with pytest.raises(RedirectError, match="without a Location header"):
+ endpoint._follow_redirect_if_any(
+ signed_in_server.session.post, "http://test/broken", {"allow_redirects": False}, resp
+ )
+
+
+def test_redirect_loop_hits_max_hops(signed_in_server: TSC.Server) -> None:
+ signed_in_server.session.max_redirects = 3
+ with requests_mock.mock() as m:
+ m.post("http://test/loop", status_code=302, headers={"Location": "http://test/loop"})
+ resp = signed_in_server.session.post("http://test/loop", allow_redirects=False)
+ from tableauserverclient.server.endpoint.endpoint import Endpoint
+
+ endpoint = Endpoint(signed_in_server)
+ with pytest.raises(RedirectError, match="Exceeded 3 redirect hops"):
+ endpoint._follow_redirect_if_any(
+ signed_in_server.session.post, "http://test/loop", {"allow_redirects": False}, resp
+ )
+
+
+def test_non_redirect_response_passes_through(signed_in_server: TSC.Server) -> None:
+ # 200 stays 200; the helper is a no-op for non-3xx.
+ with requests_mock.mock() as m:
+ m.post("http://test/ok", status_code=200, text="")
+ resp = signed_in_server.session.post("http://test/ok", allow_redirects=False)
+ from tableauserverclient.server.endpoint.endpoint import Endpoint
+
+ endpoint = Endpoint(signed_in_server)
+ final, url = endpoint._follow_redirect_if_any(
+ signed_in_server.session.post, "http://test/ok", {"allow_redirects": False}, resp
+ )
+
+ assert final.status_code == 200
+ assert url == "http://test/ok"
+
+
+def test_sign_in_after_redirect(server: TSC.Server) -> None:
+ # Integration-style: real sign-in flow across a redirect. Verifies that
+ # auth_endpoint's existing manual-redirect-of-signin still works alongside
+ # the generic _make_request redirect handling.
+ xml = _sign_in_xml()
+ with requests_mock.mock() as m:
+ m.post(
+ server.auth.baseurl + "/signin", status_code=301, headers={"Location": "http://test/api/3.6/auth/signin"}
+ )
+ m.post("http://test/api/3.6/auth/signin", text=xml)
+ tableau_auth = TSC.TableauAuth("u", "p", site_id="Samples")
+ server.auth.sign_in(tableau_auth)
+
+ assert server.auth_token is not None