Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to restore a soft-logged-out session established via SSO #25957

Closed
richvdh opened this issue Aug 11, 2023 · 6 comments · Fixed by matrix-org/matrix-react-sdk#11794
Closed
Assignees
Labels
A-OIDC O-Occasional Affects or can be seen by some users regularly or most users rarely S-Critical Prevents work, causes data loss and/or has no workaround T-Defect

Comments

@richvdh
Copy link
Member

richvdh commented Aug 11, 2023

Steps to reproduce

  1. Log into Element via SSO
  2. Have the session be soft-logged-out

Outcome

What did you expect?

I expect to be able to restore the session via another SSO login.

What happened instead?

An error saying "You cannot sign in to your account":

image

Operating system

No response

Application version

current develop (Element version: d1f7b08-react-4a9c4198b080-js-55b9116c998b Olm version: 3.2.14 or so)

How did you install the app?

No response

Homeserver

Synapse 1.90

Will you send logs?

No

@richvdh
Copy link
Member Author

richvdh commented Aug 11, 2023

This happens due to bugs in SoftLogout. In particular:

First, it inspects realQueryParams. These are the query parameters at the time the app was loaded. Since we logged in via token login, an (old) loginToken is present.

It then goes on to inspect localstorage for the URL of the homeserver that the loginToken belongs to (https://github.com/matrix-org/matrix-react-sdk/blob/v3.77.1/src/components/structures/auth/SoftLogout.tsx#L193). That setting was cleared when we did the token login.

So, it declares the situation to be untenable and shows the above error.


It's actually fine if there is an app reload between the initial login (or renewal) and the soft-logout.

@richvdh
Copy link
Member Author

richvdh commented Aug 11, 2023

matrix-org/matrix-react-sdk#11402 adds a cypress test which has to work around this.

@dbkr dbkr added S-Minor Impairs non-critical functionality or suitable workarounds exist O-Occasional Affects or can be seen by some users regularly or most users rarely labels Aug 14, 2023
@richvdh
Copy link
Member Author

richvdh commented Aug 14, 2023

I'm going to bump the severity on this. It's awful experience if you hit it because you end up vaping your crypto state.

@richvdh richvdh added S-Critical Prevents work, causes data loss and/or has no workaround and removed S-Minor Impairs non-critical functionality or suitable workarounds exist labels Aug 14, 2023
@remyb-SURF
Copy link

remyb-SURF commented Sep 5, 2023

We're also running into this issue, after querying in #element-web:matrix.org @t3chguy replied the following:


from a quick skim it seems to be thinking you're returning from your SSO provider when you hit soft logout hence the log message you're seeing, this also ends up resulting in not showing you the SSO button
image

trySsoLogin fails because you haven't gone via SSO yet to re-auth
it doesn't feel like that return should be there if trySsoLogin fails

Index: src/components/structures/auth/SoftLogout.tsx
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/components/structures/auth/SoftLogout.tsx b/src/components/structures/auth/SoftLogout.tsx
--- a/src/components/structures/auth/SoftLogout.tsx	(revision 4796b79d98a5dd2c07bd54e3ea312066806b2fb8)
+++ b/src/components/structures/auth/SoftLogout.tsx	(date 1693907928260)
@@ -117,8 +117,9 @@
         const hasAllParams = queryParams?.["loginToken"];
         if (hasAllParams) {
             this.setState({ loginView: LoginView.Loading });
-            this.trySsoLogin();
-            return;
+            if (await this.trySsoLogin()) {
+                return;
+            }
         }
 
         // Note: we don't use the existing Login class because it is heavily flow-based. We don't
@@ -186,14 +187,14 @@
         });
     };
 
-    private async trySsoLogin(): Promise<void> {
+    private async trySsoLogin(): Promise<boolean> {
         this.setState({ busy: true });
 
         const hsUrl = localStorage.getItem(SSO_HOMESERVER_URL_KEY);
         if (!hsUrl) {
             logger.error("Homeserver URL unknown for SSO login callback");
             this.setState({ busy: false, loginView: LoginView.Unsupported });
-            return;
+            return false;
         }
 
         const isUrl = localStorage.getItem(SSO_ID_SERVER_URL_KEY) || MatrixClientPeg.safeGet().getIdentityServerUrl();
@@ -209,7 +210,7 @@
         } catch (e) {
             logger.error(e);
             this.setState({ busy: false, loginView: LoginView.Unsupported });
-            return;
+            return false;
         }
 
         Lifecycle.hydrateSession(credentials)
@@ -220,6 +221,7 @@
                 logger.error(e);
                 this.setState({ busy: false, loginView: LoginView.Unsupported });
             });
+        return true;
     }
 
     private renderPasswordForm(introText: Optional<string>): JSX.Element {

something like the above might fix it

@Johennes
Copy link
Contributor

I'm adding the OIDC label to this issue as I'd like us to explore whether we could feasibly fix this in the course of https://github.com/vector-im/element-internal/issues/412 (the label gets the issue on the respective board).

CC @kittykat / @kerryarchibald

@Johennes
Copy link
Contributor

@kerryarchibald sorry I realised I hadn't actually added this to your OIDC board. Could you give it a look?

@kerryarchibald kerryarchibald self-assigned this Oct 25, 2023
gabrc52 added a commit to sipb/matrix-react-sdk that referenced this issue Nov 12, 2023
* Knock on a ask-to-join room if a module wants to join the room when navigating to a room ([\matrix-org#11787](matrix-org#11787)). Contributed by @dhenneke.
* Element-R:  Include crypto info in sentry ([\matrix-org#11798](matrix-org#11798)). Contributed by @florianduros.
* Element-R:  Include crypto info in rageshake ([\matrix-org#11797](matrix-org#11797)). Contributed by @florianduros.
* Element-R: Add current version of the rust-sdk and vodozemac ([\matrix-org#11785](matrix-org#11785)). Contributed by @florianduros.
* Fix unfederated invite dialog ([\matrix-org#9618](matrix-org#9618)). Fixes element-hq/element-meta#1466 and element-hq/element-web#22102. Contributed by @owi92.
* New right panel visual language ([\matrix-org#11664](matrix-org#11664)).
* OIDC: add friendly errors ([\matrix-org#11184](matrix-org#11184)). Fixes element-hq/element-web#25665. Contributed by @kerryarchibald.
* Fix rightpanel hiding scrollbar ([\matrix-org#11831](matrix-org#11831)). Contributed by @kerryarchibald.
* Fix multi-tab session lock on Firefox not being cleared ([\matrix-org#11800](matrix-org#11800)). Fixes element-hq/element-web#26165. Contributed by @ManuelHu.
* Deserialise spoilers back into slash command form ([\matrix-org#11805](matrix-org#11805)). Fixes element-hq/element-web#26344.
* Fix Incorrect message scaling for verification request ([\matrix-org#11793](matrix-org#11793)). Fixes element-hq/element-web#24304. Contributed by @capGoblin.
* Fix: Unable to restore a soft-logged-out session established via SSO ([\matrix-org#11794](matrix-org#11794)). Fixes element-hq/element-web#25957. Contributed by @kerryarchibald.
* Use configurable github issue links more consistently ([\matrix-org#11796](matrix-org#11796)).
* Fix io.element.late_event received_ts vs received_at ([\matrix-org#11789](matrix-org#11789)).
* Make invitation dialog scrollable when infos are too long ([\matrix-org#11753](matrix-org#11753)). Contributed by @nurjinjafar.
* Fix spoiler text-align ([\matrix-org#11790](matrix-org#11790)). Contributed by @ajbura.
* Fix: Right panel keeps showing chat when unmaximizing widget.  ([\matrix-org#11697](matrix-org#11697)). Fixes element-hq/element-web#26265. Contributed by @manancodes.
* Fix margin of invite to room button ([\matrix-org#11780](matrix-org#11780)). Fixes element-hq/element-web#26410.
* Update base64 import ([\matrix-org#11784](matrix-org#11784)).
* Set max size for Element logo in search warning ([\matrix-org#11779](matrix-org#11779)). Fixes element-hq/element-web#26408.
* Fix: emoji size in room header topic, remove obsolete emoji style ([\matrix-org#11757](matrix-org#11757)). Fixes element-hq/element-web#26326. Contributed by @kerryarchibald.
* Fix: Bubble layout design is broken ([\matrix-org#11763](matrix-org#11763)). Fixes element-hq/element-web#25818. Contributed by @manancodes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-OIDC O-Occasional Affects or can be seen by some users regularly or most users rarely S-Critical Prevents work, causes data loss and/or has no workaround T-Defect
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants