Prevent possible NPE in OIDC BackChannelLogoutHandler #29037
Conversation
| @@ -59,6 +59,9 @@ public void handle(RoutingContext context) { | |||
| LOG.debugf( | |||
| "Tenant configuration for the tenant %s is not available or does not match the backchannel logout path", | |||
| oidcTenantConfig.getTenantId().get()); | |||
| context.response().setStatusCode(400); | |||
There was a problem hiding this comment.
I wish we could include the reason in the response. Returning a client error without any reason will make it harder to debug situations where this happens
There was a problem hiding this comment.
@gastaldi I see, but note this one will be returned to Keycloak directly, not to Quarkus user; besides this is a case where the fact Quarkus can't find the matching configuration is probably not something we'd like to say to the outside world, logging it for the users to trace seems reasonable. Perhaps we can make this one an ERROR-level message though, what do you think ?
There was a problem hiding this comment.
Not sure if that's a good idea as clients could easily fill the logs with this information because the request is invalid
There was a problem hiding this comment.
@gastaldi I changed the log level to error, hope it is better
|
@sberyozkin no 2.13 backport, it's a new feature in 2.14? |
|
Sorry, forgot 2.14 has been branched :-) |
sberyozkin
force-pushed
the
back_channel_logout_npe
branch
from
9a01340 to
0ff7788
Compare
This comment has been minimized.
This comment has been minimized.
|
@gastaldi I've checked the spec, they only advise |
sberyozkin
force-pushed
the
back_channel_logout_npe
branch
from
0ff7788 to
7ed539c
Compare
As discovered by @pjgg, OIDC
BackChannelLogoutHandlercan throw NPE if no tenant configuration matching the back channel logout request has been found.BackChannelLogoutHandlerchecks if the tenant context is null and logs a message but currently forgets end the request, leading to a later NPE where this context is accessed.OIDC back channel logout spec requires
400be returned if the logout request is invalid or has failed for whatever reasons, so 400 is returned in this case, https://openid.net/specs/openid-connect-backchannel-1_0.html#BCResponse