Add OIDC SecurityEvent

[sberyozkin]

Fixes #12145

This PR adds a listener which can capture some important login/session/logout events based on the conversation with the user. Login (i.e creating a new session after a successful code flow) is of primary interest, but other events (4 of them for now, more can be added easily) will let users have a better view into what is going on.
Note I'm passing a blocking API ref as a context attribute (in case some expensive action will need to be done), @stuartwdouglas, if you prefer, I can have a dedicated parameter.

Also updated the RP-initiated doc section as the user could not make it work (was not obvious the logout path had to be authenticated)

[gsmet]

Doesn't look good:

2020-09-18T19:22:29.1068668Z [INFO] Running io.quarkus.it.keycloak.ServicePublicKeyTestCase
2020-09-18T19:22:29.1605578Z 2020-09-18 19:22:29,142 ERROR [io.qua.ver.htt.run.QuarkusErrorHandler] (vert.x-eventloop-thread-1) HTTP Request to /service/tenant-public-key failed, error id: 8f88f851-e290-4fab-88f8-3f99055a9464-1: java.lang.NullPointerException
2020-09-18T19:22:29.1608501Z 	at io.quarkus.oidc.runtime.OidcUtils.setBlockinApiAttribute(OidcUtils.java:195)
2020-09-18T19:22:29.1610843Z 	at io.quarkus.oidc.runtime.OidcUtils.validateAndCreateIdentity(OidcUtils.java:187)
2020-09-18T19:22:29.1613983Z 	at io.quarkus.oidc.runtime.OidcIdentityProvider.validateTokenWithoutOidcServer(OidcIdentityProvider.java:247)
2020-09-18T19:22:29.1617138Z 	at io.quarkus.oidc.runtime.OidcIdentityProvider.authenticate(OidcIdentityProvider.java:73)
2020-09-18T19:22:29.1619530Z 	at io.quarkus.oidc.runtime.OidcIdentityProvider.access$000(OidcIdentityProvider.java:35)
2020-09-18T19:22:29.1621595Z 	at io.quarkus.oidc.runtime.OidcIdentityProvider$1.get(OidcIdentityProvider.java:62)
2020-09-18T19:22:29.1623503Z 	at io.quarkus.oidc.runtime.OidcIdentityProvider$1.get(OidcIdentityProvider.java:50)
2020-09-18T19:22:29.1627452Z 	at io.smallrye.mutiny.operators.UniCreateFromDeferredSupplier.subscribing(UniCreateFromDeferredSupplier.java:24)
2020-09-18T19:22:29.1630766Z 	at io.smallrye.mutiny.operators.UniSerializedSubscriber.subscribe(UniSerializedSubscriber.java:43)
2020-09-18T19:22:29.1633623Z 	at io.smallrye.mutiny.operators.UniSerializedSubscriber.subscribe(UniSerializedSubscriber.java:38)
2020-09-18T19:22:29.1635976Z 	at io.smallrye.mutiny.operators.AbstractUni.subscribe(AbstractUni.java:30)
2020-09-18T19:22:29.1637983Z 	at io.smallrye.mutiny.operators.UniCache.lambda$subscribing$0(UniCache.java:37)
2020-09-18T19:22:29.1639704Z 	at io.smallrye.mutiny.operators.UniCache.subscribing(UniCache.java:65)
2020-09-18T19:22:29.1641993Z 	at io.smallrye.mutiny.operators.UniSerializedSubscriber.subscribe(UniSerializedSubscriber.java:43)
2020-09-18T19:22:29.1644833Z 	at io.smallrye.mutiny.operators.UniSerializedSubscriber.subscribe(UniSerializedSubscriber.java:38)
2020-09-18T19:22:29.1647261Z 	at io.smallrye.mutiny.groups.UniSubscribe.withSubscriber(UniSubscribe.java:49)
2020-09-18T19:22:29.1649643Z 	at io.quarkus.vertx.http.runtime.security.HttpSecurityRecorder$2.handle(HttpSecurityRecorder.java:100)
2020-09-18T19:22:29.1660839Z 	at io.quarkus.vertx.http.runtime.security.HttpSecurityRecorder$2.handle(HttpSecurityRecorder.java:49)
2020-09-18T19:22:29.1663608Z 	at io.vertx.ext.web.impl.RouteState.handleContext(RouteState.java:1034)

[sberyozkin]

Oops. I forgot to check oidc-tenancy tests, a simple fix will follow, thanks @gsmet

[pedroigor]

@sberyozkin I'm wondering if security events support shouldn't rather be provided by the security layer and extended with additional events specific for OIDC ?

[sberyozkin]

Hi @pedroigor
It sounds good in principle, but I'm not sure what kind of events can be generated at the base security layer, things like authentication or authorization failure are reported via the exceptions, so may be AUTH_SUCCESSFUL, AUTHORIZATION_SUCCESSFUL - though again it is implicitly confirmed by having a JAX-RS method invoked for example, otherwise the frequency of these events will be high.
With the OIDC the first login is done and then it will be quiet. It is OIDC specific, true, but Code flow has several event types; other mechanisms (with the exception of Form may be) will only have AUTH_SUCCESSFUL or AUTHORIZATION_SUCCESSFUL only.

That said I don't really mind creating some base SecurityEventListener, may be it is a better way to go, @stuartwdouglas hi Stuart, are you OK with this idea, I don't want to start creating a PR if you won't agree. If yes - this listener will be in quarkus-security, correct ? The other thing is that its Event enum would have to contain authentication-mechanism specific values...

[pedroigor]

@sberyozkin I see. What about CDI Event System? Can we leverage that instead of creating our own?

[sberyozkin]

Hi @pedroigor interesting, let me ask @mkouba, hi Martin, how can one, in the Quarkus extension code, send an event via Arc ?

[stuartwdouglas]

This should be done using Arc. You send them the same way you do normally (i.e. javax.enterprise.inject.spi.BeanManager#fireEvent)

[mkouba]

@sberyozkin @pedroigor Stuart is right, however keep in mind that you can't fire the CDI events until the container is started, i.e. you can't fire events during the build phase and you can fire events during STATIC_INIT bootstrap phase assuming you make sure the container is started, e.g. depend on the BeanContainerBuildItem in your build step.

For more information about CDI events in general see https://quarkus.io/guides/cdi#events-and-observers and https://docs.jboss.org/weld/reference/latest/en-US/html_single/#events.

[sberyozkin]

Hi All, @pedroigor @mkouba @stuartwdouglas

It works very well, thanks, I've never done CDI events before so missed this neat option completely :-), have a look at the update PR please

One question I have here, is, I think it is worth introducing a property like quarkus.oidc.send-events and ask Arc to fire an event only if this property is set, otherwise, in many cases, with no observer code being provided, it will add to the request cost, a bit. Do you agree ?

And I suppose a bigger question, Stuart, Pedro, do we want to move this SecurityEvent to quarkus-security So that, going forward, we can add more authentication mechanism specific event types ? It probably makes sense...
thanks

[stuartwdouglas]

@mkouba is there an easy way to figure out if an event will have any observers? This should not require a config property but skipping if there are no observers makes sense IMHO.

[mkouba]

@sberyozkin @stuartwdouglas So first of all if @Inject Event is used we cache the observers for the last event payload fired and Event.fire() is more or less a no-op if no observers are declared (one volatile read in case of cache hit). Furhtermore, it's possible to obtain a set of observers for a given payload instance via BeanManager.resolveObserverMethods(T, Annotation...). At build time you can access a collection of all observers in a BeanDeploymentValidator or via ValidationPhaseBuildItem.getContext().get(io.quarkus.arc.processor.BuildExtension.Key.OBSERVERS).

[sberyozkin]

Hi @mkouba @stuartwdouglas @pedroigor

Have a look please at how the observers are checked now. I think it is not bad but can optimize as needed, I had to create a separate build step to avoid the chain issues.

I'd like to keep this observer checking code in the OIDC extension for now as we have some concrete requirements about the events there, but we can push it down to the common security code easily as needed, potentially the security events can give a very fine grained view into what is going on within various Quarkus Security flows :-), but integrating it into the rest of Quarkus Security would be a bigger issue which can be done iteratively...

The bigger question IMHO is where to keep SecurityEvent. We can keep it in quarkus-oidc for now and then migrate the users to a new package if it gets moved. Or move to quarkus-security immediately. And then keep growing its types.

What do you think ? Thanks

[mkouba]

Looks good now.

[sberyozkin]

OK, so for now this will be an OIDC-centric SecurityEvent but we will migrate to a more common package if ever needed