Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix st2 cli client auth in st2auth proxy mode #6049

Merged
merged 3 commits into from Nov 10, 2023

Conversation

floatingstatic
Copy link
Contributor

Adding on to work in #6041 that fixed proxy auth mode for users connecting via st2web, it appears that the st2 client (st2client container in the HA helm chart) connects directly to st2auth instead of connecting through nginx. As a result it is unable to authenticate with st2auth is in proxy mode as headers required for this to work are not present. We see 401 Client Error: Unauthorized in this scenario. The st2 client does however use basic auth and we can extract the username from that in this scenario to resolve the issue.

This change modifies the proxy auth handler to use the username from the Authorization header only in the case where proxy auth mode is enabled and when remote_user is not passed to the proxy auth handler function.

@pull-request-size pull-request-size bot added the size/M PR that changes 30-99 lines. Good size to review. label Oct 25, 2023
@floatingstatic
Copy link
Contributor Author

I would also add there is a 3rd issue with proxy auth mode which has a workaround unrelated to this PR. I do not feel it is something that needs to be added to code but perhaps should be documented somewhere for others that use st2auth in proxy mode. The issue is that the web ui will always force you to the login screen because an auth-token cookie is not set in local storage. While you can just enter garbage credentials to get one set this is somewhat confusing for users. We work around this by adding something under location / in the nginx config like this as a diff to force fetching and storing an auth-token cookie when it is not set:

+  location /auth.html {
+    root      /opt/stackstorm/static/webui/;
+  }
+
   location / {
     max_ranges 0;
     root      /opt/stackstorm/static/webui/;
@@ -137,5 +147,9 @@
     sendfile on;
     tcp_nopush on;
     tcp_nodelay on;
+
+    if ($http_cookie !~* "auth-token") {
+      rewrite ^(.*)$ /auth.html last;
+    }
   }

auth.html is a custom html page that looks like:

<html>
  <script>
    base = `${window.location.protocol || 'https:'}//${window.location.host}`;

    http = new XMLHttpRequest();
    http.onreadystatechange = e => {
      if (http.readyState == 4) {
        result = JSON.parse(http.responseText);

        if (http.status == 201) {
          data = {
            server: {
              api: `${base}/api`,
              auth: `${base}/auth`,
              stream: `${base}/stream`,
              token: null
            },
            token: result
          };

          window.localStorage.setItem('st2Session', JSON.stringify(data));
          document.cookie = "auth-token"; // Needed to prevent loop
          window.location.replace(base + '/index.html');
        } else {
          console.log(http);
          document.getElementById("message").innerText = `Error while retrieving token: ${result.faultstring}`;
        }
      }
    }

    http.open("POST", `${base}/auth/tokens`);
    http.setRequestHeader("Content-Type", "application/json");
    http.send();
  </script>
  <body>
    <div id="message">Retrieving authorization token, redirecting when done...</div>
  </body>
</html>

This was borrowed from a forum post I found here. I'm not sure if this is something we would want to consider bundling with st2web but I feel it may warrant documentation (st2docs ?) for others that may be trying to use proxy auth mode.

@arm4b arm4b added this to In progress in StackStorm v3.8.1 via automation Nov 6, 2023
@arm4b arm4b added this to the 3.8.1 milestone Nov 6, 2023
CHANGELOG.rst Outdated Show resolved Hide resolved
Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@arm4b arm4b requested a review from a team November 7, 2023 13:51
@arm4b arm4b enabled auto-merge November 7, 2023 13:51
Copy link
Contributor

@amanda11 amanda11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to merge, but just added comment as not sure we need to keep having six.PY3 checks any more.

return

remote_user = split[0]
if six.PY3 and isinstance(remote_user, six.binary_type):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to have checks on whether its python3 any more?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not but i was just keeping consistent with what is used in other handlers, for example:

if six.PY3 and isinstance(username, six.binary_type):
username = username.decode("utf-8")
if six.PY3 and isinstance(password, six.binary_type):
password = password.decode("utf-8")

decode("utf-8") is needed even without the conditional in my testing.

@arm4b arm4b merged commit 32a243a into StackStorm:master Nov 10, 2023
38 checks passed
StackStorm v3.8.1 automation moved this from In progress to Done Nov 10, 2023
@hamsterdolphin
Copy link

hamsterdolphin commented Nov 18, 2023

@floatingstatic would you mind sharing your helm chart which has proxy auth enabled? I added the following to the st2 config section in values.yaml:

[auth]
mode=proxy

However, I get the following pod error while installing the chart:
Failed to authenticate with credentials provided in the config. ERROR: 401 Client Error: Unauthorized MESSAGE: Invalid or missing credentials for url: http://stackstorm-chart-st2auth:9100/tokens

Seeing a full working helm chart with proxy auth mode would be very helpful to the community :)

@floatingstatic
Copy link
Contributor Author

@hamsterdolphin I cannot share my chart as its a fork of upstream with a bunch of things I have changed for my own use case, however the only real change you need to enable proxy auth would be here:

https://github.com/StackStorm/stackstorm-k8s/blob/87425e0e3f88d9e89ff6a356162fd0fc87a52cec/values.yaml#L87-L93

st2:
  config: |
    [api]
    allow_origin = '*'
    # fixes no replicaset found bug;
    [database]
    # Connection and server selection timeout (in ms).
    connection_timeout = 5000
    [auth]
    mode = proxy

It seems like you did that already so I suspect what you are hitting is actually the fact that you need a few patches on the upstream containers you are probably pulling in for version 3.8.0. Internal to my org we pull in upstream images for 3.8.0 then build a custom image with some patches on top of the st2auth and st2web containers.

The st2auth patches are basically just the diffs from #6049 and #6041 the dockerfile looks something like:

image

Patches are basically just what you see in those two PRs

Then we build and push the image to an internal registry and point to those images in our helm chart values instead of the default upstream container images.

The st2web patches are basically what I outlined in previous comments in this PR. Hope this helps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement size/M PR that changes 30-99 lines. Good size to review.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants