flipboxfactory / saml-sp

SAML Service Provider (SP) Plugin for Craft CMS
https://saml-sp.flipboxfactory.com/
Other
19 stars 5 forks source link

Error assigning groups with ADFS #87

Closed paulbrough closed 3 years ago

paulbrough commented 3 years ago

We're trying to assign groups to users. The IdP is ADFS. Here is a sample assertion:

<samlp:Response ID="_870c3926-7fd6-45f1-bed8-d79d7c557d33" Version="2.0" IssueInstant="2020-10-07T01:02:23.281Z" Destination="https://rcis.platformb.app/sso/login/" Consent="urn:oasis:names:tc:SAML:2.0:consent:unspecified"
  xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol">
  <Issuer xmlns="urn:oasis:names:tc:SAML:2.0:assertion">...</Issuer>
  <samlp:Status>
    <samlp:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success" />
  </samlp:Status>
  <Assertion ID="..." IssueInstant="2020-10-07T01:02:23.266Z" Version="2.0"
    xmlns="urn:oasis:names:tc:SAML:2.0:assertion">
    <Issuer>...</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/2001/04/xmldsig-more#rsa-sha256" />
        <ds:Reference URI="...">
          <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/2001/04/xmlenc#sha256" />
          <ds:DigestValue>...</ds:DigestValue>
        </ds:Reference>
      </ds:SignedInfo>
      <ds:SignatureValue>...</ds:SignatureValue>
      <KeyInfo xmlns="http://www.w3.org/2000/09/xmldsig#">
        <ds:X509Data>
          <ds:X509Certificate>...</ds:X509Certificate>
        </ds:X509Data>
      </KeyInfo>
    </ds:Signature>
    <Subject>
      <NameID>...</NameID>
      <SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer">
        <SubjectConfirmationData NotOnOrAfter="2020-10-07T01:07:23.281Z" Recipient="..." />
      </SubjectConfirmation>
    </Subject>
    <Conditions NotBefore="2020-10-07T01:00:23.203Z" NotOnOrAfter="2020-10-07T01:05:23.203Z">
      <AudienceRestriction>
        <Audience>urn:External:Paulsen</Audience>
      </AudienceRestriction>
    </Conditions>
    <AttributeStatement>
      <Attribute Name="http://schemas.xmlsoap.org/ws/2005/05/identity/claims/givenname">
        <AttributeValue>FirstName</AttributeValue>
      </Attribute>
      <Attribute Name="http://schemas.xmlsoap.org/ws/2005/05/identity/claims/surname">
        <AttributeValue>LastName</AttributeValue>
      </Attribute>
      <Attribute Name="http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress">
        <AttributeValue>foo@bar.com</AttributeValue>
      </Attribute>
      <Attribute Name="http://schemas.xmlsoap.org/claims/Group">
        <AttributeValue>siteAdmins</AttributeValue>
      </Attribute>
    </AttributeStatement>
    <AuthnStatement AuthnInstant="2020-10-07T01:02:23.094Z" SessionIndex="...">
      <AuthnContext>
        <AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport</AuthnContextClassRef>
      </AuthnContext>
    </AuthnStatement>
  </Assertion>
</samlp:Response>

The saml-sp.php config file looks like this:

return [
    'groupAttributeNames' => 'http://schemas.xmlsoap.org/claims/Group'
];

We don't have access to the client's system, but here's a screenshot of the error he was getting:

SAML-error

Any thoughts on what would cause this error? They don't have a lot of control over the format of the assertion, so we kind of have to work with that format. They can change attribute names, but that's about it.

dsmrt commented 3 years ago

👋 @paulbrough !

Thanks for using the plugin!

Looks like this might be an easy one (🤞 ). Make sure that groupAttributeNames is an array.

return [
    'groupAttributeNames' => [ 'http://schemas.xmlsoap.org/claims/Group' ],
];
paulbrough commented 3 years ago

Thanks, that fixed it! I submitted a PR to update the docs: #88

dsmrt commented 3 years ago

Good catch! Sorry about the confusion in the docs! I merged in the pull request, thank you for that.