Skip to content

Add :digits option - #39

Merged
josevalim merged 10 commits into
dashbitco:masterfrom
LoicBersier:feat/totp_length
Nov 22, 2025
Merged

Add :digits option#39
josevalim merged 10 commits into
dashbitco:masterfrom
LoicBersier:feat/totp_length

Conversation

@LoicBersier

Copy link
Copy Markdown
Contributor

Add a totp_size option to allow having bigger totp tokens (limited at 10)

It required changing the verification_code and valid? as it was hard coded for a length of 6

Loïc Bersier added 2 commits November 21, 2025 15:12
…id?/3 accordingly

Added validate_digits/2 to validate the otp without using a fixed size
…code` use a totp_size of 8

Updated `returns true if it matches the verification code` name to specify that it is for the default totp_size
Comment thread test/nimble_totp_test.exs Outdated
Comment thread lib/nimble_totp.ex Outdated
*in seconds*) to be used. Default is `System.os_time(:second)`.
* `:period` - The period (in seconds) in which the code is valid. Default is `30`.
If this option is given to `verification_code/2`, it must also be given to `valid?/3`.
* `:totp_size` - The desired size of the totp. Default is 6.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Lets call it length!

Comment thread lib/nimble_totp.ex Outdated
Comment on lines +333 to +341
defp validate_digits(<<e, e_rest::binary>>, <<a, a_rest::binary>>)
when byte_size(e_rest) > 0 and byte_size(a_rest) > 0 do
bxor(e, a) ||| validate_digits(e_rest, a_rest)
end

defp validate_digits(<<e, e_rest::binary>>, <<a, a_rest::binary>>)
when byte_size(e_rest) <= 0 and byte_size(a_rest) <= 0 do
bxor(e, a)
end

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
defp validate_digits(<<e, e_rest::binary>>, <<a, a_rest::binary>>)
when byte_size(e_rest) > 0 and byte_size(a_rest) > 0 do
bxor(e, a) ||| validate_digits(e_rest, a_rest)
end
defp validate_digits(<<e, e_rest::binary>>, <<a, a_rest::binary>>)
when byte_size(e_rest) <= 0 and byte_size(a_rest) <= 0 do
bxor(e, a)
end
defp validate_digits(<<e, e_rest::binary>>, <<a, a_rest::binary>>) do
bxor(e, a) ||| validate_digits(e_rest, a_rest)
end
defp validate_digits(<<>>, <<>>) do
true
end

Comment thread lib/nimble_totp.ex Outdated

@spec validate_digits(integer(), integer()) :: :error | integer()
defp validate_digits(e, a)
when byte_size(e) !== byte_size(a), do: :error

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be a separate clause, otherwise we are checking the byte size on every operation. I'd just move it out of this function. :)

Comment thread lib/nimble_totp.ex Outdated
Comment on lines +233 to +234
totp_size > 10 && raise ArgumentError, "totp_size cannot be above 10"
totp_size <= 0 && raise ArgumentError, "totp_size cannot be 0 or under"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
totp_size > 10 && raise ArgumentError, "totp_size cannot be above 10"
totp_size <= 0 && raise ArgumentError, "totp_size cannot be 0 or under"
totp_size not in 6..10 && raise ArgumentError, "length must be between 6 and 10"

@LoicBersier

LoicBersier commented Nov 21, 2025

Copy link
Copy Markdown
Contributor Author

Thank you for the review, I realised that the otp uri has an optional digits option to specify the length of the totp token, should I add a note about it in the documentation or get it from the uri_params in otpauth_uri directly?

@josevalim

Copy link
Copy Markdown
Member

@LoicBersier I may be missing something (it has been a while), but the otpauth_uri is what we generate, so I think we need to update the docs to say "pass digits to otpauth_uri".

Btw, given it is called "digits" in the otpauth_uri, we probably want to rename "length" to "digits". Sorry for the back and forth!

@LoicBersier

Copy link
Copy Markdown
Contributor Author

No worries! Added a mention of it in the doc and an example under "Generating URIs for QR Code"

Comment thread lib/nimble_totp.ex Outdated
*in seconds*) to be used. Default is `System.os_time(:second)`.
* `:period` - The period (in seconds) in which the code is valid. Default is `30`.
If this option is given to `verification_code/2`, it must also be given to `valid?/3`.
* `:totp_digits` - The desired length of the totp. Default is 6.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry, let's call it just digits!

@josevalim josevalim changed the title Add totp_size option Add :digits option Nov 22, 2025
@josevalim

Copy link
Copy Markdown
Member

@kianmeng would you be so kind to give us a hand with CI? ❤️

@josevalim
josevalim merged commit 586c130 into dashbitco:master Nov 22, 2025
0 of 2 checks passed
@josevalim

Copy link
Copy Markdown
Member

💚 💙 💜 💛 ❤️

josevalim pushed a commit that referenced this pull request Nov 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants