Merge pull request #1069 from projectsyn/fix/post-redirect

Explicitly handle redirects when doing POST requests

commodore/helpers.py

@@ -146,7 +146,26 @@ def _lieutenant_request(
                 headers=headers,
                 params=params,
                 timeout=timeout,
+                # don't let requests handle redirects (usually if the API URL is given without
+                # https://), since requests will rewrite the method to GET for the redirect which
+                # makes no sense when we're trying to POST to a POST-only endpoint.
+                allow_redirects=False,
             )
+
+            if r.status_code in [301, 302, 307, 308]:
+                # Explicitly redo the POST if we get a 301, 302, 307 or 308 status code for the
+                # first call. We don't validate that the redirect location has the same domain as
+                # the original request, since we already unconditionally follow redirects  with the
+                # bearer token for GET requests.
+                # Note that this wouldn't be necessary if all Lieutenant APIs would redirect us with
+                # a 308 for POST requests.
+                r = requests.post(
+                    r.headers["location"],
+                    json.dumps(data),
+                    headers=headers,
+                    params=params,
+                    timeout=timeout,
+                )
         else:
             raise NotImplementedError(f"QueryType {method} not implemented")
     except ConnectionError as e:

tests/test_helpers.py

@@ -338,6 +338,50 @@ def test_lieutenant_post(request_data, response, expected):
     _verify_call_status(post_url, token=request_data["token"])
 
 
+@responses.activate
+def test_lieutenant_post_redirect():
+    base_url = "http://syn.example.com/"
+    redir_url = "https://syn.example.com/"
+
+    post_url = url_normalize(f"{base_url}/clusters/c-cluster-1234/compileMeta")
+    real_post_url = url_normalize(f"{redir_url}/clusters/c-cluster-1234/compileMeta")
+    token = "token"
+    payload = {"some": "data", "other": "data"}
+
+    # successful post response from Lieutenant API has no body
+    responses.add(
+        responses.POST,
+        real_post_url,
+        content_type="application/json",
+        status=204,
+        body=None,
+        match=[matchers.json_params_matcher(payload)],
+    )
+    responses.add(
+        responses.POST,
+        post_url,
+        content_type="application/json",
+        headers={"location": real_post_url},
+        status=302,
+        body=None,
+        match=[matchers.json_params_matcher(payload)],
+    )
+
+    helpers.lieutenant_post(
+        base_url,
+        token,
+        "clusters/c-cluster-1234",
+        "compileMeta",
+        post_data=payload,
+    )
+
+    assert len(responses.calls) == 2
+    call_initial = responses.calls[0]
+    assert call_initial.request.url == post_url
+    call_redirected = responses.calls[1]
+    assert call_redirected.request.url == real_post_url
+
+
 def test_unimplemented_query_method():
     with pytest.raises(NotImplementedError, match="QueryType PATCH not implemented"):
         helpers._lieutenant_request(