OpenConext / Mujina

A mock IDP and SP using the OpenSAML library
Apache License 2.0
369 stars 166 forks source link

Incompatible ID #23

Closed markmishaev closed 7 years ago

markmishaev commented 7 years ago

When validating SAML response with OneLogin SAML tools, the validation fails because of incorrect IDs format (response and assertion).

https://www.samltool.com/validate_response.php

thijskh commented 7 years ago

Agreed - may not start with digit. Fix is to always prefix with e.g. MU or _.

@oharsta

oharsta commented 7 years ago

https://github.com/OpenConext/Mujina/commit/4931ef02f4d22f4de49a6e972c58ff2e15e4893c

https://build.openconext.org/repository/public/releases/org/openconext/mujina-idp/5.0.6/ https://build.openconext.org/repository/public/releases/org/openconext/mujina-sp/5.0.6/

markmishaev commented 7 years ago

There is better solution: private final IdentifierGenerator idGenerator; this.idGenerator = new SecureRandomIdentifierGenerator(); private String idUnique() { return idGenerator.generateIdentifier(); } Btw, the same should be done with AssertionID: Assertion assertion = buildAssertion(principal, status, entityId); assertion.setID(idUnique());

Guys, please take a look.

oharsta commented 7 years ago

In my commit I covered all SAML ID's including the Assertion. See https://github.com/OpenConext/Mujina/blob/master/mujina-common/src/main/java/mujina/saml/SAMLBuilder.java#L117

markmishaev commented 7 years ago

I see, thanks!