[MAGNOLIA-4459] AuditLogging logs twice login attempt Created: 28/Jun/12 Updated: 15/Oct/14 Resolved: 13/Oct/14 |
|
| Status: | Closed |
| Project: | Magnolia |
| Component/s: | core |
| Affects Version/s: | 4.5, 5.0 |
| Fix Version/s: | 4.5.23, 5.3.5 |
| Type: | Bug | Priority: | Neutral |
| Reporter: | Milan Divilek | Assignee: | Christopher Zimmermann |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | support | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||
| Template: |
|
||||||||||||
| Acceptance criteria: |
Empty
|
||||||||||||
| Task DoD: |
[ ]*
Doc/release notes changes? Comment present?
[ ]*
Downstream builds green?
[ ]*
Solution information and context easily available?
[ ]*
Tests
[ ]*
FixVersion filled and not yet released
[ ] 
Architecture Decision Record (ADR)
|
||||||||||||
| Bug DoR: |
[ ]*
Steps to reproduce, expected, and actual results filled
[ ]*
Affected version filled
|
||||||||||||
| Date of First Response: | |||||||||||||
| Description |
|
Steps to reproduce:
This behaviour is happen only for "Login" log. "Logout, Activation, Move,.. " are logged correctly. |
| Comments |
| Comment by Christopher Zimmermann [ 07/Oct/14 ] |
|
Removed redundant call to AuditLoggingUtil.log in LoginFilter. |
| Comment by Jan Haderka [ 07/Oct/14 ] |
|
Excuse my ignorance, but how is change in AbstractI18nContentSupport relevant to this ticket? |
| Comment by Christopher Zimmermann [ 08/Oct/14 ] |
|
In consultation with Jan - keeping the current approach as the login status can change, and adding additional calls to uditLoggingUtil#log in the clauses did not have them. Also logging the login failure in the isMemoryLimitReached clause. |
| Comment by Jan Haderka [ 08/Oct/14 ] |
|
Looks good. Just one more case to cover: if (subject == null) { 90 log.error("Invalid login result from handler [{}] returned STATUS_SUCCEEDED but no subject", handler.getClass().getName()); 91 throw new ServletException("Invalid login result"); 92 } we log error and throw exception, but we should also log audit record about it as having successful login w/o subject is very suspicious. |
| Comment by Christopher Zimmermann [ 08/Oct/14 ] |
|
Also write failure to access log if subject=null. |
| Comment by Aleksandr Pchelintcev [ 09/Oct/14 ] |
|
Review:
Otherwise - looks good. |