Skip to content

[Review] Request from 'digitaltom' @ 'digitaltom/ruby-saml/review_130904_allow_for_settings_authn_context_and_settings_authn_context_decl_ref'#99

Closed
digitaltom wants to merge 4 commits intoSAML-Toolkits:masterfrom
digitaltom:review_130904_allow_for_settings_authn_context_and_settings_authn_context_decl_ref
Closed

[Review] Request from 'digitaltom' @ 'digitaltom/ruby-saml/review_130904_allow_for_settings_authn_context_and_settings_authn_context_decl_ref'#99
digitaltom wants to merge 4 commits intoSAML-Toolkits:masterfrom
digitaltom:review_130904_allow_for_settings_authn_context_and_settings_authn_context_decl_ref

Conversation

@digitaltom
Copy link
Copy Markdown

This adds support for a AuthnContextDeclRef element inside RequestedAuthnContext.
It looks like this:

<samlp:RequestedAuthnContext>
  <saml:AuthnContextDeclRef>suse/name/password/uri</saml:AuthnContextDeclRef>
</samlp:RequestedAuthnContext>

We need that for choosing a login page on our saml provider on sp initiated requests.

@Lordnibbler
Copy link
Copy Markdown
Contributor

@digitaltom can you please rebase off the current master (just released 0.7.3) and see if specs pass? Can you add additional tests for your fix?

@digitaltom
Copy link
Copy Markdown
Author

@Lordnibbler fixed, please check again.

@Lordnibbler
Copy link
Copy Markdown
Contributor

@luisvm @inakidelamadrid @pitbulk can you review please? looks ok to me 👍

@Lordnibbler
Copy link
Copy Markdown
Contributor

@luisvm @inakidelamadrid @pitbulk review please

@push-pull-deploy
Copy link
Copy Markdown
Contributor

👍

1 similar comment
@luisvm
Copy link
Copy Markdown
Contributor

luisvm commented Sep 25, 2014

👍

@luisvm
Copy link
Copy Markdown
Contributor

luisvm commented Sep 25, 2014

@pitbulk any word on this PR?

@Lordnibbler
Copy link
Copy Markdown
Contributor

@pitbulk please review so we can merge!

Comment thread test/request_test.rb
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Instead of 'suse' use 'http://example.com/declaration_ref'
(review the test)

@pitbulk
Copy link
Copy Markdown
Collaborator

pitbulk commented Oct 9, 2014

There is no documentation on the PR, neither example values for the fields.
I will sent a new PR with this info.

pitbulk added a commit that referenced this pull request Oct 9, 2014
@pitbulk
Copy link
Copy Markdown
Collaborator

pitbulk commented Oct 9, 2014

I gonna close this PR, we will merge #152

@pitbulk pitbulk closed this Oct 9, 2014
pitbulk added a commit that referenced this pull request Oct 31, 2014
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.

5 participants