Changeset View
Standalone View
src/app/AuthAttempt.php
- This file was added.
<?php | |||||
namespace App; | |||||
use Illuminate\Database\Eloquent\Model; | |||||
use Iatstuti\Database\Support\NullableFields; | |||||
use Carbon\Carbon; | |||||
/** | |||||
* The eloquent definition of an AuthAttempt. | |||||
* | |||||
* An AuthAttempt represents an authenticaton attempt from an application/client. | |||||
*/ | |||||
machniak: Something's wrong with this sentence ;) | |||||
class AuthAttempt extends Model | |||||
{ | |||||
use NullableFields; | |||||
// Password mismatch | |||||
public const REASON_PASSWORD = 'password'; | |||||
// Geolocation not in whitelist | |||||
public const REASON_GEOLOCATION = 'geolocation'; | |||||
private const STATUS_ACCEPTED = 'ACCEPTED'; | |||||
private const STATUS_DENIED = 'DENIED'; | |||||
protected $nullable = [ | |||||
'reason', | |||||
]; | |||||
protected $fillable = [ | |||||
'ip', | |||||
'user_id', | |||||
'status', | |||||
'reason', | |||||
'expires_at', | |||||
'last_seen', | |||||
]; | |||||
protected $casts = [ | |||||
'expires_at' => 'datetime', | |||||
'last_seen' => 'datetime' | |||||
]; | |||||
/** | |||||
* Prepare a date for array / JSON serialization. | |||||
* | |||||
* Required to not omit timezone and match the format of update_at/created_at timestamps. | |||||
* | |||||
* @param \DateTimeInterface $date | |||||
* @return string | |||||
*/ | |||||
protected function serializeDate(\DateTimeInterface $date): string | |||||
{ | |||||
return Carbon::instance($date)->toIso8601ZuluString('microseconds'); | |||||
} | |||||
/** | |||||
* Returns true if the authentication attempt is accepted. | |||||
* | |||||
* @return bool | |||||
Done Inline ActionsThese status labels could be class constants. Maybe it would be better to have isExpired() method and not include that check here. machniak: These status labels could be class constants. Maybe it would be better to have isExpired()… | |||||
*/ | |||||
public function isAccepted(): bool | |||||
{ | |||||
if ($this->status == self::STATUS_ACCEPTED && Carbon::now() < $this->expires_at) { | |||||
machniakUnsubmitted Done Inline ActionsWouldn't be better to have/use isAccepted() and isExpired() methods instead? machniak: Wouldn't be better to have/use isAccepted() and isExpired() methods instead? | |||||
mollekopfAuthorUnsubmitted Done Inline ActionsI don't think so. We're not currently using isExpired() (but we could of course), and the idea is that that the entry isAccepted until it expires (at which point it is no longer accepted. mollekopf: I don't think so. We're not currently using isExpired() (but we could of course), and the idea… | |||||
return true; | |||||
} | |||||
return false; | |||||
} | |||||
/** | |||||
* Returns true if the authentication attempt is denied. | |||||
* | |||||
* @return bool | |||||
*/ | |||||
public function isDenied(): bool | |||||
{ | |||||
return ($this->status == self::STATUS_DENIED); | |||||
} | |||||
/** | |||||
* Accept the authentication attempt. | |||||
*/ | |||||
public function accept() | |||||
{ | |||||
$this->expires_at = Carbon::now()->addHours(8); | |||||
$this->status = self::STATUS_ACCEPTED; | |||||
$this->reason = ''; | |||||
$this->save(); | |||||
} | |||||
/** | |||||
* Deny the authentication attempt. | |||||
*/ | |||||
public function deny() | |||||
{ | |||||
$this->status = self::STATUS_DENIED; | |||||
$this->reason = ''; | |||||
$this->save(); | |||||
} | |||||
/** | |||||
* Notify the user of this authentication attempt. | |||||
* | |||||
* @return bool false if there was no means to notify | |||||
*/ | |||||
public function notify(): bool | |||||
{ | |||||
return \App\CompanionApp::notifyUser($this->user_id, ['token' => $this->id]); | |||||
} | |||||
/** | |||||
* Notify the user and wait for a confirmation. | |||||
*/ | |||||
private function notifyAndWait() | |||||
{ | |||||
if (!$this->notify()) { | |||||
//FIXME if the webclient can confirm too we don't need to abort here. | |||||
\Log::warning("There is no 2fa device to notify."); | |||||
return false; | |||||
} | |||||
\Log::debug("Authentication attempt: {$this->id}"); | |||||
$confirmationTimeout = 120; | |||||
$timeout = Carbon::now()->addSeconds($confirmationTimeout); | |||||
do { | |||||
if ($this->isDenied()) { | |||||
\Log::debug("The authentication attempt was denied {$this->id}"); | |||||
return false; | |||||
} | |||||
if ($this->isAccepted()) { | |||||
\Log::debug("The authentication attempt was accepted {$this->id}"); | |||||
return true; | |||||
} | |||||
if ($timeout < Carbon::now()) { | |||||
\Log::debug("The authentication attempt timed-out: {$this->id}"); | |||||
Done Inline ActionsI think you can just do $this->refresh() and do not need the extra variable. And updating the $this state here might even be better. machniak: I think you can just do `$this->refresh()` and do not need the extra variable. And updating the… | |||||
return false; | |||||
} | |||||
sleep(2); | |||||
$this->refresh(); | |||||
Done Inline Actions"... or update existing one". Documentation for arguments, please. machniak: "... or update existing one". Documentation for arguments, please. | |||||
} while (true); | |||||
} | |||||
/** | |||||
* Record a new authentication attempt or update an existing one. | |||||
* | |||||
* @param \App\User $user The user attempting to authenticate. | |||||
* @param string $clientIp The ip the authentication attempt is coming from. | |||||
machniakUnsubmitted Done Inline ActionsFound by phpstan, should be $clientIP machniak: Found by phpstan, should be `$clientIP` | |||||
* | |||||
* @return \App\AuthAttempt | |||||
*/ | |||||
public static function recordAuthAttempt(\App\User $user, $clientIP) | |||||
{ | |||||
$authAttempt = \App\AuthAttempt::where('ip', $clientIP)->where('user_id', $user->id)->first(); | |||||
if (!$authAttempt) { | |||||
$authAttempt = new \App\AuthAttempt(); | |||||
$authAttempt->ip = $clientIP; | |||||
$authAttempt->user_id = $user->id; | |||||
} | |||||
$authAttempt->last_seen = Carbon::now(); | |||||
$authAttempt->save(); | |||||
return $authAttempt; | |||||
} | |||||
/** | |||||
* Trigger a notification if necessary and wait for confirmation. | |||||
* | |||||
* @return bool | |||||
*/ | |||||
public function waitFor2FA(): bool | |||||
{ | |||||
Done Inline ActionsReturn value is not self explanatory, please add some description. machniak: Return value is not self explanatory, please add some description. | |||||
if ($this->isAccepted()) { | |||||
return true; | |||||
} | |||||
Done Inline ActionsAgain, just $this->refresh(). machniak: Again, just `$this->refresh()`. | |||||
if ($this->isDenied()) { | |||||
return false; | |||||
} | |||||
if (!$this->notifyAndWait()) { | |||||
return false; | |||||
} | |||||
// Ensure the authAttempt is now accepted | |||||
$this->refresh(); | |||||
machniakUnsubmitted Done Inline ActionsActually I don't see why you would need a refresh here. machniak: Actually I don't see why you would need a refresh here. | |||||
mollekopfAuthorUnsubmitted Done Inline ActionsYou're right, we no longer need it because we now refresh $this. mollekopf: You're right, we no longer need it because we now refresh $this. | |||||
return $this->isAccepted(); | |||||
} | |||||
} |
Something's wrong with this sentence ;)