Add more apis for generic xml security token#4703
Conversation
StephenMolloy
left a comment
There was a problem hiding this comment.
One question about overriding with the same results... but looks fine otherwise.
| public override bool CanCreateKeyIdentifierClause<T>() => default; | ||
| public override T CreateKeyIdentifierClause<T>() => default; | ||
| public override bool MatchesKeyIdentifierClause(SecurityKeyIdentifierClause keyIdentifierClause) => default; |
There was a problem hiding this comment.
Aren't these overrides just re-defining the same results inherited from SecurityToken?
There was a problem hiding this comment.
Yes, but I think it might do something with regards to what happens when a derived class calls base.MatchesKeyIdentifierClause. When calling a base method, it can't just look up in the vtable for the abstract type and find the method, it's got to look up the method in the actual base class implementation. At least semantically there's a difference. If you didn't know this class also overrode it, you might expect it to call the implementation in the parent class of this. I suspect it would resolve correctly at runtime regardless, but semantically this is more correct.
Adding api's which are missing from the contract for GenericXmlSecurityToken and GenericXmlSecurityKeyIdentifierClause. This is needed to support the WSTrustChannelFactory usage case where customers will need to access the returned security token's TokenXml property in some cases.