Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Secret: byte[] secret confusion #83

Closed
anibyl opened this issue Dec 16, 2024 · 3 comments
Closed

Secret: byte[] secret confusion #83

anibyl opened this issue Dec 16, 2024 · 3 comments

Comments

@anibyl
Copy link
Contributor

anibyl commented Dec 16, 2024

This ticket is merely a question to start a discussion.

Currently, the library uses the following constructor for initialization (link):

public Builder(final byte[] secret) {

A shared secret is a byte sequence, also commonly represented as a Base32 string (example).

However, the builder constructor consumes neither of them, but instead a byte array which is a Base32 secret encoded with a standard charset (secret.getBytes(), e.g. here):

private final static String secret = "vv3kox7uqj4kyakohmzpph3us4cjimh6f3zknb5c2oobq6v2kiyhm27q";
...
TOTPGenerator generator = new TOTPGenerator.Builder(secret.getBytes()).withHOTPGenerator(builder -> {

It's unclear why it has to be like that, since the encoded-string secret is later converted to a correct shared secret anyway:

private final byte[] secret;
...
byte[] secretBytes = decodeBase32(secret);

I recently spent a few hours trying to understand why the library wasn't working when I was passing a correct shared secret to it.

This is confusing.

Would that be possible to add a javadoc explaining what should be passed as byte[] secret, or a Builder constructor consuming String secret, or...?

I could create a PR, but first I would like to have a conversation.

@BastiaanJansen
Copy link
Owner

Hmm thanks for your question! You make a good point as there is no specific reason the library works this way.

It would be appreciated if you make a PR with JavaDoc or another implementation!

@anibyl
Copy link
Contributor Author

anibyl commented Dec 20, 2024

Hi @BastiaanJansen, please take a look at #84 :)

BastiaanJansen added a commit that referenced this issue Dec 23, 2024
Add a String secret Builder constructor + Base32 docs and comments #83
@BastiaanJansen
Copy link
Owner

Thanks! This is included in version 2.1.0!

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

No branches or pull requests

2 participants