Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/continuous_integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ jobs:
run: npm ci
- name: Verify build
run: npm run build:check
env:
PUBLIC_AUTH_URL: http://authentication.auth-plus.app

Lint:
runs-on: ubuntu-latest
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/playwright.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ jobs:
run: npx playwright install --with-deps
- name: Run Playwright tests
run: npx playwright test
env:
PUBLIC_AUTH_URL: http://authentication.auth-plus.app
- uses: actions/upload-artifact@v4
if: ${{ !cancelled() }}
with:
Expand Down
2 changes: 1 addition & 1 deletion .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ pnpm-lock.yaml
yarn.lock
bun.lock
bun.lockb

node_modules/
# Miscellaneous
/static/
32 changes: 17 additions & 15 deletions .prettierrc
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
{
"useTabs": true,
"semi": false,
"tabWidth": 2,
"singleQuote": true,
"trailingComma": "none",
"printWidth": 100,
"plugins": ["prettier-plugin-svelte"],
"overrides": [
{
"files": "*.svelte",
"options": {
"parser": "svelte"
}
}
]
"useTabs": false,
"tabWidth": 2,
"semi": false,
"singleQuote": true,
"trailingComma": "none",
"printWidth": 100,
"bracketSameLine": true,
"bracketSpacing": true,
"plugins": ["prettier-plugin-svelte", "prettier-plugin-tailwindcss"],
"overrides": [
{
"files": "*.svelte",
"options": {
"parser": "svelte"
}
}
]
}
8 changes: 0 additions & 8 deletions e2e/Header.test.ts

This file was deleted.

82 changes: 82 additions & 0 deletions e2e/Login.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import { test, expect } from '@playwright/test'

test.describe('Login Flow', () => {
test.beforeEach(async ({ page }) => {
// Replace with your actual login route
await page.goto('/login')
})

test('should display the login form correctly', async ({ page }) => {
await expect(page.locator('h1')).toHaveText('Welcome Back')
await expect(page.locator('label[for="login-email"]')).toBeVisible()
await expect(page.locator('label[for="login-pw"]')).toBeVisible()
})

test('successful login without MFA redirects to home', async ({ page }) => {
// Intercept the API call to mock a successful login (no MFA)
await page.route('**/login', async (route) => {
// Ensure this matches your PUBLIC_AUTH_URL path
await route.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify({
token: 'mock-session-token',
email: 'test@company.com',
id: '123',
name: 'Test User',
info: { phone: '', deviceId: '', googleAuth: '' }
})
})
})

await page.fill('#login-email', 'test@company.com')
await page.fill('#login-pw', 'password123')
await page.click('button[type="submit"]')

// Verify redirection and session storage
const token = await page.evaluate(() => sessionStorage.getItem('token'))
expect(token).toBe('mock-session-token')
await expect(page).toHaveURL('/')
})
Comment on lines +36 to +40

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Race: read sessionStorage after asserting navigation, not before.

page.click('button[type="submit"]') resolves once the click is dispatched, not after submit() finishes its await credential.login(...) and goto(...). Reading sessionStorage immediately after the click can race with the async submit handler and may read null on a slow run. Wait for the URL change first, then read storage.

♻️ Proposed fix
-    // Verify redirection and session storage
-    const token = await page.evaluate(() => sessionStorage.getItem('token'))
-    await expect(page).toHaveURL('/')
-    expect(token).toBe('mock-session-token')
+    // Verify redirection first, then session storage (avoids reading before submit completes)
+    await expect(page).toHaveURL('/')
+    const token = await page.evaluate(() => sessionStorage.getItem('token'))
+    expect(token).toBe('mock-session-token')
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Verify redirection and session storage
const token = await page.evaluate(() => sessionStorage.getItem('token'))
await expect(page).toHaveURL('/')
expect(token).toBe('mock-session-token')
})
// Verify redirection first, then session storage (avoids reading before submit completes)
await expect(page).toHaveURL('/')
const token = await page.evaluate(() => sessionStorage.getItem('token'))
expect(token).toBe('mock-session-token')
})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@e2e/Login.test.ts` around lines 36 - 40, The test reads sessionStorage too
early and can race with the async submit handler; modify the test so it waits
for navigation before reading storage: call expect(page).toHaveURL('/') (or
otherwise await navigation) immediately after triggering the submit (the
existing page.click('button[type="submit"]')) and only then call
page.evaluate(() => sessionStorage.getItem('token')) to assign token and assert
token === 'mock-session-token'; update the code around the token variable and
page.evaluate usage in e2e/Login.test.ts accordingly.


test('login with MFA triggers the MFA Selection view', async ({ page }) => {
// Mock a response that returns an MFA hash and strategy list
await page.route('**/login', async (route) => {
await route.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify({
hash: 'mfa-challenge-123',
strategyList: ['email', 'totp']
})
})
})

await page.fill('#login-email', 'mfa-user@company.com')
await page.fill('#login-pw', 'password123')
await page.click('button[type="submit"]')

// Verify that the 'Default' component is hidden and 'Choose' is shown
// Note: We check for elements that would exist in choose.svelte
await expect(page.locator('form')).not.toContainText('Welcome Back')
})
Comment on lines +42 to +62

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect Strategy type and choose.svelte text/testids to confirm a stronger positive assertion.
fd -t f 'mfa.ts' --exec cat {}
fd -t f 'choose.svelte' --exec cat {}

Repository: auth-plus/auth-plus-client

Length of output: 3796


Fix mock strategyList to match Strategy enum and strengthen the assertion.

Two issues:

  1. Strategy is an uppercase string enum (EMAIL, PHONE, GOOGLE_AUTHENTICATOR), but the mock uses lowercase strings ['email', 'totp'] that don't match enum values. Additionally, 'totp' doesn't exist in the enum — it should be 'GOOGLE_AUTHENTICATOR'. This mismatch will cause rendering to fail silently, and the test will pass for the wrong reason.

  2. expect(page.locator('form')).not.toContainText('Welcome Back') is a weak negative assertion. Instead, assert the presence of text or elements unique to choose.svelte, such as the "Security Verification" heading or the strategy selector with id login-strategy.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@e2e/Login.test.ts` around lines 42 - 62, The mock response in the 'login with
MFA triggers the MFA Selection view' test uses incorrect strategy strings;
update the route.fulfill body to return strategyList matching the Strategy enum
values (e.g., ['EMAIL','GOOGLE_AUTHENTICATOR']) so the MFA UI renders correctly,
and replace the weak negative assertion await
expect(page.locator('form')).not.toContainText('Welcome Back') with a positive
assertion that checks for elements unique to choose.svelte such as await
expect(page.locator('text=Security Verification')).toBeVisible() or await
expect(page.locator('#login-strategy')).toBeVisible() to confirm the MFA
selection view appears.


test('should display error message on failed login', async ({ page }) => {
// Mock a 401 Unauthorized error
await page.route('**/login', async (route) => {
await route.fulfill({
status: 401,
contentType: 'application/json',
body: JSON.stringify({ message: 'Invalid credentials' })
})
})

await page.fill('#login-email', 'wrong@company.com')
await page.fill('#login-pw', 'wrongpassword')
await page.click('button[type="submit"]')

const errorMsg = page.locator('.text-red-800')
await expect(errorMsg).toBeVisible()
await expect(errorMsg).toContainText("Login didn't work")
})
Comment thread
coderabbitai[bot] marked this conversation as resolved.
})
54 changes: 27 additions & 27 deletions eslint.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,31 +11,31 @@ import svelteConfig from './svelte.config.js'
const gitignorePath = fileURLToPath(new URL('./.gitignore', import.meta.url))

export default defineConfig(
includeIgnoreFile(gitignorePath),
js.configs.recommended,
...ts.configs.recommended,
...svelte.configs.recommended,
prettier,
...svelte.configs.prettier,
{
languageOptions: {
globals: { ...globals.browser, ...globals.node }
},
rules: {
// typescript-eslint strongly recommend that you do not use the no-undef lint rule on TypeScript projects.
// see: https://typescript-eslint.io/troubleshooting/faqs/eslint/#i-get-errors-from-the-no-undef-rule-about-global-variables-not-being-defined-even-though-there-are-no-typescript-errors
'no-undef': 'off'
}
},
{
files: ['**/*.svelte', '**/*.svelte.ts', '**/*.svelte.js'],
languageOptions: {
parserOptions: {
projectService: true,
extraFileExtensions: ['.svelte'],
parser: ts.parser,
svelteConfig
}
}
}
includeIgnoreFile(gitignorePath),
js.configs.recommended,
...ts.configs.recommended,
...svelte.configs.recommended,
prettier,
...svelte.configs.prettier,
{
languageOptions: {
globals: { ...globals.browser, ...globals.node }
},
rules: {
// typescript-eslint strongly recommend that you do not use the no-undef lint rule on TypeScript projects.
// see: https://typescript-eslint.io/troubleshooting/faqs/eslint/#i-get-errors-from-the-no-undef-rule-about-global-variables-not-being-defined-even-though-there-are-no-typescript-errors
'no-undef': 'off'
}
},
{
files: ['**/*.svelte', '**/*.svelte.ts', '**/*.svelte.js'],
languageOptions: {
parserOptions: {
projectService: true,
extraFileExtensions: ['.svelte'],
parser: ts.parser,
svelteConfig
}
}
}
)
Loading
Loading