Alternative to #741 - Autosubmit Tweak#820
Conversation
Introduce provider-wide code length controls and an autosubmit length filter. - Add Two_Factor_Provider::get_code_length() and update get_code() to accept a null length and fall back to the provider-specific length. Providers can now use two_factor_code_length to customize default code lengths per provider. - Use self::get_code_length() in backup-codes and email providers so their default token/backup lengths are filterable. - Add two_factor_autosubmit_length filter and apply it in backup-codes, email and TOTP providers to control the data-digits attribute (setting it to 0 disables autosubmit). - Update readme.txt to document the new two_factor_code_length and two_factor_autosubmit_length filters. - Minor whitespace cleanups in TOTP packing code. These changes centralize code-length behavior across providers and make the client-side autosubmit behavior configurable.
Ensure the expectedLength value is treated as a number by parsing inputEl.dataset.digits with parseInt(..., 10). This prevents string-based comparisons or unexpected behavior when dataset values are present (or missing), while preserving the optional chaining and defaulting to 0.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @Copilot. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
To be clear, I may be overcomplicating something that provides a simpler API in the prior PR. This take just makes more sense to my brain, for what little that's worth. |
There was a problem hiding this comment.
Pull request overview
This PR introduces a more flexible and extensible approach to configuring authentication code lengths and auto-submit behavior. It adds a centralized get_code_length() method in Two_Factor_Provider that applies a new two_factor_code_length filter as the base default for all providers, and a new two_factor_autosubmit_length filter that controls the data-digits attribute used by the JavaScript to determine when to auto-submit the authentication form.
Changes:
- Added
get_code_length()static method toTwo_Factor_Providerwith atwo_factor_code_lengthfilter, and updatedget_code()to use it when no length is specified - Added
two_factor_autosubmit_lengthfilter applied in backup codes, email, and TOTPauthentication_pagemethods to independently control the auto-submit threshold; updateddata-digitsoutput accordingly - Improved JS robustness by parsing
data-digitsas an integer withparseInt()inclass-two-factor-core.php
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
providers/class-two-factor-provider.php |
Adds get_code_length() method with two_factor_code_length filter; changes get_code() default length from 8 to null (resolves via get_code_length()) |
providers/class-two-factor-backup-codes.php |
Uses get_code_length() as default for two_factor_backup_code_length; adds two_factor_autosubmit_length filter in authentication_page |
providers/class-two-factor-email.php |
Uses get_code_length() as default for two_factor_email_token_length; adds two_factor_autosubmit_length filter in authentication_page |
providers/class-two-factor-totp.php |
Adds two_factor_autosubmit_length filter in authentication_page; updates data-digits from hardcoded constant to filtered value; minor whitespace cleanup |
class-two-factor-core.php |
Parses data-digits dataset attribute with parseInt() for more reliable JS numeric comparison |
readme.txt |
Documents new two_factor_code_length and two_factor_autosubmit_length filter hooks |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * @since 0.?.0 | ||
| * | ||
| * @param int $default Default code length if not filtered. | ||
| * @param string|null $provider The provider class name. Null uses the called class. | ||
| * @return int Number of characters. | ||
| */ | ||
| public static function get_code_length( $default = 8, $provider = null ) { | ||
| /** | ||
| * Filter the length of the code for a user. | ||
| * | ||
| * @since 0.?.0 |
There was a problem hiding this comment.
The @since 0.?.0 version placeholder is present in the get_code_length() method and its filter documentation. This needs to be replaced with the actual version number before the code is released. Based on the codebase, the current version is 0.15.0 (per CHANGELOG.md), so this would be 0.16.0.
| * | ||
| * To disable autosubmit, set the digits to `0` via the core method `__return_zero`. | ||
| * | ||
| * @since 0.?.0 |
There was a problem hiding this comment.
The @since 0.?.0 version placeholder in the two_factor_autosubmit_length filter documentation needs to be replaced with the actual release version before merging.
| * @since 0.?.0 | |
| * @since 0.15.0 |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@georgestephanis I've opened a new pull request, #821, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Initial plan * Add test coverage for get_code_length() and two_factor_autosubmit_length filter Co-authored-by: georgestephanis <941023+georgestephanis@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: georgestephanis <941023+georgestephanis@users.noreply.github.com>
|
I definitely support bringing some consistency to how the code lengths are retrieved for different providers. I noticed the same issue when I was working on my PR, but decided not to try and address that issue to keep the scope small (and because I don't have enough experience with this repo to know if there was a reason for the differences). So if there's support for bigger change I'm definitely all for it. Having said that, I don't see the value in setting the number of digits for auto submission rather than a simple true/false value (and the corresponding hook names). Any value other than the code length or 0 will result in auto submission at the wrong time. Do you see other applications for altering the number other than simply turning auto submission off? |
Alternative take on #741 // cc: @eric-michel -- this is a first draft, and the code should be reasonably intelligible, but I let Copilot generate the following summary:
This pull request introduces a more flexible and extensible approach to configuring authentication code lengths and auto-submit behavior across all two-factor authentication providers. It centralizes code length logic, adds new filters for customization, and ensures that UI elements reflect these dynamic settings.
Key changes include:
Core logic improvements
get_code_lengthtoTwo_Factor_Provider, with a correspondingtwo_factor_code_lengthfilter, allowing the default code length to be set for all providers in a consistent way.get_codemethod inTwo_Factor_Providerto use the newget_code_lengthmethod when no length is specified, making code generation more flexible and consistent.Provider-specific enhancements
get_code_lengthfor determining code/token length, and updated their filters to use the new centralized logic. [1] [2]two_factor_autosubmit_lengthfilter to allow customization of the input length at which authentication forms auto-submit, and applied this filter in backup code, email, and TOTP provider authentication screens. [1] [2] [3]data-digitsattribute in TOTP provider input fields accurately reflects the filtered code length, improving UI consistency.User interface and documentation
data-digitsattribute as an integer, ensuring correct behavior with dynamic code lengths.readme.txtto document the newtwo_factor_code_lengthandtwo_factor_autosubmit_lengthfilters for developers.