Implement logic for sign in with QR #7369

hughns · 2022-10-13T23:50:57Z

This PR is dependent on #7338. Compare

Type of change

Feature
Bugfix
Technical
Other :

Content

This PR has the logic to implement the UI from #7338

Motivation and context

The motivation is as per #7338

Screenshots / GIFs

See #7338

Tests

Testing requires generating a QR code in Element Web and scanning it in Element Android.

To generate a code in Element Web:

Go to preview build of Element Web: https://pr9403--matrix-react-sdk.netlify.app/
Choose test homeserver: synapse-msc3882.rendezvous.lab.element.dev
Register a user
Go to Menu -> All settings -> Labs and enable the QR feature

Then go to Security & Privacy and see the new Sign in with QR section:

Press Show QR to generate a QR:

To scan in Android:

Launch app when signed out
Enable the QR Code Login feature in Features:

- Go back to main screen - Select "I already have an account" - Press EDIT to enter a different homeserver - Enter https://synapse-msc3882.rendezvous.lab.element.dev - Press next - You should then see a new button:

- Press Scan QR code and follow instructions

Tested devices

Physical
Emulator
OS version(s):

Checklist

Changes has been tested on an Android device or Android emulator with API 21
UI change has been tested on both light and dark themes
Accessibility has been taken into account. See https://github.com/vector-im/element-android/blob/develop/CONTRIBUTING.md#accessibility
Pull request is based on the develop branch
Pull request includes a new file under ./changelog.d. See https://github.com/vector-im/element-android/blob/develop/CONTRIBUTING.md#changelog
Pull request includes screenshots or videos if containing UI changes
Pull request includes a sign off
You've made a self review of your PR
If you have modified the screen flow, or added new screens to the application, you have updated the test UiAllScreensSanityTest.allScreensTest()

Signed-off-by: Hugh Nimmo-Smith hughns@element.io

# Conflicts: # library/ui-strings/src/main/res/values/strings.xml # library/ui-styles/src/main/res/values/stylable_sessions_list_header_view.xml # vector-app/src/debug/java/im/vector/app/features/debug/features/DebugFeaturesStateFactory.kt # vector-app/src/debug/java/im/vector/app/features/debug/features/DebugVectorFeatures.kt # vector/src/main/java/im/vector/app/core/di/MavericksViewModelModule.kt # vector/src/main/java/im/vector/app/features/VectorFeatures.kt # vector/src/main/java/im/vector/app/features/settings/devices/v2/VectorSettingsDevicesFragment.kt # vector/src/main/java/im/vector/app/features/settings/devices/v2/list/SessionsListHeaderView.kt # vector/src/main/res/layout/fragment_other_sessions.xml # vector/src/main/res/layout/fragment_settings_devices.xml

BillCarsonFr · 2022-10-18T08:18:15Z

vector/src/main/java/im/vector/app/features/login/qr/QrCodeLoginViewModel.kt

+
+        _viewEvents.post(QrCodeLoginViewEvents.NavigateToStatusScreen)
+
+        viewModelScope.launch(Dispatchers.IO) {


I think it's just calling session.coroutineDispatchers.io

.../main/java/org/matrix/android/sdk/api/rendezvous/transports/SimpleHttpRendezvousTransport.kt

bmarty

Nice work.

There is a bit of architecture rework to do on the SDK, but I would not block the PR for that.
The only blocking remark is about the logs: #7369 (comment)

library/ui-strings/src/main/res/values/strings.xml

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/rendezvous/RendezvousChannel.kt

...ndroid/src/main/java/org/matrix/android/sdk/api/rendezvous/channels/ECDHRendezvousChannel.kt

bmarty · 2022-10-18T09:00:20Z

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/rendezvous/model/RendezvousError.kt

-}
+import org.matrix.android.sdk.api.rendezvous.RendezvousFailureReason
+
+class RendezvousError(val description: String, val reason: RendezvousFailureReason) : Exception(description)


You could extend Failure.FeatureFailure() instead.

Suggested change

class RendezvousError(val description: String, val reason: RendezvousFailureReason) : Exception(description)

class RendezvousError(val description: String, val reason: RendezvousFailureReason) : Failure.FeatureFailure()

bmarty · 2022-10-18T09:01:57Z

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/rendezvous/Rendezvous.kt

+
+            val transport = SimpleHttpRendezvousTransport(parsed.rendezvous.transport.uri)
+
+            return Rendezvous(


You can return an instance of RendezvousImpl here.

bmarty · 2022-10-18T09:02:23Z

.../main/java/org/matrix/android/sdk/api/rendezvous/transports/SimpleHttpRendezvousTransport.kt

+/**
+ * Implementation of the Simple HTTP transport MSC3886: https://github.com/matrix-org/matrix-spec-proposals/pull/3886
+ */
+class SimpleHttpRendezvousTransport(rendezvousUri: String?) : RendezvousTransport {


bmarty · 2022-10-18T09:08:53Z

vector/src/main/java/im/vector/app/features/login/qr/QrCodeLoginStatusFragment.kt

-            QrCodeLoginErrorType.DEVICE_IS_NOT_SUPPORTED -> getString(R.string.qr_code_login_header_failed_device_is_not_supported_description)
-            QrCodeLoginErrorType.TIMEOUT -> getString(R.string.qr_code_login_header_failed_timeout_description)
-            QrCodeLoginErrorType.REQUEST_WAS_DENIED -> getString(R.string.qr_code_login_header_failed_denied_description)
+    private fun getErrorDescription(reason: RendezvousFailureReason): String {


I would move this method to DefaultErrorFormatter, and errorType could be a Throwable.

vector/src/main/java/im/vector/app/features/login/qr/QrCodeLoginViewModel.kt

Co-authored-by: Benoit Marty <benoitm@matrix.org>

…ndezvous/RendezvousChannel.kt Co-authored-by: Benoit Marty <benoitm@matrix.org>

…ndezvous/channels/ECDHRendezvousChannel.kt Co-authored-by: Benoit Marty <benoitm@matrix.org>

…ndezvous/RendezvousChannel.kt Co-authored-by: Benoit Marty <benoitm@matrix.org>

…tor-im/element-android into feature/hughns/qr_code_login

hughns · 2022-10-18T11:33:14Z

Nice work.

There is a bit of architecture rework to do on the SDK, but I would not block the PR for that. The only blocking remark is about the logs: #7369 (comment)

Thank you. Blocker should now be resolved.

Re: architecture - I agree this needs work and my plan is to do this when we add in the next round of capabilities about signing in another device from the settings screen in Android.

.../main/java/org/matrix/android/sdk/api/rendezvous/transports/SimpleHttpRendezvousTransport.kt

...ndroid/src/main/java/org/matrix/android/sdk/api/rendezvous/channels/ECDHRendezvousChannel.kt

…tor-im/element-android into feature/hughns/qr_code_login

sonarcloud · 2022-10-19T08:57:33Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

0.0% Coverage
0.0% Duplication

bmarty

LGTM, with the reserve of the architecture update in the future. Thanks!

hughns and others added 30 commits October 13, 2022 15:28

Support for login by m.login.token during QR code sign in

21ae4c6

Changelog

098c268

Linting

a71ecee

Retry scanning if not a QR code

0d24565

Implementations of MSC3886 and MSC3903

1c70d45

Partial implementation of QR login logic

b192fdb

Only do completeOnNewDevice if we received a confirmation code

8609008

Make initialDeviceName optional

ebb3d20

Use correct var name

5843c38

Add missing binding

579df74

Set default value for optional params

b5e81d2

Another default value fix

22b344c

Map for soft logout

8dbb1b8

Support for navigation to home screen

0111b93

QR login + E2EE set up

1ed082d

Reduce logging

560fda5

Support for login by m.login.token during QR code sign in

88238c0

Changelog

282825d

Linting

d0898a2

Retry scanning if not a QR code

3b3e11e

Implementations of MSC3886 and MSC3903

de611ca

Partial implementation of QR login logic

bfab07d

Only do completeOnNewDevice if we received a confirmation code

b032403

Make initialDeviceName optional

1e60f3c

Use correct var name

e2f3dde

Add missing binding

ca7a6ef

Set default value for optional params

ac80ae5

Another default value fix

bc0843e

hughns added 2 commits October 18, 2022 08:48

Use mutex

f297117

Always check master key when provided by verifying device

a1d2944

hughns requested review from BillCarsonFr and removed request for ElementBot October 18, 2022 08:11

BillCarsonFr requested changes Oct 18, 2022

View reviewed changes

hughns added 2 commits October 18, 2022 09:31

Whitespce

57a8dd4

Missing throws

376cd1c

hughns requested a review from BillCarsonFr October 18, 2022 08:58

bmarty requested changes Oct 18, 2022

View reviewed changes

hughns and others added 8 commits October 18, 2022 12:07

Update library/ui-strings/src/main/res/values/strings.xml

9fb0db3

Co-authored-by: Benoit Marty <benoitm@matrix.org>

Update matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/re…

0d1df3f

…ndezvous/RendezvousChannel.kt Co-authored-by: Benoit Marty <benoitm@matrix.org>

Update matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/re…

8530f8f

…ndezvous/channels/ECDHRendezvousChannel.kt Co-authored-by: Benoit Marty <benoitm@matrix.org>

Update matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/re…

a83fb8b

…ndezvous/RendezvousChannel.kt Co-authored-by: Benoit Marty <benoitm@matrix.org>

Don't log whole QR code

916ae65

Merge branch 'feature/hughns/qr_code_login' of https://github.com/vec…

0acbd9c

…tor-im/element-android into feature/hughns/qr_code_login

Reuse getDecimalCodeRepresentation from SAS instead of duplicating code

811d6d8

Remove redundant annotations

f7e0a19

hughns requested a review from bmarty October 18, 2022 11:33

hughns and others added 2 commits October 18, 2022 12:44

The one that got away

67be8c3

Fix layout after try again button is clicked.

0c52a7e

bmarty reviewed Oct 19, 2022

View reviewed changes

.../main/java/org/matrix/android/sdk/api/rendezvous/transports/SimpleHttpRendezvousTransport.kt Outdated Show resolved Hide resolved

bmarty reviewed Oct 19, 2022

View reviewed changes

...ndroid/src/main/java/org/matrix/android/sdk/api/rendezvous/channels/ECDHRendezvousChannel.kt Outdated Show resolved Hide resolved

hughns added 2 commits October 19, 2022 09:12

Request changes from review

4f652f1

Merge branch 'feature/hughns/qr_code_login' of https://github.com/vec…

025a89f

…tor-im/element-android into feature/hughns/qr_code_login

hughns requested a review from bmarty October 19, 2022 09:56

bmarty approved these changes Oct 19, 2022

View reviewed changes

Johennes merged commit 451f5f8 into develop Oct 19, 2022

Johennes deleted the feature/hughns/qr_code_login branch October 19, 2022 15:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement logic for sign in with QR #7369

Implement logic for sign in with QR #7369

hughns commented Oct 13, 2022 •

edited

Loading

BillCarsonFr Oct 18, 2022

bmarty left a comment •

edited

Loading

bmarty Oct 18, 2022

bmarty Oct 18, 2022

bmarty Oct 18, 2022

bmarty Oct 18, 2022

hughns commented Oct 18, 2022

sonarcloud bot commented Oct 19, 2022

bmarty left a comment


		_viewEvents.post(QrCodeLoginViewEvents.NavigateToStatusScreen)

		viewModelScope.launch(Dispatchers.IO) {

	class RendezvousError(val description: String, val reason: RendezvousFailureReason) : Exception(description)
	class RendezvousError(val description: String, val reason: RendezvousFailureReason) : Failure.FeatureFailure()


		val transport = SimpleHttpRendezvousTransport(parsed.rendezvous.transport.uri)

		return Rendezvous(

Implement logic for sign in with QR #7369

Implement logic for sign in with QR #7369

Conversation

hughns commented Oct 13, 2022 • edited Loading

Type of change

Content

Motivation and context

Screenshots / GIFs

Tests

Tested devices

Checklist

BillCarsonFr Oct 18, 2022

Choose a reason for hiding this comment

bmarty left a comment • edited Loading

Choose a reason for hiding this comment

bmarty Oct 18, 2022

Choose a reason for hiding this comment

bmarty Oct 18, 2022

Choose a reason for hiding this comment

bmarty Oct 18, 2022

Choose a reason for hiding this comment

bmarty Oct 18, 2022

Choose a reason for hiding this comment

hughns commented Oct 18, 2022

sonarcloud bot commented Oct 19, 2022

bmarty left a comment

Choose a reason for hiding this comment

hughns commented Oct 13, 2022 •

edited

Loading

bmarty left a comment •

edited

Loading