Support auth via HTTP request header

[tmaier]

Related discussion: #338 Closes #339, #340

This is WIP. It will not work. I am just pushing it out in the open very early on.

Design decisions:

• Configuration happens via Feature with features.yml and environment variables defined within. This allows to configure this features on infrastructure level (e.g. via kubernetes or docker compose)
• The AuthN mechanism supported are username+password, google and http auth. You can enable username+password together with google. Enabling Http auth means google and username+password will be disabled.
• When a new user arrives and clicks on "log in with Google" or when a new user arrives with the right HTTP request header set, the Person/User will be created automatically.
  • This means, the users#new and users#create controller actions are only used for password-based AuthN
  • This means http-request-header-based AuthN needs to provide username and email address as well

[krschacht]

@tmaier Looking good! I did a quick pass through it and left a handful of thoughts on things that struck me as I’m starting to wrap my mind around the new architecture.

[tmaier]

I wonder how to properly support HTTP request authentication

Situation:

• The User/Person models require email, name

Complication:

• HTTP request authentication may just set one header, which allows to find/create a user. Everything else may not be shared. The value set, does not need to be an email address.

Resolution options:

1. We assume more headers are available, which allow us to set these values
2. We make some (e.g. name optional, especially when HTTP-login is ongoing.
3. We create a new model, next to User, maybe HTTPUser

[krschacht]

I don’t quite follow this, can you say more about it:

HTTP request authentication may just set one header, which allows to find/create a user. Everything else may not be shared. The value set, does not need to be an email address.

I think what I’m hearing is that a user can be in this other system, click a link, and they’ll be directed to some route within our app which should register and authenticate them all in this moment? And this request does not pass any information about the user except a key in this header, so we won’t know anything else about them? If I’m following this correctly, how feasible is it for this request to be required to include a little more information (eg name and email).

In general the way I think we should approach this is to introduce a Credentials model. We move details from User over to Credentials and we make this table be STI so it can be the backing for EmailCredential model and a HTTPCredential model.

[tmaier]

@krschacht I was imprecise in my words:

1. We will always get something like a unique identifier for a user. In some cases it may be an email address. In some cases not.
2. We might not get any other data, like name or email address. This is the complication in respect to User and Person models

In the case of Tailscale, we get a unique identifier (looks like an email) and the name.

In the case of Reverse Proxy it depends on whatever is provided by the external AuthN system.

I am going to rename the defaults to X-WEBAUTH-USER, X-WEBAUTH-NAME and X-WEBAUTH-EMAIL, since this is what I found to be more generic (I first saw this header names at Grafana)

[krschacht]

With those three ENV you are going to rename to: is NAME the full name? And what’s the USER one?

[tmaier]

@krschacht can you have one more look at this PR? I refactored it.

However, I would still not consider it finished.

[krschacht]

@tmaier I took a quick look and I think it’s on the right track. In your mind, what parts are still remaining?

I think I’d like to land a small refactor of Features ahead of this. You need to extend its capabilities for your work here in this PR, and I also need to extend its capabilities in another branch I’m working in. So tomorrow I will try to spin up a new PR that just focuses on Features, I’ll closely reference what you’re doing in this branch to incorporate the extensions that you need. Then after you get a chance to review it and I merge into main, I can help in pulling main back into here.

[krschacht]

@tmaier I gave this PR a close read and added a few small comments. I also pulled out the Feature work into it's own PR. Give this a quick review, then I can merge into main and we can finish up this PR: https://github.com/AllYourBot/hostedgpt/pull/372

I'd love to get your PR merged in this week, if possible. I have some additional work I'm doing where I want users to be able to give their Assistant access to their Google Calendar / Gmail (optionally) and I'm building off the google auth in this PR.

[krschacht]

@tmaier I landed the Features PR into main, I then pulled main back into your branch here. When I did this, I resolved the merge conflicts, I did the find & replace on Feature. and Setting. so your code matches the new names.

Note: there is one final rename that I did:

    http_header_auth_uid: <%= ENV["HTTP_HEADER_AUTH_UID"] || "X-WEBAUTH-UID" %>

I made the default X-WEBAUTH-UID to match the symmetry there. I hope this is okay.

I am doing some google auth related work in another branch so I'm spending a little more time studying how this PR approaches that.

[krschacht]

Okay @tmaier , I merged that other PR into main and then merged main back into this. I hope this doesn't randomize your efforts too much! You have a lot of good changes in here that I tried to preserve, but I may have accidentally introduced a bug while resolving merge conflicts. Thanks for your help on all this auth stuff!

[krschacht]

Hi @tmaier do you think you’ll have time to wrap up this PR this week? You got it really close, and after peeling off the Google Auth and getting that merged in this shouldn’t take too much to get this one finished.

[krschacht]

@tmaier I took one more pass through it and flagged a couple small things.

[tmaier]

@krschacht I think everything is done now. I never worked with ActionDispatch::IntegrationTest. I hope the test cases are ok for you.

[krschacht]

@tmaier Since this is auth, I re-read through everything again one more time and tested out the various scenarios. I did a few tiny cleanup commits along the way and there is only one material change I made:

I flipped the feature flags in such a way that got me into an infinite redirect loop (directing me to root which directed me to login but I had disabled password & google so it was redirecting me to root). I fixed this by simply allowing us to render a login (and registration) page even though all the options are hidden. This has the added advantage of ensuring that I see just a single flash message explaining what's wrong, which should be easier for people to diagnose what state the system is is. I don't think the flash messages were properly persisting through the redirect.

(this is the very last commit, if you want to take a look at it)

But otherwise, I'm ready to click merge on this. Any hesitation at all before I do? Just double checking, since it feels dangerous for me to click merge after just making a last-minute change to an auth flow.

Thanks again for taking the lead on all these great auth improvements! i don't know when I would have gotten to this.

(oh, and your test looked good! just wanted to acknowledge since you had called that out)

[krschacht]

I made those last two changes, good catches. Merging it in now! Whew.

[tmaier]

Great! Thank you very much! This was a bit of a collaborative effort. I like it.