Single logout broken when multiple clients in the session.

By: Doug Harris named 10 Mar 2022 at 3:19 p.m. CST

9 Responses
Doug Harris gravatar
There are a few symptoms of this: 1. When calling the end_session service when there are multiple clients in a session, validation of the `post_logout_redirect_uri` fails for all but one of them 2. If `post_logout_redirect_uri` validation succeeds for the lucky client, the other clients don't receive their front-channel logout. Cutting to the chase, it looks like the issue was introduced with [this commit](https://github.com/GluuFederation/oxOrm/commit/557355a7b592b63c9a1ff48839009127e96d1479). With this change, `org.gluu.oxauth.model.registration.Client` now inherits the `hashCode()` and `equals()` methods from `org.gluu.persist.model.base.Entry`. This is a problem because `org.gluu.oxauth.model.registration.Client` overrides the `dn` property of `Entry`, such that `hashCode()` and `equals()` are now being computed based on `Entry`'s dn, which is `null`. This causes "very bad things" to happen with collections of `Client`s, for example: [here](https://github.com/GluuFederation/oxAuth/blob/9523eb2ed86d46d69cb39d3c79262444d0ab3157/Server/src/main/java/org/gluu/oxauth/service/ClientService.java#L109-L127) where only the first client ever gets added to the resulting `Set` since all instances of `Client` have the same hashCode and are equals() to each other.
Gluu 4.3 Rhel 7.9
closed

Answers

By Michael Schwartz Account Admin 10 Mar 2022 at 3:35 p.m. CST

Copy
Michael Schwartz gravatar
Thanks for the feedback. We'll look into this.

By Doug Harris named 11 Mar 2022 at 6:56 a.m. CST

Copy
Doug Harris gravatar
I ran a quick search and there are a number of other subclasses of `BaseEntry` (via `DeletableEntity`) which also override `dn`. E.g. * org.gluu.persist.cloud.spanner.model.Scope * org.gluu.service.cache.NativePersistenceCacheEntity * org.gluu.persist.cloud.spanner.model.SimpleCacheEntry * org.gluu.persist.sql.model.SimpleCacheEntry .. there may be others.

By Doug Harris named 11 Mar 2022 at 7 a.m. CST

Copy
Doug Harris gravatar
Note sure if it's the cleanest fix, but adding a call to the superclass setter does seem to make the problem go away. E.g. ``` public void setDn(String dn) { this.dn = dn; super.setDn(dn); } ```

By Yuriy Zabrovarnyy staff 11 Mar 2022 at 8:27 a.m. CST

Copy
Yuriy Zabrovarnyy gravatar
Thank you so much for bring it to our attention. Maybe better to drop override of dn to get it not null, I'm checking it right now.

By Yuriy Zabrovarnyy staff 11 Mar 2022 at 10 a.m. CST

Copy
Yuriy Zabrovarnyy gravatar
Bug report for reference https://github.com/GluuFederation/oxAuth/issues/1651

By Yuriy Zabrovarnyy staff 14 Mar 2022 at 3:35 a.m. CDT

Copy
Yuriy Zabrovarnyy gravatar
Issue is corrected and fix is in 4.3.2 and master. Would you be so kind to give it a try? War file can be found here: https://jenkins.gluu.org/maven/org/gluu/oxauth-server/4.3.2-SNAPSHOT/oxauth-server-4.3.2-SNAPSHOT.war

By Doug Harris named 14 Mar 2022 at 8:40 a.m. CDT

Copy
Doug Harris gravatar
The new WAR looks good. Logout is working as expected. Do you have a timeline in mind for releasing 4.3.2? We were "this close" to deploying 4.3.1 to production when we ran into this issue. Now we need to decide whether to wait for 4.3.2 or to just go ahead with 4.3.1 + the newer oxauth.war. Thanks, -Doug

By Yuriy Zabrovarnyy staff 14 Mar 2022 at 3:56 p.m. CDT

Copy
Yuriy Zabrovarnyy gravatar
Plan is to get it soon but QA process and other things may delay it. Thus I don't have dates. I would suggest to go with 4.3.1 + new war. Thank you for confirmation!

By Doug Harris named 14 Mar 2022 at 4:23 p.m. CDT

Copy
Doug Harris gravatar
Thanks

Post an answer