Bugfix/43 Apple revoke use-case

[RasDeaks]

Hello!
I've tried to implements the revocation use-case, it contains :

• Http client to call apple /revoke endpoint
• Renarde controller for /apple-revoke
• An integration test

See FroMage/quarkus-renarde-todo#7 for usage.

Let me know if this helps, I'd be glad to contribute. I'm open for reveiw, I can update my work if needed.

[FroMage]

Thanks for the contrib, let's wait for @sberyozkin 's answer on Zulip before we figure out where to take this.

[FroMage]

I assume this should also make sure that the q_session_apple cookie is cleared as well.

[RasDeaks]

That's right, it's an oversight. I just updated the test to check that. I did the same on the 'Todos' demo.

[FroMage]

I feel like this is the sort of thing Quarkus OIDC should be able to give us, since it has all that config.

[FroMage]

Perhaps we also need to revoke the refresh token? The docs do mention it.

[RasDeaks]

I thought we had choice, using the access token OR the refresh token. I might be wrong. Do you think I should make a double call to this endpoint, one for each ? Starting with the refresh_token ?

[FroMage]

I am not sure, TBH. But it looks like the docs mention having to revoke both, no? In theory, the way it works, the refresh token (if there is one) is longer-lived than the access token. But if you're logged in, both must be valid.

[RasDeaks]

I'll read again and make some test, if needed, I'll add this second call.

[RasDeaks]

Since the refresh token is supposed to be longer-lived than the access token it must be invalidated as well. I just added that part.
I also added a check for the tenant id on the back-end side.

[FroMage]

I'm pretty sure this is where we should fetch it from. In fact, for native compiled or packaged production applications, this folder will not exist. This will only work in DEV mode, I think.

[RasDeaks]

I've tried using quarkus-run.jarand it was ok but you must be right. Unfortunatelly I didn't find a better solution. I guess quarkus-oidc lib got something to help on that as it already read this file. WDYT ?

[sberyozkin]

@RasDeaks It is a nicely done PR, but indeed, hopefully we can get Quarkus do it for Renarde, as @FroMage also suggests.

At the moment, the option which will work is to configure OIDC client, for example:

quarkus.oidc-client.apple.auth-server-url=${quarkus.oidc.apple.auth-server-url}
#etc for other properties which are injected in this PR in the revocation controller 

and then in the controller:

@NamedOidcClient("apple")
@Inject
OidcClient appleClient;

@Inject AccessTokenCredential at;

// and then
appleClient.revokeAccessToken(at.getToken());

The above should handle it, the revocation endpoint is expected to be discovered because Apple supports it and therefore it should be included in the discovery doc.

But I was thinking that may be quarkus-oidc could help it as well, without having to bring quarkus-oidc-client.
quarkus-oidc already allows to inject OidcSession, which one can use to do the local logout and clear the session cookie:

@Inject OidcSession session;

@GET
void logout() {
    session.logout();
}

So may be we can have something like:

@Inject OidcSession session;

@GET
void revokeAccessAndLogout() {
   // revoke the access token and then clear the local session cookie
    session.revokeAccessAndLogout();
}

How about that ? That would be the simplest option for sure.

Or if you prefer to handle the revocation and the local session cookie deletion separately, we can do:

@Inject OidcSession session;

@GET
void revokeAccessAndLogout() {
    session.revokeAccess();
    session.logout();
}

[RasDeaks]

Thanks for your help @sberyozkin ,
If I understand correctly, you're making a proposal to add this revoke call implementation in a future version of quarkus, right ?
It would be great, I think I prefer your second proposal. Let me know if I can help to implement this call on quarkus-oidc side. At least I'll follow the progress.

[sberyozkin]

Thanks @RasDeaks for the feedback on Zulip. Indeed, having a standalone OidcSession.revokeAccess() seems best as one may want to treat the logout in a separate action. It should be of general help not only to Apple users.

If it can be of interest, then please open a Quarkus enhancement request and work on the PR (OidcSessionImpl has a resolver injected which you can use to get the current OIDC tenant context and get to OidcProvider which uses OidcProviderClient - both should have methods to support the revocation, I guess they will return Uni<Void>, and OidcConfigurationMetadata should be updated to keep the revocation endpoint address which one can get from the discovered metadata)

[sberyozkin]

@RasDeaks I was just typing my response and noticed your comment. It would be great if you could look at the Quarkus enhancement, it should be an interesting PR to look at, we can help you with Steph along the way