[feat] Add X-Remote-User header #135
Conversation
internal/route/auth.go
Outdated
|
|
||
| func (h *TaskcafeHandler) xHeaderAuthenticate(w http.ResponseWriter, r *http.Request) { | ||
| xRemoteUser := r.Header.Get("X-Remote-User") |
There was a problem hiding this comment.
I think it's worth making this configurable from the start- my proxy actually passes Remote-User (no X-), and I've seen examples implying several other names for this header. Does Go have an optional string type? that could work instead of a boolean for server_remote_user_header, with absence indicating "feature is turned off".
There was a problem hiding this comment.
yeap, was thinking about it already :)
In some cases there are needs to authenticate user not in Taskcafe itself. For this reason option server.remote_user_header was added. ```toml [server] remote_user_header = "X-Remote-User" ``` With turned on Taskcafe listens X-Remote-User http header and skip password checking. But still check user existence and activity flag.
MistaTwista
force-pushed
the
feature/add_remote_authorize
branch
from
481630b
to
c12a745
Compare
| @@ -139,9 +148,47 @@ func (h *TaskcafeHandler) LoginHandler(w http.ResponseWriter, r *http.Request) { | |||
| authCreatedAt := time.Now().UTC() | |||
| authExpiresAt := authCreatedAt.AddDate(0, 0, 1) | |||
| authToken, err := h.repo.CreateAuthToken(r.Context(), db.CreateAuthTokenParams{user.UserID, authCreatedAt, authExpiresAt}) | |||
| if err != nil { | |||
| w.WriteHeader(http.StatusInternalServerError) | |||
| // TODO: should we return here? | |||
There was a problem hiding this comment.
@JordanKnott is this a feature?
There was a problem hiding this comment.
If you're talking about not returning early on an error, it is not
|
Based on the discussions in #49, we should have the remote user header be accepted in place of the current access cookie rather than implementing a separate authentication path in This way the authentication flow would be So the changes would likely mostly be in the authentication middleware. However, one problem with going this route would be that public routes would no longer be public since the proxy would require authentication. I am also a little hesitant on adding features without some platform that I can actually test the implementation. @aroberts - what proxy setup were you planning on using with this added feature? Perhaps I can setup something similar for testing. |
|
I'll be using this with Authelia. There's a "getting started" compose file with documentation about defining users statically via file, rather than having to set up ldap. I'm not intending this service to be usable by anyone not logged in, but if I did, Authelia lets me scope the auth challenge to only specific path prefixes. I think that's a fairly common pattern (auth on specific prefixes only) in this space, due to this exact problem. |
|
@JordanKnott do you see a path forward for this? I don’t think the public routes are an issue- if I wanted them to remain public, I’d just configure my authentication proxy not to apply the auth challenge to those routes. This is a feature of virtually every tool that sits in this role (auth proxy) because of exactly the problem you point out. |
|
I think the way forward would be to change the authentication logic to what I described in the my last comment - and leave handling the public routes up to the user (like you described). |
|
Any luck here? I'd love to see this feature make it in. |
In some cases there are needs to Authorize user not in Taskcafe itself.
For this reason option server.remote_user_header was added.
With turned on Taskcafe listens X-Remote-User http header and skip
password checking. But still check user existence and active flag.
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
This was in discussion Feature: Authenticate via "Remote User" header #49
What is the current behavior? (You can also link to an open issue here)
Check user and password at login handler. Add token in DB. Return cookie.
What is the new behavior (if this is a feature change)?
Add an option server.remote_user_header (bool). Default is false.
If it's true – taskcafe will not check user password, but still find it in DB and check it's activeness.
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
no
Other information: