Stateful OIDC token management

[Sergey Beryozkin]

HI,

Michal Vavrik has opened the following PR:

https://github.com/quarkusio/quarkus/pull/36175

Let me describe the current situation first.

Right now, Quarkus OIDC is completely stateless, it keeps all the tokens it acquires during the authorization code flow in the session cookie, which is now also encrypted by default.

This aligns well with the cloud native and the general web scalability strategies.

It has some, arguably, minor costs:

* cookie encryption, done by default, has its cost, and unless users manage their own encryption keys, they may get warnings of the weak encryption if the fallback secret keys used to to encrypt the cookie such as client id are less than 16 characters

* In some rare cases, when MTLS or client_jwt_private authentication is used between Quarkus and OIDC provider like Keycloak, and users have not configured the encryption key, Quarkus will generate the one by default - which might pose session decryption problems in a multi-pod endpoint

* cookie size can be up to 4K as 3 tokens can be included, ID, access and refresh tokens and augmented with the encryption metadata - if the cookie size is larger than 4K, users will have to configure Quarkus to split the session cookie

There are all easily manageable problems when they arise though.

Another problem may be a bit more serious. If we have new users migrating to Quarkus who are used to the stateful session management done in their applications, the fact the tokens are stored in cookies, even in the encrypted form, might pose one too many challenges.

The fact is not all Quarkus users want to run their applications in Kubernetes, they may want just to run simple JVM endpoints with a limited number of users. They may not really care about having the encrypted tokens stored by the browser.

As it happens, in Quarkus OIDC, it has been possible to customize the way the tokens are stored for a long while. Default `TokenStateManager` interface implementation sores the tokens in the session cookie, but users can simply register custom `TokenStateManager` bean, store the tokens in sone cache, DB, encrypted FS, and return some pointer to that storage as a cookie value.

What Michal did is about letting such users do it by merely updating the project dependencies and Quarkus will manage such tokens in the DB if it is what they want.

We will continue recommending the stateless strategy but IMHO we should make it easy for users who don't like this idea to follow the stateful one without having to write any code of their own.

Please comment at https://github.com/quarkusio/quarkus/pull/36175

Cheers Sergey

[Sergey Beryozkin]

On Wed, Sep 27, 2023 at 6:46 PM Sergey Beryozkin <sbia...@redhat.com> wrote:

HI,

Michal Vavrik has opened the following PR:

https://github.com/quarkusio/quarkus/pull/36175

Let me describe the current situation first.

Right now, Quarkus OIDC is completely stateless, it keeps all the tokens it acquires during the authorization code flow in the session cookie, which is now also encrypted by default.

This aligns well with the cloud native and the general web scalability strategies.

It has some, arguably, minor costs:

* cookie encryption, done by default, has its cost, and unless users manage their own encryption keys, they may get warnings of the weak encryption if the fallback secret keys used to to encrypt the cookie such as client id

such as the `client secret` that is...

[Ladislav Thon]

Hi,

I've been meaning to reply on this thread for a few weeks, but I keep getting distracted...

Anyway, I've been meaning to articulate one thing that I find incredibly important: SESSIONS ARE FINE. Let's use sessions. To that end, I've been working on Quarkus support for Vert.x Web sessions, which I believe is now ready for review: https://github.com/quarkusio/quarkus/pull/36310 In that PR, sessions can be stored in memory, in Redis, or in Infinispan. I intentionally don't support storing session data in a cookie, because 1. I think it's a bad idea in general, 2. the Vert.x Web session cookie implementation has limitations (the session cookie is signed but not encrypted, so not a good idea for sensitive data, and it is just a single cookie and the data store to it are Base64 encoded, so there's a limit of roughly 2.5 KB of data).

This could be, I believe, very easily adapted to storing OIDC tokens. In fact, considering the SPI is fairly simple and provides direct access to the Vert.x Web `RoutingContext`, I think a session-based implementation of the `TokenStateManager` would be this simple:

public class WebSessionTokenStateManager implements TokenStateManager {

    private static final String ID_TOKEN_PREFIX = "idToken.";
    private static final String ACCESS_TOKEN_PREFIX = "accessToken.";
    private static final String REFRESH_TOKEN_PREFIX = "refreshToken.";

    @Override
    public Uni<String> createTokenState(RoutingContext routingContext, OidcTenantConfig oidcConfig,
            AuthorizationCodeTokens tokens, OidcRequestContext<String> requestContext) {
        Session session = routingContext.session();
        if (session == null) {
            return noSession();
        }

        String id = UUID.randomUUID().toString(); // to avoid predictable keys, not terribly important
        session.put(ID_TOKEN_PREFIX + id, tokens.getIdToken());
        session.put(ACCESS_TOKEN_PREFIX + id, tokens.getAccessToken());
        session.put(REFRESH_TOKEN_PREFIX + id, tokens.getRefreshToken());
        return Uni.createFrom().item(id);
    }

    @Override
    public Uni<AuthorizationCodeTokens> getTokens(RoutingContext routingContext, OidcTenantConfig oidcConfig, String tokenState, OidcRequestContext<AuthorizationCodeTokens> requestContext) {
        Session session = routingContext.session();
        if (session == null) {
            return noSession();
        }

        String idToken = session.get(ID_TOKEN_PREFIX + tokenState);
        String accessToken = session.get(ACCESS_TOKEN_PREFIX + tokenState);
        String refreshToken = session.get(REFRESH_TOKEN_PREFIX + tokenState);
        return Uni.createFrom().item(new AuthorizationCodeTokens(idToken, accessToken, refreshToken));
    }

    @Override
    public Uni<Void> deleteTokens(RoutingContext routingContext, OidcTenantConfig oidcConfig, String tokenState, OidcRequestContext<Void> requestContext) {
        Session session = routingContext.session();
        if (session == null) {
            return noSession();
        }

        session.remove(ID_TOKEN_PREFIX + tokenState);
        session.remove(ACCESS_TOKEN_PREFIX + tokenState);
        session.remove(REFRESH_TOKEN_PREFIX + tokenState);
        return Uni.createFrom().voidItem();
    }

    private static <T> Uni<T> noSession() {
        return Uni.createFrom().failure(new UnsupportedOperationException("No active session or support for sessions disabled"));
    }
}

Ideas? Thoughts? I'd also welcome any reviews on the sessions PR itself, even though that's unrelated to this thread :-)

Thanks,

LT

st 27. 9. 2023 v 19:47 odesílatel Sergey Beryozkin <sbia...@redhat.com> napsal:

--
You received this message because you are subscribed to the Google Groups "Quarkus Development mailing list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to quarkus-dev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/CAMsYBfUZ_gYTu_16NXXSP_5pfkbRhmtp8apoqYY001Sj%3D%2BM1%2Bg%40mail.gmail.com.

[Michal Vavřík]

I think this is fantastic idea Ladislav, I meant to follow-up with this after your PR is merged, but now I think you should do it. 

I have one question - https://vertx.io/docs/vertx-web/java/#_handling_sessions says that there is no guarantee the data is fully persisted before the client receives the response. I understand that storing session in REDIS / Infinispan is most likely going to be faster than next request comes, but is it good idea to rely on this? So my question is - shouldn't your implementation force flush?

Michal

[Ladislav Thon]

Great question! Unfortunately, if you flush manually, Vert.x Web won't persist session data automatically at the time when response is being written. So explicit flushing in the token state manager would mean users would also have to flush explicitly, if they use the session.

I tried to find a way how to make session storing synchronous with response writing, but I couldn't figure it out. Maybe it's possible, I'm no expert on Vert.x Web.

I'm afraid I don't have a better answer at the moment 

LT

Dne út 24. 10. 2023 18:28 uživatel Michal Vavřík <mva...@redhat.com> napsal:

To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/048f9225-4684-48e6-abe7-5260104351fcn%40googlegroups.com.

[Sergey Beryozkin]

Hey Ladislav,

On Tue, Oct 24, 2023 at 4:48 PM Ladislav Thon <lad...@gmail.com> wrote:

Hi,

I've been meaning to reply on this thread for a few weeks, but I keep getting distracted...

For very good reasons :-).

Before commenting further, I'd like to say, your Vert.x Web session integration has been beautifully done, 🍀, agree with Michal.

Anyway, I've been meaning to articulate one thing that I find incredibly important: SESSIONS ARE FINE. Let's use sessions.

Right, stateful sessions are not something OIDC has been opposing in principle, we'd like of course the Quarkus endpoints of the Google and Amazon level scale to the Web maximum, which is what a cookie based approach can help with, but as we've discussed, first of all Quarkus OIDC has had an option for users to do the stateful session for a long while with the custom `TokenStateManager`, except that they had to implement it manually.

In fact this thread was about announcing Michal has implemented the first stateful `TokenStateManager` as a quarkus extension, in a fashion done exactly as you prototyped below, but by storing tokens in the database.

 

To that end, I've been working on Quarkus support for Vert.x Web sessions, which I believe is now ready for review: https://github.com/quarkusio/quarkus/pull/36310 In that PR, sessions can be stored in memory, in Redis, or in Infinispan.

Nicely done indeed

 

I intentionally don't support storing session data in a cookie, because 1. I think it's a bad idea in general,

It just has to be done carefully. If the state does not hold secrets, signed content is sufficient. Otherwise it must be encrypted

 

2. the Vert.x Web session cookie implementation has limitations (the session cookie is signed but not encrypted, so not a good idea for sensitive data,

FYI, we've reacted to the earlier feedback and now Quarkus OIDC encrypts the session cookie by default

 

and it is just a single cookie and the data store to it are Base64 encoded, so there's a limit of roughly 2.5 KB of data).

FYI, Qarkus OIDC allows users to request that the session cookie is split into 2 or 3 encrypted cookies.

From the practical point of view it seems the complete JWE encryption is causing Quarkus to log a warning sometimes about the large session cookie size, I'm considering supporting a `dir` algorithm, where the content encryption secret is not generated - and the configured encryption key is used directly (as opposed to encrypting this generated key), this is what Auth0 does for its access tokens if they are in opaque formats. This is a minor detail though. 

I'd be happy to ship a code like this directly with quarkus-oidc given how seamless the Vert.x Web Session integration is now, and let users choose it with a single switch, as an alternative to the default cookie store based solution, and if users would  not prefer to use the DB-based one which we also ship now.

Here is a question, in addition to what Michal asked, since we have 3 tokens to manage, I guess it should be all be done atomically, I would not mind having an option to write:

session.put(id, tokens);

session.get(id, AuthorizationCodeTokens.class); 
or at least 

(AuthorizationCodeTokens)session.get(id);

 where `AuthorizationCodeTokens` implement some interface like SessionState, turning 3 tokens into a single string and parsing 3 tokens back into the bean ?

How does it look to you ?

Ideas? Thoughts? I'd also welcome any reviews on the sessions PR itself, even though that's unrelated to this thread :-)

Thanks

Sergey

[Sergey Beryozkin]

Probably, rather than complicating things, we can create the string manually, when saving/retrieving :-)

[Ladislav Thon]

út 24. 10. 2023 v 19:57 odesílatel Sergey Beryozkin <sbia...@redhat.com> napsal:

Here is a question, in addition to what Michal asked, since we have 3 tokens to manage, I guess it should be all be done atomically, I would not mind having an option to write:

session.put(id, tokens);

session.get(id, AuthorizationCodeTokens.class); 
or at least 

(AuthorizationCodeTokens)session.get(id);

 where `AuthorizationCodeTokens` implement some interface like SessionState, turning 3 tokens into a single string and parsing 3 tokens back into the bean ?

How does it look to you ?

That would be doable, but it would also not be necessary at the moment. The thing is, Vert.x Web doesn't allow fine-grained (e.g. per attribute) access to the session, it's always accessed as a whole. At the beginning of a request, the `SessionHandler` reads the entire session from the `SessionStore`, so you have direct access to it and you can modify it as you wish, and at the end of the request, the entire session is written back. The session has a version number, so that if multiple requests access the same session concurrently, only one of them will be able to persist it back to the `SessionStore`.

LT

 

To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/CAMsYBfV80Z-ghia%2BLW9Ftf4AQrbALFCoZuLqSHTdamDowLhF0Q%40mail.gmail.com.

[Sergey Beryozkin]

On Tue, Oct 24, 2023 at 6:36 PM Ladislav Thon <lad...@gmail.com> wrote:

Great question! Unfortunately, if you flush manually, Vert.x Web won't persist session data automatically at the time when response is being written. So explicit flushing in the token state manager would mean users would also have to flush explicitly, if they use the session.

I tried to find a way how to make session storing synchronous with response writing, but I couldn't figure it out. Maybe it's possible, I'm no expert on Vert.x Web.

I'm afraid I don't have a better answer at the moment 

Like Michal suggested, I'm hoping that by the time the user gets a response after the authentication and decides to click on some button or link to access some application data, 

the session state representing that authentication has already reached its destination in Infinispan, Redis. It is probably more theoretical than practically reproducible that the returning authenticated user won't have the matching state, if it ever happens, the user will have to re authenticate, but it should be rare indeed.

Thanks Sergey

 

To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/CALbocOm8Pv2PQtu_eJmOLut4TDjGDLJ-9Qsgs%2BO1wosNgq6qZQ%40mail.gmail.com.