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

feat(auth): support for forwarded auth provider #874

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ajgon
Copy link

@ajgon ajgon commented Apr 6, 2024

What type of PR is this?

  • feature

What this PR does / why we need it:

This PR introduces a new authentication provider, called "ForwardAuth". It's written with forward-authentication supporting reverse proxies (like nginx or traefik) and accomodanting authentication apps (like authelia, authentik and probably many more).

With this changes, if homebox is configured in one of Fw-Auth setups, it will automatically sign in (and register, depending on configuration) user forwarded by initial authentication backend, thus removing necessity of logging in twice.

This is done by special header (also configurable, by default Remote-Email) which tells homebox, that authentication was already be done, and it can trust that given email is already valid. Of course, a malicious user, my try to send this user, by themself trying to impersonate the real one - but in properly configured fw-auth setups it shouldn't be possible. Anyway, to avoid any mistakes, there is another configurable field, defining list of IPs which are allowed to be source of fw-auth traffic. By default it's empty, meaning every source is blacklisted - thus feature is disabled. This also protects installations which doesn't need this feature, to not accidentally expose themselves.

Which issue(s) this PR fixes:

Implements #270

Testing

I deployed updated code to my cluster, and tested everything with authelia.

Release Notes

- Support for forward-authentication applications like authelia.

@ajgon
Copy link
Author

ajgon commented Apr 6, 2024

After I pushed this PR, I noticed there was another attempt before, but closed because of expected login providers refactor. Since this is (I assume) done, I think we can get back to the problem, and try to implement it again?

@verybadsoldier - I'm tagging you, if you have any input/opinion about this, since you are the author of the original one :)

@hay-kot
Copy link
Owner

hay-kot commented Apr 19, 2024

I deployed updated code to my cluster, and tested everything with authelia.

Hey, this looks great! Since I don't have any infrastructure for running or testing this, I won't be able to merge it without any kind of automated testing.

What options do we have there for implementing automated testing in CI? Ideally, something that we can run locally would be great as well, but CI is definitely a requirement.

@hay-kot hay-kot marked this pull request as draft April 28, 2024 16:08
@seiferma
Copy link

@hay-kot Which types of tests do you have in mind? I am interested in the feature and willing to help.

I only had a quick look through the repository. To me, it looks like there are unit tests at the moment. Creating unit tests for the contributions should not be a big deal and I could create them if @ajgon would like to have support on this.

However, I assume that you are aiming for integration tests or even system tests based on your comment regarding test infrastructure. For integration tests, a mock of the authentication endpoint would be sufficient from my point of view. Though, there would be not much benefit from doing this compared to unit tests from my point of view.

Tests regarding compatibility with real authentication endpoints feel like system tests to me. We would have to spin up the application, a reverse proxy and the authentication endpoint. Of course, this is possible but before I start working on this, I would like to be sure that this is what you are aiming for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants