mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-06 16:02:23 +00:00
fix: OAuth popup login reliability + auto-login Safari edge cases (#8971)
- Login.svelte: poll whoami after popup opens as a safety net for Safari
ITP — when the popup is opened without a fresh user gesture, cookies
and localStorage can be partitioned, leaving the existing postMessage
/ storage signaling unable to reach the parent. Polling is independent
of partitioning since it runs in the parent's own session. An
oauthFlowDone flag guards the three terminal paths (postMessage,
storage, poll) so onLoginSuccess fires exactly once. Adds compact
"oauth: signaled via {postMessage|storage|poll}" diagnostic logs.
- routes/user/login_callback/[client_name]/+page.svelte: replace `??`
with `||` on the cookie/localStorage fallback. The cookie check
returns a boolean, so `??` never fell through and the localStorage
branch was dead code.
- InstanceSettings.svelte: per-category save/discard for the
Auth/OAuth/SAML tab now sees auto_login_provider and
disable_password_login. getSettingsForCategory was returning only
scimSamlSetting for that tab, leaving the dirty check and per-category
save unable to detect changes to those fields.
- vite.config.js: drop a stale personal dev hostname from allowedHosts.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
bef0a36c55
commit
3c3c03455d
@@ -371,7 +371,7 @@
|
||||
|
||||
function getSettingsForCategory(category: string) {
|
||||
if (category === 'Auth/OAuth/SAML') {
|
||||
return scimSamlSetting
|
||||
return [...(settings[category] ?? []), ...scimSamlSetting]
|
||||
}
|
||||
const base = settings[category] ?? []
|
||||
// In quick setup, reorder Core: base settings (without license_key), then extras from Jobs
|
||||
@@ -502,27 +502,20 @@
|
||||
}
|
||||
|
||||
export function discardCategory(category: string) {
|
||||
const categorySettings = getSettingsForCategory(category)
|
||||
for (const s of categorySettings) {
|
||||
const v = initialValues[s.key]
|
||||
$values[s.key] = v !== undefined ? JSON.parse(JSON.stringify(v)) : undefined
|
||||
}
|
||||
if (category === 'Auth/OAuth/SAML') {
|
||||
for (const s of scimSamlSetting) {
|
||||
const v = initialValues[s.key]
|
||||
$values[s.key] = v !== undefined ? JSON.parse(JSON.stringify(v)) : undefined
|
||||
}
|
||||
oauths = JSON.parse(JSON.stringify(initialOauths))
|
||||
requirePreexistingUserForOauth = initialRequirePreexistingUserForOauth
|
||||
const account_identifier =
|
||||
initialOauths?.snowflake_oauth?.connect_config?.extra_params?.account_identifier
|
||||
snowflakeAccountIdentifier = account_identifier ?? ''
|
||||
} else {
|
||||
const categorySettings = getSettingsForCategory(category)
|
||||
for (const s of categorySettings) {
|
||||
const v = initialValues[s.key]
|
||||
$values[s.key] = v !== undefined ? JSON.parse(JSON.stringify(v)) : undefined
|
||||
}
|
||||
if (category === 'Registries') {
|
||||
const v = initialValues['workspace_registries']
|
||||
$values['workspace_registries'] =
|
||||
v !== undefined ? JSON.parse(JSON.stringify(v)) : undefined
|
||||
}
|
||||
} else if (category === 'Registries') {
|
||||
const v = initialValues['workspace_registries']
|
||||
$values['workspace_registries'] = v !== undefined ? JSON.parse(JSON.stringify(v)) : undefined
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -96,6 +96,7 @@
|
||||
let smtpConfigured: boolean | undefined = $state(undefined)
|
||||
let disablePasswordLogin = $state(false)
|
||||
let autoRedirecting = $state(false)
|
||||
let oauthFlowDone = false
|
||||
|
||||
type OAuthLogin = {
|
||||
type: string
|
||||
@@ -301,18 +302,23 @@
|
||||
if (data.type === 'error') {
|
||||
sendUserToast(data.error, true)
|
||||
} else if (data.type === 'success') {
|
||||
onLoginSuccess?.()
|
||||
finishOauthFlow('postMessage')
|
||||
}
|
||||
}
|
||||
|
||||
function handleStorageEvent(event) {
|
||||
if (event.key === 'oauth-success') {
|
||||
try {
|
||||
processPopupData(JSON.parse(event.newValue))
|
||||
const data = JSON.parse(event.newValue)
|
||||
console.log('oauth-success from storage')
|
||||
// Clean up
|
||||
localStorage.removeItem('oauth-success')
|
||||
window.removeEventListener('storage', handleStorageEvent)
|
||||
if (data?.type === 'success') {
|
||||
finishOauthFlow('storage')
|
||||
} else {
|
||||
processPopupData(data)
|
||||
}
|
||||
} catch (e) {
|
||||
console.error('Could not process oauth-success from storage', e)
|
||||
}
|
||||
@@ -321,6 +327,16 @@
|
||||
}
|
||||
}
|
||||
|
||||
function finishOauthFlow(via: 'postMessage' | 'storage' | 'poll', win?: Window) {
|
||||
if (oauthFlowDone) return
|
||||
oauthFlowDone = true
|
||||
console.log(`oauth: signaled via ${via}`)
|
||||
if (win && !win.closed) win.close()
|
||||
window.removeEventListener('message', popupListener)
|
||||
window.removeEventListener('storage', handleStorageEvent)
|
||||
onLoginSuccess?.()
|
||||
}
|
||||
|
||||
onDestroy(() => {
|
||||
window.removeEventListener('message', popupListener)
|
||||
window.removeEventListener('storage', handleStorageEvent)
|
||||
@@ -351,6 +367,13 @@
|
||||
window.removeEventListener('storage', handleStorageEvent)
|
||||
return false
|
||||
}
|
||||
// Safety net for Safari: when the popup is opened without a fresh user
|
||||
// gesture (auto-login), ITP can partition cookies/localStorage between
|
||||
// popup and parent, so neither the close cookie, the postMessage, nor
|
||||
// the localStorage 'oauth-success' signal reaches us. The session
|
||||
// cookie is set same-origin and isn't subject to that partitioning, so
|
||||
// polling whoami catches the success and lets us force-close the popup.
|
||||
pollForLoginSuccess(win)
|
||||
return true
|
||||
} else {
|
||||
localStorage.setItem('closeUponLogin', 'false')
|
||||
@@ -359,6 +382,33 @@
|
||||
}
|
||||
}
|
||||
|
||||
function pollForLoginSuccess(win: Window) {
|
||||
const startedAt = Date.now()
|
||||
const interval = setInterval(async () => {
|
||||
if (oauthFlowDone) {
|
||||
clearInterval(interval)
|
||||
return
|
||||
}
|
||||
if (Date.now() - startedAt > 5 * 60 * 1000) {
|
||||
clearInterval(interval)
|
||||
console.log('oauth: poll timed out after 5 minutes')
|
||||
return
|
||||
}
|
||||
if (win.closed) {
|
||||
clearInterval(interval)
|
||||
console.log('oauth: popup closed before login completed')
|
||||
return
|
||||
}
|
||||
try {
|
||||
await UserService.getCurrentEmail()
|
||||
} catch {
|
||||
return
|
||||
}
|
||||
clearInterval(interval)
|
||||
finishOauthFlow('poll', win)
|
||||
}, 1500)
|
||||
}
|
||||
|
||||
function redirectSaml(): boolean {
|
||||
if (!saml) {
|
||||
sendUserToast('No SAML login available', true)
|
||||
|
||||
@@ -21,15 +21,13 @@
|
||||
let state = page.url.searchParams.get('state') ?? undefined
|
||||
|
||||
onMount(async () => {
|
||||
// const closeCookie = getAndDeleteCookie('close')
|
||||
// console.log('closeCookie', closeCookie)
|
||||
const rawRd = localStorage.getItem('rd')
|
||||
if (rawRd) {
|
||||
localStorage.removeItem('rd')
|
||||
}
|
||||
const rd = rawRd?.startsWith('http') && !isValidLogoutRedirect(rawRd) ? null : rawRd
|
||||
const cookieCloseUponLogin = getCookie('close') == 'true'
|
||||
const closeUponLogin = cookieCloseUponLogin ?? localStorage.getItem('closeUponLogin') == 'true'
|
||||
const closeUponLogin =
|
||||
getCookie('close') == 'true' || localStorage.getItem('closeUponLogin') == 'true'
|
||||
if (error) {
|
||||
sendUserToast(`Error trying to login with ${clientName} ${error}`, true)
|
||||
if (closeUponLogin) {
|
||||
|
||||
@@ -35,8 +35,7 @@ const config = {
|
||||
'rubendev.wimill.xyz',
|
||||
'windmill.xyz',
|
||||
'app.windmill.xyz',
|
||||
'public.windmill.xyz',
|
||||
'hugo.ngrok.pro'
|
||||
'public.windmill.xyz'
|
||||
],
|
||||
port: parseInt(process.env.FRONTEND_PORT) || 3000,
|
||||
cors: { origin: '*' },
|
||||
|
||||
Reference in New Issue
Block a user