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

fix: AccessTokens authenticator records all accesses to database #843

Merged
merged 3 commits into from
Sep 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
# Upgrade Guide

## Version 1.0.0-beta.6 to 1.0.0-beta.7

### Install New Config AuthToken.php

A new Config file **AuthToken.php** has been introduced. Run `php spark shield:setup`
again to install it into **app/Config/**, or install it manually.

Then change the default settings as necessary. When using Token authentication,
the default value has been changed from all accesses to be recorded in the
``token_logins`` table to only accesses that fail authentication to be recorded.

## Version 1.0.0-beta.3 to 1.0.0-beta.4

### Important Password Changes
Expand Down
52 changes: 36 additions & 16 deletions src/Authentication/Authenticators/AccessTokens.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use CodeIgniter\I18n\Time;
use CodeIgniter\Shield\Authentication\AuthenticationException;
use CodeIgniter\Shield\Authentication\AuthenticatorInterface;
use CodeIgniter\Shield\Config\Auth;
use CodeIgniter\Shield\Entities\User;
use CodeIgniter\Shield\Exceptions\InvalidArgumentException;
use CodeIgniter\Shield\Models\TokenLoginModel;
Expand Down Expand Up @@ -42,6 +43,8 @@ public function __construct(UserModel $provider)
*/
public function attempt(array $credentials): Result
{
$config = config('AuthToken');

/** @var IncomingRequest $request */
$request = service('request');

Expand All @@ -51,21 +54,35 @@ public function attempt(array $credentials): Result
$result = $this->check($credentials);

if (! $result->isOK()) {
// Always record a login attempt, whether success or not.
$this->loginModel->recordLoginAttempt(
self::ID_TYPE_ACCESS_TOKEN,
$credentials['token'] ?? '',
false,
$ipAddress,
$userAgent
);
if ($config->recordLoginAttempt >= Auth::RECORD_LOGIN_ATTEMPT_FAILURE) {
// Record all failed login attempts.
$this->loginModel->recordLoginAttempt(
self::ID_TYPE_ACCESS_TOKEN,
$credentials['token'] ?? '',
false,
$ipAddress,
$userAgent
);
}

return $result;
}

$user = $result->extraInfo();

if ($user->isBanned()) {
if ($config->recordLoginAttempt >= Auth::RECORD_LOGIN_ATTEMPT_FAILURE) {
// Record a banned login attempt.
$this->loginModel->recordLoginAttempt(
self::ID_TYPE_ACCESS_TOKEN,
$credentials['token'] ?? '',
false,
$ipAddress,
$userAgent,
$user->id
);
}

$this->user = null;

return new Result([
Expand All @@ -80,14 +97,17 @@ public function attempt(array $credentials): Result

$this->login($user);

$this->loginModel->recordLoginAttempt(
self::ID_TYPE_ACCESS_TOKEN,
$credentials['token'] ?? '',
true,
$ipAddress,
$userAgent,
$this->user->id
);
if ($config->recordLoginAttempt === Auth::RECORD_LOGIN_ATTEMPT_ALL) {
// Record a successful login attempt.
$this->loginModel->recordLoginAttempt(
self::ID_TYPE_ACCESS_TOKEN,
$credentials['token'] ?? '',
true,
$ipAddress,
$userAgent,
$this->user->id
);
}

return $result;
}
Expand Down
4 changes: 2 additions & 2 deletions src/Config/AuthToken.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@
use CodeIgniter\Config\BaseConfig;

/**
* Authenticator Configuration for Token Auth and HMAC Auth
* Configuration for Token Auth and HMAC Auth
*/
class AuthToken extends BaseConfig
{
/**
* --------------------------------------------------------------------
* Record Login Attempts for Token and HMAC Authorization
* Record Login Attempts for Token Auth and HMAC Auth
* --------------------------------------------------------------------
* Specify which login attempts are recorded in the database.
*
Expand Down
2 changes: 1 addition & 1 deletion src/Filters/TokenAuth.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class TokenAuth implements FilterInterface
{
/**
* Do whatever processing this filter needs to do.
* By default it should not return anything during
* By default, it should not return anything during
* normal execution. However, when an abnormal state
* is found, it should return an instance of
* CodeIgniter\HTTP\Response. If it does, script
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
$this->assertNull($this->auth->getUser());
}

public function testLoginByIdNoToken(): void

Check warning on line 65 in tests/Authentication/Authenticators/AccessTokenAuthenticatorTest.php

View workflow job for this annotation

GitHub Actions / phpunit / PHP 7.4 - OCI8 - highest

Took 1.80s from 0.50s limit to run Tests\\Authentication\\Authenticators\\AccessTokenAuthenticatorTest::testLoginByIdNoToken
{
$user = fake(UserModel::class);

Expand Down Expand Up @@ -174,7 +174,7 @@
$this->assertFalse($result->isOK());
$this->assertSame(lang('Auth.badToken'), $result->reason());

// A login attempt should have always been recorded
// A failed login attempt should have been recorded by default.
$this->seeInDatabase($this->tables['token_logins'], [
'id_type' => AccessTokens::ID_TYPE_ACCESS_TOKEN,
'identifier' => 'abc123',
Expand Down Expand Up @@ -202,8 +202,8 @@
$this->assertInstanceOf(AccessToken::class, $foundUser->currentAccessToken());
$this->assertSame($token->token, $foundUser->currentAccessToken()->token);

// A login attempt should have been recorded
$this->seeInDatabase($this->tables['token_logins'], [
// A successful login attempt is not recorded by default.
$this->dontSeeInDatabase($this->tables['token_logins'], [
'id_type' => AccessTokens::ID_TYPE_ACCESS_TOKEN,
'identifier' => $token->raw_token,
'success' => 1,
Expand Down
Loading