[keycloak-dev] Better support authenticationSessions in multiple browser tabs

[Stian Thorgersen]

I would target this to 3.4.2. I don't want to delay the 3.4.1 release if we
can help it.

I'd also suggest some (short if possible) random key (or a counter?!)
rather than relying on protocol specific values. 'state' is not actually
required in OAuth right? It's just recommended.

On 29 November 2017 at 13:00, Marek Posolda <mposolda at redhat.com> wrote:

> I am looking at https://issues.jboss.org/browse/KEYCLOAK-5797 issue now.
> It's about the fact, that when user has opened browser with multiple
> browser tabs, then after successful login in tab1 (clientA), he may be
> redirected to the incorrect client (clientB, which was opened in tab2).
>
> The thing is, that authenticationSession was tracked by the cookie and
> didn't support multiple clients. So both browser tabs "tab1" and "tab2"
> used same authenticationSession, which can reference just one of the
> clients, hence there could be the conflict and one of the tabs
> redirected to incorrect client after authentication.
>
> I am working on a fix, that allow better support for multiple clients.
> What I am doing is, that there is "RootAuthenticationSessionModel",
> which is now referenced by the ID from the cookie. That root session can
> reference more actual authentication sessions through the map like
> "Map<String, AuthenticationSessionModel>" . The key is the client UUID.
> In the Authentication SPI, there are no changes. Those still use the
> AuthenticationSessionModel as before.
>
> This is easily possible as we have "client-id" parameter already
> available during authentication flows in every tab. So every browser tab
> can reference correct client and redirect to it.
>
> However even with the fix, there is another corner case about support
> for multiple browser tabs with *same* client. This will be still an
> issue, especially for the javascript clients. I've created another JIRA
> https://issues.jboss.org/browse/KEYCLOAK-5938 with the example issue,
> which can be simulated with our admin console. To properly support
> multiple tabs of same client, the key will need to be like: "client-id +
> state" instead of just "client-id"  The "state" will need to be either
> the OIDC/SAML state or some randomly generated string (As in both the
> OAuth2 and SAML, the state/relayState parameter is not mandatory AFAIK),
> which will need to be added to the URL during authentication flows as well.
>
> Fixing this will take me another few days of work (maybe 2?) as there
> will need to be change in many files for adding the new parameter + some
> more authenticationSession model changes etc. So I wonder if we want to:
> 1) Fix this in 3.4.1 . Will likely mean to delay the release?
> 2) Fix this in 3.4.2. It will affect many files and there is some chance
> of regression (hopefully not big as we have lots of the tests for
> various other corner cases)
> 3) Fix this later in 4.X .
>
> My vote is 1 or 2. WDYT? Any other possibility?
>
> Marek
>
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>