-
-
Notifications
You must be signed in to change notification settings - Fork 263
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: support for forwardAuth #580
base: develop
Are you sure you want to change the base?
Conversation
557af26
to
c9efbb9
Compare
This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged. |
ef325ba
to
b4143ef
Compare
b4143ef
to
386293c
Compare
386293c
to
01665c4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please add a bit of documentation for this?
|
||
This works by passing the authenticated user e-mail in `X-Forwarded-User` header by the auth server, therefore enabling single-sign-on (SSO) login. | ||
|
||
> The user has to exist, it will not be created automatically. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably better to use a warning instead of a citation.
> The user has to exist, it will not be created automatically. | |
:::warning | |
The user has to exist, it will not be created automatically. | |
::: |
if (user) { | ||
req.user = user; | ||
} | ||
|
||
req.locale = user?.settings?.locale | ||
? user.settings.locale | ||
: settings.main.locale; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not necessary, it's a duplicate from below.
enableForwardAuth: | ||
type: boolean | ||
example: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You wrote the setting here, but it is not available anywhere.
You should add this as a general setting, by adding a new properties in the MainSettings
type in server/lib/settings.ts
and adding a new input in the settings page in src/components/Settings/SettingsMain/index.tsx
. You should also check that this setting is enabled before allowing login with forwardAuth.
Or did you intend to not make a setting and set it enabled by default?
@@ -6939,3 +6947,4 @@ paths: | |||
security: | |||
- cookieAuth: [] | |||
- apiKey: [] | |||
- forwardAuth: [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not in the OpenAPI spec?
https://spec.openapis.org/oas/latest.html#security-scheme-object-0
import type { GetServerSidePropsContext, PreviewData } from 'next/types'; | ||
import type { ParsedUrlQuery } from 'querystring'; | ||
|
||
export const getRequestHeaders = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this function be called something like getAuthHeaders
instead of getRequestHeaders
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wdyt about renaming this file to something like serverSidePropsHelpers
?
Description
Add support for forwardAuth available with nginx, caddy, traefik, etc. which allows use of SSO without exposing any jellyseerr services to unauthenticated users.
Inspired by similar PR in overseer but slightly simplified, requiring existing user to authenticate.
To-Dos
yarn build