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

Custom context in createLoginRequest Callback is Ignored #549

Open
omidraha opened this issue Oct 30, 2024 · 3 comments
Open

Custom context in createLoginRequest Callback is Ignored #549

omidraha opened this issue Oct 30, 2024 · 3 comments

Comments

@omidraha
Copy link

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
Copy link
Author

omidraha commented Nov 7, 2024

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.

samlify/src/libsaml.ts

Lines 142 to 145 in 7a04e81

const defaultLoginRequestTemplate = {
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}" ProtocolBinding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" AssertionConsumerServiceURL="{AssertionConsumerServiceURL}"><saml:Issuer>{Issuer}</saml:Issuer><samlp:NameIDPolicy Format="{NameIDFormat}" AllowCreate="{AllowCreate}"/></samlp:AuthnRequest>',
};
/**

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.

rawSamlRequest = libsaml.replaceTagsByValue(libsaml.defaultLoginRequestTemplate.context, {
ID: id,
Destination: base,
Issuer: metadata.sp.getEntityID(),
IssueInstant: new Date().toISOString(),
NameIDFormat: selectedNameIDFormat,
AssertionConsumerServiceURL: metadata.sp.getAssertionConsumerService(binding.post),
EntityID: metadata.sp.getEntityID(),
AllowCreate: spSetting.allowCreate,
} as any);

@omidraha
Copy link
Author

omidraha commented Nov 7, 2024

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 omidraha closed this as completed Nov 7, 2024
@omidraha
Copy link
Author

omidraha commented Nov 8, 2024

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>'
}

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

1 participant