Subtle Change in Authenticator Assertion Verification Logic #451
Description
I realise you should key assertion results on the returned credential (and userHandle associated with it). However, I’ve hit a confusing scenario, and I’m trying to understand whether the change in FinishAssertionSteps from 2.5.1 to 2.8.1 is intentional.
In 2.5.1, I don’t think the AssertionResult could contain a different username than the one in the AssertionRequest. In Step 6, the library checked:
- if the request contained a username
- and the response contained a userHandle
- then the userHandle derived from the request’s username must match the response’s userHandle
If the username in the request either never had credentials or no longer had any credentials, getUserHandleForUsername() returned an empty Optional, which did not equal the response userHandle — causing the assertion to fail. This seemed sensible, if an unlikely scenario.
In 2.8.1, the logic has changed slightly. If the request contains a username but no userHandle, the library now has the logic:
If the username does not correspond to a stored credential (e.g., no entry in the repository), then:
- effectiveRequestUserHandle is empty
- userHandleDerivedFromUsername is false
- the mismatch check is not executed and the final username becomes the username from the request, even though the authenticated credential belongs to a different user
I assume this is a pretty unlikely scenario, and it might be intentional, but the result from the older version seems less confusing (arguably safer). Although possible that the original check was overly strict compared to the spec.
The validation logic seems to imply it detects this kind of mismatch (as the old one did), and I’d still assume the result to contain a username that matches the userhandle in the verified response.
Phil