Expose UserIdentity in AssertionResult #291
Conversation
There was a problem hiding this comment.
Thanks, but I don't think this is a good approach. The library has no reason to need the UserIdentity type in authentication ceremonies, so it shouldn't appear in the relevant interfaces. At best, passing UserIdentity through in AssertionResult would provide the RP with additional information that the RP already knows by inference. PR #293
(now closed) would have changed that, but I don't think that's the right way to go either.
| username.flatMap( | ||
| un -> | ||
| assertedUserHandle.map( | ||
| uh -> UserIdentity.builder().name(un).displayName(un).id(uh).build())); |
There was a problem hiding this comment.
.name(un).displayName(un)
This is incorrect, and cannot be resolved without adding additional methods to CredentialRepository just for the sake of this feature. For this reason I don't think the UserIdentity type should be involved at all in authentication ceremonies - the library has no reason to need it, so it should not be required by the supported interfaces.
This is part of the
UserIdentitywork I proposed in #289 (comment); I am breaking it down into smaller parts for ease of review.This PR simply builds a
UserIdentityin step 6 ofFinishAssertionSteps(from the existing information) and passes it through to the finalAssertionResult. For now, this does not enable additional functionality; however, it is necessary groundwork for being able to specifyUserIdentityinStartAssertionOptionsin the future.I've also taken the liberty of restructuring step 6 a bit to make it easier to follow.