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

Creating AuthRequests with nil RelayState sends empty RelayState URL param #437

Closed
vincentwoo opened this issue Feb 2, 2018 · 2 comments

Comments

@vincentwoo
Copy link
Contributor

Doing OneLogin::RubySaml::Authrequest.new.create(settings, RelayState: nil) still creates an empty RelayState url param. Ideally nil (or perhaps even falsy) RelayState would cause ruby-saml to not output a param. This caused an issue for a customer integration recently. SimpleSaml (the PHP library) apparently decided that we had an improperly signed request with the empty param.

@pitbulk
Copy link
Collaborator

pitbulk commented Feb 2, 2018

We should definitely avoid to generate TX with empty RelayState URL parameters.

When we generate the url_string we use the build_query that doesn't add the RelayState parameter if there is no content.

but the issue is after that, at the

params.each_pair do |key, value|
    request_params[key] = value.to_s
end

where params has the RelayState value, so if null set a null value for that parameter to request_params.

Similar issue happens at LogoutRequest and LogoutResponse

@vincentwoo are you able to provide a PR to fix that? Maybe makes sense some refactor.

@vincentwoo
Copy link
Contributor Author

Scratch last comment, got tests working w/ bundle exec

shyam-habarakada pushed a commit to 10Kft/ruby-saml that referenced this issue May 24, 2018
…ponses with nil RelayState sends empty RelayState URL param
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

No branches or pull requests

2 participants