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(server): trusted header authentication #8778

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

truongsinh
Copy link
Contributor

@truongsinh truongsinh commented Apr 13, 2024

This PR implements trusted header authentication

To quote from https://www.authelia.com/integration/trusted-header-sso/introduction/

This authentication method is referred to by many names; notably trusted header authentication, header authentication, header sso, and probably many more.

API test included.

To use it:

  • IMMICH_TRUSTED_REMOTE_NETWORKS must be set, for example "10.3.3.0/24, fd00:3::/32, 172.3.4.0/24, 172.3.2.0/24"
  • remote-email (note, email, not user) must exist in the HTTP request.

@truongsinh truongsinh changed the title [server]: trusted header authentication feat(server): trusted header authentication Apr 13, 2024
@bo0tzz
Copy link
Member

bo0tzz commented Apr 14, 2024

What is the use-case or problem solved by this? What are the security implications, for example if a potentially malicious client has the ability to set this header themselves?

@truongsinh
Copy link
Contributor Author

truongsinh commented Apr 14, 2024

What is the use-case or problem solved by this?

One use case is to use with reverse proxy and dedicated IAM. For example, Caddy as reverse proxy, and authelia/authentik/keyclock as IAM. See:

To quote from https://caddy.community/t/securing-web-apps-with-caddy-and-authelia-in-docker-compose-an-opinionated-practical-and-minimal-production-ready-login-portal-guide/20465:

Tight integration to the server itself may reduce footguns (of which there were indeed some encountered in the course of formulating this guide!), and may reduce complexity (of which there is indeed an added amount by virtue of configuring, running, and maintaining multiple separate services).

There are some other use cases of having auth header as well #1604, though I may misunderstand what the OP means.

What are the security implications, for example if a potentially malicious client has the ability to set this header themselves?

The implication would be that the operator of the stack (proxy, IAM server, and immich), if choosing to use this feature, must configure correctly, for example, https://www.authelia.com/integration/trusted-header-sso/introduction/#trusted-remote-networks . That is, if the operator blindly set IMMICH_TRUSTED_REMOTE_NETWORKS, and the proxy does not use IAM or sanitize remote-email header, the malicious client can successfully set this header themselves. When configured correctly, any remote-* header would be sanitized and/or reset by proxy before being forwarded to immich.

On the other hand, be default, this feature is not enabled, and if the malicious client set the header, it it not considered at all (test cases https://github.com/immich-app/immich/pull/8778/files#diff-e70b055978691a42a3ac156ff070103a5263c60e0b1343da4255e0954b28195aR388)

Copy link
Contributor

@jrasm91 jrasm91 left a comment

Choose a reason for hiding this comment

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

I think we can add this feature, but can you clean up the pull request before I look at it?

  • All the automated checks are failing
  • There is a describe.only block
  • The web changes likely don't work in the production build (env variables don't mean anything for a static build).
  • Like all other auth settings, this should be system config instead of an environment variable.
  • You should add an e2e test for this feature

@bo0tzz
Copy link
Member

bo0tzz commented Apr 15, 2024

I don't really see a case for this feature. Authentication via external IAM is already better covered with the existing oauth implementation, and the request in #1604 is not solved here - that request is for the ability to configure a header inside of the mobile app.

@jrasm91
Copy link
Contributor

jrasm91 commented Apr 15, 2024

@truongsinh - can you explain why you would need to use this authentication mechanism over the already existing OAuth implementation and integration? Does that also not allow external IAM via authelia/authentik/keyclock?

@truongsinh
Copy link
Contributor Author

@bo0tzz @jrasm91

Authentication via external IAM is already better covered with the existing oauth implementation
can you explain why you would need to use this authentication mechanism over the already existing OAuth implementation and integration

While Immich already supports OAuth, trusted header authentication presents unique advantages for our self-hosted community. This method allows seamless integration into personal network setups with reverse proxies that handle authentication, enhancing user convenience significantly.

  1. Ease of Use in Secured LANs: In a secure and trusted LAN, where ease of access is crucial, especially for users like elderly family members who prefer simplicity over complex logins, trusted headers allow a password-free (neither immich nor "sign in with google" password), "click-and-see" experience. This setup lets them easily access content like family videos without remembering complex passwords.
  2. Decentralized Security Management: By leveraging trusted header authentication, the responsibility for security and cryptography can shift from Immich to dedicated, specialized apps designed for security. This reduces the security burden on Immich while utilizing more robust, dedicated tools that are already managing network security.
  3. Efficient Single Logout (SLO): Unlike OAuth 2.0, trusted header authentication supports true single logout capabilities, making it simpler and more straightforward than implementing SLO with SAML or OIDC. This ease extends not only to Immich contributors but also to operators, simplifying system management and enhancing security.

Given we're discussing the need of trusted auth header, I'll hold off for now. If there is a consensus, I'll continue to address all the comment above (btw I'm a little embarrassed about having describe.only in the PR 😅)

@zackpollard zackpollard marked this pull request as draft May 14, 2024 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants