Allow authn request without relayState if relayState is empty

[ThePetrov]

Currently it's not possible to have an authn request without a relayState. When a null relayState is provided, then a self routed URL is used instead. If an empty string is provided, then relayState is appended as a query parameter without value. A request without a relayState is a valid case and relayState without value does not work ie in ADFS (it results in an MSIS7000 error).

To mitigate this without changing the API too much I've changed login to treat an empty returnTo (relayState) parameter as no relay state. A null returnTo will still result in a self routed URL. I also added a test case for this scenario and extended the Javadocs to inform about the behavior.

[coveralls]

Coverage increased (+0.002%) to 96.59% when pulling 9a4ad27 on ThePetrov:allow-authn-request-without-relaystate into 3fcda2c on onelogin:v2.0.0.

[pitbulk]

I think is an acceptable behavior, but let me review rest of the toolkits and also review how this affect to the signature process before merge. (also we should extend that for the LogoutRequest).

[ThePetrov]

Alright, thanks for the feedback @pitbulk . I've changed the logout method so that it uses the same behavior.

[coveralls]

Coverage increased (+0.003%) to 96.592% when pulling 560e3e2 on ThePetrov:allow-authn-request-without-relaystate into 3fcda2c on onelogin:v2.0.0.

[coveralls]

Coverage increased (+0.003%) to 96.592% when pulling 84df311 on ThePetrov:allow-authn-request-without-relaystate into 3fcda2c on onelogin:v2.0.0.

[miszobi]

@pitbulk anything left to be done here before merging?

[pitbulk]

No sorry, I was on vacation mode. I will review it again in a couple of hours and merge it.

[miszobi]

No worries, thanks!

[pitbulk]

If we don't add a GET['RelayState'] parameter if relayState var is empty, we should update the buildSignature method, and don't calculate the signature based on the relayState.

currently we only check that is not null

if (relayState != null) {
    msg += "&RelayState=" + Util.urlEncoder(relayState);
}

The specs says:

if there is no RelayState value, the entire parameter should be omitted from the signature
computation (and not included as an empty parameter name), resulting in a string of one
of these

SAMLRequest=value&SigAlg=value
SAMLResponse=value&SigAlg=value

@ThePetrov can you fix that?

[pitbulk]

@ThePetrov any chance to update the PR with my suggestions and rebase?

[ThePetrov]

@pitbulk - sorry for the delay, I updated the signature building and added tests for it

[coveralls]

Coverage increased (+0.003%) to 96.639% when pulling e0eeb43 on ThePetrov:allow-authn-request-without-relaystate into cca5d21 on onelogin:v2.0.0.