Allow missing "active" field in check_token/introspect response.

[fcrespel]

This PR amends the changes made to RemoteTokenServices in #838 to allow a missing "active" field in the check_token (introspect) response, in order to restore backward compatibility.

Indeed, prior to #1070, the check_token endpoint did NOT return the "active" field, so any server using an older version of spring-security-oauth (or any third-party server not complying with RFC 7662) no longer works when used in conjuction with a more recent client, that expects the "active" field to be always present.

This is exactly what happened to one of my projects when we updated spring-security-oauth from a 2.0.x to a 2.2.x version. We saw a "check_token returned active attribute: null" message in the logs, followed by an InvalidTokenException. So IMHO, backward compatibility should be respected and only enforce the "active"=true check if the attribute is actually there.

Additionally, this PR also adds support for both Boolean and String values for this field (related to #1440 but with a different implementation).

[pivotal-issuemaster]

@fcrespel Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

[pivotal-issuemaster]

@fcrespel Thank you for signing the Contributor License Agreement!

[diegofurtados]

if Boolean.FALSE.equals(map.get("active")) thrown ....

[fcrespel]

No, because "active" could also be null or a String instead of a false boolean...

[diegofurtados]

and you just want to throw the exception if it's false, no?

[fcrespel]

I guess so, but taking String values into account. So would you prefer the following?

Suggested change

	if (map.containsKey("active") && !"true".equals(String.valueOf(map.get("active")))) {
	if (map.containsKey("active") && "false".equals(String.valueOf(map.get("active")))) {

[diegofurtados]

I'm not sure. I'd keep it simple. We're facing an issue with it here.
To me feels like you wanna to throw an exception just if it was explicitly disabled. If you have something else there it should be fine.

[fcrespel]

The only difference with this change, is that a null active value (or any arbitrary value other than false) would be considered as a valid token... which may be a bit risky? Checking if it's true is more restrictive and looks safer to me.

[jgrandja]

Thanks for the PR @fcrespel ! This is now in master.
And apologies for the changes introduced in #838 as this indeed broke compatibility.
FYI, I'm planning on a release next week.

[amblume]

Can you consider an option on the OAuth API server side to suppress this "active" field? Or some kind of extension point where I can suppress it, optionally?
(With the previous code change, I can't figure out any kind of extension point to do so.)

We've unfortunately built up a huge amount of clients validate our access tokens and they have brittle (i.e default Jackson) JSON parsing when they validate OAuth access tokens using our OAuth APIs, and they all broke once I upgraded our OAuth API stack from version 2.1.0.RELEASE to version 2.1.1.RELEASE or newer.
So effectively, we're now stuck on version 2.1.0.RELEASE now, or until I come up with an ugly solution to strip that attribute out of all responses using a servlet filter or something.

Thanks,
-Alex

[fcrespel]

Can you consider an option on the OAuth API server side to suppress this "active" field? Or some kind of extension point where I can suppress it, optionally?

@amblume please see #1591 (backported to 2.1.4.RELEASE), which seems to introduce a way to do it. Nevertheless, I'd recommend that you begin updating your clients to allow unknown fields when parsing the check_token response, for better compatibility with any OAuth authorization server.