overseerr
overseerr copied to clipboard
OIDC Support - Attempt 2
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 allowsaud
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
- [x] Successful build
yarn build
- [x] Translation keys
yarn i18n:extract
- [-] Database migration (if required)
Issues Fixed or Closed
- Fixes: https://github.com/sct/overseerr/pull/2792
@sct Let's try this again, this should only include the OIDC changes now, plus some small bug fixes.
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)$
@sct got eslint passing (for code I touched) and addressed log injection risk.
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
Not stale
@sct can we get someone to look at this?
merged latest develop into the branch.
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.]
Has this pull request stalled as well?
I'm not 100% sure I got this right but it does work to some extent. When I tested one of my plex users, on the first run, if they're logged via authentik already, they're still prompted with a choice to login with plex or authentik on the overseerr page (when i expected the oauth to see them as an existing plex user). When i check the console in Chrome i see:
GET https://req.mydomain.com/api/v1/auth/me 403 (Forbidden) {"status":403,"error":"You do not have permission to access this endpoint"}
If i have that user click on the authentik login it works from that point on. Is there some setting on authentik that I should know about to translate the username or email to overseerr?
This would be an amazing feature. Why does this keep getting ignored?
Hello owner!!! Please review!!!
@sct @samwiseg0 @OwsleyJr @danshilm @TheCatLady Please, help to merge.
@lenaxia This branch is out-of-date with the base branch