tngan / samlify

Node.js library for SAML SSO
https://samlify.js.org
MIT License
611 stars 217 forks source link

Custom context in createLoginRequest Callback is Ignored #549

Open omidraha opened 3 weeks ago

omidraha commented 3 weeks ago

I've encountered an issue with the createLoginRequest method in samlify. When using a custom callback for modifying the SAML request template, the context returned by the callback appears to be ignored. Instead, the context used by samlify is automatically generated, and any custom values specified in the callback seem to be overridden. Here’s a simplified version of my code:

async generateSAMLRequest(
    idp: IdentityProviderInstance,
    sp: ServiceProviderInstance,
    binding: string,
  ): Promise<{ id: string; context: string }> {
    const { id, context } = await sp.createLoginRequest(
      idp,
      binding,
      (template) => {
        return {
          id,
          context: 'NEVER USED', // This value is ignored
        };
      },
    );
    console.log('context:', context);
    return { id: id, context: context };
  }
}

In the above example, even though the callback specifies context: 'NEVER USED', the actual context value logged in the console comes from the samlify library's internal handling, not the callback’s return value.

Expected Behavior: The context provided by the callback should be used as the final output when returned, instead of being replaced by an internally generated context.

Actual Behavior: The context set within the callback is overridden by samlify’s internally generated SAML request.

Is there any suggested approach to ensure the custom context value from the callback is respected?

My goal is to replace ProtocolBinding in the SAML request with the appropriate value, either urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect or urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST. However, it always defaults to urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST.

Here’s a simplified version of the code I’m using:

async generateSAMLRequest(
    idp: IdentityProviderInstance,
    sp: ServiceProviderInstance,
    binding: string,
  ): Promise<{ id: string; context: string }> {
    const { id, context } = await sp.createLoginRequest(
      idp,
      binding,
      (template) => {
        // Set ProtocolBinding based on binding type
        const protocolBindingValue =
          binding === 'redirect'
            ? 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect'
            : 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST';

        const updatedTemplate = template.replace(
          `/ProtocolBinding="urn:oasis:names:tc:SAML:2.0:bindings:[^"]+"/`,
          `ProtocolBinding="${protocolBindingValue}"`,
        );

        console.log('Updated Template:', updatedTemplate);

        return {
          id,
          context: updatedTemplate,
        };
      },
    );
    console.log('context:', context);
    return { id: id, context: context };
  }
}

Observed Output

Despite modifying the ProtocolBinding in the callback, the generated SAML request always includes ProtocolBinding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST", even when binding is set to redirect. Here’s an example of the

<samlp:AuthnRequest xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" ID="_5f3a43d9-815d-4e8d-ae4d-9e22d7aa4dde" 
Version="2.0" IssueInstant="2024-10-30T20:10:10.440Z" 
Destination="http://localhost:3000/saml/idp/authenticate/uid/" 
ProtocolBinding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" 
AssertionConsumerServiceURL="http://localhost:3000/saml/idp/authenticate/uid/">
  <saml:Issuer>SPExampleEntityID</saml:Issuer>
  <samlp:NameIDPolicy Format="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress" AllowCreate="" />
</samlp:AuthnRequest>
omidraha commented 2 weeks ago

The default behavior is due to the default login request template set in the library (libsaml in libsaml.ts). Specifically, the defaultLoginRequestTemplate in this file has ProtocolBinding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" as the default protocol.

https://github.com/tngan/samlify/blob/7a04e81811dd5c869ced51329639e4be414c5e63/src/libsaml.ts#L142-L145

In this case, similar to other fields that are dynamically created, the ProtocolBinding field is not assigned a specific value and always remains set to the default value, which is POST.

https://github.com/tngan/samlify/blob/7a04e81811dd5c869ced51329639e4be414c5e63/src/binding-redirect.ts#L103-L112

omidraha commented 2 weeks ago

About sp.createLoginRequest with custom template question:

This filed should be define in ServiceProviderSettings:

      loginRequestTemplate: {
        context: `<samlp:AuthnRequest xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" \
                                xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" \
                                ID="{ID}" \
                                Version="2.0" \
                                IssueInstant="{IssueInstant}" \
                                Destination="{Destination}" \
                                AssertionConsumerServiceURL="{AssertionConsumerServiceURL}"> \
              <saml:Issuer>{Issuer}</saml:Issuer> \
              <samlp:NameIDPolicy Format="{NameIDFormat}" AllowCreate="{AllowCreate}" /> \
            </samlp:AuthnRequest>`,
      },

About ProtocolBinding questing:

ProtocolBinding:

The ProtocolBinding attribute on AuthnRequest is used to specify the expected binding to be used by the IdP when sending their SAML Response XML. HTTP-Redirect isn't a valid option to use here, because of the possible length restriction on the URL querystring; a SAML Response,especially if it's signed, can be pretty lengthy.

https://docs.oasis-open.org/security/saml/v2.0/saml-profiles-2.0-os.pdf

  1. Identity Provider issues to Service Provider In step 5, the identity provider issues a message to be delivered by the user agent to the service provider. Either the HTTP POST, or HTTP Artifact binding can be used to transfer the message to the service provider through the user agent. The message may indicate an error, or will include (at least) an authentication assertion. The HTTP Redirect binding MUST NOT be used, as the response will typically exceed the URL length permitted by most user agents.
omidraha commented 2 weeks ago

Considering that the loginRequestTemplate type is SAMLDocumentTemplate and should be assigned as follows:

// Custom loginRequestTemplate
loginRequestTemplate: {
        context: `<samlp:AuthnRequest xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" \
                                xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" \
                                ID="{ID}" \
                                Version="2.0" \
                                IssueInstant="{IssueInstant}" \
                                Destination="{Destination}" \
                                AssertionConsumerServiceURL="{AssertionConsumerServiceURL}"> \
              <saml:Issuer>{Issuer}</saml:Issuer> \
              <samlp:NameIDPolicy Format="{NameIDFormat}" AllowCreate="{AllowCreate}" /> \
            </samlp:AuthnRequest>`,
      },
    };

But why the type of template is defined as a string?

createLoginRequest(idp: IdentityProvider, binding?: string, customTagReplacement?: (template: string) => BindingContext): BindingContext | PostBindingContext | SimpleSignBindingContext;

When printing its type and value in the customTagReplacement method:

  async generateSAMLRequest(
    idp: IdentityProviderInstance,
    sp: ServiceProviderInstance,
    binding: string
  ): Promise<{ id: string; context: string }> {

    const customTagReplacement = (template: string): BindingContext => {
      console.log(
        typeof template,
        template
      );
      return {
        id: sp.entitySetting.generateID(),
        // should be template.context
        context: template,
      };
    };
    const { id, context } = sp.createLoginRequest(
      idp,
      binding,
      customTagReplacement
    );
    return { id, context };
  }
  );

It appears as an object and is displayed as follows:

{
  context: '<samlp:AuthnRequest xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol"                                 xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"                                 ID="{ID}"                                 Version="2.0"                                 IssueInstant="{IssueInstant}"                                 Destination="{Destination}"                                 AssertionConsumerServiceURL="{AssertionConsumerServiceURL}">               <saml:Issuer>{Issuer}</saml:Issuer>               <samlp:NameIDPolicy Format="{NameIDFormat}" AllowCreate="{AllowCreate}" />             </samlp:AuthnRequest>'
}