Fix rejected headless authentication path#62634
Conversation
30d449a to
43d9a80
Compare
43d9a80 to
2bef5d5
Compare
| var mfaResp *proto.MFAAuthenticateResponse | ||
| switch req.Action { | ||
| case "accept": | ||
| if req.MFAResponse != nil { |
There was a problem hiding this comment.
So here, we implicitly assume that MFAResponse will always be non-nil if Action is "accept". What if it's nil, then? Previously it would lead to a panic in all cases (though as you wrote, it was only when the request was denied). Now, even in case of "accept", we would move on and mark the state to approved with a nil mfaResp. Perhaps we should return an error if the response is nil?
There was a problem hiding this comment.
If the Action is accept and the mfaResp is nil, an audit event is emitted and an error is returned by the UpdateHeadlessAuthenticationState call that happens just after this switch statement. I thought it was best to leave it to the update function to handle the error and emit audit events. What do you think?
|
Forgot to merge this one, and it was solved by #63189, closing |
Since v17, if a user rejects a headless authentication request, the call to update the headless authentication state on the backend would fail due to calling a function on a nil pointer. This would leave the headless authentication object in a pending state (cleaned up after 3 minutes) and no audit log showing rejection.
Manual tests:
changelog: Fixed a server panic when denying a headless authentication attempt