Allow OIDC as default login #3617
Conversation
johnnyaug
added
the
include-changelog
| @@ -57,6 +57,7 @@ This reference uses `.` to denote the nesting of values. | |||
| * `auth.ldap.default_user_group` `(string : )` - Create all LDAP users in this group. Defaults to `Viewers`. | |||
| * `auth.ldap.user_filter` `(string : )` - Additional filter for users. | |||
| * `auth.oidc.enabled` `(boolean : false)` - Set to true to enable authentication with an external OIDC provider. | |||
| * `auth.oidc.is_default_login` `(boolean : false)` - If true, the lakeFS login page will redirect to the external provider by default. | |||
There was a problem hiding this comment.
This is a great way to add a property! I even complained about the added line on swagger.yml (then had to delete that comment when I saw this documentation...).
webui/src/pages/auth/login.jsx
Outdated
| if (!error && response && response.state !== SETUP_STATE_INITIALIZED) { | ||
| router.push({pathname: '/setup', query: router.query}) | ||
| } | ||
| if (!error && response && response.oidc_default_login) { | ||
| window.location = OIDC_LOGIN_URL; | ||
| } |
There was a problem hiding this comment.
The logic here is a bit worrying: it seems like a both ifs could be taken. Now I know that they cannot, but it still looks weird.
What makes it even more confusing is that when oidc_default_login, we do a client-side redirect but also return a <Layout>. AFAIU that React component will never be rendered, so perhaps return after line 88? And then move lines 87..89 about lines 84..86???
I think something like this might be easier (if it is indeed correct, of course!):
| if (!error && response && response.state !== SETUP_STATE_INITIALIZED) { | |
| router.push({pathname: '/setup', query: router.query}) | |
| } | |
| if (!error && response && response.oidc_default_login) { | |
| window.location = OIDC_LOGIN_URL; | |
| } | |
| if (!error && response && response.oidc_default_login) { | |
| window.location = OIDC_LOGIN_URL; | |
| return null; | |
| } | |
| if (!error && response && response.state !== SETUP_STATE_INITIALIZED) { | |
| router.push({pathname: '/setup', query: router.query}) | |
| } | |
|
@arielshaqed - your comment is very important. I changed it so that if you explicitly browse to "/auth/login" you get the old, internal login. If you are redirected from another page - we redirect to OIDC login. PTAL. |
Still looks great! If I want to use username+password then I have to type in |
|
@arielshaqed, thanks! The OIDC page is at the control of the external provider - the user may choose to add a link there to "/auth/login". |
Configure lakeFS to redirect to OIDC login automatically.