added feature for oidc auto redirection #6066
Conversation
client/src/components/Auth/Login.tsx
Outdated
| const redirectAttemptedRef = useRef(false); | ||
|
|
||
| // Auto-redirect to OpenID provider if enabled | ||
| // This is controlled by the OPENID_AUTO_REDIRECT environment variable | ||
| // When enabled, users will be automatically redirected to the OpenID provider | ||
| // without seeing the login form at all | ||
| useEffect(() => { | ||
| // Simple check if redirect is needed and not yet attempted | ||
| if ( | ||
| !redirectAttemptedRef.current && | ||
| startupConfig?.openidLoginEnabled && | ||
| startupConfig?.openidAutoRedirect && | ||
| startupConfig?.serverDomain | ||
| ) { | ||
| // Mark that we've attempted to redirect | ||
| redirectAttemptedRef.current = true; | ||
|
|
||
| // Log and redirect | ||
| console.log('Auto-redirecting to OpenID provider...'); | ||
| window.location.href = `${startupConfig.serverDomain}/oauth/openid`; | ||
| } | ||
| }, [startupConfig]); | ||
|
|
There was a problem hiding this comment.
what if the callback fails and returns back to the login page? then we have a infantent loop right? and then the OpenID endpoint will then finally return with a 429 status?
There was a problem hiding this comment.
Good catch!
I added some cookies and cooldown logic to prevent that.
It works, however I would really be greatful if someone can review it thoroughly for security questions.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -22,6 +22,13 @@ | |||
| return; | |||
| } | |||
| await setAuthTokens(req.user._id, res); | |||
|
|
|||
Check failure
Code scanning / ESLint
Disallow trailing whitespace at the end of lines Error
| @@ -22,6 +22,13 @@ | |||
| return; | |||
| } | |||
| await setAuthTokens(req.user._id, res); | |||
|
|
|||
| // On successful login, let's clear any openid redirect flags | |||
| res.cookie('successful_login', 'true', { | |||
Check failure
Code scanning / ESLint
Disallow trailing whitespace at the end of lines Error
| // On successful login, let's clear any openid redirect flags | ||
| res.cookie('successful_login', 'true', { | ||
| maxAge: 1000, // very short-lived, just for client-side detection | ||
| httpOnly: false // client needs to read this |
Check failure
Code scanning / ESLint
Require or disallow trailing commas Error
| maxAge: 1000, // very short-lived, just for client-side detection | ||
| httpOnly: false // client needs to read this | ||
| }); | ||
|
|
Check failure
Code scanning / ESLint
Disallow trailing whitespace at the end of lines Error
| @@ -31,7 +38,9 @@ | |||
| router.get('/error', (req, res) => { | |||
| // A single error message is pushed by passport when authentication fails. | |||
| logger.error('Error in OAuth authentication:', { message: req.session.messages.pop() }); | |||
| res.redirect(`${domains.client}/login`); | |||
|
|
|||
Check failure
Code scanning / ESLint
Disallow trailing whitespace at the end of lines Error
| authFailed | ||
| }) |
Check failure
Code scanning / ESLint
Require or disallow trailing commas Error
| if (successfulLogin) { | ||
| // Clear the redirect flag in localStorage | ||
| clearOpenIDRedirectFlag(); | ||
|
|
Check failure
Code scanning / ESLint
Disallow trailing whitespace at the end of lines Error
| // Get timestamp of last redirect attempt from localStorage | ||
| const lastRedirectAttempt = localStorage.getItem(OPENID_REDIRECT_KEY); | ||
| const currentTime = Date.now(); | ||
|
|
Check failure
Code scanning / ESLint
Disallow trailing whitespace at the end of lines Error
| localStorage.setItem(OPENID_REDIRECT_KEY, currentTime.toString()); | ||
| return true; | ||
| } | ||
|
|
Check failure
Code scanning / ESLint
Disallow trailing whitespace at the end of lines Error
|
@leondape thank you for updating the code. What do you think about instead of using the default url for redirecting have a parameter in the url to trigger this? So we don't need to make too many changes in the code? This will also solve the infinite redirect loop? |
|
@rubentalstra could you elaborate your idea? I don’t quite get what you mean. :) |
As described in #5683
Goal
If OIDC is the only option the click on the button is a little redundant.
This option enables automatic redirection to the OIDC provider if set to true
Change Type
Basically the Login page and config types were changed. Please review if any security questions have been touched. If think t should be fine.
Documented in:
Please delete any irrelevant options.