Accept a device ID to the login fallback endpoint. #7629

clokep · 2020-06-03T18:35:45Z

This modifies the login fallback page to accept a query parameter (device_id) which will get passed into the login endpoint during the login process.

This also reworks some of the login.js code to reduce duplicated code.

Fixes #5755

MSC: matrix-org/matrix-spec-proposals#2604

Questions

Should we also accept an initial_device_display_name query parameter? It does not look any of the other parameters would be useful to accept since they're part of the login process.

clokep · 2020-06-03T18:39:39Z

Also -- I wasn't really sure about the styling of the JavaScript code, but kind of just followed what was there.

clokep · 2020-06-03T19:04:44Z

synapse/static/client/login/js/login.js

-        token: loginToken
-    };
+    // Add the login type.
+    data.type = type;


This modifies the input value which is kind of meh, but I think this code is contained enough that it is OK + we control the values being sent to this function.

clokep · 2020-06-03T19:08:21Z

synapse/static/client/login/js/login.js

-    $("#sso_redirect_url").val(this_page);
+    // Set the redirect to come back to this page, a login token will get added
+    // and handled after the redirect.
+    $("#sso_redirect_url").val(window.location.href);


This change means that we use the whole URL as the redirect, which is necessary to pass the query parameters through SSO.

passing the query params through SSO feels like the sort of thing we shouldn't be doing, for fear of CSRF. can we stash them in a cookie or something instead?

Hmm...the concern being the request will be modified before it gets back to us? I think that would be "bad", but I don't see a security issue with it. We can probably save it in a cookie though. I'll take a look!

clokep · 2020-06-04T11:48:55Z

Should we also accept an initial_device_display_name query parameter? It does not look any of the other parameters would be useful to accept since they're part of the login process.

Had a brief chat about this and it seems there's no reason not to include it. I've added it to this PR.

clokep · 2020-06-05T18:02:40Z

@richvdh I modified this to save the query string parameters to a cookie in the case of SSO. (In the case of a password I don't think this is necessary. I'm hoping the code is clear, but for SSO the following happens:

The query string is parsed to an object.
The object is serialized to JSON.
The JSON is stored into a cookie
✨ SSO occurs ✨
The cookie is retrieved and de-JSON-ified

This let us keep the code paths similar to the password version where we just follow the first step (parse the query string to an object).

I also cleaned-up some more code and added a few more comments while messing around in here.

richvdh

lgtm

synapse/static/client/login/js/login.js

richvdh · 2020-06-08T13:12:14Z

synapse/static/client/login/js/login.js

-    }).fail(errorFunc);
-};
+// Titles get updated through the process to give users feedback.
+var TITLE_PRE_AUTH = "Log in with one of the following methods";


const is a thing in modern javascript https://caniuse.com/#feat=const

🤷

I debated about changing it... IE < 11 didn't support it and I didn't want to think too hard about our policy here. I'll do a separate PR that cleans up the style of this file a bit!

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

@dklimpel

…rg-hotfixes Synapse 1.15.0rc1 (2020-06-09) ============================== Features -------- - Advertise support for Client-Server API r0.6.0 and remove related unstable feature flags. ([\#6585](#6585)) - Add an option to disable autojoining rooms for guest accounts. ([\#6637](#6637)) - For SAML authentication, add the ability to pass email addresses to be added to new users' accounts via SAML attributes. Contributed by Christopher Cooper. ([\#7385](#7385)) - Add admin APIs to allow server admins to manage users' devices. Contributed by @dklimpel. ([\#7481](#7481)) - Add support for generating thumbnails for WebP images. Previously, users would see an empty box instead of preview image. ([\#7586](#7586)) - Support the standardized `m.login.sso` user-interactive authentication flow. ([\#7630](#7630)) Bugfixes -------- - Allow new users to be registered via the admin API even if the monthly active user limit has been reached. Contributed by @dkimpel. ([\#7263](#7263)) - Fix email notifications not being enabled for new users when created via the Admin API. ([\#7267](#7267)) - Fix str placeholders in an instance of `PrepareDatabaseException`. Introduced in Synapse v1.8.0. ([\#7575](#7575)) - Fix a bug in automatic user creation during first time login with `m.login.jwt`. Regression in v1.6.0. Contributed by @olof. ([\#7585](#7585)) - Fix a bug causing the cross-signing keys to be ignored when resyncing a device list. ([\#7594](#7594)) - Fix metrics failing when there is a large number of active background processes. ([\#7597](#7597)) - Fix bug where returning rooms for a group would fail if it included a room that the server was not in. ([\#7599](#7599)) - Fix duplicate key violation when persisting read markers. ([\#7607](#7607)) - Prevent an entire iteration of the device list resync loop from failing if one server responds with a malformed result. ([\#7609](#7609)) - Fix exceptions when fetching events from a remote host fails. ([\#7622](#7622)) - Make `synctl restart` start synapse if it wasn't running. ([\#7624](#7624)) - Pass device information through to the login endpoint when using the login fallback. ([\#7629](#7629)) - Advertise the `m.login.token` login flow when OpenID Connect is enabled. ([\#7631](#7631)) - Fix bug in account data replication stream. ([\#7656](#7656)) Improved Documentation ---------------------- - Update the OpenBSD installation instructions. ([\#7587](#7587)) - Advertise Python 3.8 support in `setup.py`. ([\#7602](#7602)) - Add a link to `#synapse:matrix.org` in the troubleshooting section of the README. ([\#7603](#7603)) - Clarifications to the admin api documentation. ([\#7647](#7647)) Internal Changes ---------------- - Convert the identity handler to async/await. ([\#7561](#7561)) - Improve query performance for fetching state from a PostgreSQL database. ([\#7567](#7567)) - Speed up processing of federation stream RDATA rows. ([\#7584](#7584)) - Add comment to systemd example to show postgresql dependency. ([\#7591](#7591)) - Refactor `Ratelimiter` to limit the amount of expensive config value accesses. ([\#7595](#7595)) - Convert groups handlers to async/await. ([\#7600](#7600)) - Clean up exception handling in `SAML2ResponseResource`. ([\#7614](#7614)) - Check that all asynchronous tasks succeed and general cleanup of `MonthlyActiveUsersTestCase` and `TestMauLimit`. ([\#7619](#7619)) - Convert `get_user_id_by_threepid` to async/await. ([\#7620](#7620)) - Switch to upstream `dh-virtualenv` rather than our fork for Debian package builds. ([\#7621](#7621)) - Update CI scripts to check the number in the newsfile fragment. ([\#7623](#7623)) - Check if the localpart of a Matrix ID is reserved for guest users earlier in the registration flow, as well as when responding to requests to `/register/available`. ([\#7625](#7625)) - Minor cleanups to OpenID Connect integration. ([\#7628](#7628)) - Attempt to fix flaky test: `PhoneHomeStatsTestCase.test_performance_100`. ([\#7634](#7634)) - Fix typos of `m.olm.curve25519-aes-sha2` and `m.megolm.v1.aes-sha2` in comments, test files. ([\#7637](#7637)) - Convert user directory, state deltas, and stats handlers to async/await. ([\#7640](#7640)) - Remove some unused constants. ([\#7644](#7644)) - Fix type information on `assert_*_is_admin` methods. ([\#7645](#7645)) - Convert registration handler to async/await. ([\#7649](#7649))

…#7629)

…dinsic-release-v1.15.x * 'release-v1.15.0' of github.com:matrix-org/synapse: (55 commits) 1.15.0 Fix some attributions Update CHANGES.md 1.15.0rc1 Revert "1.15.0rc1" 1.15.0rc1 Fix bug in account data replication stream. (#7656) Convert the registration handler to async/await. (#7649) Accept device information at the login fallback endpoint. (#7629) Convert user directory handler and related classes to async/await. (#7640) Add an option to disable autojoin for guest accounts (#6637) Clarifications to the admin api documentation (#7647) Update to the stable SSO prefix for UI Auth. (#7630) Fix type information on `assert_*_is_admin` methods (#7645) Remove some unused constants. (#7644) Typo fixes. Allow new users to be registered via the admin API even if the monthly active user limit has been reached (#7263) Add device management to admin API (#7481) Attempt to fix PhoneHomeStatsTestCase.test_performance_100 being flaky. (#7634) Support CS API v0.6.0 (#6585) ...

clokep force-pushed the clokep/device-id-fallback-login branch from 93dfee8 to d7311fa Compare June 3, 2020 18:39

clokep requested a review from a team June 3, 2020 19:04

clokep marked this pull request as ready for review June 3, 2020 19:04

clokep commented Jun 3, 2020

View reviewed changes

Accept a device ID to the login fallback endpoint.

0d41ffc

clokep force-pushed the clokep/device-id-fallback-login branch from d7311fa to 0d41ffc Compare June 3, 2020 19:05

clokep commented Jun 3, 2020

View reviewed changes

Also pass the device display name.

f5de2fa

clokep mentioned this pull request Jun 5, 2020

MSC2604: Accept device information for the login fallback endpoint matrix-org/matrix-spec-proposals#2604

Merged

richvdh removed the request for review from a team June 5, 2020 14:27

Store the query parameters in a cookie during the SSO flow.

3c9e512

clokep requested a review from richvdh June 5, 2020 18:02

richvdh approved these changes Jun 8, 2020

View reviewed changes

Fix tense in comment.

12f3b7c

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

clokep merged commit 375ca0c into develop Jun 8, 2020

clokep deleted the clokep/device-id-fallback-login branch June 8, 2020 14:13

clokep mentioned this pull request Jun 8, 2020

Clean-up the fallback login code #7657

Merged

phil-flex pushed a commit to phil-flex/synapse that referenced this pull request Jun 16, 2020

Accept device information at the login fallback endpoint. (matrix-org…

9f55172

…#7629)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Accept a device ID to the login fallback endpoint. #7629

Accept a device ID to the login fallback endpoint. #7629

clokep commented Jun 3, 2020 •

edited by richvdh

Loading

clokep commented Jun 3, 2020

clokep Jun 3, 2020

clokep Jun 3, 2020

richvdh Jun 5, 2020

clokep Jun 5, 2020

clokep commented Jun 4, 2020

clokep commented Jun 5, 2020

richvdh left a comment

richvdh Jun 8, 2020

clokep Jun 8, 2020

Accept a device ID to the login fallback endpoint. #7629

Accept a device ID to the login fallback endpoint. #7629

Conversation

clokep commented Jun 3, 2020 • edited by richvdh Loading

Questions

clokep commented Jun 3, 2020

clokep Jun 3, 2020

Choose a reason for hiding this comment

clokep Jun 3, 2020

Choose a reason for hiding this comment

richvdh Jun 5, 2020

Choose a reason for hiding this comment

clokep Jun 5, 2020

Choose a reason for hiding this comment

clokep commented Jun 4, 2020

clokep commented Jun 5, 2020

richvdh left a comment

Choose a reason for hiding this comment

richvdh Jun 8, 2020

Choose a reason for hiding this comment

clokep Jun 8, 2020

Choose a reason for hiding this comment

clokep commented Jun 3, 2020 •

edited by richvdh

Loading