diff --git a/frontend/src/lib/components/CenteredModal.svelte b/frontend/src/lib/components/CenteredModal.svelte index a72df74002..e3f8ea1604 100644 --- a/frontend/src/lib/components/CenteredModal.svelte +++ b/frontend/src/lib/components/CenteredModal.svelte @@ -1,14 +1,12 @@ + -
- {#if autoRedirecting} -

Signing you in…

- {/if} -
+ +{#snippet errorMessage()} + +{/snippet} + + +{#snippet lastUsedBadge()} + +
+ + Last used + +
+{/snippet} + +{#snippet providerButtons()} +
{#if !logins} {#each Array(4) as _} {/each} {:else} - {#each providers as { type, icon }} - {#if logins?.some((login) => login.type === type)} + {#each orderedThirdParty as entry (entry.method.kind === 'saml' ? 'saml:' : `oauth:${entry.method.provider}`)} +
+ {#if sameLoginMethod(lastUsed, entry.method)} + {@render lastUsedBadge()} + {/if} - {/if} +
{/each} - {#each logins.filter((login) => !providersType?.includes(login.type)) as login} - - {/each} - {/if} - {#if saml} - {/if}
- {#if !autoRedirecting && !disablePasswordLogin && (saml || (logins && logins.length > 0))} -
0 ? 'mt-6' : '')}> - -
+{/snippet} + +{#snippet orDivider()} +
+
+ or +
+
+{/snippet} + +
+ {#if autoRedirecting} +

Signing you in…

+ {/if} + + {#if !passwordFirst} + {@render providerButtons()} + {#if !autoRedirecting && !disablePasswordLogin && (saml || (logins && logins.length > 0))} + {@render orDivider()} + + {#if !showPassword} +
+ +
+ {/if} + {/if} {/if} {#if !autoRedirecting && showPassword && !disablePasswordLogin} @@ -525,51 +725,73 @@ Welcome! Default credentials admin@windmill.dev / changeme have been prefilled.

{/if} -
- {#if isCloudHosted()} +
+ {#if cloudHosted}

To get credentials without the OAuth providers above, send an email at contact@windmill.dev

{/if} -
- -
- { - // Only move on once the field holds something: while the browser's - // credential dropdown is open, Enter belongs to the dropdown - if (e.key === 'Enter' && !e.isComposing && !e.repeat && e.currentTarget.value) { - e.preventDefault() - passwordField?.focus() +
+
+ +
+ { + // Only move on once the field holds something: while the browser's + // credential dropdown is open, Enter belongs to the dropdown + if (e.key === 'Enter' && !e.isComposing && !e.repeat && e.currentTarget.value) { + e.preventDefault() + passwordField?.focus() + } } - } - }} - /> + }} + /> +
+ +
+ +
+ +
+ +
+ {@render errorMessage()}
-
- -
- -
+ {/if} + + {#if passwordFirst && (saml || (logins && logins.length > 0))} + {@render orDivider()} + {@render providerButtons()} + {/if}
diff --git a/frontend/src/lib/components/LoginHeading.svelte b/frontend/src/lib/components/LoginHeading.svelte new file mode 100644 index 0000000000..478a5e3a53 --- /dev/null +++ b/frontend/src/lib/components/LoginHeading.svelte @@ -0,0 +1,28 @@ + + + +
+ {#if hasThirdParty !== undefined} +

+ {hasThirdParty ? `Log in or sign up to ${instanceName}` : `Log in to ${instanceName}`} +

+

+ {hasThirdParty + ? 'Log in or sign up with any of the methods below' + : 'Log in with your email and password'} +

+ {/if} +
diff --git a/frontend/src/lib/components/LoginPageHeader.svelte b/frontend/src/lib/components/LoginPageHeader.svelte index d841f48aa1..7b49c97e34 100644 --- a/frontend/src/lib/components/LoginPageHeader.svelte +++ b/frontend/src/lib/components/LoginPageHeader.svelte @@ -1,11 +1,34 @@ - -
-
+
+ +
+ {#if showBrand} + {#if $whitelabelNameStore} + {capitalize($whitelabelNameStore)} + {:else} + + Windmill + {/if} + {/if} +
+ +
diff --git a/frontend/src/lib/components/Password.svelte b/frontend/src/lib/components/Password.svelte index 72e7e43024..569150f7b4 100644 --- a/frontend/src/lib/components/Password.svelte +++ b/frontend/src/lib/components/Password.svelte @@ -18,6 +18,10 @@ autocomplete?: HTMLInputAttributes['autocomplete'] /** Off for login-style fields: keeps Enter free to submit. Overrides `minRows`. */ allowMultiline?: boolean + /** Renders the field in its error state; the message itself is the caller's to display. */ + error?: boolean + /** id of the element holding that message, wired up as aria-describedby. */ + describedBy?: string onKeyDown?: (event: KeyboardEvent) => void onBlur?: (event: FocusEvent) => void } @@ -32,11 +36,14 @@ id, autocomplete = 'new-password', allowMultiline = true, + error = false, + describedBy = undefined, onKeyDown, onBlur }: Props = $props() let red = $derived(required && (password == '' || password == undefined)) + let hasError = $derived(red || error) let hideValue = $state(true) let forceMultiline = $state(false) let isMultiline = $derived( @@ -76,7 +83,7 @@ onBlur?.(e), onkeydown: (e) => { onKeyDown?.(e) @@ -99,13 +108,15 @@ onBlur?.(e), onkeydown: (e) => { if (allowMultiline && e.key === 'Enter') { diff --git a/frontend/src/lib/components/icons/GitlabIcon.svelte b/frontend/src/lib/components/icons/GitlabIcon.svelte index 88fa898a3d..d8aaf12e0e 100644 --- a/frontend/src/lib/components/icons/GitlabIcon.svelte +++ b/frontend/src/lib/components/icons/GitlabIcon.svelte @@ -1,16 +1,30 @@ + import { twMerge } from 'tailwind-merge' + interface Props { - height?: string - width?: string + size?: number + height?: number + width?: number + class?: string } - let { height = '24px', width = '24px' }: Props = $props() + let { + size = undefined, + height: heightProp = 24, + width: widthProp = 24, + class: clazz = '' + }: Props = $props() + + const { width, height } = $derived( + size ? { width: size, height: size } : { width: widthProp, height: heightProp } + ) - interface Props { - size?: number - style?: string - class?: string - } - - let { size = 16, style = 'fill: white;', class: clazz = '' }: Props = $props() - - - - auth0-svg - - diff --git a/frontend/src/lib/components/icons/brands/Github.svelte b/frontend/src/lib/components/icons/brands/Github.svelte deleted file mode 100644 index 35d8319f7f..0000000000 --- a/frontend/src/lib/components/icons/brands/Github.svelte +++ /dev/null @@ -1,23 +0,0 @@ - - - - - - diff --git a/frontend/src/lib/components/icons/brands/Gitlab.svelte b/frontend/src/lib/components/icons/brands/Gitlab.svelte deleted file mode 100644 index 8b4a1e828f..0000000000 --- a/frontend/src/lib/components/icons/brands/Gitlab.svelte +++ /dev/null @@ -1,23 +0,0 @@ - - - - - - diff --git a/frontend/src/lib/components/icons/brands/Google.svelte b/frontend/src/lib/components/icons/brands/Google.svelte deleted file mode 100644 index 9798da0c36..0000000000 --- a/frontend/src/lib/components/icons/brands/Google.svelte +++ /dev/null @@ -1,23 +0,0 @@ - - - - - - diff --git a/frontend/src/lib/components/icons/brands/Microsoft.svelte b/frontend/src/lib/components/icons/brands/Microsoft.svelte deleted file mode 100644 index 3c4acbb9ec..0000000000 --- a/frontend/src/lib/components/icons/brands/Microsoft.svelte +++ /dev/null @@ -1,23 +0,0 @@ - - - - > - - diff --git a/frontend/src/lib/components/icons/brands/Okta.svelte b/frontend/src/lib/components/icons/brands/Okta.svelte deleted file mode 100644 index 469b60d76c..0000000000 --- a/frontend/src/lib/components/icons/brands/Okta.svelte +++ /dev/null @@ -1,26 +0,0 @@ - - - - oktaddd-svg - - diff --git a/frontend/src/lib/components/icons/index.ts b/frontend/src/lib/components/icons/index.ts index d5a7f1df4a..825f671e43 100644 --- a/frontend/src/lib/components/icons/index.ts +++ b/frontend/src/lib/components/icons/index.ts @@ -337,6 +337,12 @@ import type { Component } from 'svelte' * Most variants are the pre-audit artwork painted with currentColor. GoogleCloudIcon is * greyscale instead — four grey tones rather than one flat colour — because Google's * cloud loses its internal shape when flattened. If the artwork changes, change both. + * + * Brand marks carry a viewBox that centres the artwork in a box 24/22 of its bounding + * size, so the mark occupies the same safe area a lucide glyph does on its 24 grid. + * Vendor SVGs come with whatever padding the vendor chose — none for Google, 10% for + * GitHub — so a raw viewBox makes them render at visibly different sizes from each other + * and from the lucide icons beside them. Re-derive the viewBox when replacing artwork. */ export const APP_TO_ICON_COMPONENT = { diff --git a/frontend/src/lib/lastLoginMethod.test.ts b/frontend/src/lib/lastLoginMethod.test.ts new file mode 100644 index 0000000000..ffe1714f93 --- /dev/null +++ b/frontend/src/lib/lastLoginMethod.test.ts @@ -0,0 +1,47 @@ +import { describe, it, expect, beforeEach } from 'vitest' +import { + clearPendingLoginMethod, + confirmPendingLoginMethod, + getLastLoginMethod, + markLoginMethodPending, + rememberLoginMethod +} from './lastLoginMethod' + +describe('lastLoginMethod', () => { + beforeEach(() => localStorage.clear()) + + it('only promotes a pending method when told a session exists', () => { + markLoginMethodPending({ kind: 'oauth', provider: 'gitlab' }) + expect(getLastLoginMethod()).toBeUndefined() + + confirmPendingLoginMethod() + expect(getLastLoginMethod()).toEqual({ kind: 'oauth', provider: 'gitlab' }) + + // the pending slot is spent, so a later confirm cannot re-promote it + localStorage.removeItem('lastLoginMethod') + confirmPendingLoginMethod() + expect(getLastLoginMethod()).toBeUndefined() + }) + + it('forgets a pending method that never became a login', () => { + rememberLoginMethod({ kind: 'password' }) + markLoginMethodPending({ kind: 'oauth', provider: 'github' }) + clearPendingLoginMethod() + + confirmPendingLoginMethod() + expect(getLastLoginMethod()).toEqual({ kind: 'password' }) + }) + + it('ignores stored values it does not recognise', () => { + for (const stored of [ + 'not json', + '{}', + '"password"', + '{"kind":"oauth"}', + '{"kind":"carrier"}' + ]) { + localStorage.setItem('lastLoginMethod', stored) + expect(getLastLoginMethod()).toBeUndefined() + } + }) +}) diff --git a/frontend/src/lib/lastLoginMethod.ts b/frontend/src/lib/lastLoginMethod.ts new file mode 100644 index 0000000000..db1010911b --- /dev/null +++ b/frontend/src/lib/lastLoginMethod.ts @@ -0,0 +1,61 @@ +// The login method that last worked on this browser, so the card can put it first and badge it. +// Purely a hint: it is never read for anything but ordering and a label. +export type LastLoginMethod = + | { kind: 'password' } + | { kind: 'oauth'; provider: string } + | { kind: 'saml' } + +const CONFIRMED_KEY = 'lastLoginMethod' +// OAuth and SAML leave the page before the outcome is known, so the method is parked here and +// only promoted once a session exists — otherwise an abandoned provider would claim the badge. +const PENDING_KEY = 'lastLoginMethodPending' + +function read(key: string): LastLoginMethod | undefined { + try { + const raw = localStorage.getItem(key) + if (!raw) return undefined + const parsed = JSON.parse(raw) + if (parsed?.kind === 'password' || parsed?.kind === 'saml') return parsed + if (parsed?.kind === 'oauth' && typeof parsed.provider === 'string') return parsed + return undefined + } catch { + return undefined + } +} + +function write(key: string, method: LastLoginMethod | undefined) { + try { + if (method) localStorage.setItem(key, JSON.stringify(method)) + else localStorage.removeItem(key) + } catch (e) { + console.error('Could not record the last login method', e) + } +} + +export function getLastLoginMethod(): LastLoginMethod | undefined { + return read(CONFIRMED_KEY) +} + +export function rememberLoginMethod(method: LastLoginMethod) { + write(CONFIRMED_KEY, method) + write(PENDING_KEY, undefined) +} + +export function markLoginMethodPending(method: LastLoginMethod) { + write(PENDING_KEY, method) +} + +export function clearPendingLoginMethod() { + write(PENDING_KEY, undefined) +} + +/** Call only where a session is proven: whatever redirect was in flight is what worked. */ +export function confirmPendingLoginMethod() { + const pending = read(PENDING_KEY) + if (pending) rememberLoginMethod(pending) +} + +export function sameLoginMethod(a: LastLoginMethod | undefined, b: LastLoginMethod): boolean { + if (!a || a.kind !== b.kind) return false + return a.kind !== 'oauth' || a.provider === (b as { provider: string }).provider +} diff --git a/frontend/src/lib/loginError.test.ts b/frontend/src/lib/loginError.test.ts new file mode 100644 index 0000000000..cf76dca5ec --- /dev/null +++ b/frontend/src/lib/loginError.test.ts @@ -0,0 +1,40 @@ +import { describe, it, expect } from 'vitest' +import { loginErrorMessage } from './loginError' + +describe('loginErrorMessage', () => { + it('maps the backend rejection to one message for a wrong email and a wrong password', () => { + expect(loginErrorMessage({ status: 400, body: 'Bad request: Invalid login' })).toBe( + 'Invalid email or password.' + ) + }) + + it('surfaces the messages the endpoint is known to produce', () => { + expect( + loginErrorMessage({ + status: 400, + body: 'Bad request: Password login is disabled on this instance' + }) + ).toBe('Password login is disabled on this instance') + // The rate limiter's own wording is not echoed back: a 429 always gets this sentence. + expect(loginErrorMessage({ status: 429, body: 'Bad request: slow down' })).toBe( + 'Too many login attempts. Please try again later.' + ) + }) + + it('never surfaces server text it does not recognise, to an unauthenticated visitor', () => { + const sqlError = { + status: 400, + body: 'Bad request: SqlErr: error returned from database: relation "password" does not exist @backend/windmill-api-users/src/users.rs:123' + } + expect(loginErrorMessage(sqlError)).toBe('Could not sign you in. Please try again.') + expect( + loginErrorMessage({ status: 502, body: '502 Bad Gateway' }) + ).toBe('Could not sign you in. Please try again.') + expect(loginErrorMessage(new TypeError('Failed to fetch'))).toBe( + 'Could not sign you in. Please try again.' + ) + expect(loginErrorMessage({ status: 400, body: { error: { message: { nested: true } } } })).toBe( + 'Could not sign you in. Please try again.' + ) + }) +}) diff --git a/frontend/src/lib/loginError.ts b/frontend/src/lib/loginError.ts new file mode 100644 index 0000000000..d26f44abd6 --- /dev/null +++ b/frontend/src/lib/loginError.ts @@ -0,0 +1,24 @@ +// Only messages the login endpoint is known to produce are shown. Anything else — a SQL error +// (which the API also returns as a 400, with the query and a source location), a proxy's HTML +// error page — would otherwise be printed verbatim to an unauthenticated visitor. +const KNOWN_LOGIN_ERRORS = ['Password login is disabled on this instance'] + +const GENERIC_LOGIN_ERROR = 'Could not sign you in. Please try again.' + +export function loginErrorMessage(err: any): string { + // The API returns errors as plain text, prefixed by their class (e.g. "Bad request: Invalid + // login"); ApiError.message is only the HTTP status text. + const raw = typeof err?.body === 'string' ? err.body : err?.body?.error?.message + const body = typeof raw === 'string' ? raw : '' + const detail = body.replace(/^(Bad request|Internal|Error): /, '').trim() + if (detail === 'Invalid login') { + return 'Invalid email or password.' + } + if (err?.status === 429) { + return 'Too many login attempts. Please try again later.' + } + if (KNOWN_LOGIN_ERRORS.includes(detail)) { + return detail + } + return GENERIC_LOGIN_ERROR +} diff --git a/frontend/src/routes/(root)/(logged)/+layout.svelte b/frontend/src/routes/(root)/(logged)/+layout.svelte index e55cfd54a9..257af0fbbd 100644 --- a/frontend/src/routes/(root)/(logged)/+layout.svelte +++ b/frontend/src/routes/(root)/(logged)/+layout.svelte @@ -63,6 +63,7 @@ import { syncTutorialsTodos } from '$lib/tutorialUtils' import { PanelLeftClose, PanelLeftOpen, Home, Play, Search, WandSparkles } from 'lucide-svelte' import { getUserExt } from '$lib/user' + import { confirmPendingLoginMethod } from '$lib/lastLoginMethod' import { deepEqual } from 'fast-equals' import { twMerge } from 'tailwind-merge' import OperatorMenu from '$lib/components/sidebar/OperatorMenu.svelte' @@ -315,6 +316,9 @@ } } const user = await getUserExt(workspace) + // getUserExt resolves to undefined on failure, so a user is the only proof of a + // session: without it a cancelled SSO round trip would claim the "Last used" badge. + if (user) confirmPendingLoginMethod() // Every workspace change starts a fetch without cancelling the one before it, // so a slow response can land after a faster one for the workspace the user // has since moved to. The store must describe the active workspace: letting a diff --git a/frontend/src/routes/(root)/(logged)/user/(user)/login/+page.svelte b/frontend/src/routes/(root)/(logged)/user/(user)/login/+page.svelte index 2ea7ab1c89..d7520bd873 100644 --- a/frontend/src/routes/(root)/(logged)/user/(user)/login/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/user/(user)/login/+page.svelte @@ -10,17 +10,18 @@ enterpriseLicense, whitelabelNameStore } from '$lib/stores' - import { classNames, emptyString, parseQueryParams } from '$lib/utils' + import { emptyString, parseQueryParams } from '$lib/utils' import { getUserExt } from '$lib/user' - import { WindmillIcon } from '$lib/components/icons' import LoginPageHeader from '$lib/components/LoginPageHeader.svelte' - import DarkModeToggle from '$lib/components/sidebar/DarkModeToggle.svelte' + import { WindmillIcon } from '$lib/components/icons' import { clearStores } from '$lib/storeUtils' import { setLicense } from '$lib/enterpriseUtils' import Login from '$lib/components/Login.svelte' + import LoginHeading from '$lib/components/LoginHeading.svelte' import { onMount } from 'svelte' import { refreshSuperadmin } from '$lib/refreshUser' import { isValidLogoutRedirect, toSameOriginRelativePath } from '$lib/logoutRedirect' + import { confirmPendingLoginMethod } from '$lib/lastLoginMethod' const email = page.url.searchParams.get('email') ?? '' const password = page.url.searchParams.get('password') ?? '' @@ -37,8 +38,10 @@ const sameOriginRd = toSameOriginRelativePath(rawRd) const rd = sameOriginRd ?? rawRd - let showPassword = false let firstTime = $state(false) + // A third-party login creates the account on first use, so the page only offers sign-up + // once the instance has one configured. undefined until the card reports what it loaded. + let hasThirdParty = $state(undefined) function clearWindmillCloudCookies() { const domain = window.location.hostname @@ -118,6 +121,8 @@ async function redirectIfNecessary() { await UserService.getCurrentEmail() + // Reached only with a session: an SSO round trip that landed back here worked. + confirmPendingLoginMethod() redirectUser() } @@ -134,30 +139,32 @@ } -
- -
+ +
+ + +
{#if !$enterpriseLicense || !$whitelabelNameStore} - + {/if}
-

- Log in or sign up -

-

- Log in or sign up with any of the methods below -

+
+ +
-
-
- -
- +
+ (hasThirdParty = options.hasThirdParty)} + />
diff --git a/frontend/src/routes/approve/[workspace]/[job]/+page.svelte b/frontend/src/routes/approve/[workspace]/[job]/+page.svelte index 6b88204c08..34e704d6fa 100644 --- a/frontend/src/routes/approve/[workspace]/[job]/+page.svelte +++ b/frontend/src/routes/approve/[workspace]/[job]/+page.svelte @@ -212,7 +212,6 @@ {#if error} diff --git a/frontend/src/routes/approve/[workspace]/[job]/[resume]/[hmac]/+page.svelte b/frontend/src/routes/approve/[workspace]/[job]/[resume]/[hmac]/+page.svelte index 5a2f0b4746..18fe489a9a 100644 --- a/frontend/src/routes/approve/[workspace]/[job]/[resume]/[hmac]/+page.svelte +++ b/frontend/src/routes/approve/[workspace]/[job]/[resume]/[hmac]/+page.svelte @@ -173,7 +173,7 @@ - + {#if error}
{#if error.startsWith('Not authorized:')} diff --git a/frontend/src/routes/kitchen_sink/login/+page.svelte b/frontend/src/routes/kitchen_sink/login/+page.svelte new file mode 100644 index 0000000000..0c51f65bec --- /dev/null +++ b/frontend/src/routes/kitchen_sink/login/+page.svelte @@ -0,0 +1,210 @@ + + +{#if enabled} +
+
+
+

Login page states

+

+ Real cards, fixed instance config, no API calls. Sign in always fails so the error state + is one click away (click twice for the shake). +

+
+
+
Card width
+