search

steffow / meteor-accounts-saml

SAML SP tested with OpenAM
27 stars 29 forks source link

Add support for encrypted assertions (by PR) #29

Closed gerbsen closed 6 years ago

gerbsen commented 6 years ago

hey @steffow, we have installed rocket.chat in a pretty large environment with possibly 60k+ users. since our IT department forces us to use encrypted assertions (please forgive me if the wording doesn't make 100% sense, I currently feel responsible but did not code so far on this project) and your plugin does not support it (I've been told), we hacked it into the the meteor-accounts-saml lib/file that is delivered with Rocket.Chat. Since things have settled down a little and we also experienced some bugs with our installation, we thought it's time to give back. We are in the process of preparing a pull request and I wanted to ask you if you would willing to accept it, if of course it meets your quality criteria. Cheers and thanks for your work, daniel

steffow commented 6 years ago

Hey Daniel.

Sure - happy to receive PR. Couple of things which saves my time:

a. Could y do a rebase and then run yr tests? b. By encry assertion you mean an assertion that uses XMLenc to encrypt data, right? Can this be done using the metadata we currently use? c. Make sure you send PR against dev branch or fork off dev branch (eg using git-flow) d. Are you aware of things that might backward compat?

Do let me know, if you have questions

Best

Steffo

On 29. Mar 2018, at 19:46, gerbsen notifications@github.com wrote:

hey @steffow https://github.com/steffow, we have installed rocket.chat in a pretty large environment with possibly 60k+ users. since our IT department forces us to use encrypted assertions (please forgive me if the wording doesn't make 100% sense, I currently feel responsible but did not code so far on this project) and your plugin does not support it (I've been told), we hacked it into the the meteor-accounts-saml lib/file that is delivered with Rocket.Chat. Since things have settled down a little and we also experienced some bugs with our installation, we thought it's time to give back. We are in the process of preparing a pull request and I wanted to ask you if you would willing to accept it, if of course it meets your quality criteria. Cheers and thanks for your work, daniel

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/steffow/meteor-accounts-saml/issues/29, or mute the thread https://github.com/notifications/unsubscribe-auth/AGosJQZUMK_PP9aj0hbwvJhvG-lsCt5yks5tjR31gaJpZM4TAuYW.

anicoa commented 6 years ago

Hey Steffo,

b. it's XMLenc encrypted data and we are using xml-encryption to decrypt. It can be done with current metadata. In our case encryption is forced on IDP side and settings in 'our' metadata regarding encrypted assertion/nameid are ignored. A next step could be to provide encryption in the local metadata but thats no necessary.

regards nico

gerbsen commented 6 years ago

hey @steffow I wanted to check up on the current status. Cheers, Daniel

steffow commented 6 years ago

Yep. Just tested - looks good. One minor comment. Could y also pls add sth to the README on how to use this?

steffow commented 6 years ago

Tagged 0.0.16