ondemand icon indicating copy to clipboard operation
ondemand copied to clipboard

host_based_profiles don't work correctly

Open johrstrom opened this issue 1 year ago • 6 comments

From discourse: https://discourse.openondemand.org/t/url-based-profile-selection/3278

and confirmed in dev. I believe it's this section of the ood portal conf that's always redirecting back to the servername so you can never access the 2nd profile.

https://github.com/OSC/ondemand/blob/25d3d02f15501b76cd28afcd16d6a8932e59359f/ood-portal-generator/templates/ood-portal.conf.erb#L93-L95

johrstrom avatar Mar 21 '24 16:03 johrstrom

OK - it's actually 2 issues. The first being the redirects. I was able to access the server_alias, but I had to specify the port 443 in the URL, which should not be the case, so there's something off with that regex requiring a port.

(I've replaced the actual hostnames in this log message with servername and server_alias[0] to obsfucate them)

[Thu Mar 21 13:20:01.212209 2024] [auth_openidc:error] [pid 1544337:tid 139761498965760] [client 10.2.9.72:49708] oidc_authenticate_user: the URL hostname (servername) of the configured OIDCRedirectURI does not match the URL hostname of the URL being accessed (server_alias[0]): the "state" and "session" cookies will not be shared between the two!

johrstrom avatar Mar 21 '24 17:03 johrstrom

I cannot reproduce the redirect issue in 3.1 when accessing directly a server _alias.

What I think is happening is the HTTP to HTTPS redirect that happens if you type a server_alias as http://server_alias in the browser. This redirect has not been updated to support aliases: https://github.com/OSC/ondemand/blob/25d3d02f15501b76cd28afcd16d6a8932e59359f/ood-portal-generator/templates/ood-portal.conf.erb#L66

abujeda avatar Mar 21 '24 18:03 abujeda

Yea that could have been it. I know the OIDC thing is an issue for sure - but yea just using the server_alias on the first go must have defaulted to plain http in my browser.

johrstrom avatar Mar 21 '24 18:03 johrstrom

ChatGPT says it should be something like this:

RewriteEngine On
RewriteRule ^(.*) <%= @ssl ? "https" : "http" %>://%{HTTP_HOST}%:<%= @port %>$1 [R=301,NE,L]

abujeda avatar Mar 21 '24 18:03 abujeda

Seems right - though I don't know if HTTP_HOST is the proxy server in case OOD is behind a load balancer.

johrstrom avatar Mar 21 '24 18:03 johrstrom

Yes, when using a proxy that changes the domain, that would not work. I think it is difficult to cater for all cases. This is an idea based on 2 new variables:

  • https_redirect_use_host: use the host variable on the HTTP to HTTPS redirect
  • https_redirect: array with Apache directives to fit your particular use case.

By default is the same behaviour.

  <%- if @https_redirect_use_host -%>
  RewriteEngine On
  RewriteRule ^(.*) <%= @ssl ? "https" : "http" %>://%{HTTP_HOST}%:<%= @port %>$1 [R=301,NE,L]
  <%- elsif @https_redirect -%>
  <%- @https_redirect.each do |line| -%>
  <%= line %>
  <%- end -%>
  <%- else -%>
  RewriteEngine On
  RewriteRule ^(.*) <%= @ssl ? "https" : "http" %>://<%= @proxy_server %>:<%= @port %>$1 [R=301,NE,L]
  <%- end -%>

abujeda avatar Mar 21 '24 22:03 abujeda

I realize I'm a bit late to the party, but I only noticed when the commit landed in master. The committed change https://github.com/OSC/ondemand/commit/12375e036bc524ac4b7b3ac047c30c87450d28ab is actually worse for our use case, and that's why I used @proxy_server deliberately when committing https://github.com/OSC/ondemand/commit/22ae7d3a962106c41456d3f669a071a323f17a07 .

It would be nice to see something configurable, similar to what @abujeda suggested. There is an extra trailing % in the %{HTTP_HOST}% string, but otherwise the given example would work nicely for us, and the use case described in the issue originally.

I think it would be nice if the templates were as generic as possible, to allow for customization without forking and fixing in post. Especially for configuring third-party software, i.e., httpd and nginx, where there are possibilities to do a lot of things around OOD, that the upstream hasn't maybe encountered yet. https://github.com/OSC/ondemand/issues/708 was a step in the right direction.

CSC-swesters avatar Apr 15 '24 13:04 CSC-swesters

🤦‍♂️ I didn't remember #2870 and that #3508 just reverts that... OK. Looks like we need a new config.

johrstrom avatar Apr 15 '24 13:04 johrstrom

Thanks for considering this!

I personally prefer lots of configuration options, as long as the defaults are sane. Then if one ever notices the need to tweak something, there is some way of affecting the deployment without needing to change the code or do some hacky workaround :slightly_smiling_face:

CSC-swesters avatar Apr 15 '24 13:04 CSC-swesters

We could do something like this to cater for complex cases and let the admins decide how to fix it.

http_to_https_directives: New array with Apache directives to fit your particular use case. By default is the same behaviour.

  <%- if @http_to_https_directives.empty? -%>
  RewriteEngine On
  RewriteRule ^(.*) <%= @ssl ? "https" : "http" %>://<%= @proxy_server %>:<%= @port %>$1 [R=301,NE,L]
  <%- else -%>
  <%- @http_to_https_directives.each do |line| -%>
  <%= line %>
  <%- end -%>
  <%- end -%>

abujeda avatar Apr 17 '24 10:04 abujeda

@abujeda yes, that looks viable :+1: @johrstrom would probably like the default to be %{HTTP_HOST} instead of <%= @proxy_server %>, based on the most recent changes. Our site can manage with the manual override via http_to_https_directives :slightly_smiling_face:

CSC-swesters avatar Apr 17 '24 11:04 CSC-swesters

Do you need multiple lines? Or can it just be 1 string configuration?

johrstrom avatar Apr 17 '24 15:04 johrstrom

I've got #3515 for a configurable string. I'm not sure why folks would need an array of redirects.

johrstrom avatar Apr 17 '24 17:04 johrstrom

I leave it to @CSC-swesters to reply. For us the existing redirect to %{HTTP_HOST} is good enough.

abujeda avatar Apr 17 '24 17:04 abujeda

I like the idea of being able to sneak in other lines in that context too, instead of limiting the fix to just this redirect host name. That way, anyone would be able to do anything with the provided template. It's the same reasoning as in my previous comment: https://github.com/OSC/ondemand/issues/3442#issuecomment-2056896819

Currently there is no way of doing whatever one wants in the HTTP VirtualHost context. @abujeda's suggestion would make that possible, which is good for everybody in my opinion. It's frustrating as an admin to fight against restrictive tools, so I'd like to see OOD keep the templates as flexible as possible. Every site's environment is unique, after all.


That said, if you still insist on keeping the template locked down, I'd at least appreciate if the variable name was more descriptive: https://github.com/OSC/ondemand/pull/3515/files#diff-206c33c15be4d445d715b215232f9ea6e0a30d127659164388d0f096694f98cbR42

I'd name it http_redirect_host, to indicate that it only affects the HTTP (non-S) VirtualHost's context. There are other redirects in, e.g., maintenance mode and HTTPS host name fixes.

CSC-swesters avatar Apr 18 '24 07:04 CSC-swesters

That said, if you still insist on keeping the template locked down, I'd at least appreciate if the variable name was more descriptive:

I opted for this route - renaming to http_redirect_host. I figure if someone needs say a http_custom_vhost_directives then we can add that should someone need it.

johrstrom avatar Apr 19 '24 13:04 johrstrom