Skip to content

Lower some session warnings to debug level - #41195

Closed
solracsf wants to merge 1 commit into
masterfrom
lowerToDebug
Closed

Lower some session warnings to debug level#41195
solracsf wants to merge 1 commit into
masterfrom
lowerToDebug

Conversation

@solracsf

@solracsfsolracsf commented Oct 30, 2023•
edited
Loading

Copy link
Copy Markdown
Member

Summary

No need to Warn about these, and they can be noisy.

Resolves#41156

Checklist

@solracsf
Lower some warnings to debug level
a0c5444
Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com>
@solracsfsolracsf added the 3. to review Waiting for reviews label Oct 30, 2023
@solracsfsolracsf added this to the Nextcloud 28 milestone Oct 30, 2023
@solracsfsolracsf linked an issue Oct 30, 2023 that may be closed by this pull request
[Bug]: OC\Authentication\Exceptions\InvalidTokenException: Token is too short for a generated token, should be the password during basic auth #41156
Closed
8 tasks
@solracsf

solracsf commented Oct 30, 2023

Copy link
Copy Markdown
MemberAuthor

/backport to stable27

@solracsf

solracsf commented Oct 30, 2023

Copy link
Copy Markdown
MemberAuthor

/backport to stable26

@brianjmurrell

brianjmurrell commented Oct 30, 2023

Copy link
Copy Markdown

Are either of the changes in this PR for the specific message being spammed in #41156? The message in that issue seems to be:

Token is too short for a generated token, should be the password during basic auth

ChristophWurst
ChristophWurst reviewed Oct 30, 2023
View reviewed changes
Comment threadlib/private/User/Session.php
return false;
}

if (!$this->checkTokenCredentials($dbToken, $token)) {

@ChristophWurstChristophWurstOct 30, 2023

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.

inside this method there are critical paths that might still be worth logging. That is every time \OC\Authentication\Token\IProvider::invalidateToken or markPasswordInvalid is called, for example.

@solracsfsolracsfOct 30, 2023

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Maybe in a follow up PR? Otherwise, feel free to commit them as output messages would require a deeper knowledge of what they exactly accomplish, which I don't have. 🦺

@skjnldsvskjnldsv mentioned this pull request Nov 1, 2023
Merged
This was referenced Nov 6, 2023
Merged
Merged
@solracsf

solracsf commented Nov 10, 2023

Copy link
Copy Markdown
MemberAuthor

Closing as #41302 has been merged.

@solracsf
solracsf deleted the lowerToDebug branch November 10, 2023 15:49
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ChristophWurstChristophWurstChristophWurst left review comments
@AltahrimAltahrimAwaiting requested review from Altahrim
@come-nccome-ncAwaiting requested review from come-nc

Assignees

No one assigned

Labels

3. to reviewWaiting for reviews

Projects

None yet

Milestone

Nextcloud 28

Development

Successfully merging this pull request may close these issues.

[Bug]: OC\Authentication\Exceptions\InvalidTokenException: Token is too short for a generated token, should be the password during basic auth

4 participants

@solracsf@brianjmurrell@ChristophWurst@skjnldsv