Desktop: Change Joplin Cloud login process to allow MFA via browser

[pedr]

This is a working-in-progress PR for the new login flow that we are going to use for Joplin Cloud.

This change is required to make the 2FA work in the Desktop.

With this PR, when the user tries to synchronise to Joplin Cloud we will show a new page that will instruct the user on how to authorise the application over Joplin Cloud. This process will involve the creation of a unique value that Joplin Cloud will use to identify the application that allowed the use of the recently created credentials.

Images

There are some missing things for now:

• Generate a proper unique_login_code
• Test if I didn't break anything for Dropbox or OneDrive
• Test login without 2FA (it shouldn't interfere, just a sanity check)
• Include information about the client in the URL for login (platform and type)
• Bug: when clicking in synchronize I'm creating 4 session records in db.
  • Check if this wasn't an issue before.
• Review code from syncWizard/Dialog.tsx, there are things there aren't necessary anymore.
• Make code stateless and move to library to be used in CLI and Mobile.
  • Not sure exactly how much I could move to a library, some parts will be similar like the request the request to api/applications with an interval, but a lot of code here is platform-specific.
• Update platform, type and version on new requests to /api/sessions
  • We probably just need to update only the version value since the other won't change
  • Need to discuss it how do it Desktop: Change Joplin Cloud login process to allow MFA via browser #9445 (comment)

Testing

I still need to add automated tests, maybe I will need to mock so much stuff that won't really be very useful?

Some manual tests that are good to be done:

• Make the proper process, authorize, see synchronisation working.
• Open JoplinCloudLogin page two times and copy the URL, the unique login code should not be unique, not the same
• The page should only start making requests to api/applications after one of the buttons is clicked
• The page should not start a new interval of requests if one is already active
• Closing the page should clear the interval of requests
• Clicking the Synchronise button in the Sidebar when the SyncTarget is set to Joplin Cloud but the login process wasn't completed should open the JopliCloudLogin page

Testing upgrading version without 2FA

• Start an old version of Desktop
• Synchronise to Joplin Cloud
• Upgrade Desktop version
• Check if synchronisation still works (it should, but won't be using application credentials yet, only when user redo the login process)

Testing upgrading version and enabling 2FA

• Start an old version of Desktop
• Synchronise to Joplin Cloud
• Enable 2FA in Joplin Cloud website
• Upgrade Desktop version
• Synchronisation should not be working until user go through the new login process.

[pedr]

I'm struggling to avoid a problem in the new SyncTargetJoplinCloud login.

I'm writing this down so you maybe can help me think about it, or maybe I just to help myself think what are the solutions here. I think this is more of a rambling than a thoughtful explanation of what I'm struggling with, I'm sorry!

---

The scenario that I'm dealing with is what happens when the user is logged-in with an application credentials and his credentials get revoked (excluded via the website interface, for example). If this happens while the user is synching its file, we will get a request a api/sessions to each resource that needs to be downloaded. We need a process for the user to understand what happens (he needs to login again), to avoid the application of crashing (by making a hundreds of requests) and to avoid flooding Joplin Cloud servers with useless requests.

What I'm trying to rethink right now is the process, thinking in two steps of verifying if the credentials are correct (would be something similar to checkConfig) and the process of renewing the token when it expires. What I'm struggling is to understand what should happen when/where.

What I thought as a strategy:

• Have a checkConfig() method in the SyncTargetJoplinServer that can set or disable consequent requests. If the API call fails too many times, we set a flag and disable it while the username/password doesn't change or the application is not restarted, maybe?
  • what happens if the user were offline and is online again? He will need to restart the application to make it work again? Not the best strategy

I don't think this used to be a problem because until now user only had one credential, but now with application credentials I think this might turn out to be an issue.

[pedr]

I'm struggling to avoid a problem in the new SyncTargetJoplinCloud login.

A similar error happens when

• User without 2FA is logged in desktop application in the older version
• Enables 2FA in Joplin Cloud
• Upgrade version of the desktop app
• Tries to synchronise

Again, it can be fixed by going to the synchronisation screen and login-in into Joplin Cloud, but might not be obvious (there is no error message) besides console logs

[pedr]

I pushed another "feature":

When Joplin Cloud is selected as the synchronisation target and the users clicks "Check synchronisation configuration", we check if the user is Authenticated, if it is not we redirect to JoplinCloudLogin screen, if he is we show the happy message:

joplin/packages/app-desktop/gui/ConfigScreen/ConfigScreen.tsx

Lines 63 to 74 in f6f8787

	private async checkSyncConfig_() {
	if (this.state.settings['sync.target'] === SyncTargetRegistry.nameToId('joplinCloud')) {
	const isAuthenticated = await reg.syncTarget().isAuthenticated();
	if (!isAuthenticated) {
	return this.props.dispatch({
	type: 'NAV_GO',
	routeName: 'JoplinCloudLogin',
	});
	}
	}
	await shared.checkSyncConfig(this, this.state.settings);
	}

[pedr]

I update the branch with the fixes and merged with upstream dev.

[pedr]

I changed the implementation to be able to cancel the setInterval and show the error message when the checkIfLoginWasSuccessful function throws an error, I think this is more inline to what is expected (only handle error before showing to the user).

"Failed to fetch" is the error message that is returned when the server is offline, this part will change depending in the error that Joplin Cloud throws.

[pedr]

This PR is ready for review again.

[laurent22]

Does it mean that when trying to fetch unique_login_code but it's not ready yet the server throws an error? If that's what it is I think it should be changed - instead it should tell the status like "in progress", "done", etc. with 200 status code.

Errors should be only when there's an actual error, for example if the code doesn't exist or the server is down. And we need to report these errors to the user so that function should throw an exception with the statusText or whatever info the server provides.

[pedr]

I made the change to follow this principle. I wasn't sure if it was ok to keep the message log, I guess it doesn't make sense from what you saying.

[pedr]

Last changes:

• Flatten the body of request to api/sessions
• Change the URLs for the new Joplin Cloud login process

I also add images about how the feature looks right now at the PR description

[pedr]

Last changes:

• Renamed api/applications endpoint to api/application_auth
• Changed the unique login code value name to application auth id