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

TokenResponse.IsExpired returns true at one minute after token expiration instead of one minute before #503

Closed
GoogleCodeExporter opened this issue Apr 14, 2015 · 7 comments
Assignees

Comments

@GoogleCodeExporter
Copy link

What steps will reproduce the problem?

Create a program that includes the OAuth library and does something like this:

var credentialInitializer = new 
ServiceAccountCredential.Initializer(clientId).FromCertificate(certificate);
credentialInitializer.Scopes = scopes.ToArray();
credentialInitializer.User = user;

credential = new ServiceAccountCredential(credentialInitializer);
await credential.RequestAccessTokenAsync(CancellationToken.None);

Where "clientId", "scopes", "certificate", and "user" are all valid for use 
with a service account.

The credential.Token property is a TokenResponse object with a IsExpired 
method. Here are some return values for that method for different situations:

Issued | Expires | Current Time | IsExpired?
--------------------------------------------
10:00  | 11:00   | 10:58:00     | False
10:00  | 11:00   | 10:59:00     | False
10:00  | 11:00   | 11:00:00     | False
10:00  | 11:00   | 11:01:00     | True
10:00  | 11:00   | 11:02:00     | True

What is the expected output? What do you see instead?

I expect the method to return true a minute before the expiration time, not a 
minute afterward:

Issued | Expires | Current Time | IsExpired?
--------------------------------------------
10:00  | 11:00   | 10:58:00     | False
10:00  | 11:00   | 10:59:00     | True
10:00  | 11:00   | 11:00:00     | True
10:00  | 11:00   | 11:01:00     | True
10:00  | 11:00   | 11:02:00     | True

ALSO: DateTime.Now is used in the TokenResponse code - DateTime.UtcNow might be 
more appropriate.

What version of the product are you using? 1.8.2
What is your operating system? Windows 8.1
What is your IDE? Visual Studio 2013
What is the .NET framework version? 4.5.2

Original issue reported on code.google.com by brett.e...@gmail.com on 10 Nov 2014 at 7:29

@GoogleCodeExporter
Copy link
Author

I think that's in purpose. One minute before the token is going to expired we 
already trying to refresh it.

I'm not sure why we did it exactly. The Java library has the same behavior. 
I'll think if we should avoid that.
Any suggestions?

Thanks!


Original comment by pele...@google.com on 10 Nov 2014 at 10:17

  • Changed state: Accepted

@GoogleCodeExporter
Copy link
Author

I think that's in purpose. One minute before the token is going to expired we 
already trying to refresh it.

I'm not sure why we did it exactly. The Java library has the same behavior. 
I'll think if we should avoid that.
Any suggestions?

Thanks!


Original comment by pele...@google.com on 10 Nov 2014 at 10:17

@GoogleCodeExporter
Copy link
Author

I think it is fine to consider the token "expired" 1 minute before the actual 
expiration time. The code in the IsExpired method does the opposite, though - 
it does not return true until 1 minute after the actual expiration time.

Original comment by brett.e...@gmail.com on 10 Nov 2014 at 10:50

@GoogleCodeExporter
Copy link
Author

ohhhhh crap... I'll check that. Thanks!

Original comment by pele...@google.com on 11 Nov 2014 at 9:40

  • Added labels: Milestone-Release1.9.1

@GoogleCodeExporter
Copy link
Author

Change the test first -
https://code.google.com/p/google-api-dotnet-client/source/browse/Src/GoogleApis.
Auth.Tests/OAuth2/Responses/TokenResponseTests.cs
and fix the code.

Original comment by pele...@google.com on 11 Nov 2014 at 9:44

@GoogleCodeExporter
Copy link
Author

https://codereview.appspot.com/184430043

Original comment by pele...@google.com on 14 Dec 2014 at 3:22

  • Changed state: Started

@GoogleCodeExporter
Copy link
Author

Original comment by pele...@google.com on 16 Dec 2014 at 11:26

  • Changed state: Fixed

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

1 participant