Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid code duplication in extensibility testing #3041

Merged

Conversation

iNinja
Copy link
Contributor

@iNinja iNinja commented Nov 27, 2024

Avoid code duplication in extensibility testing

  • Modified ValidateTokenAsync in SAML and SAML2 token handlers to receive SecurityToken instead of SamlSecurityToken / Saml2SecurityToken
  • Added test interface to unify JsonWebTokenHandler, SamlSecurityTokenHandler and Saml2SecurityTokenHandler's ValidateTokenAsync and CreateToken methods under a shared API
  • Added ExtensibilityTheoryData as the base for all extensibility theory data.
  • Added ValidateTokenAsyncExtensibility to test all extensibility aspects across JWT, SAML, and SAML2
  • Added issuer specific theory data and extensibility test
  • Removed all duplicated code in issuer extensibility testing across JWT, SAML, and SAML2 using the new test classes

…ve SecurityToken instead of Saml(2)SecurityToken
…andler and Saml2SecurityTokenHandler's ValidateTokenAsync and CreateToken methods under a shared API
…y data. Added ValidateTokenAsyncExtensibility to test all extensibility aspects across JWT, SAML, and SAML2
…T, SAML, and SAML2 using the new test classes
@iNinja iNinja requested a review from a team as a code owner November 27, 2024 16:20
@brentschmaltz
Copy link
Member

brentschmaltz commented Nov 28, 2024

This is a good pass at code reduction.
The internals in Saml2SecurityTokenHandler and SamlSecurityTokenHandler should be put in TokenHandler is possible.
If adding TokenHandler is not possible, because of previous work, we will need new API's.

@brentschmaltz brentschmaltz merged commit eb0daac into dev Nov 28, 2024
5 of 6 checks passed
@armandRobled
Copy link

armandRobled commented Nov 28, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants