mbg / wai-saml2 Goto Github PK

It seems from this line that this package only supports SAML responses with encrypted assertions:

This is further indicated by the fact that saml2PrivateKey is required in SAML2Config:

We have SAML responses with assertions that aren't encrypted, is it possible to support this use-case?

Stackage Nightly is using zlib-0.7, so wai-saml2 can't be used with the other packages. It would be nice if wai-saml2 accepted zlib-0.7.

I find it slightly confusing because it's called NameID in specification. Probably now is the best opportunity given that the subjectNameId field is inaccessible in the latest Hackage release.

There have been quite a few changes since the last release (thanks @fumieval and @Philonous!) and it would be good to get a new version published to Hackage. There are a good number of breaking changes, so it will be a major version release (0.4). There are three open PRs currently which would be nice to include in this release:

• #33
• #35
• #37
• #42

Once they are finished and merged, I will prepare a new release.

As you can see on version history, GHC 9.10 has containers-0.7. However , this version is excluded by wai-saml2.cabal. It would nice to accept this version. That might unblock using wai-saml2 on GHC 9.10.

To check the validity of an Assertion, we should also check AudienceRestrictions. (This doesn't seem to happen at the moment)

To quote [1] (lines 922 - 925)

Note that multiple elements MAY be included in a single assertion, and each
MUST be evaluated independently. The effect of this requirement and the preceding definition is that
within a given condition, the audiences form a disjunction (an "OR") while multiple conditions form a
conjunction (an "AND").

As I understand it, this means:

• The Conditions can contain multiple (zero or more) <AudienceRestriction> elements
• Each AudienceRestriction can include multiple (one or more) Audience elements
• To validate an AudienceRestriction, we have to accept one of the Audiences in it (OR), but all of the AudienceRestrictions have be validated (AND)

I'll work on a PR

References:

• [1] https://docs.oasis-open.org/security/saml/v2.0/saml-core-2.0-os.pdf#page=23

This broke Stackage Nightly, so allow the testsuite to fail for now.

Test suite failure for package wai-saml2-0.2.1.1
    wai-saml2-tests:  exited with: ExitFailure 1
Full log available at /var/stackage/work/unpack-dir/.stack-work/logs/wai-saml2-0.2.1.1-test.log

    wai-saml2-tests: test-data/sp.pem: openBinaryFile: does not exist (No such file or directory)

edit: I see the testsuite was just added

Stackage runs your package's tests from the hackage tarball, not from github source. We encountered the following error running the wai-saml2-0.3.0.0 test suite:

      keycloak: FAIL
        Error parsing XML file tests/data/keycloak.xml: tests/data/keycloak.xml: openBinaryFile: does not exist (No such file or directory)
        Use -p '/keycloak/' to rerun this test only.
      okta:     FAIL
        Error parsing XML file tests/data/okta.xml: tests/data/okta.xml: openBinaryFile: does not exist (No such file or directory)
        Use -p '/okta/' to rerun this test only.

I believe this can be resolved by adding the test xml files to extra-source-files

Compare content of this repository with https://hackage.haskell.org/package/wai-saml2-0.4/src/.

validateResponse checks that the response status is Success, however, there is only one status code defined: Success, so the check is a no-op.

Presence of any other status code is handled as a parse failure instead of a validation failure.

While this isn't directly a problem, I think it would be more consistent to parse status codes and let the validation check for success.

• Reference to StatusCode element with well-known status codes: https://docs.oasis-open.org/security/saml/v2.0/saml-core-2.0-os.pdf#page=39