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

Get time from Clock [SDK-1973] #350

Closed
skjolber opened this issue Sep 21, 2020 · 3 comments · Fixed by #358
Closed

Get time from Clock [SDK-1973] #350

skjolber opened this issue Sep 21, 2020 · 3 comments · Fixed by #358
Labels
needs investigation An issue that has more questions to answer or otherwise needs work to fully understand the issue

Comments

@skjolber
Copy link

Describe the problem you'd like to have solved

We're using a NTP library to get the current time.

Describe the ideal solution

Get current time from a java.time.Clock instance instead of System.currentTimeMillis(). Probably unit-testing will be more simple as well.

Alternatives and current work-arounds

Subclassing and/or cloning classes. Explicitly passing in the current time to methods.

Additional context

SecureCredentialsManager

@lbalmaceda
Copy link
Contributor

lbalmaceda commented Sep 22, 2020

That would be ideal, but the java.time.Clock class does not seem to be available until Android API 26, and this library targets as min API 15. We plan to raise this minimum but is not going to be 26, unfortunately. I guess we could go with a solution like the one java-jwt has.

The only reason to push back on a change that will let the developer pass the Clock instance is that you can put as much documentation and warning flags as you want, but anyone could still mistakenly use it and break the application.

Is this only for testing purposes or your use case requires to use that NTP library to avoid time sync issues across apps/services?

@lbalmaceda lbalmaceda added the waiting for customer This issue is waiting for a response from the issue or PR author label Sep 22, 2020
@skjolber
Copy link
Author

We are going to have consumer apps, and can not controll their internal clock, and we know from experience that some of them are off (by minutes or more). So the plan is to use an NTP library to get the correct current time, so that the app is unaffected by the time skew.

I missed that backwards compatible target - then java.time.Clock is not going to work. The java-jwt approach is a good alternative.

@lbalmaceda lbalmaceda added needs investigation An issue that has more questions to answer or otherwise needs work to fully understand the issue and removed waiting for customer This issue is waiting for a response from the issue or PR author labels Sep 25, 2020
@lbalmaceda lbalmaceda changed the title Get time from Clock Get time from Clock [SDK-1973] Sep 25, 2020
@lbalmaceda
Copy link
Contributor

I think the java-jwt solution is an easy path forward. I need to evaluate if this can be set in a global way, maybe through the Auth0 object, so it can also be considered when validating ID tokens, for example.

I'm tracking this as SDK-1973.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs investigation An issue that has more questions to answer or otherwise needs work to fully understand the issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants