Skip to content

Commit

Permalink
Merge pull request #373 from silinternational/idp-781-update-last_log…
Browse files Browse the repository at this point in the history
…in-after-mfa

Idp 781 update last login after mfa
  • Loading branch information
hobbitronics authored Sep 19, 2024
2 parents bce267d + 0b47032 commit 692e2dd
Show file tree
Hide file tree
Showing 8 changed files with 69 additions and 21 deletions.
19 changes: 19 additions & 0 deletions api.raml
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,25 @@ types:
description: A server-side error occurred.
body:
type: Error
/update-last-login:
put:
description: Update the last login timestamp of a user based on their employee ID.
responses:
200:
description: Successfully updated the last login time.
body:
properties:
employee_id: string
last_login_utc: string
example: |
{
"employee_id": "12345",
"last_login_utc": "2024-09-18T12:34:56Z"
}
404:
description: Not found. The user with the specified `employeeId` was not found.
500:
description: Server error. Failed to update the last login time.
/mfa:
get:
description: Get a list of configured MFA devices for user
Expand Down
8 changes: 2 additions & 6 deletions application/common/models/Authentication.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

namespace common\models;

use common\components\Emailer;
use common\helpers\MySqlDateTime;
use yii\web\HttpException;

/**
Expand Down Expand Up @@ -91,7 +89,7 @@ protected function authenticateByInvite($invite)
/**
* Run User validation rules. If all rules pass, $this->authenticatedUser will be a
* clone of the User, and the User record in the database will be updated with new
* login and reminder dates.
* reminder dates.
*
* @param User $user
*/
Expand All @@ -100,15 +98,13 @@ protected function validateUser(User $user)
if ($user->validate()) {
$this->authenticatedUser = clone $user;

$user->last_login_utc = MySqlDateTime::now();

$user->updateProfileReviewDates();

$user->checkAndProcessHIBP();

if (!$user->save()) {
\Yii::error([
'action' => 'save last_login_utc and nag dates for user after authentication',
'action' => 'save nag dates for user after authentication',
'status' => 'error',
'message' => $user->getFirstErrors(),
]);
Expand Down
9 changes: 7 additions & 2 deletions application/common/models/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

use Br33f\Ga4\MeasurementProtocol\Dto\Event\BaseEvent;
use Closure;
use common\components\Emailer;
use common\components\HIBP;
use common\components\Sheets;
use common\helpers\MySqlDateTime;
Expand All @@ -13,7 +12,6 @@
use Ramsey\Uuid\Uuid;
use Sil\EmailService\Client\EmailServiceClientException;
use Sil\PhpArrayDotNotation\DotNotation;
use TheIconic\Tracking\GoogleAnalytics\Analytics;
use Yii;
use yii\behaviors\AttributeBehavior;
use yii\data\ActiveDataProvider;
Expand Down Expand Up @@ -803,6 +801,13 @@ public function save($runValidation = true, $attributeNames = null)
return parent::save($runValidation, $attributeNames);
}

public function updateLastLogin()
{
$this->last_login_utc = MySqlDateTime::now();

return $this->save(false, ['last_login_utc']);
}

/**
* @param bool $isNewUser
* @param array $changedAttributes
Expand Down
3 changes: 1 addition & 2 deletions application/features/authentication.feature
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@ Feature: Authentication
| active | yes |
| locked | no |
And a record still exists with an employee_id of "123"
# TODO: check that last_login_utc was updated and current
# And none of the data has changed
# TODO: check that none of the data has changed

Scenario: Attempt to authenticate an unknown user
Given I provide the following valid data:
Expand Down
8 changes: 4 additions & 4 deletions application/features/bootstrap/FeatureContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -452,9 +452,9 @@ public function shouldBeStoredAsNowUTC($property)
$minAcceptable = $expectedNow - self::ACCEPTABLE_DELTA_IN_SECONDS;
$maxAcceptable = $expectedNow + self::ACCEPTABLE_DELTA_IN_SECONDS;

$storedNow = strtotime($this->userFromDb->$property);
$storedNow = $this->userFromDb->$property ? strtotime($this->userFromDb->$property) : 0;

Assert::range($storedNow, $minAcceptable, $maxAcceptable);
Assert::range($storedNow, $minAcceptable, $maxAcceptable, "Stored time $storedNow is not within acceptable range of $minAcceptable to $maxAcceptable");
}

/**
Expand Down Expand Up @@ -490,8 +490,8 @@ public function theOnlyPropertyToChangeShouldBe($property)
$previous = $this->userFromDbBefore->$name;
$current = $this->userFromDb->$name;

$name === $property ? Assert::notEq($current, $previous)
: Assert::eq($current, $previous);
$name === $property ? Assert::notEq($current, $previous, "$property is equal. Previous: $previous, Current: $current")
: Assert::eq($current, $previous, "$property is not equal. Previous: $previous, Current: $current");
}
}

Expand Down
12 changes: 11 additions & 1 deletion application/features/user.feature
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,16 @@ Feature: User
| hide | yes |
| groups | mensa,hackers |

Scenario: update the last_login_utc of an existing user
Given the requester is authorized
And I add a user with an employee_id of "123"
And a record exists with an employee_id of "123"
When I request "/user/123/update-last-login" be updated
Then the response status code should be "200"
And a record exists with an employee_id of "123"
And last_login_utc should be stored as now UTC
And the only property to change should be last_login_utc

Scenario: Deactivate an existing user
Given a record does not exist with an employee_id of "123"
And the requester is authorized
Expand Down Expand Up @@ -771,4 +781,4 @@ Feature: User
Examples:
| active | isOrIsNot |
| no | is NOT |
| yes | is |
| yes | is |
13 changes: 7 additions & 6 deletions application/frontend/config/main.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,13 @@
/*
* User routes
*/
'GET user' => 'user/index',
'GET user/<employeeId:\w+>' => 'user/view',
'POST user' => 'user/create',
'PUT user/<employeeId:\w+>' => 'user/update',
'PUT user/<employeeId:\w+>/password' => 'user/update-password',
'PUT user/<employeeId:\w+>/password/assess' => 'user/assess-password',
'GET user' => 'user/index',
'GET user/<employeeId:\w+>' => 'user/view',
'POST user' => 'user/create',
'PUT user/<employeeId:\w+>' => 'user/update',
'PUT user/<employeeId:\w+>/update-last-login' => 'user/update-last-login',
'PUT user/<employeeId:\w+>/password' => 'user/update-password',
'PUT user/<employeeId:\w+>/password/assess' => 'user/assess-password',

/*
* Authentication routes
Expand Down
18 changes: 18 additions & 0 deletions application/frontend/controllers/UserController.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,24 @@ public function actionUpdate(string $employeeId)
return $user;
}

public function actionUpdateLastLogin(string $employeeId)
{
$user = User::findOne(condition: ['employee_id' => $employeeId]);

if ($user === null) {
Yii::$app->response->statusCode = 404;

return null;
}

if (!$user->updateLastLogin()) {
Yii::$app->response->statusCode = 500;
return null;
}

return ['employee_id' => $user->employee_id, 'last_login_utc' => $user->last_login_utc];
}

public function actionUpdatePassword(string $employeeId)
{
$user = User::findOne(['employee_id' => $employeeId]);
Expand Down

0 comments on commit 692e2dd

Please sign in to comment.