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

Added RSAKeyValue to KeyInfo element #75

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Added RSAKeyValue to KeyInfo element #75

wants to merge 3 commits into from

Conversation

hiddewie
Copy link
Contributor

@hiddewie hiddewie commented Sep 3, 2015

In Issue #74 I saw a feature was requested to add a RSAKeyValue to a signature. This piece of code adds that feature.

I've also updated a single test.

@hiddewie
Copy link
Contributor Author

Is this PR being looked at? This repository does not seem very active.

@gfaust-qb
Copy link
Contributor

@robrichards is mostly very busy but @Maks3w often looks, categorizes and reviews very often, but it takes time until Rob merges the PRs.

@robrichards
Copy link
Owner

@hiddewie I have looked at it but am making some changes to it as I've run into a couple cases where it fails to work against certain backends when the cert is also included so need to make those optional. I just haven't had time to finish the code and go through testing it yet. Limited time for another 3 more weeks

@hiddewie
Copy link
Contributor Author

Thank you for the quick response.

No problem if it takes some time, as long as people look into it.

@jlopezmazzeira
Copy link

Hi, how do I implement this method? What function should I use, I can not find the way to implement it, greetings

@hiddewie
Copy link
Contributor Author

hiddewie commented Apr 6, 2017

This is a pull request so the functionality is still under consideration by the package maintainer @robrichards.

Please open a separate issue or send an email to the author for library specific questions.

@solidevolution
Copy link

What about the PR? Or did someone a fork? This is 2 years old!

@hiddewie
Copy link
Contributor Author

hiddewie commented Oct 9, 2017

As far as I am concerned I am open for comments on the PR. However, the maintainer has commented no more since the first comment so I am unsure of the current status or the existance of a fork.

@robrichards
Copy link
Owner

@hiddewie Can you just make adding those optional where default behavior is not to add them so that current behavior is maintained

@hiddewie
Copy link
Contributor Author

@robrichards I've added an option, which preserves the original behavour. The original test has been restored (xml-sign.phpt) and an extra one has been added with the updated XML content (the public key info).

src/XMLSecurityDSig.php Outdated Show resolved Hide resolved
@robrichards robrichards mentioned this pull request Oct 1, 2020
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.

6 participants