[ELY-2127] Add client side TRACE logging after successful authentication #1532
Conversation
|
Hello, Ashpan. I'm waiting for one of the admins to verify this patch with /ok-to-test in a comment. |
|
/ok-to-test |
| @@ -2074,6 +2074,7 @@ SecurityIdentity getSourceIdentity() { | |||
| boolean verifyEvidence(final Evidence evidence) throws RealmUnavailableException { | |||
| // At this stage, we just verify that the evidence principal matches, and verify it with the realm. | |||
| final Principal evidencePrincipal = evidence.getDecodedPrincipal(); | |||
| if (evidencePrincipal == null) { log.tracef("Authentication Failed"); } | |||
There was a problem hiding this comment.
I think this can be removed. At this point, we haven't called verifyEvidence yet so we don't know whether authentication has failed here.
| @@ -2428,6 +2429,7 @@ void succeed() { | |||
| SecurityRealm.safeHandleRealmEvent(getRealmInfo().getSecurityRealm(), new RealmSuccessfulAuthenticationEvent(realmIdentity, authorizedIdentity.getAuthorizationIdentity(), null, null)); | |||
| SecurityDomain.safeHandleSecurityEvent(authorizedIdentity.getSecurityDomain(), new SecurityAuthenticationSuccessfulEvent(authorizedIdentity)); | |||
| realmIdentity.dispose(); | |||
| log.tracef("Authentication Completed"); | |||
There was a problem hiding this comment.
This is just a very minor comment but since a similar message ("Handling AuthenticationCompleteCallback: succeed") is already logged when handling the AuthenticationCompleteCallback, we can probably get away without this log message here.
| @@ -206,6 +206,7 @@ public void setNegotiationState(final int newState) { | |||
| */ | |||
| public void negotiationComplete() { | |||
| state = COMPLETE_STATE; | |||
| log.tracef("SASL Negotiation Succeeded"); | |||
There was a problem hiding this comment.
Just another minor comment, but I think it would be better to use "SASL Negotiation Completed" here.
There was a problem hiding this comment.
Just noticed that there are a couple SASL client implementations that call setNegotiationState(COMPLETE_STATE) instead of negotiationComplete() (see OAuth2SaslClient and ScramSaslClient). You could update these to call negotiationComplete() instead of setNegotiationState(COMPLETE_STATE) to make sure your message gets logged for these cases too.
https://issues.redhat.com/browse/ELY-2127