Skip to content

Commit 3e1b0d2

Browse files
MaanilVermacoderabbitai[bot]CodeRabbitchristian-byrne
authored
test: cover the cloud auth surface (#14987)
## Summary Adds the missing test coverage for the cloud sign-in and sign-up surface. Four files had no tests at all. ## Changes - **What**: Tests only, no product code. Previously uncovered: the `beforeEnter` route guards and their `switchAccount` escape hatch, `usePostAuthRedirect`, the sign-in password schema, and the terms notice. The rest extends existing files rather than duplicating their setup. Covered here: route guards on both `/cloud/login` and `/cloud/signup`, post-auth redirect precedence and its open-redirect guard, password boundaries and character classes, the terms notice and its outbound-link safety, the sign-in form's validation states and autofill attributes, the turnstile gate on sign-up, and the insecure-context warning for self-hosted HTTP origins. - **Breaking**: None. ## Review Focus The route-guard tests drive both `/cloud/login` and `/cloud/signup` through the same `describe.for`. Those guards are byte-identical duplicates, so testing one would leave the other free to regress. One assumption from the plan turned out to be wrong, and the test pins the real behaviour instead: a pristine sign-in button is **enabled**, not disabled. PrimeVue leaves `$form.valid` undefined until a field is touched, so submitting a pristine form is what surfaces the errors. `globalThis.isSecureContext` is `undefined` under happy-dom rather than `true`, so the insecure-context test stubs the *secure* case. The value is a setup-time snapshot, so the stub has to precede mount. ## Testing 90 tests. Every one was mutation-checked: the behaviour it pins was broken in the source, the test confirmed failing, then reverted. **No survivors.** | Area | Cases | Representative mutation → result | | --- | --- | --- | | Password + email schema | 19 | drop any one complexity regex → that case fails; move the refine `path` → mismatch fails | | Open-redirect guard | 7 | read the last repeated query entry instead of the first → fails; decode fallback returns a `/` path → fails | | Post-auth redirect | 7 | `router.replace` → `push` → fails; drop the OAuth-resume early return → deep-link precedence fails | | Route guards | 19 | negate `isLoggedIn` → both routes fail; `if (!to.query.switchAccount)` → `if (true)` → escape hatch fails | | Terms notice | 5 | drop `rel="noopener noreferrer"` → fails; hard-code any locale fragment → fails | | Sign-in form | 12 | drop `:disabled="!$form.valid"` → fails; `current-password` → `new-password` → fails | | Sign-up form | 15 | remove `useThrottleFn` → double-submit fails; make `resetTurnstile` a no-op → fails | | Login view | 6 | insecure-context warning `v-if="false"` → fails; force the webview notice → fails | Three guards are unkillable in isolation and die only once the outer gate is also removed, which is the signature of defence-in-depth rather than dead code: `CloudSignInForm.onSubmit`'s `event.valid`, `SignUpForm.onSubmit`'s `waitingForTurnstile`, and both forms' loading gate behind `Button`'s `disabled || loading`. No e2e spec. The two flows worth driving end to end — the deep-link round trip and the already-signed-in redirect — are pinned closer to the logic in `usePostAuthRedirect.test.ts` and `onboardingCloudRoutes.test.ts`, the latter on both routes. A browser spec added a slower path to the same assertions. --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: CodeRabbit <noreply@coderabbit.ai> Co-authored-by: Christian Byrne <cbyrne@comfy.org>
1 parent 03cce8f commit 3e1b0d2

8 files changed

Lines changed: 846 additions & 32 deletions

File tree

src/components/dialog/content/signin/SignUpForm.test.ts

Lines changed: 68 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,6 @@ const mockReset = vi.fn()
4343
let emitTurnstileToken: ((token: string) => void) | undefined
4444
let emitTurnstileUnavailable: ((unavailable: boolean) => void) | undefined
4545

46-
// The reset-on-toggle behavior lives in useTurnstileGate itself (see
47-
// useTurnstile.test.ts); this fake just wires token/unavailable through to
48-
// `waiting` the same way so SignUpForm's submit gating can be exercised.
4946
vi.mock('@/composables/auth/useTurnstile', () => ({
5047
useTurnstile: () => ({
5148
enabled: mockTurnstileEnabled
@@ -62,9 +59,8 @@ vi.mock('@/composables/auth/useTurnstile', () => ({
6259
})
6360
}))
6461

65-
// Stub the real widget (which loads the external Turnstile script) with one that
66-
// exposes a spyable reset() and lets a test drive the v-model token/unavailable
67-
// the way a solved challenge (or a broken/slow widget) would.
62+
// The real widget loads an external Turnstile script; this stub exposes a
63+
// spyable reset() and lets a test drive the token/unavailable v-models.
6864
vi.mock('./TurnstileWidget.vue', async () => {
6965
const { defineComponent: defineMock } = await import('vue')
7066
return {
@@ -118,8 +114,6 @@ describe('SignUpForm', () => {
118114
return { ...utils, user }
119115
}
120116

121-
/** Render through a host that keeps a ref, so the parent-facing exposed
122-
* `resetTurnstile()` can be invoked the way SignInContent would. */
123117
function renderWithRef() {
124118
const formRef = ref<{ resetTurnstile: () => void } | null>(null)
125119
const Host = defineComponent({
@@ -246,11 +240,6 @@ describe('SignUpForm', () => {
246240
})
247241
})
248242

249-
// Regression coverage for the shadow-mode race: previously submit was only
250-
// gated in 'enforce' mode, so most real signups in 'shadow' mode raced
251-
// ahead of the async Cloudflare challenge and reached the backend with an
252-
// empty token. Gating now depends only on whether the widget is enabled
253-
// (shadow or enforce both render it), so both modes behave identically here.
254243
describe('Turnstile submit gating', () => {
255244
it('disables the submit button until a token is present', async () => {
256245
mockTurnstileEnabled.value = true
@@ -268,7 +257,10 @@ describe('SignUpForm', () => {
268257

269258
await user.click(screen.getByRole('button', { name: signUpButton }))
270259

271-
expect(onSubmit).not.toHaveBeenCalled()
260+
expect(
261+
onSubmit,
262+
'gating on enabled (not enforce) is what stops a shadow-mode signup racing ahead with an empty token'
263+
).not.toHaveBeenCalled()
272264
})
273265

274266
it('emits submit with the token once the challenge is solved', async () => {
@@ -297,4 +289,66 @@ describe('SignUpForm', () => {
297289
expect(onSubmit).toHaveBeenCalledWith(expectedValues, undefined)
298290
})
299291
})
292+
293+
describe('Turnstile wait hint accessibility', () => {
294+
it('announces the wait politely while the challenge is pending', async () => {
295+
mockTurnstileEnabled.value = true
296+
renderComponent()
297+
await nextTick()
298+
299+
const hint = screen.getByRole('status')
300+
expect(
301+
hint,
302+
'the hint is the only thing telling a screen-reader user why submit is unavailable'
303+
).toHaveTextContent(enMessages.auth.turnstile.submitBlockedHint)
304+
expect(hint).toHaveAttribute('aria-live', 'polite')
305+
})
306+
307+
it('points the disabled submit button at the hint', async () => {
308+
mockTurnstileEnabled.value = true
309+
const { user } = renderComponent()
310+
await fillValidSignup(user)
311+
await nextTick()
312+
313+
const submit = screen.getByRole('button', { name: signUpButton })
314+
expect(
315+
submit,
316+
'an otherwise-valid form must stay disabled while the challenge is pending'
317+
).toBeDisabled()
318+
expect(submit).toHaveAttribute(
319+
'aria-describedby',
320+
screen.getByRole('status').id
321+
)
322+
})
323+
324+
it('drops the description once the challenge resolves', async () => {
325+
mockTurnstileEnabled.value = true
326+
renderComponent()
327+
await nextTick()
328+
329+
emitTurnstileToken!('token-xyz')
330+
await nextTick()
331+
332+
expect(
333+
screen.getByRole('button', { name: signUpButton })
334+
).not.toHaveAttribute('aria-describedby')
335+
})
336+
})
337+
338+
describe('double-submit throttling', () => {
339+
it('emits once when the button is clicked twice in quick succession', async () => {
340+
const onSubmit = vi.fn()
341+
const { user } = renderComponent({ onSubmit })
342+
await fillValidSignup(user)
343+
const submit = screen.getByRole('button', { name: signUpButton })
344+
345+
await user.click(submit)
346+
await user.click(submit)
347+
348+
expect(
349+
onSubmit,
350+
'an impatient double-click would otherwise create the account twice'
351+
).toHaveBeenCalledOnce()
352+
})
353+
})
300354
})

src/platform/cloud/onboarding/CloudLoginView.test.ts

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,17 @@ const FREE_RUN_MESSAGES = {
2727
login: {
2828
cloudNewUser: 'New to Comfy?',
2929
cloudSignUp: 'Sign up here',
30-
freeRunsSuffix: 'to get {count} free run. | to get {count} free runs.'
30+
freeRunsSuffix: 'to get {count} free run. | to get {count} free runs.',
31+
insecureContextWarning: 'This connection is insecure'
3132
}
3233
}
3334
}
3435

3536
async function renderLoginView(
3637
url = '/cloud/login',
37-
messages: Partial<typeof FREE_RUN_MESSAGES> = {}
38+
messages: {
39+
auth?: { login?: Partial<typeof FREE_RUN_MESSAGES.auth.login> }
40+
} = {}
3841
) {
3942
const router = createRouter({
4043
history: createMemoryHistory(),
@@ -56,7 +59,6 @@ async function renderLoginView(
5659
createI18n({ legacy: false, locale: 'en', messages: { en: messages } })
5760
],
5861
stubs: {
59-
Message: true,
6062
CloudSignInForm: { template: '<form data-testid="signin-form" />' }
6163
}
6264
}
@@ -65,6 +67,7 @@ async function renderLoginView(
6567

6668
afterEach(() => {
6769
isEmbeddedWebView.value = false
70+
vi.unstubAllGlobals()
6871
})
6972

7073
describe('CloudLoginView', () => {
@@ -107,6 +110,29 @@ describe('CloudLoginView', () => {
107110
).not.toBeInTheDocument()
108111
})
109112

113+
it.for([true, false])(
114+
'renders the insecure-context warning only over plain HTTP (secure: %s)',
115+
async (secure: boolean) => {
116+
vi.stubGlobal('isSecureContext', secure)
117+
const { unmount } = await renderLoginView('/cloud/login', {
118+
auth: {
119+
login: { insecureContextWarning: 'This connection is insecure' }
120+
}
121+
})
122+
123+
const warning = screen.queryByText('This connection is insecure')
124+
if (secure) {
125+
expect(warning).not.toBeInTheDocument()
126+
} else {
127+
expect(
128+
warning,
129+
'a self-hosted HTTP origin can have credentials intercepted, so the warning must render'
130+
).toBeInTheDocument()
131+
}
132+
unmount()
133+
}
134+
)
135+
110136
it('does not region-gate sign-in, because an existing account already completed sign-up', async () => {
111137
const user = (await import('@testing-library/user-event')).default.setup()
112138
await renderLoginView()
Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
import userEvent from '@testing-library/user-event'
2+
import { render, screen, waitFor } from '@testing-library/vue'
3+
import PrimeVue from 'primevue/config'
4+
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
import { createI18n } from 'vue-i18n'
6+
import { createMemoryHistory, createRouter } from 'vue-router'
7+
8+
import enMessages from '@/locales/en/main.json' with { type: 'json' }
9+
import CloudSignInForm from '@/platform/cloud/onboarding/components/CloudSignInForm.vue'
10+
11+
const loading = vi.hoisted(() => ({ value: false }))
12+
vi.mock('@/stores/authStore', () => ({
13+
useAuthStore: () => ({
14+
get loading() {
15+
return loading.value
16+
}
17+
})
18+
}))
19+
20+
const LOGIN_COPY = enMessages.auth.login
21+
22+
function renderForm(
23+
authError?: string,
24+
messages: Record<string, object | string> = {}
25+
) {
26+
const router = createRouter({
27+
history: createMemoryHistory(),
28+
routes: [
29+
{
30+
path: '/cloud/forgot-password',
31+
name: 'cloud-forgot-password',
32+
component: { template: '<div />' }
33+
}
34+
]
35+
})
36+
return render(CloudSignInForm, {
37+
props: { authError },
38+
global: {
39+
plugins: [
40+
router,
41+
PrimeVue,
42+
createI18n({ legacy: false, locale: 'en', messages: { en: messages } })
43+
]
44+
}
45+
})
46+
}
47+
48+
const renderRealForm = () => renderForm(undefined, enMessages)
49+
50+
const emailField = () =>
51+
screen.getByPlaceholderText(LOGIN_COPY.emailPlaceholder)
52+
const passwordField = () =>
53+
screen.getByPlaceholderText(LOGIN_COPY.passwordPlaceholder)
54+
const submitButton = () =>
55+
screen.getByRole('button', { name: LOGIN_COPY.loginButton })
56+
57+
beforeEach(() => {
58+
loading.value = false
59+
})
60+
61+
describe('CloudSignInForm', () => {
62+
it('renders the auth error inline', () => {
63+
renderForm('The password you entered is incorrect.')
64+
65+
expect(
66+
screen.getByText('The password you entered is incorrect.')
67+
).toBeInTheDocument()
68+
})
69+
70+
it('renders nothing when there is no auth error', () => {
71+
renderForm()
72+
73+
expect(
74+
screen.queryByText('The password you entered is incorrect.')
75+
).not.toBeInTheDocument()
76+
})
77+
})
78+
79+
describe('CloudSignInForm password manager support', () => {
80+
it('marks the email field for autofill with a stable id', () => {
81+
renderRealForm()
82+
83+
expect(emailField()).toHaveAttribute('id', 'cloud-sign-in-email')
84+
expect(emailField()).toHaveAttribute('name', 'email')
85+
expect(emailField()).toHaveAttribute('autocomplete', 'email')
86+
})
87+
88+
it('marks the password field as current-password, not new-password', () => {
89+
renderRealForm()
90+
91+
expect(passwordField()).toHaveAttribute('id', 'cloud-sign-in-password')
92+
expect(passwordField()).toHaveAttribute('autocomplete', 'current-password')
93+
})
94+
95+
it('binds both labels to their inputs', () => {
96+
renderRealForm()
97+
98+
expect(screen.getByLabelText(LOGIN_COPY.emailLabel)).toBe(emailField())
99+
expect(screen.getByLabelText(LOGIN_COPY.passwordLabel)).toBe(
100+
passwordField()
101+
)
102+
})
103+
})
104+
105+
describe('CloudSignInForm submit gating', () => {
106+
// PrimeVue leaves `$form.valid` undefined until a field is touched, so the
107+
// pristine button is enabled by design and is not asserted here.
108+
it('disables submit once a field is touched and invalid', async () => {
109+
const user = userEvent.setup()
110+
renderRealForm()
111+
112+
await user.type(emailField(), 'not-an-email')
113+
114+
await waitFor(() => {
115+
expect(submitButton()).toBeDisabled()
116+
})
117+
})
118+
119+
it('enables submit once both fields are filled', async () => {
120+
const user = userEvent.setup()
121+
renderRealForm()
122+
123+
await user.type(emailField(), 'user@example.com')
124+
await user.type(passwordField(), 'Password1!')
125+
126+
await waitFor(() => {
127+
expect(submitButton()).toBeEnabled()
128+
})
129+
})
130+
131+
it('surfaces a field error for a malformed email', async () => {
132+
const user = userEvent.setup()
133+
renderRealForm()
134+
135+
await user.type(emailField(), 'not-an-email')
136+
await user.type(passwordField(), 'Password1!')
137+
await user.click(submitButton())
138+
139+
await waitFor(() => {
140+
expect(
141+
screen.getByText(enMessages.validation.invalidEmail)
142+
).toBeInTheDocument()
143+
})
144+
})
145+
146+
it('does not emit submit for a malformed email, by button or by Enter', async () => {
147+
const user = userEvent.setup()
148+
const { emitted } = renderRealForm()
149+
150+
await user.type(emailField(), 'not-an-email')
151+
await user.type(passwordField(), 'Password1!')
152+
await user.click(submitButton())
153+
await user.type(passwordField(), '{Enter}')
154+
155+
expect(emitted().submit).toBeUndefined()
156+
})
157+
158+
it('submits on Enter from the password field', async () => {
159+
const user = userEvent.setup()
160+
const { emitted } = renderRealForm()
161+
162+
await user.type(emailField(), 'user@example.com')
163+
await user.type(passwordField(), 'Password1!{Enter}')
164+
165+
await waitFor(() => {
166+
expect(
167+
emitted().submit,
168+
'a handler wired only to the button click would never fire for the Enter key most people submit with'
169+
).toBeTruthy()
170+
})
171+
expect(emitted().submit[0]).toEqual([
172+
{ email: 'user@example.com', password: 'Password1!' }
173+
])
174+
})
175+
})
176+
177+
describe('CloudSignInForm in-flight state', () => {
178+
it('disables submit and marks it busy while an auth action runs', () => {
179+
loading.value = true
180+
renderRealForm()
181+
182+
expect(submitButton()).toBeDisabled()
183+
expect(submitButton()).toHaveAttribute('aria-busy', 'true')
184+
})
185+
186+
it('does not emit submit while loading', async () => {
187+
const user = userEvent.setup()
188+
loading.value = true
189+
const { emitted } = renderRealForm()
190+
191+
await user.click(submitButton())
192+
193+
expect(emitted().submit).toBeUndefined()
194+
})
195+
})

0 commit comments

Comments
 (0)