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

OIDC Support - Attempt 2 #3746

Open
wants to merge 33 commits into
base: develop
Choose a base branch
from
Open

Conversation

lenaxia
Copy link

@lenaxia lenaxia commented Jan 10, 2024

Description

This PR rebases the oidc changes to the current Overseerr mainline (develop), and fixes some improper OIDC implementation. Because of this, it now works with authelia. Changes have also been revalidated to work with a basic configuration of Authentik. I have not tested this with other OIDC providers.

Fixes include:

  • Adding support for scope and error parameters in the oidc-callback endpoint (these are all optional parameters)
  • fixing callback redirect_uri protocol generation to base on the initiating protocol, as opposed to assumping https, which is what it did previously
  • add support for the aud (audience) callback parameter being an array. the OIDC spec allows aud to be either a string, or an array of strings. Previous implementation here only allowed string when doing a oidc validation. This is now fixed to support both string and array
  • Added improved logging and error handling to make future debugging easier.
  • change default logging level to be info if LOG_LEVEL env variable is not defined (previously was debug)
  • Improved JWT token validation robustness

Bug Fix:

  • Plex New Login button was incorrectly tied to the localLogin settings flag. It also was disabled unnecessarily and did not match the formatting of the local login (and now OIDC) buttons. This has been fixed.

Screenshot (if UI-related)

To-Dos

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

Issues Fixed or Closed

@lenaxia lenaxia mentioned this pull request Jan 10, 2024
2 tasks
@lenaxia
Copy link
Author

lenaxia commented Jan 10, 2024

@sct Let's try this again, this should only include the OIDC changes now, plus some small bug fixes.

server/index.ts Outdated Show resolved Hide resolved
@lenaxia
Copy link
Author

lenaxia commented Jan 10, 2024

Okay, addressed eslint and logging injection attack. Further, improved middleware logging to scrub sensitive data from log messages and sanitize against injection attacks.

ES Lint output:


/home/ubuntu/workspace/overseerr-oidc/server/api/rating/imdbRadarrProxy.ts
   20:20  error  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
   30:11  error  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  137:11  error  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  138:13  error  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  139:17  error  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  140:10  error  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any

✖ 6 problems (6 errors, 0 warnings)

yarn build output:

ubuntu@terraform:~/workspace/overseerr-oidc(⎈|prod:networking)$ docker build --build-arg COMMIT_TAG=b4256043424658626e02bd002de1d4369f541bdb -t overseerr-oidc .
[+] Building 596.0s (18/18) FINISHED                                                                                                                               docker:default
 => [internal] load build definition from Dockerfile                                                                                                                         0.1s
 => => transferring dockerfile: 996B                                                                                                                                         0.0s
 => [internal] load .dockerignore                                                                                                                                            0.1s
 => => transferring context: 392B                                                                                                                                            0.0s
 => [internal] load metadata for docker.io/library/node:18.18-alpine                                                                                                         1.2s
 => [internal] load build context                                                                                                                                            0.2s
 => => transferring context: 53.21kB                                                                                                                                         0.2s
 => [build_image  1/11] FROM docker.io/library/node:18.18-alpine@sha256:16b46e5ea9fb5c2d13dda36f0feb670fa89de6a412725007555f2eee9a126b60                                     0.0s
 => CACHED [build_image  2/11] WORKDIR /app                                                                                                                                  0.0s
 => CACHED [build_image  3/11] RUN   case "linux/amd64" in   'linux/arm64' | 'linux/arm/v7')   apk update &&   apk add --no-cache python3 make g++ gcc libc6-compat bash &&  0.0s
 => CACHED [build_image  4/11] COPY package.json yarn.lock ./                                                                                                                0.0s
 => CACHED [build_image  5/11] RUN CYPRESS_INSTALL_BINARY=0 yarn install --frozen-lockfile --network-timeout 1000000                                                         0.0s
 => [build_image  6/11] COPY . ./                                                                                                                                            0.6s
 => [build_image  7/11] RUN yarn build                                                                                                                                     483.0s
 => [build_image  8/11] RUN yarn install --production --ignore-scripts --prefer-offline                                                                                     21.8s
 => [build_image  9/11] RUN rm -rf src server .next/cache                                                                                                                    0.7s
 => [build_image 10/11] RUN touch config/DOCKER                                                                                                                              0.7s
 => [build_image 11/11] RUN echo "{"commitTag": "b4256043424658626e02bd002de1d4369f541bdb"}" > committag.json                                                                0.6s
 => CACHED [stage-1 3/4] RUN apk add --no-cache tzdata tini && rm -rf /tmp/*                                                                                                 0.0s
 => [stage-1 4/4] COPY --from=BUILD_IMAGE /app ./                                                                                                                           33.3s
 => exporting to image                                                                                                                                                      24.2s
 => => exporting layers                                                                                                                                                     24.1s
 => => writing image sha256:bc03784e0d0e1914db7968888b72233a1e81cd9ac9ca2672cf4fec8a41631865                                                                                 0.0s
 => => naming to docker.io/library/overseerr-oidc                                                                                                                            0.0s
ubuntu@terraform:~/workspace/overseerr-oidc(⎈|prod:networking)$

@lenaxia
Copy link
Author

lenaxia commented Jan 10, 2024

@sct got eslint passing (for code I touched) and addressed log injection risk.

This comment was marked as resolved.

@stale stale bot added the stale label Mar 13, 2024
@lenaxia
Copy link
Author

lenaxia commented Mar 14, 2024

Not stale

@sct can we get someone to look at this?

@stale stale bot removed the stale label Mar 14, 2024
@lenaxia
Copy link
Author

lenaxia commented Mar 22, 2024

merged latest develop into the branch.

@tenfourty
Copy link

tenfourty commented Apr 22, 2024

I've just tested this fork with my Authelia setup, and it works; thank you, @lenaxia, for the docker image and for keeping this PR up to date.

Some quick thoughts after using this:

  • The setting "Enable New Plex Sign-In (Allow Plex users to sign in without first being imported)" could be changed to include OIDC users that log in.
  • I would love it if there was a way to disable sign-in via Plex to only allow OIDC logins, just like there is an option to disable local logins. (The logic could allow admins to disable sign-in methods like local, Plex, and OIDC as long as one method is enabled).

@sct is there any reason why this PR isn't being accepted?

[Edit: I realised you might be waiting for #3015 to be merged before accepting this one - thanks for your work on this cool project. I recognise you are doing this in your spare time, so please don't take my comment as an attempt to hassle you. More to understand the logic/sequence you had in mind.]

@nebb00
Copy link

nebb00 commented Jun 25, 2024

Has this pull request stalled as well?

@xTerm1nus
Copy link

This would be an amazing feature. Why does this keep getting ignored?

@alexsalex
Copy link

Hello owner!!! Please review!!!

@alexsalex
Copy link

@sct @samwiseg0 @OwsleyJr @danshilm @TheCatLady Please, help to merge.

@alexsalex
Copy link

@lenaxia This branch is out-of-date with the base branch

@Flight777
Copy link

@lenaxia Will this still be updated?

@edbourque0
Copy link

@sct This would be an awesome feature

@scoobydoofus
Copy link

I've just tested this fork with my Authelia setup, and it works; thank you, @lenaxia, for the docker image and for keeping this PR up to date.

I can't get it working with Authelia. I click the "sign in with Authelia" option, I get the consent request popup, I accept, and it loads a blank page with error "{"message":"Unexpected token < in JSON at position 0"}" at the top.

I've tried all sorts of changes to the Authelia config but nothing gets me any further than this. Could you share your Authelia client config for Overseerr? And I assume my OIDC domain in the Overseerr settings is just overseer.domain.com?

@jkossis
Copy link

jkossis commented Oct 31, 2024

Seconding this, @sct could we get some eyes/approval on this?

@lenaxia
Copy link
Author

lenaxia commented Nov 6, 2024

@scoobydoofus

I've just tested this fork with my Authelia setup, and it works; thank you, @lenaxia, for the docker image and for keeping this PR up to date.

I can't get it working with Authelia. I click the "sign in with Authelia" option, I get the consent request popup, I accept, and it loads a blank page with error "{"message":"Unexpected token < in JSON at position 0"}" at the top.

I've tried all sorts of changes to the Authelia config but nothing gets me any further than this. Could you share your Authelia client config for Overseerr? And I assume my OIDC domain in the Overseerr settings is just overseer.domain.com?

      #- id: overseerr
      #  description: overseerr
      #  secret: ${SECRET_OVERSEERR_OAUTH_CLIENT_SECRET_HASHED}
      #  public: false
      #  authorization_policy: one_factor
      #  consent_mode: implicit
      #  pre_configured_consent_duration: 1y
      #  scopes: ["openid", "profile", "email"]
      #  redirect_uris:
      #    - "https://request.${SECRET_DEV_DOMAIN}/api/v1/auth/oidc-callback"
      #  userinfo_signing_algorithm: none

@gnome161
Copy link

gnome161 commented Nov 9, 2024

I'm having exactly the same issue. My client setup in Authelia is basically identical to yours but I'm receiving the below error when trying to sign in on Overseerr.

{"message":"Unexpected token < in JSON at position 0"}

feat: oidc 2

feat: oidc
@h3mmy
Copy link

h3mmy commented Nov 9, 2024

I'm having exactly the same issue. My client setup in Authelia is basically identical to yours but I'm receiving the below error when trying to sign in on Overseerr.

{"message":"Unexpected token < in JSON at position 0"}

Sounds like it's getting an html response instead of json. I see these with 401s in my work but getting a look at the underlying message would probably help pinpoint what is wrong

@scoobydoofus
Copy link

Yeah my Authelia config is in a slightly different format than lenanxia's example but it matches closely (I'm confident my format is valid, it works with another application), and I'm still getting that error.

Sounds like it's getting an html response instead of json. I see these with 401s in my work but getting a look at the underlying message would probably help pinpoint what is wrong

What do you mean by "the underlying message"? When I check the console the error it's returning is "Failed to load resource: the server responded with a status of 500 ()". The URL is "https://overseerr.mydomain.com/api/v1/auth/oidc-callback?code=authelia_ac_[REDACTED]&iss=https%3A%2F%2Fauth.mydomain.com&scope=openid+profile+email&state=[REDACTED]".

@gnome161
Copy link

gnome161 commented Nov 16, 2024

I did a bit more digging on this today. As scoobydoofus mentions I'm also getting a 500 response from the https://overseerr.{MYDOMAIN}/api/v1/auth/oidc-login endpoint which is presumably what's causing the problem.

Looking at the logs in authelia it seems it's not getting passed the correct information as all the logs are suggesting the requests are coming from a source that isn't authenticated with the correct credentials but I'm definitely logged into Authelia. The below is what I get in my Authelia logs whenever I try to login.

"Access to https://overseer.{MYDOMAIN}.app/{...} (method GET) is not authorized to user , responding with status code 401"

Unless there is something special about Overseerr, I don't think anything is off in my configuration, as it works fine for OIDC login to all my other services like Komga, Immich, etc.

@shelldandy
Copy link

I tried using the ghcr.io/lenaxia/overseerr-oidc:oidc-support with authentik but im also getting the error:

{"message":"Unexpected token < in JSON at position 4"}

The logs from the docker container are:

2024-12-02T04:35:09.852Z [error]: Failed to initiate OIDC login {"path":"/oidc-login","error":"Unexpected token < in JSON at position 4"}

So not super helpful, maybe we're doing something very wrong?

@shelldandy
Copy link

I actually got it working, so for future reference this is how i got it working with Authentik:

Some placeholders:
auth.company => FQDN of your authentik install
overseer.company => FQDN of your overseerr install

Authentik

  • Create a new Oauth2 provider, call it overseerr and set the redirect URI to: https://overseer.company/api/v1/auth/oidc-callback
  • Take note of the client_id and client_secret
  • Now create an application that uses the overseerr provider and call it overseerr as well with a slug of overseerr and give it a Launch URL of: https://overseer.company (this may not be optimal but it works, lol)

Overseerr

  • Go to Settings / General and click enable proxy support click save and restart the server
  • Got to Settings / Users and set the following:
  • Enable OIDC Signin (checked)
  • OIDC Provider Name: Authentik
  • OIDC Domain: auth.company/application/o/overseerr
  • Then add the client id and client secret from the authentik provider

Save and enjoy!

@BerserkeR-Git
Copy link

This feature seems to be highly anticipated. It would be a shame if it went stale or outdated on the main branch again. @sct

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.