8:36 a.m.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
Stian,
I went ahead and did the changes based on the following assumptions:
3) KeycloakSecurityContext should have a "realm" property. Added test
for this and changed the callers to pass the realm to the constructor.
4) Leave the adapter unconfigured in case of configuration mismatch
(ie: resolver specified but invalid + keycloak.json present). The
cases are described as comments in the code
6 and 7) I assume this is not a problem. I cannot find a situation
where this would lead to a non-existing user in one tenant to
successfully access an unauthorized resource. In any case, I've added
a couple of test cases, where an user from tenant1 tries to login to
tenant2 and where an already logged in user from tenant1 tries to
access tenant2.
A new PR was sent with all the changes from your review.
- - Juca.
On 10/29/2014 11:23 AM, Juraci Paixão Kröhling wrote:
Stian,
Agree with all the comments. I'm doing the changes, but I've got a
question and a few comments. See below.
On 10/28/2014 04:54 PM, Stian Thorgersen wrote:
> 3. CatalinaSessionTokenStore.checkCurrentToken - can you figure
> out the realm if the session was serialized? when adapter is
> clustered we support serializing the session
I'm then changing one of the SecurityContext's to include the
realm, so that it gets de-serialized with this information. Now,
the question is whether it is more appropriate to add it to
KeycloakSecurityContext or RefreshableKeycloakSecurityContext. On
the superclass (KeycloakSecurityContext), I have access only to
IdToken and AccessToken. I believe both have ways to retrieve the
realm (issuer, I believe), but I don't know how reliable this is. I
remember seeing a post from Bill on keycloak-user that it should be
changed to an URL, not the realm name. On the subclass, however, I
have access to the KeycloakDeployment, which provides the realm on
the exact way that it was originally configured.
> 4. KeycloakAuthenticatorValve.initInternal - just do a warn +
> unconfigured to make it behave same as if keycloak-server.json is
> missing, and drop the comment
Ok, but just to confirm: I should add a new warn with the same text
as the other and *not* attempt to load a keycloak.json, right? The
question is because there might be a non-existing resolver
specified *and* a keycloak.json . As it is, there will be a warn on
the log, saying that the resolver will not be used, but as there is
a keycloak.json, a KeycloakDeployment is available.
Based on your comment, I assume that this is a configuration
problem, hence, it should not attempt to load keycloak.json if a
resolver is specified.
> 6. CatalinaCookieTokenStore.checkPrincipalFromCookie - if cookie
> has different realm I'd say the log should at least be a warn, is
> this not a significant problem? 7.
> CatalinaSessionTokenStore.checkCurrentToken - same as above
> isn't
cookie with different realm a pretty big issue?
I don't think it's a significant problem. It will happen when one
user has access to two realms, is logged in on the first and then
tries to access the same application on a second tenant. On this
case, the cookie sent is for the first tenant, so, we should
consider it invalid for this request, meaning, the principal for
this cookie for this realm is unknown (return null). On this
scenario, Keycloak will redirect the user for logging in on the
second realm, which will effectively override the first cookie.
This scenario is tested on
MultiTenancyTest#testTenantsWithoutLoggingOut .
- Juca.
_______________________________________________ keycloak-dev
mailing list keycloak-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/keycloak-dev
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
iQEcBAEBCgAGBQJUUO1BAAoJEDnJtskdmzLMIv4H/R0NTJ46mK0dbDXkwNb68xQS
W6zeQ+jzBmAvwu7/i8/kSu7AXSPaGxR/EBbjSBKj6l5zVQLjlPQ0CFUzQSDGDASy
WKyMEKk64Df0dNZlkuqHHFaCyuZ+3rz2k9XiGRmpIXej/uzUjvXUpIKBmUOpDxR6
dmqViOkRBuLFbBvwbCgid85Mip92wd2YXUHl0dmPNZN/j9bJPp8GYTHinJgv1pZs
j6hTYduasl/T4jcgWWHYKftuOpbpOWtusjCwuGwxIS1TsIYNZt4G88LL6tWTD9WF
8LznY1cxXuRA7YAvGKOrZY/e9yECQGJokWSZxkQrjNuvzLx1TXkaU7bM7E1oPXs=
=Vpjp
-----END PGP SIGNATURE-----