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

Fails to recognize ADFS SAML response #993

Closed
chongkong opened this issue Jul 18, 2016 · 6 comments
Closed

Fails to recognize ADFS SAML response #993

chongkong opened this issue Jul 18, 2016 · 6 comments

Comments

@chongkong
Copy link

chongkong commented Jul 18, 2016

I'm using Spinnaker in AWS. I used Spinnaker provided AMI version 42, and I just run sudo apt-get upgrade so I'm quite sure I'm running a latest version of it.

I'm trying to use ADFS SAML for authentication, and I'm stuck on a bug (or more likely, my misconfiguration) that I can't process SAML response in gate.

here's the log from /var/log/spinnaker/gate/gate.log

groovy.lang.MissingMethodException: No signature of method: org.opensaml.xml.schema.impl.XSAnyImpl.getValue() is applicable for argument types: () values: []
Possible solutions: getClass(), getAt(java.lang.String), detach(), getDOM(), getParent()
        at org.codehaus.groovy.runtime.ScriptBytecodeAdapter.unwrap(ScriptBytecodeAdapter.java:58)
        at org.codehaus.groovy.runtime.ScriptBytecodeAdapter.unwrap(ScriptBytecodeAdapter.java:64)
        at org.codehaus.groovy.runtime.callsite.PogoGetPropertySite.getProperty(PogoGetPropertySite.java:52)
        at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callGetProperty(AbstractCallSite.java:296)
...

and following the log, it ends up in this line

My SAMLResponse looks somewhat like this:

<samlp:Response Consent="urn:oasis:names:tc:SAML:2.0:consent:unspecified" Destination="https://spinnaker.mintnote.com/gate/saml/SSO" ID="_4a4f1bca-60fd-440d-9b65-c57501186e99" InResponseTo="a2a7ffab437gbc4a2da7ff7ce99g6f9" IssueInstant="2016-07-15T09:38:17.817Z" Version="2.0" xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol">
  <Issuer xmlns="urn:oasis:names:tc:SAML:2.0:assertion">http://adfs.vcnc.co.kr/adfs/services/trust</Issuer>
  <samlp:Status>
    <samlp:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success"/>
  </samlp:Status>
  <Assertion ID="_f303f03f-36dd-4cf8-bbc2-266050924bc1" IssueInstant="2016-07-15T09:38:17.817Z" Version="2.0" xmlns="urn:oasis:names:tc:SAML:2.0:assertion">
    <Issuer>http://adfs.vcnc.co.kr/adfs/services/trust</Issuer>
    <ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
      <ds:SignedInfo>
        <ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
        <ds:SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1"/>
        <ds:Reference URI="#_f303f03f-36dd-4cf8-bbc2-266050924bc1">
          <ds:Transforms>
            <ds:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/>
            <ds:Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
          </ds:Transforms>
          <ds:DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"/>
          <ds:DigestValue>xOfjPii8GMQrqLhH9bxHh4em7xE=</ds:DigestValue>
        </ds:Reference>
      </ds:SignedInfo>
      <ds:SignatureValue>hRJzBw9rJPNCaGY56n/ENW+Ee75ZAstGZh/YgQ6MLxhrPybukgVKyVJO+S7ImIqidQCqdKM5na0TbRklfFwTBLOcUKtgED1jb7Zww1F7A5mEmb26CEy4rAhwaHuTcCmqVMlVa45BWPbQDZF+7JpA1t2ptfsqHQpsltke1x+emtSdZuC4SkFxQOvOZbQD4qtB6SN4XFnhQkfNwUXjqxsBUs/Pj7avrAgYvdxHBhW8bQ5CBAigGg//uCxVFFq7SM4wDb89WTj2QKYTvIQ0X6hmhITfJrPhGxdyDA44SZM9a/rF2GXmu2aVFzae7a6sbUi6OuYmJx/aAhklCIFwHBjX+g==</ds:SignatureValue>
      <KeyInfo xmlns="http://www.w3.org/2000/09/xmldsig#">
        <ds:X509Data>
          <ds:X509Certificate>MIIC2jCCAcKgAwIBAgIQMs9+izs/3bpDb050w/kJpzANBgkqhkiG9w0BAQsFADApMScwJQYDVQQDEx5BREZTIFNpZ25pbmcgLSBhZGZzLnZjbmMuY28ua3IwHhcNMTUwMTI3MDQ0MDI4WhcNMTgwMTI2MDQ0MDI4WjApMScwJQYDVQQDEx5BREZTIFNpZ25pbmcgLSBhZGZzLnZjbmMuY28ua3IwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC2+Eu73hIbljw3+ro+KCyuJKKOuq+Qf6tQhedcjE7SBNtmDwfDECP41N6L2X12y6WpwidGFOnzsc+/71lHNxoYN4Bafj20y0S6Ioo/jb6qk+WF0c4A1+gKGQ9nXE9FayFedJtm56KBjU0Mo34SQBvog0YDeUC/7Mb2C+iDeWIwqz9TQZ9eP0YhZSfe+J/XQVriEzTXYAzLQENmaVuYp6KsrqhOCd8c3y544tYlIB7kKVvnZfZw5qp3280jG6sF+XT2MEmg0YIEZEIlVHuQeNUbGj70QtJFLm79aPBoyTJCaVsJfnNPyleq9FkYFL8U/FD1vqiVBaMIF5NECxQSTI0tAgMBAAEwDQYJKoZIhvcNAQELBQADggEBAAktAwOqkgVqeU/OuVCMFSQ3qKsT607cEOixbgLeyk6sktSSQh1tLFYVBUl0kFlTo/duO88Riy4+7EUlC5OaOf2NOJJgUoNssrCnD1B29QVBfW9rJxjxDdVJPvAkFWlCbMe1ES7WAV0+4WODH2yiwP+NyY9VUa9FbV9eMhpL02HkbSowUZV7DP9MlDibmkmER2EMHOlOQm8s0vILozRHlFlIuVWCv8BTM1f19OzYfX+rp9rZasAKPUrOyIFA+BPLIEyYgIjCRsrsWDqnG0w1430Hfcp5koMlHE5S4uFBwuweBZxgqvluq4JCz2eZMsps+FfVy1jmkfOzSWyHyuIZqKU=</ds:X509Certificate>
        </ds:X509Data>
      </KeyInfo>
    </ds:Signature>
    <Subject>
      <NameID Format="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress">aiden@between.us</NameID>
      <SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer">
        <SubjectConfirmationData InResponseTo="a2a7ffab437gbc4a2da7ff7ce99g6f9" NotOnOrAfter="2016-07-15T09:43:17.817Z" Recipient="https://spinnaker.mintnote.com/gate/saml/SSO"/>
      </SubjectConfirmation>
    </Subject>
    <Conditions NotBefore="2016-07-15T09:38:17.815Z" NotOnOrAfter="2016-07-15T10:38:17.815Z">
      <AudienceRestriction>
        <Audience>spinnaker.mintnote.com</Audience>
      </AudienceRestriction>
    </Conditions>
    <AttributeStatement>
      <Attribute Name="http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress">
        <AttributeValue>aiden@between.us</AttributeValue>
      </Attribute>
      <Attribute Name="http://schemas.xmlsoap.org/claims/Group">
        <AttributeValue>corp\Domain Users</AttributeValue>
        <AttributeValue>corp\Dev Team</AttributeValue>
        <AttributeValue>corp\Banner Admin Login Users</AttributeValue>
        <AttributeValue>corp\Between Community Admin Login Users</AttributeValue>
        <AttributeValue>corp\Spinnaker Login Users</AttributeValue>
      </Attribute>
    </AttributeStatement>
    <AuthnStatement AuthnInstant="2016-07-15T09:35:22.645Z" SessionIndex="_f303f03f-36dd-4cf8-bbc2-266050924bc1">
      <AuthnContext>
        <AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport</AuthnContextClassRef>
      </AuthnContext>
    </AuthnStatement>
  </Assertion>
</samlp:Response>

and its getting error in this part:

    <AttributeStatement>
      <Attribute Name="http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress">
        <AttributeValue>aiden@between.us</AttributeValue>
      </Attribute>
      <Attribute Name="http://schemas.xmlsoap.org/claims/Group">
        <AttributeValue>corp\Domain Users</AttributeValue>
        <AttributeValue>corp\Dev Team</AttributeValue>
        <AttributeValue>corp\Banner Admin Login Users</AttributeValue>
        <AttributeValue>corp\Between Community Admin Login Users</AttributeValue>
        <AttributeValue>corp\Spinnaker Login Users</AttributeValue>
      </Attribute>
    </AttributeStatement>
@chongkong
Copy link
Author

chongkong commented Jul 19, 2016

For convenience, I'll borrow code segment from above link.

assertion.attributeStatements*.attributes.flatten().each { Attribute attribute ->
  def name = attribute.name
  def values = attribute.attributeValues.collect { (it as XSString)?.value }
  attributes[name] = values
}

I found that list of AttributeValue in attribute.attributeValues are instance of XSAnyImpl, and it as XSString provides XSAnyImpl1_groovyProxy. (I'm not familiar with groovy so I don't know what it exactly does, but maybe lazy evaluation of type casting?) The point is, (it as XSString)?.value certainly does provides above error

@chongkong
Copy link
Author

So I think above code should be changed to something like this:

assertion.attributeStatements*.attributes.flatten().each { Attribute attribute ->
  def name = attribute.name
  def values = attribute.attributeValues.collect { (it as XSAny)?.textContent }
  attributes[name] = values
}

This is because I've printed it.class.getSimpleName() and it all gave me XSAnyImpl, but for defensive coding:

assertion.attributeStatements*.attributes.flatten().each { Attribute attribute ->
  def name = attribute.name
  def values = attribute.attributeValues.collect {
    if (it instanceof XSString) {
      return (it as XSString)?.value
    } else if (it instanceof XSAny) {
      return (it as XSAny)?.textContent
    } else {
      return null
    }
  } findAll { it != null }
  attributes[name] = values
}

and both gave me correct XAML behavior.

Which one seems better? May I send a pull request for this?

@ttomsu
Copy link
Member

ttomsu commented Jul 19, 2016

@chongkong - I suggest making a small PR to capture your proposed change.

@ajordens - can you test it out when the PR is ready?

@ajordens
Copy link
Contributor

I'm out of the country right now but happy to verify the proposed changes
against our SAML integration (not ADFS tho) when I get back.
On Tue, Jul 19, 2016 at 12:27 PM Travis Tomsu notifications@github.com
wrote:

@chongkong https://github.com/chongkong - I suggest making a small PR
to capture your proposed change.

@ajordens https://github.com/ajordens - can you test it out when the PR
is ready?


You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#993 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAXuLLUL_8i3GPieVkG7WCkN4MF9FqhZks5qXQkIgaJpZM4JOcSG
.

@ttomsu
Copy link
Member

ttomsu commented Jul 20, 2016

@chongkong - also, consider something like:

def values = attribute.attributeValues.findResults {
    switch(it) {
      case XSString:
        return (it as XSString)?.value
        break
      case XSAny:
        return (it as XSAny)?.textContent
        break
      default: 
        return null
    }
  } ?: []

@chongkong
Copy link
Author

@ttomsu Thanks! I just applied 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

3 participants