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 the PR #99 #152

Merged
merged 5 commits into from
Oct 31, 2014
Merged

Fix the PR #99 #152

merged 5 commits into from
Oct 31, 2014

Conversation

pitbulk
Copy link
Collaborator

@pitbulk pitbulk commented Oct 9, 2014

Add AuthnContextDeclRef and AuthContext comparison support.
Remove AuthContext from LogoutRequest

@pitbulk pitbulk mentioned this pull request Oct 9, 2014
Closed
@pitbulk
Copy link
Collaborator Author

pitbulk commented Oct 9, 2014

@luisvm , @Lordnibbler can you review it?

@Lordnibbler
Copy link
Contributor

@pitbulk the build on TravisCI is failing for Ruby 1.8.7, can you look into why?

@pitbulk
Copy link
Collaborator Author

pitbulk commented Oct 27, 2014

Is strange, First time I execute bundle exec rake

/home/pitbulk/.rvm/rubies/ruby-1.8.7-head/bin/ruby -I"lib:lib:test" -I"/home/pitbulk/.rvm/gems/ruby-1.8.7-head/gems/rake-10.3.2/lib" "/home/pitbulk/.rvm/gems/ruby-1.8.7-head/gems/rake-10.3.2/lib/rake/rake_test_loader.rb" "test/**/*_test.rb" 
  * DEFERRED: XmlSecurity::SignedDocument #extract_inclusive_namespaces should support inclusive canonicalization. 
Loaded suite /home/pitbulk/.rvm/gems/ruby-1.8.7-head/gems/rake-10.3.2/lib/rake/rake_test_loader
Started
...........FF.......................................................................................
Finished in 0.980707 seconds.
  1) Failure:
test: Authrequest should create the saml:AuthnContextClassRef with comparison exact. (RequestTest)
    [/home/pitbulk/proyectos/ruby-saml/test/request_test.rb:159:in `__bind_1414427782_383156'
     /home/pitbulk/.rvm/gems/ruby-1.8.7-head/gems/shoulda-2.11.3/lib/shoulda/context.rb:382:in `call'
     /home/pitbulk/.rvm/gems/ruby-1.8.7-head/gems/shoulda-2.11.3/lib/shoulda/context.rb:382:in `test: Authrequest should create the saml:AuthnContextClassRef with comparison exact. '
     /home/pitbulk/.rvm/gems/ruby-1.8.7-head/gems/mocha-0.14.0/lib/mocha/integration/test_unit/ruby_version_186_and_above.rb:29:in `__send__'
     /home/pitbulk/.rvm/gems/ruby-1.8.7-head/gems/mocha-0.14.0/lib/mocha/integration/test_unit/ruby_version_186_and_above.rb:29:in `run']:
<nil> is not true.

  2) Failure:
test: Authrequest should create the saml:AuthnContextClassRef with comparison minimun. (RequestTest)
    [/home/pitbulk/proyectos/ruby-saml/test/request_test.rb:169:in `__bind_1414427782_384138'
     /home/pitbulk/.rvm/gems/ruby-1.8.7-head/gems/shoulda-2.11.3/lib/shoulda/context.rb:382:in `call'
     /home/pitbulk/.rvm/gems/ruby-1.8.7-head/gems/shoulda-2.11.3/lib/shoulda/context.rb:382:in `test: Authrequest should create the saml:AuthnContextClassRef with comparison minimun. '
     /home/pitbulk/.rvm/gems/ruby-1.8.7-head/gems/mocha-0.14.0/lib/mocha/integration/test_unit/ruby_version_186_and_above.rb:29:in `__send__'
     /home/pitbulk/.rvm/gems/ruby-1.8.7-head/gems/mocha-0.14.0/lib/mocha/integration/test_unit/ruby_version_186_and_above.rb:29:in `run']:
<nil> is not true.

Same output than Travis, but If I execute the test again:

bundle exec rake
/home/pitbulk/.rvm/rubies/ruby-1.8.7-head/bin/ruby -I"lib:lib:test" -I"/home/pitbulk/.rvm/gems/ruby-1.8.7-head/gems/rake-10.3.2/lib" "/home/pitbulk/.rvm/gems/ruby-1.8.7-head/gems/rake-10.3.2/lib/rake/rake_test_loader.rb" "test/**/*_test.rb" 
  * DEFERRED: XmlSecurity::SignedDocument #extract_inclusive_namespaces should support inclusive canonicalization. 
Loaded suite /home/pitbulk/.rvm/gems/ruby-1.8.7-head/gems/rake-10.3.2/lib/rake/rake_test_loader
Started
....................................................................................................
Finished in 0.961662 seconds.
100 tests, 167 assertions, 0 failures, 0 errors

@pitbulk
Copy link
Collaborator Author

pitbulk commented Oct 27, 2014

Is a problem with the regular expression

@pitbulk
Copy link
Collaborator Author

pitbulk commented Oct 27, 2014

@luisvm , @Lordnibbler can you review it?

Lordnibbler
Lordnibbler reviewed Oct 27, 2014
View reviewed changes
README.md
@@ -104,6 +104,10 @@ response.settings = saml_settings
response.attributes[:username]
```

The saml:AuthnContextClassRef of the AuthNRequest can be provided by settings.authn_context , possible values are described at [SAMLAuthnCxt]. The comparison method can be set using the parameter settings.authn_context_comparison (the possible values are: 'exact', 'better', 'maximum' and 'minimum'), 'exact' is the default value.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would recommend wrapping code snippets in ticks like settings.authn_context_decl_ref (see raw source)

@Lordnibbler
Copy link
Contributor

👍 looks good other than my readme comments

pitbulk added a commit that referenced this pull request Oct 31, 2014
@pitbulk
Merge pull request #152 from onelogin/contextauth
5143b44 
Fix the PR #99
@pitbulk pitbulk merged commit 5143b44 into master Oct 31, 2014
@pitbulk pitbulk deleted the contextauth branch October 31, 2014 01:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pitbulk @Lordnibbler