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

Improve documentation of generator regarding counter-parameter #43

Open
uweschaefer opened this issue May 8, 2021 · 7 comments
Open

Comments

@uweschaefer
Copy link

uweschaefer commented May 8, 2021

Hi & Thanks for this lib. We use the generator & verifier to generate simple expiring OTPs (not looking at the full MFA usecase).

One difficulty we had, was the use of the generator, especially what to pass for the counter parameter. It is easy to figure out that it should be relying on the TimeProvider, but in order of our usecase to work, we need to pass

Math.floorDiv(timeProvider.getTime(), timePeriodInSeconds)

and i guess we're not alone there. This is hard to figure out until you read the code of the verifier.

I was wondering if documentation can be improved here, or (maybe even better) the API can be augmented to be more usable.
For instance, why not have a

public String generate(String key, DefaultCodeVerifier counter) throws CodeGenerationException

so that OTPs can be generated according to the parameters of the verifier (which is weird, because we're not using the interface CodeVerifier here. Maybe extending this with the necessary parameters would help.

Another way to do this would be to have a

public String generate(String key, TimeProvider tprov, int timePeriod) throws CodeGenerationException

But as people look at the method with the min number of params first, some javadoc would really help nevertheless.

What do you think?

PS: i'd be open to create a PR if you want me to.

@uweschaefer
Copy link
Author

anyone?

@skapral
Copy link

skapral commented Aug 24, 2021

@uweschaefer +1, stumbled exactly on the same thing.

@Bas83
Copy link

Bas83 commented Sep 14, 2021

Luckily I decided to check the open issues before starting to debug this. I had the same issue, I just figured I had to pass the time obtained from the timeProvider in there. Doesn't help that the params are apparently called String s, long l.

@ClisthenesPimentel
Copy link

Same fight here, glad I found this.

@villordo
Copy link

villordo commented Jul 1, 2022

Buenardo

@Krokochik
Copy link

Thanks!!

@barnesm999
Copy link

+1

Great lib, but I spent a lot of time here trying to understand why the generated codes were not working correctly.

Using the lib to write some integration tests against AWS Cognito which requires a 30s token window. Final code to generate valid tokens that Cognito could verify was:

final CodeGenerator codeGenerator = new DefaultCodeGenerator(HashingAlgorithm.SHA1);
final TimeProvider timeProvider = new SystemTimeProvider();
final var code = codeGenerator.generate(secretCode, Math.floorDiv(timeProvider.getTime(), 30));

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

7 participants