8:09 a.m.
On 29/11/17 14:44, Stian Thorgersen wrote:
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.
Yes, it's not
required. And same for SAML. Was wondering about the same.
Will use the random key or counter. Thinking if counter doesn't have
some corner case issues (EG. If 2 tabs are opened concurrently after
logout and will try to use same counter value as authSession update from
tab2 won't be yet visible in tab1).
Marek
On 29 November 2017 at 13:00, Marek Posolda <mposolda(a)redhat.com
<mailto:mposolda@redhat.com>> wrote:
I am looking at https://issues.jboss.org/browse/KEYCLOAK-5797
<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
<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(a)lists.jboss.org <mailto:keycloak-dev@lists.jboss.org>
https://lists.jboss.org/mailman/listinfo/keycloak-dev
<https://lists.jboss.org/mailman/listinfo/keycloak-dev>