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): add ForwardAuth support via X-Plex-Token header #3105

Open
wants to merge 1 commit into
base: feature/plex-decouple
Choose a base branch
from

Conversation

tobz
Copy link

@tobz tobz commented Nov 1, 2022

Description

This is an attempt to partially add ForwardAuth support to Overseerr, specifically for Plex-based logins.

#1555 and #1638 have already covered much of the ground on the wants/needs of Overseerr users when it comes to authentication, but this solution focuses purely on the ForwardAuth usecase: an application running behind a reverse proxy (or similar) system that derives authentication information itself (or via some other mechanism) and "forwards" that authentication to the proxied application. This PR does not concern itself with augmenting Overseerr's existing authentication methods with support for external identity providers, and so on.

In the ForwardAuth model, "trusted headers" are sent to the underlying application which carry the authentication information: username, e-mail, etc. In this PR, we're exclusively focusing on support for passing an existing Plex token as a trusted header, which drives newly-added logic to automatically create and/or log in the user based on that Plex token.

This new feature obeys the following rules/logic:

  • we do not create a new user if an imported one does not already exist and "Enable New Plex Sign-In" is disabled
  • we have added a new setting, "Enable Forward Auth Via Plex Token", which must be enabled for any of this new logic to take effect
  • we always attempt to create and/or login a user via this mechanism so long as the above constraints are meant, regardless of which page they access

Essentially, the login flow is skipped entirely if the above constraints are met: on a normal load to /, the first call to /api/v1/auth/me would log the user in via their Plex token, create and initialize their session, set their session cookie, and allow the UI to send them straight to the normal dashboard landing page.

To say it out loud: this is predicated on at least sending the ForwardAuth header along to the /api/v1/auth/me endpoint, but generally ForwardAuth implementations will send their authentication headers uniformly to all requests being proxied.

Beyond that, I've tried to refactor some of the Plex login code to be reusable between the actual Plex login endpoint and the code I've added for ForwardAuth support. I'm not a TypeScript developer by trade, so apologies if things are in the wrong place, not idiomatic, etc.

Screenshot (if UI-related)

To-Dos

  • Successful build yarn build
  • Translation keys yarn i18n:extract
  • Database migration (if required)

@tobz tobz force-pushed the tobz/plex-forward-auth branch 2 times, most recently from b2982a0 to dd9d362 Compare November 1, 2022 03:02
@lenaxia
Copy link

lenaxia commented Dec 14, 2022

Request to make the header configurable. To simplify it should be configurable via environment variable (no need for something as fancy as a UX). I use Authelia which uses the Remote-User and would absolutely be onboard for using this. I just added a comment in #1638 revisiting why forward auth is superior to OIDC, however it really limits the use of this feature if its limited to just Plex's token. Not all my users have plex accounts (in my case they share a plex pass account) since Plex doesn't allow loading of local accounts, but they do have their own accounts for other services.

@tobz
Copy link
Author

tobz commented Dec 14, 2022

Request to make the header configurable. To simplify it should be configurable via environment variable (no need for something as fancy as a UX). I use Authelia which uses the Remote-User and would absolutely be onboard for using this. I just added a comment in #1638 revisiting why forward auth is superior to OIDC, however it really limits the use of this feature if its limited to just Plex's token. Not all my users have plex accounts (in my case they share a plex pass account) since Plex doesn't allow loading of local accounts, but they do have their own accounts for other services.

I can understand your use case, and it makes sense. That said, adding support here to additionally support local accounts via ForwardAuth would be a significant increase in logic on top of what I've already built... which, given the seemingly low chance that this PR ever gets a real review and approved/merged, is just that much more work for (potentially) nothing.

@stale

This comment was marked as outdated.

@stale stale bot added the stale label Feb 18, 2023
@tobz
Copy link
Author

tobz commented Feb 19, 2023

@sct @TheCatLady @danshilm

Apologies for the direct ping, but, is it possible to review this? Or, if nothing else, to tell me whether or not you're even willing to accept this feature?

@stale stale bot removed the stale label Feb 19, 2023
@sct
Copy link
Owner

sct commented Feb 20, 2023

Hi @tobz. Sorry for the wait. We are still very busy, but we want to get to this! I am also in the process of refactoring some login and user type logic with the update to decouple plex in #3015. This will likely affect some of the logic you have here. So please give us a little longer.

@tobz
Copy link
Author

tobz commented Feb 20, 2023

Hi @tobz. Sorry for the wait. We are still very busy, but we want to get to this! I am also in the process of refactoring some login and user type logic with the update to decouple plex in #3015. This will likely affect some of the logic you have here. So please give us a little longer.

Understandable. 👍🏻

I'll rebase my branch on top of yours at some point and see what it looks like, and get it as close as I reasonably can for whenever #3015 lands.

sdsadas

fix sct#1638
@tobz tobz force-pushed the tobz/plex-forward-auth branch from dd9d362 to 31debf6 Compare February 21, 2023 19:02
@tobz tobz changed the base branch from develop to feature/plex-decouple February 21, 2023 19:04
@stale

This comment was marked as outdated.

@tobz
Copy link
Author

tobz commented Oct 15, 2023

@sct Checking in again.

It looks like your work on the Plex decoupling stuff stalled out around 2-3 months ago. No judgment at all whatsoever, but given that we talked about that work landing before coming back to revisit this PR getting merged... could we potentially come to a decision around whether to merge this or not?

The PR has been open for almost a year at this point. 😅

@geman220
Copy link

Would love to see this implemented

@tobz tobz force-pushed the tobz/plex-forward-auth branch from 0178509 to 31debf6 Compare May 6, 2024 00:09
@tobz tobz requested a review from OwsleyJr as a code owner May 6, 2024 00:09
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.

5 participants