-2

I'm looking for a lean way to compare two times. One is the time, someone requested a new password(FOS UserBundle provides a Getter for that) and the other is the current time minus e.g. 10 minutes.

if($user->getPasswordRequestedAt() <= date("Y-m-d H:i:s", strtotime('-10 minutes'))){
    return $this->render('@User/Resetting/no_resend.html.twig', array( 'username' => $username));
  }
  else {
  return $this->render('@User/Resetting/check_email.html.twig', array(
      'tokenLifetime' => ceil($this->container->getParameter('fos_user.resetting.retry_ttl') / 60),
  ));

So if someone requested a password already 10 minute ago, he gets to the page saying "You already requested a password, please wait 10 minute to retry.". If the request was longer than 10 minutes ago, the pages says "Email for password reset has been sent".

I would think the comparison is right that way but it's somehow always going to the "else" part.

Where's the mistake?

sonja
  • 924
  • 1
  • 21
  • 52
  • what does `$user->getPasswordRequestedAt()` return? And why are you comparing strings when it's times you want to compare? – Andreas Jan 09 '18 at 15:19
  • 1
    As Andreas said, the issue is caused by comparing a DateTime object and a string. The FOS User class also provides a method called `isPasswordRequestNonExpired`, which might be a better approach than writing your own logic. – iainn Jan 09 '18 at 15:20
  • Your comparison works (even if it's a bad idea to compare strings) see here. (change time to your timezone) https://3v4l.org/JQF6o but most likely `$user->getPasswordRequestedAt()` does not return what you think it does. – Andreas Jan 09 '18 at 15:23
  • Take a look at the nesbot/carbon library. – svgrafov Jan 09 '18 at 15:27
  • @Andreas it's returning a DateTime object. – sonja Jan 09 '18 at 15:29
  • @iainn I found that method, but I'm not sure how I'd correctly use it? do you have a suggestion? – sonja Jan 09 '18 at 15:30
  • I haven't used it myself, but it looks like it just takes a number of seconds, so for 10 minutes you'd do something like `if ($user->isPasswordRequestNonExpired(600))...` – iainn Jan 09 '18 at 15:33
  • @iainn somehow that's not working.. but logic wise it should, right? – sonja Jan 09 '18 at 16:18

3 Answers3

0
if($user->getPasswordRequestedAt() >= date("Y-m-d H:i:s", strtotime('-10 minutes'))){

Should do the trick.

Since you want to check if the last request was done in the last 10 minutes or newer

gurks0r
  • 46
  • 4
  • That way, it's already at the first time going to the "resend not possible" page. – sonja Jan 09 '18 at 15:30
  • It might not working at the first time because $user->getPasswordRequestedAt() has no date or a wrong date when executed the first time – gurks0r Jan 09 '18 at 15:43
  • Ok, I dumped it and there actually is a value for getPasswordRequestedAt() so that can't be the problem. @gurks0r – sonja Jan 09 '18 at 16:00
  • I only saw your mistack in the comparison. since getPasswordRequestedAt is returning a DateTime object Johannes Krauß's answer should work. My last idea is that your code is running to late and the PasswordRequestedAt is overriden with the current time before your code is executed? – gurks0r Jan 09 '18 at 16:20
0

As far as I know $user->getPasswordRequestedAt() (since being processed through Doctrine) is an instance of \DateTime.

So use another datetime to compare it:

$limit = \new DateTime();
$limit->sub(new \DateInterval("P600S"));

if($user->getPasswordRequestedAt() >= $limit){
    return $this->render('@User/Resetting/no_resend.html.twig', array( 'username' => $username));
}
else {
    return $this->render('@User/Resetting/check_email.html.twig', array(
      'tokenLifetime' => ceil($this->container->getParameter('fos_user.resetting.retry_ttl') / 60),
));

With this you keep the OOP style and have a clean if clause. The $limit holds a timestamp from 10 minutes ago. So when you requested your password at 14:10 and it is 14:19 by now, $limit will hold 14:09. As long as the requested password date is bigger than the timestamp 10 minutes ago, there won't be a resend.

EDIT: Since you are using the FOS UserBundle it is indeed possible to make it even shorter as iainn pointed out:

if($user->isPasswordRequestNonExpired(600)){
    return $this->render('@User/Resetting/no_resend.html.twig', array( 'username' => $username));
}
else {
    return $this->render('@User/Resetting/check_email.html.twig', array(
          'tokenLifetime' => ceil($this->container->getParameter('fos_user.resetting.retry_ttl') / 60),
));

The first and only argument of this method is a amount of seconds that have to pass so that the password can be requested again.

Johannes Klauß
  • 10,676
  • 16
  • 68
  • 122
  • When trying your second way with isPasswordRequestNonExpired () it's going to that page even when I'm requesting the password for the first time.. I'll try your first way as well but the second one seems leaner and more correct to me, so there must be a way how to achieve it with that method, I'd think.. – sonja Jan 09 '18 at 15:52
  • Well, I'm guessing that even the registration will set the `passwordRequestedAt` field in the user table. So after the registration you have to wait ten minutes until you can request the password again. When unsure, try setting the `passwordRequestedAt` field for your test user in the database so that it's more than 10 minutes ago and check if you can request it through your app. – Johannes Klauß Jan 09 '18 at 15:54
  • I'm trying it with an already registered user on my local machine, so it's acting like the live system itself.. – sonja Jan 09 '18 at 15:56
  • And what is the current value of the field in the database? – Johannes Klauß Jan 09 '18 at 15:59
  • I'm pretty sure I'm not on the same time zone you are, but it seems like this is your current local time, right? So the ten minutes haven't passed yet. Either wait ten minutes or change the field to ten minutes ago. – Johannes Klauß Jan 09 '18 at 16:04
  • but why would I change it? I don't see yet where this would lead me? – sonja Jan 09 '18 at 16:05
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/162828/discussion-between-johannes-klauss-and-sonja). – Johannes Klauß Jan 09 '18 at 16:06
  • Would you like to post your solution with the parameter as an answer so that I can accept it? It's working perfectly fine now! I can provide you my code if you want! – sonja Jan 09 '18 at 16:59
0

Since I haven't used symfony I can't know for sure.
But this is my guess. See if it works.

$dt = $user->getPasswordRequestedAt();
If($dt->getTimestamp() <= strtotime(time())-600){
    return $this->render('@User/Resetting/no_resend.html.twig', array( 'username' => $username));
}else {
     return $this->render('@User/Resetting/check_email.html.twig', array(
  'tokenLifetime' => ceil($this->container->getParameter('fos_user.resetting.retry_ttl') / 60),
));

This will create the datetime object to $dt variable.
In the if() I use get_timestamp to get the Unix value of the datetime object and compare it to Unix time -600 seconds (10 minutes).

The reason I choose Unix values is because they are great for comparison.

Andreas
  • 23,610
  • 6
  • 30
  • 62