feat: add remember me functionality and improve OTP input accessibili… #5
Conversation
…ty in TwoFactorsAuthPlugin and TwoFactorsSetup component
There was a problem hiding this comment.
Pull Request Overview
This PR adds "remember me" functionality to the two-factor authentication system and improves accessibility for OTP input fields. The changes allow users to stay logged in for extended periods when the remember me option is selected and enhance the OTP input experience for users with assistive technologies.
- Integrates remember me functionality into the 2FA flow by passing rememberMe state through JWT tokens and setting appropriate cookie expiration times
- Improves OTP input accessibility by adding proper ARIA labels, autocomplete attributes, and semantic naming
- Enhances UI layout and styling for better user experience in both TwoFactorsSetup and TwoFactorsConfirmation components
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| index.ts | Adds remember me functionality by extracting rememberMe from request body, calculating expiration days, and passing through JWT tokens to set appropriate auth cookie expiration |
| custom/TwoFactorsSetup.vue | Improves OTP input accessibility with proper ARIA labels and autocomplete attributes, updates styling and layout |
| custom/TwoFactorsConfirmation.vue | Enhances OTP input accessibility, cleans up unused code, and improves component structure |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
index.ts
Outdated
| const authPk = authResource.columns.find((col)=>col.primaryKey).name | ||
| const userPk = adminUser.dbUser[authPk] | ||
| const rememberMe = extra?.body?.rememberMe || false; | ||
| const rememberMeDays = rememberMe ? adminforth.config.auth.rememberMeDays || 30 : process.env.ADMINFORTH_REMEMBER_ME_DAYS || 1; |
There was a problem hiding this comment.
The fallback logic mixes configuration and environment variables inconsistently. When rememberMe is false, it should use a consistent default (like 1 day) rather than checking process.env.ADMINFORTH_REMEMBER_ME_DAYS, which seems intended for the rememberMe=true case.
| const rememberMeDays = rememberMe ? adminforth.config.auth.rememberMeDays || 30 : process.env.ADMINFORTH_REMEMBER_ME_DAYS || 1; | |
| const rememberMeDays = rememberMe | |
| ? (adminforth.config.auth.rememberMeDays || process.env.ADMINFORTH_REMEMBER_ME_DAYS || 30) | |
| : 1; |
custom/TwoFactorsSetup.vue
Outdated
| container-class="grid grid-cols-6 gap-3 w-full" | ||
| input-classes="otp-input bg-gray-50 text-center border leading-none border-gray-300 text-gray-900 text-sm rounded-lg focus:ring-blue-500 focus:border-blue-500 block h-[43.33px] w-full dark:bg-gray-700 dark:border-gray-600 dark:placeholder-gray-400 dark:text-white dark:focus:ring-blue-500 dark:focus:border-blue-500" | ||
| :num-inputs="6" | ||
| inputType="text" |
There was a problem hiding this comment.
Changing inputType from 'number' to 'text' while keeping inputmode='numeric' is inconsistent. For OTP codes, inputType should remain 'number' to ensure proper mobile keyboard display and input validation.
| inputType="text" | |
| inputType="number" |
| tagOtpInputs(); | ||
| }); | ||
| onBeforeUnmount(() => { |
There was a problem hiding this comment.
The onBeforeUnmount function references handlePaste in the removeEventListener call, but handlePaste function is not defined in this component and the addEventListener call was removed from onMounted.
…ation in TwoFactorsSetup component
…ty in TwoFactorsAuthPlugin and TwoFactorsSetup component