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

Force escape downcasing for Azure SLO #627

Merged
merged 5 commits into from
Jan 31, 2022

Conversation

alagos
Copy link
Contributor

@alagos alagos commented Nov 11, 2021

I followed the Single Log Out guide for my Azure AD integration and it worked all good until implementing that idp_logout_request method where I received requests from Azure to log out a user.
The requests come signed and trying to validate the signature it was failing, then I realised that the problem is that they are encoding the request parameters with downcase encoding characters (like using %2f instead of %2F) and they use the parameters downcased to generate the signature, therefore in validation time, when signed parameters are restored with CGI.escape they were different than the originals sent by Azure (all upcased).
To solve this problem, I added a force_escape_downcasing option for OneLogin::RubySaml::SloLogoutrequest.new, so all the signature verification is done with downcased encoded parameters.
I'm not sure where to add in the readme this option, specially because the SLO example I followed isn't verifying signed requests, but I'm open to suggestions.

@pitbulk
Copy link
Collaborator

pitbulk commented Nov 27, 2021

In php-saml we named this setting: lowercaseUrlencoding

Can you:

  • Rename it and make it available at settings level instead be a options of the constructor?
  • Apply it as well to LogoutResponse validations

@alagos
Copy link
Contributor Author

alagos commented Dec 14, 2021

@pitbulk about your request:

Rename it and make it available at settings level instead be a options of the constructor?

added as settings.security[:lowercase_url_encoding], so done.

Apply it as well to LogoutResponse validations

I guess you meant to change the opposite method where the signature is unescaped, right?
I don't think that will be really necessary, as CGI.unescape doesn't make any difference about the received string casing, so it will return always the same result:

irb(main):008:0> CGI.unescape('%c3%a1sdf')
=> "ásdf"
irb(main):009:0> CGI.unescape('%C3%A1sdf')
=> "ásdf"

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