CAS 6.5.0: Error 500 instead of 403 Forbidden when calling REST service for issuing ST

[Petr Bodnár]

Hi,

I originally posted this to https://groups.google.com/a/apereo.org/g/cas-user/c/sQeZD0NV4jw, but apparently quite a different problem was reported there...

So, I discovered an error present since CAS 6.5.0, which happens when calling REST service "/cas/v1/tickets/{TGT}" and passing it "service" which is not matched against any service registered in a given CAS instance.

Expected behavior: error " 403 Forbidden" is returned - analogical to when an authenticated user tries to access "/cas/login?service=<not-registered-service>" (in that case, this problem is caught early on - in the

org.apereo.cas.services.web.support.RegisteredServiceResponseHeadersEnforcementFilter.prepareFilterBeforeExecution() - apparently a filter which is not used for REST services).

Actual behavior: error "500 Internal Server Error" is returned, caused by NullPointerException from ensureServiceSsoAccessIsAllowed(), called from  grantServiceTicket().

This problem seems to be caused by this commit, where inside grantServiceTicket() a call to enforceRegisteredServiceAccess() (basically includes checking "registeredService" is found) was replaced by a "too late" call to an overloaded variant of this method.

Can CAS authors please confirm and suggest a solution for this? By quickly analyzing the code, I guess it could be solved either by adding a null check into ensureServiceSsoAccessIsAllowed(), or by returning the original call to enforceRegisteredServiceAccess(), but I'm not sure without a deeper investigation of the CAS code. (For example, I would expect that grantServiceTicket() will be structurally similar to grantProxyTicket(), but it isn't and I have no clue why.)

(I also wonder, possibly a topic for another thread, provided that I read it correctly, whether it is really correct that enforceRegisteredServiceAccess() can currently NOT throw an exception even if passing it registeredService equal to null (see checks including calling a Groovy script here). In previous versions, there was a simple null check and only then all other checks, which makes more sense to me, because follow-up code, like inside grantServiceTicket(), seems to expect registeredService is not null anyway.)

[Petr Bodnár]

A small correction: the aforementioned RegisteredServiceResponseHeadersEnforcementFilter seems to be used even for REST, yet it doesn't stop further processing. So it needs to be something "down the chain" which blocks calling of the grantServiceTicket() method within the login web flow.

Anyway, to fix this REST issue, we simply added a call to RegisteredServiceAccessStrategyUtils.ensureServiceAccessIsAllowed(service, registeredService). Not sure if it is ideal, but it does its job of correctly throwing the appropriate exception and not NPE...