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

Add support for encrypted name id in encrypted assertion #594

Merged

Conversation

joonlabs
Copy link

Hey @pitbulk,

this is the PR for the issue #318. I added a check for the existence of an EncryptedID element during the decryption of the assertion. If it exists, the same method can be called recursively to decrypt the encrypted name id.
I also removed the validation error, that throws when an encrypted name id is detected in an encrypted assertion, since this is supported with this PR.
I added a signed SAML response with encrypted assertion that contains an encrypted name id to the test data and wrote a test case to ensure the validation of the response and the correct retrieving of the name id.

Let me know if that works for you, or you miss anything.

Best
Julius

@pitbulk
Copy link
Contributor

pitbulk commented Sep 13, 2024

Can yo fix the lint error?

@joonlabs
Copy link
Author

Hey @pitbulk,

I saw the run failed due to PHP Sniffer. I use a different intended / formatting tool at my IDEA. I'll fix the 'if' statement and request again.
Is there a way for me to check the rest of the pipeline by myself before I commit?

Best
Julius

@joonlabs
Copy link
Author

@pitbulk fixed the lint error. Please allow the workflow to run again.

@joonlabs
Copy link
Author

@pitbulk quick check in, could you please run the workflow again and merge the pull request if successful?

Best
Julius

@pitbulk pitbulk merged commit f40fbed into SAML-Toolkits:4.x-dev Sep 29, 2024
6 checks passed
@joonlabs
Copy link
Author

@pitbulk thank you for merging. When will this be included in a release?

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.

2 participants