[BUG] Invalid document signature

[royalcala]

Could you help me to know why I have this error after we updated from passport-saml to @node-saml/passport-saml:

Error: Invalid document signature
at SAML.validatePostResponseAsync (/usr/src/server/node_modules/@node-saml/node-saml/lib/saml.js:510:23)
at runMicrotasks ()
at processTicksAndRejections (node:internal/process/task_queues:96:5)

[cjbarth]

Perhaps your issue is the same as #839 or #816? If not, please post more information about the versions your running and a minimal test case. Ideally we'd like to see a PR with a test that fails.

[catamphetamine]

We also encountered such error and the workaround was adding the following options:

    // After updating from `passport-saml` to `@node-saml/passport-saml`,
    // it started throwing `Error: Invalid document signature`
    // when testing SSO using Acadeum's Google SAML server.
    // Turns out that's a common issue and the workaround is to pass
    // `wantAssertionsSigned: false` / `wantAuthnResponseSigned: false` flags.
    // https://github.com/node-saml/passport-saml/issues/839
    // https://github.com/node-saml/node-saml/blob/master/CHANGELOG.md#v400-2022-10-28
    wantAssertionsSigned: false,
    wantAuthnResponseSigned: false,

[edwandr]

I encountered the same issue and after quite some digging in code this seems to be indeed because in validatePostResponseAsync method, the default value in this.options.wantAssertionsSigned is true if not specified in SamlConfig whereas it was false in v3.

@cjbarth are you sure this is an intended behavior ? I feel wantAssertionsSigned should be false if not specified in config

[cjbarth]

The change was intended as it is in harmony with the secure-by-default philosophy that we have here. Please feel free to put up a PR to help us with our README.

[edwandr]

Ok I understand but then indeed it would be great to specify it in documentation, I can open a PR for that 👍
What would have been really helpful also is to add it as a breaking change in the changelog, I don't know if it's automatically generated or if it's possible to also update it now

[markstos]

I agree this would be useful to highlight as a breaking change in the Changelog.

[edwandr]

I opened a PR to update both doc and changelog: #883