Skip to content

feat: Add more debug output when checkTokenCredentials fails #63428

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.

Sign up for GitHub

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

Open
CarlSchwan wants to merge 3 commits into
base: master
Choose a base branch
from
Open

feat: Add more debug output when checkTokenCredentials fails #63428

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0a14c50
feat: Add more debug output when checkTokenCredentials fails
CarlSchwan Aug 20, 2026
71847ff
fix: Add backtrace when login fails
CarlSchwan Aug 20, 2026
e320b06
chore: Add more debug output
CarlSchwan Aug 20, 2026
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
6 changes: 5 additions & 1 deletion lib/private/Authentication/Token/PublicKeyToken.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,11 @@ public function getLoginName(): string {
*/
#[\Override]
public function getPassword(): ?string {
return parent::getPassword();
$password = parent::getPassword();
if ($password === '') {
return null;
}
return $password;
}

#[\Override]
Expand Down
30 changes: 22 additions & 8 deletions lib/private/User/Session.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ public function logClientIn($user,
}

private function handleLoginFailed(IThrottler $throttler, int $currentDelay, string $remoteAddress, string $user, ?string $password) {
$this->logger->warning("Login failed: '" . $user . "' (Remote IP: '" . $remoteAddress . "')", ['app' => 'core']);
$this->logger->warning("Login failed: '" . $user . "' (Remote IP: '" . $remoteAddress . "')", ['app' => 'core', 'exception' => new \Exception()]);

@joshtrichards joshtrichards Aug 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need an exception generated and logged for every invalid attempt? At least stick the trace at debug log level.


$throttler->registerAttempt('login', $remoteAddress, ['user' => $user]);
$this->dispatcher->dispatchTyped(new LoginFailed($user, $password));
Expand Down Expand Up @@ -702,7 +702,7 @@ private function getPassword($password) {
* @param string $token
* @return boolean
*/
private function checkTokenCredentials(IToken $dbToken, $token) {
private function checkTokenCredentials(IToken $dbToken, $token, array &$reason) {
// Check whether login credentials are still valid and the user was not disabled
// This check is performed each 5 minutes
$lastCheck = $dbToken->getLastCheck() ? : 0;
Expand All @@ -715,12 +715,19 @@ private function checkTokenCredentials(IToken $dbToken, $token) {
try {
$pwd = $this->tokenProvider->getPassword($dbToken, $token);
} catch (InvalidTokenException $ex) {
$reason = [
'exception' => $ex,
];

// An invalid token password was used -> log user out
return false;
} catch (PasswordlessTokenException $ex) {
// Token has no password

if (!is_null($this->activeUser) && !$this->activeUser->isEnabled()) {
$reason = [
'exception' => $ex,
'additional_message' => 'Passwordless token exception with no active or disabled user',
];
$this->tokenProvider->invalidateToken($token);
return false;
}
Expand All @@ -731,12 +738,18 @@ private function checkTokenCredentials(IToken $dbToken, $token) {
// Invalidate token if the user is no longer active
if (!is_null($this->activeUser) && !$this->activeUser->isEnabled()) {
$this->tokenProvider->invalidateToken($token);
$reason = [
'additional_message' => 'Invalidate token as the user is no longer active',
];
return false;
}

// If the token password is no longer valid mark it as such
if ($this->manager->checkPassword($dbToken->getLoginName(), $pwd) === false) {
$this->tokenProvider->markPasswordInvalid($dbToken, $token);
$reason = [
'additional_message' => 'The token password is no longer valid and is ' . (empty($pwd) ? 'empty' : 'not empty'),
];
// User is logged out
return false;
}
Expand All @@ -754,11 +767,9 @@ private function checkTokenCredentials(IToken $dbToken, $token) {
*
* Invalidates the token if checks fail
*
* @param string $token
* @param string $user login name
* @return boolean
* @param ?string $user The login name
*/
private function validateToken($token, $user = null) {
private function validateToken(string $token, ?string $user = null): bool {
try {
$dbToken = $this->tokenProvider->getToken($token);
} catch (InvalidTokenException $ex) {
Expand All @@ -774,10 +785,13 @@ private function validateToken($token, $user = null) {
return false;
}

if (!$this->checkTokenCredentials($dbToken, $token)) {
$reason = [];
if (!$this->checkTokenCredentials($dbToken, $token, $reason)) {
$this->logger->warning('Session token credentials are invalid', [
'app' => 'core',
'user' => $user,
'token name' => $dbToken->getName(),
...$reason,
]);
return false;
}
Expand Down
Loading