Added code to have support for multiple SAML response encryption algorithms.#253
Open
ebsi-pjoshi wants to merge 2 commits into
Open
Added code to have support for multiple SAML response encryption algorithms.#253ebsi-pjoshi wants to merge 2 commits into
ebsi-pjoshi wants to merge 2 commits into
Conversation
|
@Revsgaard please review |
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for multiple SAML response encryption algorithms by introducing new encryption algorithm constants, validation logic, and configuration options. The changes allow users to specify different encryption algorithms instead of being limited to the hardcoded AES256.
- Added comprehensive set of encryption algorithm constants for various AES modes and key sizes
- Introduced encryption algorithm validation with specific error handling for unsupported algorithms
- Updated the encryption process to use configurable algorithms instead of hardcoded AES256
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Saml2EncryptionAlgorithms.cs | New file defining encryption algorithm URL constants for AES, RSA, and SHA variants |
| Saml2AuthnResponse.cs | Added encryption algorithm property and validation call during message encryption |
| Saml2EncryptedXml.cs | Modified encryption method to accept algorithm parameter and set appropriate key sizes |
| EncryptionAlgorithm.cs | New validation class that checks algorithm support and throws specific errors |
| Saml2Configuration.cs | Added EncryptionAlgorithm property with AES256 as default |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Comment on lines
+36
to
+43
| else if (encryptionAlgorithm.Equals(Saml2EncryptionAlgorithms.XmlEncSHA256Url, StringComparison.InvariantCulture)) | ||
| { | ||
| return; | ||
| } | ||
| else if (encryptionAlgorithm.Equals(Saml2EncryptionAlgorithms.XmlEncSHA512Url, StringComparison.InvariantCulture)) | ||
| { | ||
| return; | ||
| } |
There was a problem hiding this comment.
The validation allows SHA256 and SHA512 algorithms to pass as valid encryption methods, but these are message digest algorithms, not encryption algorithms. They should not be accepted for SAML assertion encryption.
Suggested change
| else if (encryptionAlgorithm.Equals(Saml2EncryptionAlgorithms.XmlEncSHA256Url, StringComparison.InvariantCulture)) | |
| { | |
| return; | |
| } | |
| else if (encryptionAlgorithm.Equals(Saml2EncryptionAlgorithms.XmlEncSHA512Url, StringComparison.InvariantCulture)) | |
| { | |
| return; | |
| } |
…ports only 128, 192 and 256.
ebsi-ssundar
approved these changes
Sep 15, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.