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.

Sign up for GitHub

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

Jump to bottom

[fix] Flaky test #220

Merged
merged 2 commits into from
Sep 17, 2024
Merged

[fix] Flaky test #220

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1bcd250
Decode AuthnRequest params to XML format before pass to mock
Sep 17, 2024
02e7d88
Drop test case check
Sep 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions saml_idp.gemspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ Gem::Specification.new do |s|
s.add_development_dependency('capybara', '>= 2.16')
s.add_development_dependency('rails', '>= 5.2')
s.add_development_dependency('rake')
s.add_development_dependency('debug')
s.add_development_dependency('rspec', '>= 3.7.0')
s.add_development_dependency('ruby-saml', '>= 1.7.2')
s.add_development_dependency('simplecov')
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/saml_idp/controller_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def params
end

it 'should call xml signature validation method' do
signed_doc = SamlIdp::XMLSecurity::SignedDocument.new(params[:SAMLRequest])
signed_doc = SamlIdp::XMLSecurity::SignedDocument.new(decode_saml_request(params[:SAMLRequest]))
allow(signed_doc).to receive(:validate).and_return(true)
allow(SamlIdp::XMLSecurity::SignedDocument).to receive(:new).and_return(signed_doc)
validate_saml_request
Expand Down
1 change: 0 additions & 1 deletion spec/spec_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
# encoding: utf-8
require 'simplecov'
SimpleCov.minimum_coverage 96.45
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was too abstract a number based on current coverage. Also, current test unit tests are not really helping the protect SAML 2.0 workflow. Let’s postpone it until we convert the gem more focused to the SAML 2.0 protocol.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe it was just too precise.. you could have dropped the number to the new coverage instead of removing the line

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that. But most of the unit tests were meaningless not checking the logic regarding SAML 2.0.
I think once we have proper SAML 2.0 workflow tests and we can re-introduce it.

SimpleCov.start do
add_filter "/spec/"
end
Expand Down
15 changes: 13 additions & 2 deletions spec/support/saml_request_macros.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
module SamlRequestMacros
def make_saml_request(requested_saml_acs_url = "https://foo.example.com/saml/consume", enable_secure_options = false)
auth_request = OneLogin::RubySaml::Authrequest.new
auth_url = auth_request.create(saml_settings(requested_saml_acs_url, enable_secure_options))
CGI.unescape(auth_url.split("=").last)
auth_url = auth_request.create_params(saml_settings(requested_saml_acs_url, enable_secure_options))
auth_url['SAMLRequest']
end

def make_saml_logout_request(requested_saml_logout_url = 'https://foo.example.com/saml/logout')
Expand Down Expand Up @@ -90,6 +90,17 @@ def idp_configure(saml_acs_url = "https://foo.example.com/saml/consume", enable_
end
end

def decode_saml_request(saml_request)
decoded_request = Base64.decode64(saml_request)
begin
# Try to decompress, since SAMLRequest might be compressed
Zlib::Inflate.new(-Zlib::MAX_WBITS).inflate(decoded_request)
rescue Zlib::DataError
# If it's not compressed, just return the decoded request
decoded_request
end
end

def print_pretty_xml(xml_string)
doc = REXML::Document.new xml_string
outbuf = ""
Expand Down