Closed jennyf19 closed 1 month ago
There are a lot of API's for a single logical control. Following this model, we could have an explosion of apis for every new parameter.
Can we limit this to API's that take SecurityTokenDescriptor when creating a token? Have we considered how this will work with SAML or SAML2? Could we introduce a call back to get the kid?
There are a lot of API's for a single logical control. Following this model, we could have an explosion of apis for every new parameter. Can we limit this to API's that take SecurityTokenDescriptor when creating a token?
I'm fine with this. It will clean things up here. Theoretically everything should just go through the SecurityTokenDescriptor
API.
Have we considered how this will work with SAML or SAML2?
This I'm not sure on. I would assume we don't want to touch SAML/2.
Could we introduce a call back to get the kid?
Can you expand on this? IssuerSigningKeyResolver
provides the kid
as input.
SecurityTokenDescriptor
I'm fine with this. It will clean things up here. Theoretically everything should just go through the SecurityTokenDescriptor API.
Agree, consider designing for what is needed today i.e. optionally opting out of adding kid for JsonWebTokenHandler alone.
This I'm not sure on. I would assume we don't want to touch SAML/2.
Correct. This change is expected to be backward compatible.
This I'm not sure on. I would assume we don't want to touch SAML/2. SAML/2 have CreateToken methods that take a SecurityTokenDescriptor, how do we describe this flag to those api's?
Can you expand on this? IssuerSigningKeyResolver provides the kid as input. Reasoning is that this just affects JWT token, so I am trying to think of how to restrict to just the JsonWebTokenHandler. This would require a callback something like JsonWebTokenHandler.WriteKid(ref Utf8JsonWriter, SigningCredentials). It seems complicated.
Perhaps we should just comment in the SecurityTokenDescriptor that this only applies to JWTs.
The source code LGTM. Consider adding tests to ensure the changes introduced work as expected.
2960