Repository navigation
Audit-log LinOTP login success/failure (REST and RADIUS) - #462
Open
terryozaki-wq wants to merge 1 commit into
Open
terryozaki-wq wants to merge 1 commit into
terryozaki-wq wants to merge 1 commit into
Conversation
UserResource.loginLinOTP had no security-audit-trail instrumentation at all, unlike loginNative/loginLDAP. Adds the same UserAuditTrailUtils.recordLoginFailed calls loginNative/loginLDAP make, at every genuine failure outcome: the "LinOTP login is not allowed" gate, a non-200 response (invalid credentials/OTP, RADIUS/LinOTP server unreachable, misconfiguration), and an unexpected exception. The successful login's audit entry is not added explicitly - addAccessToken -> userTrackingLogin already calls UserAuditTrailUtils.recordLogin for every provider, LinOTP included, confirmed by reading that path rather than assumed, to avoid double logging. A plain operational log line is still added on success for visibility. The intermediate "please enter your OTP" challenge-response step (a 200 response with no token yet) is deliberately NOT written to the audit trail - it is neither a success nor a failure, only the eventual accept/reject outcome is. Neither the submitted pin/password/OTP, nor a RADIUS shared secret, is ever written to any log. Companion to the new RADIUS auth mode added to LinOTPUtil in SEMOSS/Semoss PR #3192 - this covers both auth modes identically since both return through the same LinOTPResponse. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
UserResource.loginLinOTPhad no security-audit-trail instrumentation at all, unlikeloginNative/loginLDAPUserAuditTrailUtils.recordLoginFailedcalls those methods already make, at every genuine failure outcome: the "LinOTP login is not allowed" gate, a non-200 response (invalid credentials/OTP, RADIUS/LinOTP server unreachable, misconfiguration), and an unexpected exceptionaddAccessToken -> userTrackingLoginalready callsUserAuditTrailUtils.recordLoginfor every provider, LinOTP included (confirmed by reading that path, not assumed - avoids double-logging)Companion PR
This pairs with SEMOSS/Semoss#3192, which adds a RADIUS authentication mode to
LinOTPUtil. This change covers both auth modes identically, since both return through the sameLinOTPResponse- it is not RADIUS-specific itself.Test plan
mvn compilesucceeds against adev-built SEMOSS jarUserResourcein this repo; manual/integration verification of the audit trail rows is still worth doing before relying on this in production🤖 Generated with Claude Code