[BUG] Entity\AuthorizationCode::setExpires should not UTC Time.

[liverbool]

https://github.com/bshaffer/oauth2-server-bundle/blob/develop/Storage/AuthorizationCode.php#L99

From above line you set UTC time, But in your OAuth2\GrantType/AuthorizationCode::validateRequest you have check by default time zone.

It should be: (new \DateTime())->setTimestamp($expires).

https://github.com/bshaffer/oauth2-server-php/blob/develop/src/OAuth2/GrantType/AuthorizationCode.php#L63

[bshaffer]

Thank you for bringing this up. Can you explain to me at the difference is? It seems to me that in both cases, the result will be the same, as the $expires variable is a Unix timestamp, and so is not affected by the default time zone.

[liverbool]

My server (Asia time +7) new \DateTime('@' . $expires) force to UTC time (+0) and time() return server's timezone. We should remove @ symbol to working base on server timezone.

[bshaffer]

I don't understand what you just said. Can you show me an example when the @ gives a different result than the ->setTimestamp?

[liverbool]

new \DateTime('@' . $expires); // => this will alway return UTC time (+00:00)
time() // => will return timestamp depends on server timezone

My server set default timezone to Asia timezone

new \DateTime('@' . $expires); // => 123
(new \DateTime())->setTimestamp($expires) // => 123 + 07:00
time() // => 123 + 07:00

Sorry, for my poor en. :)

[bshaffer]

@liverbool

The timestamps for both of these are equal. If you look at the comparison, it gets the timestamp from the DateTime object. See the following test:

$expires = time()+3600;
$date1 = new DateTime('@' . $expires);
$date2 = (new DateTime)->setTimestamp($expires);
echo $date1->getTimestamp(); // 1409123250
echo $date2->getTimestamp(); // 1409123250

So, I don't understand where the problem is occurring. Could you write a failing test to demonstrate?

[liverbool]

bshaffer, try to change your timezone to other like: date_default_timezone_set('Asia/Bangkok');

[bdecarne]

:+1: Same problem here with Europe/Paris (+02:00)

The Datetime in the database is UTC (https://github.com/bshaffer/oauth2-server-bundle/blob/develop/Storage/AccessToken.php#L86) but the server compare it to localized timestamp (https://github.com/bshaffer/oauth2-server-php/blob/develop/src/OAuth2/Controller/ResourceController.php#L-85)

The access token is always invalid.

[bshaffer]

I still do not understand the issue. Timestamps are timezone-agnostic, so as long as the time is set correctly on your server and in your database, this should work.

Can you submit a PR with either a fix, or a test to duplicate the issue (preferably the latter)? If I can create a repeatable failing case, I'll be happy to fix the issue.

[liverbool]

It's depends on php.ini timezone setting, maybe this's php bug too! (cannot remember my php version) we cannot test by just change date_default_timezone_set at a runtime. I just fix by using Datetime::setTimestamp instead of using @timestamp

[bshaffer]

Well, as frustrating as this is to not be able to recreate, the fix is easy enough that I'll just code it and be done with it.

[bshaffer]

@liverbool I changed my php.ini date.timezone to Europe/Paris and still could not duplicate the issue.

See #31 for the fix

[liverbool]

I cannot confirm for now, @bdecarne do you can confirm?

[bdecarne]

I can't confirm now, perhaps tomorrow.

I suspect Doctrine2 datetime type. From the doc :

By default Doctrine assumes that you are working with a default timezone.

When the datetime new \DateTime('@' . $expires) is persisted in the db, it become a mysql DATETIME like 2014-10-15 10:00:00. After that, when Doctrine fetch the value, perhaps it does a new DateTime('2014-10-15 10:00:00'). This object is timezone related : http://codepad.viper-7.com/gdfhM7

What do you think ?

[liverbool]

@bdecarne you are right! I also think cause of Doctrine datetime convert (when persist to db) and i tried @bshaffer see https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Types/DateTimeType.php

// this my simple test
$time = time();
$format = 'Y-m-d H:i:s';
echo date_default_timezone_get() ."\n";

var_dump('UTC');
$date = new DateTime('@' . $time);
var_dump($date->getOffset());
var_dump($date->format($format));
var_dump($date->getTimestamp());
$diff = \DateTime::createFromFormat($format, $date->format($format));
var_dump($diff->getTimestamp());
var_dump($time);
echo "\n\n";
var_dump('DEFAULT');
$date = (new DateTime())->setTimestamp($time);
var_dump($date->getOffset());
var_dump($date->format($format));
var_dump($date->getTimestamp());
$diff = \DateTime::createFromFormat($format, $date->format($format));
var_dump($diff->getTimestamp());
var_dump($time);

/**
US/Pacific
string(3) "UTC"
int(0)
string(19) "2014-10-15 18:14:49"
int(1413396889)
int(1413422089) <=== not match
int(1413396889)

string(7) "DEFAULT"
int(-25200)
string(19) "2014-10-15 11:14:49"
int(1413396889)
int(1413396889)
int(1413396889)

**/

[bshaffer]

THANK YOU!!!

Okay, it all makes sense now. I would actually say then this is a doctrine bug, as doctrine is expecting the DateTime coming in to be using the PHP Default time zone. In the case that it isn't, the hydrating coming back out of the database will be incorrect (as Doctrine always converts the date time string using the default time zone).

I will fix this in our library, and then file an issue / pr on the doctrine side. Thank you guys for helping track this down!

[bshaffer]

This page here explains that Doctrine expects ALL DateTime objects to use the default_timezone specified in php.ini. Why they do it this way, I cannot say. But alas, we must fix it anyway.

[bshaffer]

This will be part of the v0.3 release

[bdecarne]

Happy to have been helpful on a project as promising. Next month i'll try to give some help on some missing parts.