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

SamlReponse only supports one level of StatusCode #374

Closed
reggieb opened this issue Feb 7, 2017 · 5 comments
Closed

SamlReponse only supports one level of StatusCode #374

reggieb opened this issue Feb 7, 2017 · 5 comments

Comments

@reggieb
Copy link

reggieb commented Feb 7, 2017

SAML supports two levels of StatusCode, and on failure it is the inner one that has the most pertinent information. For example, here is a NoAuthnContext failure:

  <saml2p:Status xsi:type="saml2p:StatusType">
    <saml2p:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Responder" xsi:type="saml2p:StatusCodeType">
      <saml2p:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:NoAuthnContext" xsi:type="saml2p:StatusCodeType"/>
    </saml2p:StatusCode>
  </saml2p:Status>

However the method OneLogin::RubySaml::Response#status_code will only output if there is just one StatusCode:

def status_code
    @status_code ||= begin
      nodes = REXML::XPath.match(
        document,
        "/p:Response/p:Status/p:StatusCode",
        { "p" => PROTOCOL }
      )
      if nodes.size == 1
        node = nodes[0]
        node.attributes["Value"] if node && node.attributes
      end
   end
 end

Could this be replaced with something that either returned an array of statuses or concatenated multiple statuses. For example:

def status_code
  @status_code ||= begin
     nodes = REXML::XPath.match(
        document,
        "/p:Response/p:Status//p:StatusCode",
        { "p" => PROTOCOL }
      )
      statuses = nodes.collect do |node|
        value = node.attributes["Value"]
        value.split(':').last
      end
      statuses.join(" : ")
  end
end
@pitbulk
Copy link
Collaborator

pitbulk commented Feb 7, 2017

Sure, are you able to provide the PR with that change and unit test?
If not I will implement it during the week.

@reggieb
Copy link
Author

reggieb commented Mar 17, 2017

Hi @pitbulk, I hadn't spotted that you'd responded to this issue.

I don't have a problem creating a PR, but the PR I currently have in place seems to be failing a CI test due to what looks like a nokogiri version issue in JRuby. I think if I submit another PR it will have the same issue.

@pitbulk
Copy link
Collaborator

pitbulk commented Mar 17, 2017

you can send that PR... it seems that in JRuby sometimes there are random false failing CI test

@pitbulk
Copy link
Collaborator

pitbulk commented Apr 15, 2017

any progress?

@reggieb
Copy link
Author

reggieb commented Apr 21, 2017

Sorry - I've moved to another project, and we managed to work around the problem in the project where the fix was required.

However, I should be able to get this into a PR (especially now that my other PR is progressing), so at the moment it is a case of finding the time to write it.

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