feat(front): auth forms - #134
Merged
Merged
Conversation
Walkthrough認証フォームに TanStack Form + ArkType によるバリデーションを導入し、zxcvbn-ts を Elysia SSR 側に集約した Changes認証フォーム刷新とパスワード強度判定
Sequence Diagram(s)sequenceDiagram
participant ユーザー
participant SignUpForm
participant usePasswordStrength
participant ElysiaSSR as Elysia SSR
rect rgba(100, 150, 255, 0.5)
note over ユーザー,SignUpForm: パスワード入力フェーズ
ユーザー->>SignUpForm: パスワード入力
SignUpForm->>usePasswordStrength: password ref 更新
usePasswordStrength->>usePasswordStrength: 300ms デバウンス待機<br/>(seq++)
end
rect rgba(100, 200, 150, 0.5)
note over usePasswordStrength,ElysiaSSR: 強度取得フェーズ
usePasswordStrength->>ElysiaSSR: POST /internal/password-strength<br/>{password}
ElysiaSSR->>ElysiaSSR: zxcvbn(password)
ElysiaSSR->>ElysiaSSR: scoreToStrength(score)
ElysiaSSR-->>usePasswordStrength: {strength: "low"|"medium"|"high"}
usePasswordStrength->>usePasswordStrength: seq チェック<br/>(stale は破棄)
usePasswordStrength-->>SignUpForm: strength ref 更新
end
rect rgba(255, 180, 100, 0.5)
note over SignUpForm,ユーザー: UI 更新フェーズ
SignUpForm->>ユーザー: PasswordStrengthBar<br/>を強度に応じて描画
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60分 Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
sousuke0422
force-pushed
the
feat/front-auth-forms
branch
from
June 15, 2026 13:34
55c77c1 to
6ad9d2a
Compare
sousuke0422
marked this pull request as ready for review
June 15, 2026 13:35
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/frontend/server/elysia.ts`:
- Around line 40-42: The password field in the `/internal/password-strength`
endpoint's request body schema at lines 40-42 uses t.String() without any length
constraints, which allows arbitrarily long inputs to be processed by the
expensive zxcvbn() password strength calculation, creating a DoS vulnerability.
Add a maxLength constraint to the password field definition in the t.Object body
schema to enforce a reasonable maximum length and reject oversized inputs early
at the validation layer.
In `@apps/frontend/src/components/auth/SignInForm.vue`:
- Line 26: Plain text logging of sensitive authentication data (passwords) is
present in two sign-in/sign-up form components, creating a data leakage risk in
both development and production environments. In
apps/frontend/src/components/auth/SignInForm.vue at line 26, remove the
console.log('signin stub', value) statement since value contains the password,
and replace it with an audit log that excludes sensitive information if logging
is needed. Similarly, in apps/frontend/src/components/auth/SignUpForm.vue at
line 37, remove the console.log('signup stub', value) statement and replace it
with a safe audit log that masks or excludes the sensitive authentication data.
- Around line 24-27: The authentication form submissions are stubbed with
console.log and TODO comments instead of implementing actual API functionality.
In apps/frontend/src/components/auth/SignInForm.vue at the onSubmit function
(lines 24-27), replace the stub implementation with a POST request to the
/v1/auth/login endpoint, add error handling to display validation or error
messages to the user, and navigate to the appropriate page on successful
authentication. Apply the same pattern to
apps/frontend/src/components/auth/SignUpForm.vue at lines 34-38, implementing
the sign-up API call to /v1/auth/register with corresponding error handling and
success-path navigation.
In `@apps/frontend/src/components/originui/input-group/InputGroupAddon.vue`:
- Around line 23-24: The querySelector on line 24 in InputGroupAddonVue is
hardcoded to find only 'input' elements, which causes focus to not be
transferred when using InputGroupTextarea. Replace the selector 'input' with
'[data-slot="input-group-control"]' to target the appropriate control element
regardless of whether it is an input or textarea, ensuring focus is properly set
for both component types.
In `@apps/frontend/src/composables/__tests__/usePasswordStrength.test.ts`:
- Around line 107-117: Add a new test case after the existing 'API がエラーを返したとき
strength を更新しない' test to cover the scenario where fetch rejects with an
exception (network failure), rather than just returning an HTTP error response.
Mock fetch to throw/reject an error instead of returning a failed response, then
verify that strength.value remains empty after the async operation completes,
ensuring the composable handles uncaught fetch exceptions gracefully without
regressions.
In `@apps/frontend/src/composables/usePasswordStrength.ts`:
- Around line 33-43: The fetch call and response.json() operation in the
composable lack error handling, causing unhandled exceptions when the network
request or JSON parsing fails. Wrap the entire fetch and JSON parsing logic
(starting from the fetch call for '/internal/password-strength' through the
response.json() call) in a try/catch block. Maintain the existing condition that
checks response.ok and verifies that id matches seq to ensure only the latest
request updates the state, and handle any caught exceptions gracefully by simply
returning early to prevent state updates on error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 317c0788-697b-42f4-81df-b40dc3af7453
⛔ Files ignored due to path filters (1)
apps/frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
apps/frontend/package.jsonapps/frontend/server/elysia.tsapps/frontend/src/components/auth/PasswordInput.vueapps/frontend/src/components/auth/PasswordStrengthBar.vueapps/frontend/src/components/auth/SignInForm.vueapps/frontend/src/components/auth/SignUpForm.vueapps/frontend/src/components/originui/input-group/InputGroup.vueapps/frontend/src/components/originui/input-group/InputGroupAddon.vueapps/frontend/src/components/originui/input-group/InputGroupButton.vueapps/frontend/src/components/originui/input-group/InputGroupInput.vueapps/frontend/src/components/originui/input-group/InputGroupText.vueapps/frontend/src/components/originui/input-group/InputGroupTextarea.vueapps/frontend/src/components/originui/input-group/index.tsapps/frontend/src/components/ui/textarea/Textarea.vueapps/frontend/src/components/ui/textarea/index.tsapps/frontend/src/composables/__tests__/usePasswordStrength.test.tsapps/frontend/src/composables/usePasswordStrength.tsapps/frontend/vite.config.tsdocs/frontend/auth-forms.md
sousuke0422
force-pushed
the
feat/front-auth-forms
branch
from
June 15, 2026 17:16
a942dae to
5876f80
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
apps/frontend/src/components/originui/input-group/InputGroupAddon.vue (1)
23-24:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
InputGroupTextarea構成でフォーカス移譲が機能しません。Line 24 が
input固定検索のため、InputGroupTextarea利用時にアドオンクリックでフォーカスできません。data-slot="input-group-control"を対象にしてください。修正案
- if (currentTarget && currentTarget?.parentElement) { - currentTarget.parentElement?.querySelector('input')?.focus(); - } + if (currentTarget?.parentElement) { + currentTarget.parentElement + .querySelector<HTMLElement>('[data-slot="input-group-control"]') + ?.focus(); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/frontend/src/components/originui/input-group/InputGroupAddon.vue` around lines 23 - 24, The querySelector on line 24 in InputGroupAddon.vue is hardcoded to search for 'input' elements only, which causes focus delegation to fail when using InputGroupTextarea (which uses textarea instead). Replace the querySelector('input') call with querySelector('[data-slot="input-group-control"]') to target the appropriate control element by its data attribute, which will work correctly for both input and textarea elements in the component tree.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/frontend/src/components/auth/SignUpForm.vue`:
- Line 26: The `hasSubmitted` ref variable is declared but never used in the
template or script logic. Remove the `const hasSubmitted = ref(false);`
declaration from the component and also remove any assignment to `hasSubmitted`
(such as `hasSubmitted.value = true;`) in the `onSubmit` function if present. If
you plan to use this for future functionality like displaying success messages
after form submission, keep it; otherwise, delete it entirely to keep the code
clean.
In `@apps/frontend/src/composables/usePasswordStrength.ts`:
- Around line 29-43: The async watch callback in usePasswordStrength composable
currently sends POST requests to /internal/password-strength even when the
password exceeds the server constraint of 256 characters, causing unnecessary
requests and stale strength display values. Add a client-side validation check
in the callback body (after the empty value check but before the fetch call) to
verify that the password length does not exceed 256 characters; if it does
exceed 256 characters, reset strength.value to an empty string and return early
to prevent the unnecessary POST request. Consider adding a test case to verify
this behavior prevents regressions.
In `@docs/frontend/auth-forms.md`:
- Around line 50-51: The documentation at lines 50-51 references `+server.ts`
which does not match the actual implementation. The actual SSR entry point in
the codebase is `apps/frontend/server/elysia.ts`. Replace the `+server.ts`
notation in the documentation with the correct implementation filename
`elysia.ts` (or the full path if needed for clarity) to accurately reflect how
the Elysia server is configured and avoid confusing readers who might expect a
different framework.
- Around line 42-48: The fenced code block in the documentation is missing a
language specification, which triggers lint warnings. Add the language
identifier `text` to the opening backticks of the code fence (change ``` to
```text) to make the intent clear and comply with linting standards.
---
Duplicate comments:
In `@apps/frontend/src/components/originui/input-group/InputGroupAddon.vue`:
- Around line 23-24: The querySelector on line 24 in InputGroupAddon.vue is
hardcoded to search for 'input' elements only, which causes focus delegation to
fail when using InputGroupTextarea (which uses textarea instead). Replace the
querySelector('input') call with
querySelector('[data-slot="input-group-control"]') to target the appropriate
control element by its data attribute, which will work correctly for both input
and textarea elements in the component tree.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: eb85c3a2-c50c-4581-b086-821b092aa96c
⛔ Files ignored due to path filters (1)
apps/frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
apps/frontend/package.jsonapps/frontend/server/elysia.tsapps/frontend/src/components/auth/PasswordInput.vueapps/frontend/src/components/auth/PasswordStrengthBar.vueapps/frontend/src/components/auth/SignInForm.vueapps/frontend/src/components/auth/SignUpForm.vueapps/frontend/src/components/originui/input-group/InputGroup.vueapps/frontend/src/components/originui/input-group/InputGroupAddon.vueapps/frontend/src/components/originui/input-group/InputGroupButton.vueapps/frontend/src/components/originui/input-group/InputGroupInput.vueapps/frontend/src/components/originui/input-group/InputGroupText.vueapps/frontend/src/components/originui/input-group/InputGroupTextarea.vueapps/frontend/src/components/originui/input-group/index.tsapps/frontend/src/components/ui/textarea/Textarea.vueapps/frontend/src/components/ui/textarea/index.tsapps/frontend/src/composables/__tests__/usePasswordStrength.test.tsapps/frontend/src/composables/usePasswordStrength.tsapps/frontend/vite.config.tsdocs/frontend/auth-forms.md
sousuke0422
added a commit
that referenced
this pull request
Jun 15, 2026
- SignUpForm: add TODO comment on hasSubmitted (declared but not yet read in template) - docs/auth-forms: add language spec to fenced code block (text) - docs/auth-forms: fix incorrect +server.ts reference → elysia.ts Assisted-by: multi-agent-shogun-aki-tweak
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/frontend/src/components/auth/SignInForm.vue (1)
66-70:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
PasswordInputのname受け渡しがフォーム側で未適用です。
PasswordInput.vueでnameprop を追加していますが、利用側2箇所で未指定のため、実入力要素にnameが設定されず、パスワードマネージャー/自動入力の一致率が低下します。Line 66とLine 100付近で:name="field.name"を渡してください。
apps/frontend/src/components/auth/SignInForm.vue#L66-L70:<PasswordInput ... :name="field.name" />を追加してサインイン入力のnameを明示してください。apps/frontend/src/components/auth/SignUpForm.vue#L100-L104:<PasswordInput ... :name="field.name" />を追加して登録入力のnameを明示してください。🔧 修正例
--- a/apps/frontend/src/components/auth/SignInForm.vue +++ b/apps/frontend/src/components/auth/SignInForm.vue @@ <PasswordInput :id="field.name" + :name="field.name" autocomplete="current-password" :model-value="field.state.value" `@update`:model-value="field.handleChange" `@blur`="field.handleBlur" />--- a/apps/frontend/src/components/auth/SignUpForm.vue +++ b/apps/frontend/src/components/auth/SignUpForm.vue @@ <PasswordInput :id="field.name" + :name="field.name" autocomplete="new-password" :model-value="field.state.value" `@update`:model-value=" (v: string) => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/frontend/src/components/auth/SignInForm.vue` around lines 66 - 70, The PasswordInput component now accepts a name prop, but it is not being passed in the two locations where the component is used, preventing the actual input element from having a name attribute set, which breaks password manager and autofill functionality. In apps/frontend/src/components/auth/SignInForm.vue at lines 66-70, add the `:name="field.name"` binding to the PasswordInput component. Additionally, in apps/frontend/src/components/auth/SignUpForm.vue at lines 100-104, add the same `:name="field.name"` binding to the PasswordInput component to ensure both sign-in and sign-up forms properly expose the name attribute to autofill features.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@apps/frontend/src/components/auth/SignInForm.vue`:
- Around line 66-70: The PasswordInput component now accepts a name prop, but it
is not being passed in the two locations where the component is used, preventing
the actual input element from having a name attribute set, which breaks password
manager and autofill functionality. In
apps/frontend/src/components/auth/SignInForm.vue at lines 66-70, add the
`:name="field.name"` binding to the PasswordInput component. Additionally, in
apps/frontend/src/components/auth/SignUpForm.vue at lines 100-104, add the same
`:name="field.name"` binding to the PasswordInput component to ensure both
sign-in and sign-up forms properly expose the name attribute to autofill
features.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 997506d8-31dd-4531-8825-44fea0de1e17
📒 Files selected for processing (5)
apps/frontend/src/components/auth/PasswordInput.vueapps/frontend/src/components/auth/SignInForm.vueapps/frontend/src/components/auth/SignUpForm.vueapps/frontend/src/lib/__tests__/auth-validation.test.tsapps/frontend/src/lib/auth-validation.ts
|
Note Docstrings generation - SUCCESS |
coderabbitai Bot
added a commit
that referenced
this pull request
Jun 15, 2026
Docstrings generation was requested by @sousuke0422. * #134 (comment) The following files were modified: * `apps/frontend/server/elysia.ts` * `apps/frontend/src/composables/usePasswordStrength.ts` * `apps/frontend/src/lib/auth-validation.ts`
Merged
Assisted-by: multi-agent-shogun-aki-tweak
Assisted-by: multi-agent-shogun-aki-tweak
Assisted-by: multi-agent-shogun-aki-tweak
Assisted-by: multi-agent-shogun-aki-tweak
フォーカス前からユーザー名・パスワードの要件を常時表示する。 ユーザー名フィールドはエラー非表示時に「3文字以上」、 パスワードフィールドはエラー非表示かつ強度バー非表示時に「8文字以上」を表示。 Assisted-by: multi-agent-shogun-aki-tweak
Assisted-by: multi-agent-shogun-aki-tweak
…rejection test
elysia.ts: t.String({ maxLength: 256 }) で過大入力を早期拒否（DoS耐性）
usePasswordStrength.test.ts: fetch がネットワークエラーで reject した場合のテストを追加
Assisted-by: multi-agent-shogun-aki-tweak
…rors 未捕捉例外を防ぐため fetch と response.json() を try/catch で囲む。 エラー時は strength を更新せず前の値を維持する。 Assisted-by: multi-agent-shogun-aki-tweak
…andling ネットワークエラーで reject するケースと、nginx 等が HTML を返す ケース（JSON パース失敗）で strength が更新されないことを確認する。 Assisted-by: multi-agent-shogun-aki-tweak
- SignUpForm: add TODO comment on hasSubmitted (declared but not yet read in template) - docs/auth-forms: add language spec to fenced code block (text) - docs/auth-forms: fix incorrect +server.ts reference → elysia.ts Assisted-by: multi-agent-shogun-aki-tweak
…Strength Deferred until backend enforces a concrete limit. Assisted-by: multi-agent-shogun-aki-tweak
- Extract shared arkMessage() to src/lib/auth-validation.ts - Remove duplicate definitions from SignInForm and SignUpForm - Add name prop to PasswordInput for password manager support Assisted-by: multi-agent-shogun-aki-tweak
Assisted-by: multi-agent-shogun-aki-tweak
Assisted-by: multi-agent-shogun-aki-tweak
Assisted-by: multi-agent-shogun-aki-tweak
sousuke0422
force-pushed
the
feat/front-auth-forms
branch
from
June 16, 2026 04:54
bc99450 to
c9d461d
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (3)
apps/frontend/src/composables/usePasswordStrength.ts (1)
29-44:⚠️ Potential issue | 🟠 Major | ⚡ Quick winサーバ契約の
maxLength: 256をクライアント側でも先に判定してください。Line 31 の TODO が未実装のため、256 文字超でも POST が走ります。サーバ拒否時に
strengthが直前値のまま残り、表示が実入力と不整合になります。value.length > 256で即時リセット＋早期 return を入れてください（同ケースの回帰テスト追加も必要です）。差分案
watchDebounced( password, async (value) => { if (!value) return; - // TODO: add client-side maxLength guard once backend enforces a limit (e.g. 256) - const id = ++seq; + if (value.length > 256) { + strength.value = ''; + return; + } + try { const response = await fetch('/internal/password-strength', { method: 'POST',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/frontend/src/composables/usePasswordStrength.ts` around lines 29 - 44, The TODO comment on line 31 for client-side maxLength validation is not implemented, allowing POST requests to be made for passwords exceeding 256 characters. When the server rejects these requests, the strength value remains unchanged, causing a display mismatch with the actual input. Implement the maxLength guard by adding a check after the initial `if (!value) return;` statement to validate that value.length does not exceed 256 characters; if it does exceed this limit, reset the strength value to a default state and return early before attempting the fetch request. Additionally, add regression tests to verify that strength is properly reset when password length exceeds 256 characters.apps/frontend/src/components/auth/SignInForm.vue (2)
19-22:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift送信処理が両フォームでスタブ化され、認証フローが停止しています。
根因は、
onSubmitが TODO のままで実API連携を持たないことです。
apps/frontend/src/components/auth/SignInForm.vue#L19-L22:/v1/auth/login呼び出し、失敗時表示、成功時遷移を実装してください。apps/frontend/src/components/auth/SignUpForm.vue#L28-L32:/v1/auth/register呼び出し、失敗時表示、成功時遷移を実装してください。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/frontend/src/components/auth/SignInForm.vue` around lines 19 - 22, The authentication flow is incomplete because both form submission handlers are stubbed with TODO comments and console.log statements. You must implement the onSubmit handlers in both files to complete the authentication flow. In apps/frontend/src/components/auth/SignInForm.vue (lines 19-22), replace the console.log stub in the onSubmit handler with an actual API call to POST /v1/auth/login using the form value, handle errors by displaying them to the user, and on success navigate to the authenticated user's dashboard or home page. In apps/frontend/src/components/auth/SignUpForm.vue (lines 28-32), implement the same pattern but call POST /v1/auth/register instead, with failure error display and success navigation to complete the sign-up flow.
21-21:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win機微情報（パスワード）が両フォームで平文ログ出力されています。
根因は、
value全体をconsole.logしていることです。
apps/frontend/src/components/auth/SignInForm.vue#L21-L21:console.log('signin stub', value)を削除し、必要なら機微情報を除外した監査ログへ置換してください。apps/frontend/src/components/auth/SignUpForm.vue#L31-L31:console.log('signup stub', value)を削除し、同様に機微情報を除外したログへ置換してください。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/frontend/src/components/auth/SignInForm.vue` at line 21, Sensitive information (passwords) from the form value object are being logged in plain text in both authentication forms. In apps/frontend/src/components/auth/SignInForm.vue at lines 21-21, remove the console.log statement that logs the entire value object. In apps/frontend/src/components/auth/SignUpForm.vue at lines 31-31, similarly remove the console.log statement that logs the entire value object. If audit logging is needed, replace these statements with logs that explicitly exclude sensitive fields like passwords and only log non-sensitive metadata or user identification information.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@apps/frontend/src/components/auth/SignInForm.vue`:
- Around line 19-22: The authentication flow is incomplete because both form
submission handlers are stubbed with TODO comments and console.log statements.
You must implement the onSubmit handlers in both files to complete the
authentication flow. In apps/frontend/src/components/auth/SignInForm.vue (lines
19-22), replace the console.log stub in the onSubmit handler with an actual API
call to POST /v1/auth/login using the form value, handle errors by displaying
them to the user, and on success navigate to the authenticated user's dashboard
or home page. In apps/frontend/src/components/auth/SignUpForm.vue (lines 28-32),
implement the same pattern but call POST /v1/auth/register instead, with failure
error display and success navigation to complete the sign-up flow.
- Line 21: Sensitive information (passwords) from the form value object are
being logged in plain text in both authentication forms. In
apps/frontend/src/components/auth/SignInForm.vue at lines 21-21, remove the
console.log statement that logs the entire value object. In
apps/frontend/src/components/auth/SignUpForm.vue at lines 31-31, similarly
remove the console.log statement that logs the entire value object. If audit
logging is needed, replace these statements with logs that explicitly exclude
sensitive fields like passwords and only log non-sensitive metadata or user
identification information.
In `@apps/frontend/src/composables/usePasswordStrength.ts`:
- Around line 29-44: The TODO comment on line 31 for client-side maxLength
validation is not implemented, allowing POST requests to be made for passwords
exceeding 256 characters. When the server rejects these requests, the strength
value remains unchanged, causing a display mismatch with the actual input.
Implement the maxLength guard by adding a check after the initial `if (!value)
return;` statement to validate that value.length does not exceed 256 characters;
if it does exceed this limit, reset the strength value to a default state and
return early before attempting the fetch request. Additionally, add regression
tests to verify that strength is properly reset when password length exceeds 256
characters.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e53a9199-3f33-49fc-a6d8-04318a6ea779
⛔ Files ignored due to path filters (1)
apps/frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
apps/frontend/package.jsonapps/frontend/server/elysia.tsapps/frontend/src/components/auth/PasswordInput.vueapps/frontend/src/components/auth/PasswordStrengthBar.vueapps/frontend/src/components/auth/SignInForm.vueapps/frontend/src/components/auth/SignUpForm.vueapps/frontend/src/components/originui/input-group/InputGroup.vueapps/frontend/src/components/originui/input-group/InputGroupAddon.vueapps/frontend/src/components/originui/input-group/InputGroupButton.vueapps/frontend/src/components/originui/input-group/InputGroupInput.vueapps/frontend/src/components/originui/input-group/InputGroupText.vueapps/frontend/src/components/originui/input-group/InputGroupTextarea.vueapps/frontend/src/components/originui/input-group/index.tsapps/frontend/src/components/ui/textarea/Textarea.vueapps/frontend/src/components/ui/textarea/index.tsapps/frontend/src/composables/__tests__/usePasswordStrength.test.tsapps/frontend/src/composables/usePasswordStrength.tsapps/frontend/src/lib/__tests__/auth-validation.test.tsapps/frontend/src/lib/auth-validation.tsapps/frontend/vite.config.tsdocs/frontend/auth-forms.md
Contributor
|
/opencode explain このPRを説明して |
Contributor
PR概要認証フォーム（サインイン/サインアップ）の刷新PRです。 主な変更技術スタック移行
新コンポーネント
アーキテクチャ
未実装（次のPR）
CodeRabbitの指摘事項
|
Contributor
|
/opencode review このPRをレビューして |
Contributor
PR Review: feat(front): auth forms全体的によく整理されたPRです。設計書も含めて丁寧に作られています。以下に指摘事項をまとめます。 既に修正済みの項目CodeRabbitの指摘の多くは既に対応済みです:
未解決の問題1. // TODO: add client-side maxLength guard once backend enforces a limit (e.g. 256)サーバーは if (value.length > 256) {
strength.value = '';
return;
}テストも追加すべきです。 2. const hasSubmitted = ref(false); // TODO: use to display success message after registration
3. export function arkMessage(msg: string): string {
if (msg.includes('at least length 3')) return '3文字以上で入力してください。';
if (msg.includes('email address')) return 'メールアドレスの形式が正しくありません。';
if (msg.includes('at least length 8')) return '8文字以上で入力してください。';
return msg;
}
将来的にはarktypeのエラーツリーを直接カスタマイズする方が堅牢です。 4.
実装では 軽微な指摘5. CodeRabbitが指摘済みですが、 6. function scoreToStrength(score: number): PasswordStrength {
if (score <= 1) return 'low';
if (score <= 3) return 'medium';
return 'high';
}zxcvbnのscoreは0-4で、設計書のマッピングと一致しています。ただし 7. サインインフォームのバリデーションタイミング — :validators="{ onBlur: type('string.email') }"
:validators="{ onBlur: type('string >= 8') }"SignUpFormと異なり、SignInFormには 良い点
総合API連携は次PR予定のためスタブは許容範囲ですが、maxLengthガードのTODO と |
Contributor
|
Q. hasSubmitted ref が宣言されているが使われていない（SignUpForm.vue:26） |
Contributor
|
/opencode review このPRをレビューして |
Contributor
PR Review: feat(front): auth forms既に対応済みの指摘が多いですが、未解決の問題が3件残っています。 未解決の問題1. console.log('signin stub', value); // value.password が平文で出力される
console.log('signup stub', value);PRコメントで「削除済み」と報告されていますが、実際には残っています。本番・開発問わず漏えいリスクがあるため、削除するか 2. // TODO: add client-side maxLength guard once backend enforces a limit (e.g. 256)サーバーは if (value.length > 256) {
strength.value = '';
return;
}3. currentTarget.parentElement?.querySelector('input')?.focus();
軽微な指摘4. 宣言・代入されていますが、テンプレートでもバリデーションタイミングでも参照されていません。PRコメントでは「将来のメッセージ表示用」とのことですが、ドキュメント ( 5.
良い点
総合console.logのパスワード出力とmaxLengthガードはこのPRで解消すべきです。残りは軽微で次PR対応可能なため、この2件を修正すればマージ可能と思います。 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#8 のPhase 1
パスワード辞書をバンドルしないためにelysiaで処理
2faは後発
blocking by #114 (#113)done計画
3.5. api連携?
4.5. api連携?
Summary by CodeRabbit
リリースノート