All: Add SAML support #11865

ttcchhmm · 2025-02-20T10:16:34Z

This PR adds SAML support to Joplin.

Server

Based on the samlify library that provides the SAML logic flow for Joplin Server.

This adds the following environment variables used as configuration parameters to Joplin Server :

SAML_ENABLED: If set to true, enables SAML support.
DISABLE_BUILTIN_LOGIN_FLOW: If set to true, all auth requests MUST go though SAML. Users can't log-in using Joplin-specific credentials and/or LDAP.
SAML_IDP_CONFIG_FILE: Should be a path to an XML file containing the metadata for the Identity Provider (IDP).
SAML_SP_CONFIG_FILE: Should be a path to an XML file containing the metadata for the Service Provider (SP, in this case Joplin).
SAML_ORGANIZATION_DISPLAY_NAME: Name of the organization, as shown on the log-in screen. Optional.

The XML files are standard SAML IDP/SP metadata that should be created by the identity solution.

Clients

As for the clients themselves, no additional libraries are needed, since the actual log-in process is happening in a web browser, outside of Joplin itself.

It also adds a new sync target, based on the one for Joplin Server: "Joplin Server (Beta, SAML)". We kept "Beta" in the name for this since the main Joplin Server target itself is currently considered as such.

Important

The log-in flow uses a callback to a joplin:// URL, and thus requires that only one instance of Joplin is running at any given time. This is important for the desktop client, since the single instance lock is not enforced in the dev environment.

Log-in flow

The log-in process differs slightly if started from within a client or within the server web interface.

    graph TB;
        12([From the server web interface]) --> 13[The user clicks 'Sign-in using your organization account']

        13 --> 5

        0([From a client]) --> 1

        1[The user chooses the SAML sync target] --> 2[They enter the server's address]
        2 --> 3[They click 'Sign-in using your organization account']
        3 --> 4[The Joplin client opens the default web browser to an URL pointing to Joplin Server]
        4 --> 5[Joplin Server reads its configuration and redirects the browser to the Identity Provider sign-in page]
        5 --> 6[The user signs-in]
        6 --> 7[The Identity Provider generates and signs a SAML payload, and redirects the browser to Joplin Server]
        7 --> 8[Joplin Server validates the payload signature]

        8 -- Client --- 9[If the signature is valid, Joplin Server creates a new token for this user]
        9 --> 10[Using a joplin:// URL, the browser calls back into the client, with the new token]

        8 -- Server web interface --- 11[If the signature is valid, a new session is created]

Testing and development

For development purposes, we used saml-idp as the Identity Provider, as it allows to quickly create new users on the fly and is simple to set up. After generating the keypair (look at the saml-idp documentation to see how), just running npx saml-idp --acsUrl 'http://localhost:22300/api/saml' --audience http://localhost:22300 --issuer 'saml-idp' is enough to get a test Identity Provider running, assuming that Joplin Server is running on localhost:22300.

Since saml-idp does not support generating SP metadata, here is a sample configuration for the Service Provider part :

<?xml version="1.0"?>
<md:EntityDescriptor xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata"
                     validUntil="2024-12-15T08:05:25Z"
                     cacheDuration="PT604800S"
                     entityID="Joplin">
    <md:SPSSODescriptor AuthnRequestsSigned="false" WantAssertionsSigned="false" protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol">
        <md:NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress</md:NameIDFormat>
        <md:AssertionConsumerService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST"
                                     Location="http://localhost:22300/api/saml"
                                     index="1" />
    </md:SPSSODescriptor>
</md:EntityDescriptor>

This is due to changes in the lib package caused by adding SAML support. Currently, the CLI does not support SAML auth, this only fixes regular Joplin Server sync.

github-actions · 2025-02-20T10:16:50Z

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

ttcchhmm · 2025-02-20T10:37:57Z

I have read the CLA Document and I hereby sign the CLA

laurent22 · 2025-02-20T16:23:25Z

Thanks for creating this pull request! At the moment it has some issues related to the linter, which you should be able to fix by running yarn linter ./ from the root. Would you mind fixing this so that CI passes? As it's a large PR it might take a while before we can review it but as a first step it would help if it doesn't have any CI error

ttcchhmm · 2025-02-21T09:03:47Z

I fixed the issues related to the linter.

However, the server image does not build since the XML schema validator (@authenio/samlify-xsd-schema-validator) has a dependency on a Java environment that is missing in the CI. If needed I can replace this dependency with something else since samlify supports several validators, but this validator is the one recommended in the samlify README file, so I hope this is not an issue?

laurent22 · 2025-02-21T10:46:59Z

Hmm, if it's just to validate an XML schema I guess it's not worth adding a Java dependency? From their doc it looks like there's a TypeScript package too?

xsd-schema-validator is replaced with xmllint-wasm, to remove any dependency to another program

ttcchhmm · 2025-02-21T13:34:08Z

I got rid of the Java dependency, and replaced the schema validator with @authenio/samlify-xmllint-wasm, which doesn't require any native code. This allows the server image to build.

ttcchhmm · 2025-03-26T15:37:38Z

Sorry to bump this, but is there any update about this?

laurent22 · 2025-03-26T18:51:00Z

Hello, sorry for the lack of feedback yet. As this is a large pull request I will need more time to review it.

The fact that it is deeply integrated to both the server and apps mean there will be maintenance concerns since unlike for example a sync target that works independently, the new code will have to be maintained by us probably over time. I don't assume it's possible to make things a bit more modular? i.e. most of the code in new files, and a few integration points here and there;

Add comments, and clean up the code a bit

ttcchhmm · 2025-03-27T15:18:20Z

I don't think I can make the code more modular than it currently is. I refactored it a bit to help with your review (mostly by adding comments, but also by doing a bit of cleanup), and merged the current dev branch into mine. The only conflict I got was on a dictionary file, and after testing everything was still working as intended (on the SAML side, at least), so even if it's only one limited example it seems rather positive.

The new SAML sync target extends the base Joplin Server one, so on this topic I think I made things more modular than they were before by allowing the sync target to provide a session to JoplinServerApi, in addition to the regular login/password combo.

Unfortunately since the SAML login process relies on a web browser, I didn't have a choice but to rely on a callback from a web page which means that I needed to use the x-callback-url scheme, and thus I had to add a new command that can be called externally (which means modifying part of the existing code). I don't think I can do this in a different way other than using a webview within the apps, and doing this would defeat part of the security benefit that comes with SAML. Furthermore, doing it that way would need even more code in the clients to handle this, so I think the current approach is the best one.

And on the server, I had to modify UserModel since the only way to authenticate before was with a username and a password. This was required since the response from the Identity Provider is trusted (and we don't have the password anyways), and we need to create new users dynamically if they are not already known by the server. I also had to modify the /login route to add redirects as needed. Otherwise, the other modifications should be rather minor, and most of the actual SAML-related logic is within routes/api/login.ts and utils/saml.ts, which are new files.

If you have any tips about making this better, feel free to tell me so I can implement them.

laurent22 · 2025-03-27T21:03:53Z

Thank you for the detailed explanation, it does help. I also need to go back to your top post and check the diagram again so I understand things at a higher level before diving into the code review. I will try to do that as soon as possible and will get back to you.

laurent22

Thank you for the pull request. It looks good overall, although my main concern is whether it could add new security vulnerabilities, but I didn't notice anything obvious.

Regarding the use of x-callback-url - this is often not very reliable. I'm not sure it works in all operating systems and most browsers don't render these links for security reasons. Also does it work on mobile?

As an alternative, at least on desktop, have you considered launching a small server from the Joplin app and having the browser call some URL on that server? This is what we do for OneDrive in onedrive-api-node-utils.js for example.

Another solution could to be have the browser display some code that the user would need to copy and paste into the app? (that's what Dropbox does)

laurent22 · 2025-04-01T13:26:48Z

packages/app-desktop/main.js

+if (process.defaultApp) {
+	if (process.argv.length >= 2) {
+		electronApp.setAsDefaultProtocolClient('joplin', process.execPath, [path.resolve(process.argv[1])]);
+	}
+} else {
+	electronApp.setAsDefaultProtocolClient('joplin');
+}


We need comments here to explain what this does.

I'm a bit concerned about security issues too as I know we had one with xCallbackUrl before

I originally did this to allows the joplin:// protocol to work properly within my development environment, since otherwise the protocol was not properly registered (it registered the electron binary, but not the main script needed to start Joplin). In production using just electronApp.setAsDefaultProtocolClient('joplin') is fine since the executable bundles both Electron and the main script. I took this snippet from the Electron docs: https://www.electronjs.org/docs/latest/tutorial/launch-app-from-url-in-another-app.

Since I removed the dependency on x-callback-url, I also reverted this, since this is likely out of the scope of this PR now.

laurent22 · 2025-04-01T13:30:54Z

packages/app-mobile/components/screens/ConfigScreen/ConfigScreen.tsx

+			if (canOpen) {
+				await Linking.openURL(link);
+			} else {
+				Alert.alert(_('Warning'), _('No web browser is installed on your device.'));


Is that even possible? And if it, do we need to handle that error?

And the comment above mentions "sso-saml-app" - so isn't it what we want support for?

FYI: As someone who has debugged an issue where the problem was a device without a browser, this is definitely possible, especially with devices that are company owned

laurent22 · 2025-04-01T13:48:36Z

packages/app-mobile/components/screens/ConfigScreen/ConfigScreen.tsx

@@ -460,6 +485,12 @@ class ConfigScreenComponent extends BaseScreenComponent<ConfigScreenProps, Confi

 					if (settings['sync.target'] === SyncTargetRegistry.nameToId('joplinCloud')) {
 						addSettingButton('go_to_joplin_cloud_login_button', _('Connect to Joplin Cloud'), this.goToJoplinCloudLogin_);
+					} else if (settings['sync.target'] === SyncTargetRegistry.nameToId('joplinServerSaml')) {
+						addSettingButton('login_joplin_server_saml_button', _('Connect using your organization account'), this.goToJoplinServerSamlLogin_);


Suggested change

addSettingButton('login_joplin_server_saml_button', _('Connect using your organization account'), this.goToJoplinServerSamlLogin_);

addSettingButton('login_joplin_server_saml_button', _('Connect using your organisation account'), this.goToJoplinServerSamlLogin_);

laurent22 · 2025-04-01T13:49:12Z

packages/app-mobile/components/screens/ConfigScreen/ConfigScreen.tsx

+						addSettingButton('login_joplin_server_saml_button', _('Connect using your organization account'), this.goToJoplinServerSamlLogin_);
+
+						if (Setting.value('sync.11.id') !== '' || Setting.value('sync.11.user_id') !== '') {
+							addSettingButton('logout_joplin_server_saml_button', _('Log out of Joplin Server'), this.logoutJoplinServerSaml_);


Suggested change

addSettingButton('logout_joplin_server_saml_button', _('Log out of Joplin Server'), this.logoutJoplinServerSaml_);

addSettingButton('logout_joplin_server_saml_button', _('Logout'), this.logoutJoplinServerSaml_);

laurent22 · 2025-04-01T13:49:33Z

packages/app-mobile/root.tsx

@@ -1197,6 +1199,10 @@ class AppComponent extends React.Component {
 			});
 			break;

+		case CallbackUrlCommand.SamlLogin:
+			saveTokens(params.id, params.user_id);
+			Alert.alert(_('Synchronization'), _('You are now logged into your organization account.'));


Suggested change

Alert.alert(_('Synchronization'), _('You are now logged into your organization account.'));

Alert.alert(_('Synchronisation'), _('You are now logged into your organisation account.'));

laurent22 · 2025-04-01T14:36:21Z

packages/server/src/utils/auth.ts

+	// If no hash is stored for a given user, that means that this user was created using a SSO solution
+	// such as a SAML login, and is expected to log in through that.
+	if (!hash) {
+		return false;
+	}
+


I'm not a fan of this. Why would this function be used if the user doesn't have a password? Maybe the caller needs to check for this?

laurent22 · 2025-04-01T14:37:22Z

packages/server/src/utils/saml.ts

+// Checks if SAML support is enabled.
+//
+// Throws an error otherwise.
+function checkIfSamlIsEnabled() {


Please use arrow functions only (see coding style guide). All functions here should be converted to arrow functions

laurent22 · 2025-04-01T14:37:37Z

packages/server/src/utils/saml.ts

+// @param relayState The relay state to use for any subsequent login requests.
+// @returns A ServiceProvider object.


no jsdoc - applies to all code in this file

laurent22 · 2025-04-01T14:38:27Z

packages/server/src/utils/saml.ts

+    <title>${_('Joplin SSO Authentication')}</title>
+</head>
+<body>
+    <p>${_('Please wait while we load your organization sign-in page...')}</p>


We use British English in user facing strings - organisation (in code we use US English)

laurent22 · 2025-04-01T14:40:53Z

packages/server/src/views/index/login.mustache

+
+		<div class="columns">
+			<form action="{{{global.baseUrl}}}/login" method="POST" class="column">
+				<h2 class="title">Login to {{global.appName}}</h1>
+				<p class="subtitle">Please input your details to login to {{global.appName}}</p>
+
+				<div class="field">
+					<label class="label">Email</label>
+					<div class="control">
+						<input class="input" type="email" name="email"/>
+					</div>
+				</div>
+				<div class="field">
+					<label class="label">Password</label>
+					<div class="control">
+						<input class="input" type="password" name="password"/>
+					</div>
+					<p class="help"><a href="{{{global.baseUrl}}}/password/forgot">I forgot my password</a></p>


Any reason why the default login method was changed? I would expect only the SAML block needs to be added?

I changed it to display the SAML block on the right of the default login form.

ttcchhmm · 2025-04-10T08:59:09Z

I updated my branch with your feedback. I removed the use of x-callback-url in favor of a code-based authentication flow, like the way Dropbox works. I did it that way to keep the same experience between the desktop and mobile clients.

This required additional modifications to the server side of things to handle auth codes (mostly UserModel.ts and api/login.ts). I also added a new task that runs every 15 minutes that removes expired auth codes from the database.

On the clients, the removal of the x-callback-url based flow makes things a bit cleaner. I added a new screen dedicated to opening the login page and entering an auth code. I made it modular (it's not specific to SAML), so it's possible to port the Dropbox login screen to it in the future.

This is tested on desktop (Windows 10) and mobile (both Android and iOS).

laurent22

Thanks @ttcchhmm, that looks very good. I left a few comments which hopefully shouldn't be hard to address. If you have any question feel free to let me know!

laurent22 · 2025-04-24T15:48:21Z

packages/app-desktop/gui/MainScreen.tsx

+
+		if (Object.keys(params).length === 1 && params.id) { // Single argument
+			void CommandService.instance().execute(command.toString(), params.id);
+		} else { // Multiple arguments
+			void CommandService.instance().execute(command.toString(), ...Object.values(params));
+		}


Is you no longer use x-callback-url, do we still need this? (and other x-callback-url related changes)

laurent22 · 2025-04-24T15:49:35Z

packages/app-desktop/gui/SsoLoginScreen.tsx

+		<div style={{ display: 'flex', flexDirection: 'column', height: '100%' }}>
+			<div style={containerStyle}>
+				<p style={theme.textStyle}>{_('To allow Joplin to synchronise with your account, please follow these steps:')}</p>
+				<ol>
+					<li style={listItemStyle}>
+						<button style={buttonStyle} onClick={props.shared.openLoginPage}>{_('Log in with your web browser')}</button>
+					</li>
+					<li style={listItemStyle}>
+						<div>
+							<label htmlFor='sso-code' style={theme.textStyle}>{_('Enter the code:')}</label>
+							<input id='sso-code' type='text' style={inputStyle} value={code} onChange={e => setCode(e.target.value)} placeholder='###-###-###' />
+						</div>
+					</li>
+					<li style={listItemStyle}>
+						<button type='submit' onClick={submit} disabled={!props.shared.isLoginCodeValid(code)} style={buttonStyle}>{_('Continue')}</button>
+					</li>
+				</ol>
+			</div>
+
+			<ButtonBar onCancelClick={back} />
+		</div>
+	);


this would need to be styled using RSCSS. See here:

https://joplinapp.org/help/dev/spec/desktop_styling/

laurent22 · 2025-04-24T15:53:38Z

packages/lib/models/settings/builtInMetadata.ts

+			value: '',
+			type: SettingItemType.String,
+			section: 'sync',
+			// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied


For new code please don't use "any". I believe you can get the type here using BuiltInMetadataKeys?

The show function gets the type of its any parameter from the SettingItem interface, and if I change that to BuiltInMetadataKeys I get an error since that creates a circular dependency within the types themselves. This change would also require additional modifications to code that use this function (like the other settings entries in this file, or anything that relies on config-shared.ts). I don't know what to do here, so maybe you have a clue on where to start?

laurent22 · 2025-04-24T15:55:46Z

packages/server/src/utils/types.ts

+	identityProviderConfigFile?: string;
+	serviceProviderConfigFile?: string;
+	organizationDisplayName?: string;


Please make all these properties compulsory: https://joplinapp.org/help/dev/coding_style/#avoid-default-and-optional-parameters

Just set them to an empty string if not in use

ttcchhmm · 2025-04-25T10:46:33Z

Do I have to do something to fix the CLA CI job? I did sign the CLA when I started this pull request, so this should pass.

ttcchhmm added 3 commits February 20, 2025 09:17

Server: Add SAML support

1f3394d

Desktop, Mobile: Add SAML support

680b816

CLI: Fix Joplin Server sync due to lib changes

f751849

This is due to changes in the lib package caused by adding SAML support. Currently, the CLI does not support SAML auth, this only fixes regular Joplin Server sync.

github-actions bot added a commit that referenced this pull request Feb 20, 2025

@ttcchhmm has signed the CLA in #11865

5a81e2c

Server: Fix linter errors

7a53630

Server: Remove Java dependency

b90724d

xsd-schema-validator is replaced with xmllint-wasm, to remove any dependency to another program

ttcchhmm added 2 commits March 27, 2025 09:50

Merge branch 'dev' into saml

798c85d

All: Refactor SAML-related code

c9b8d49

Add comments, and clean up the code a bit

laurent22 reviewed Apr 1, 2025

View reviewed changes

ttcchhmm added 7 commits April 2, 2025 09:21

Merge branch 'dev' into saml

ab8e4e4

Update spelling

1365d0c

Fix coding style

39314a8

Remove empty password check

4fecb62

Add comments regarding protocol registration

1018344

Switch to a code-based auth flow instead of using a callback

06ffe8b

Merge branch 'dev' into saml

7093f0b

Clean up

c3833a9

laurent22 reviewed Apr 24, 2025

View reviewed changes

Merge branch 'dev' into saml

353a0b9

ttcchhmm added 3 commits April 25, 2025 09:51

Remove a x-callback-url related change

fa060f3

Fix config typings

8806381

Fix the styling of SsoLoginScreen

acc1db7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

All: Add SAML support #11865

All: Add SAML support #11865

ttcchhmm commented Feb 20, 2025

github-actions bot commented Feb 20, 2025 •

edited

Loading

ttcchhmm commented Feb 20, 2025

laurent22 commented Feb 20, 2025

ttcchhmm commented Feb 21, 2025

laurent22 commented Feb 21, 2025

ttcchhmm commented Feb 21, 2025

ttcchhmm commented Mar 26, 2025

laurent22 commented Mar 26, 2025

ttcchhmm commented Mar 27, 2025

laurent22 commented Mar 27, 2025

laurent22 left a comment

laurent22 Apr 1, 2025

laurent22 Apr 1, 2025

ttcchhmm Apr 10, 2025

laurent22 Apr 1, 2025

JifScharp Apr 7, 2025

laurent22 Apr 1, 2025

laurent22 Apr 1, 2025

laurent22 Apr 1, 2025

laurent22 Apr 1, 2025

laurent22 Apr 1, 2025

laurent22 Apr 1, 2025

laurent22 Apr 1, 2025

laurent22 Apr 1, 2025

ttcchhmm Apr 10, 2025

ttcchhmm commented Apr 10, 2025

laurent22 left a comment

laurent22 Apr 24, 2025

laurent22 Apr 24, 2025

laurent22 Apr 24, 2025

ttcchhmm Apr 25, 2025

laurent22 Apr 24, 2025

ttcchhmm commented Apr 25, 2025

	addSettingButton('login_joplin_server_saml_button', _('Connect using your organization account'), this.goToJoplinServerSamlLogin_);
	addSettingButton('login_joplin_server_saml_button', _('Connect using your organisation account'), this.goToJoplinServerSamlLogin_);

	addSettingButton('logout_joplin_server_saml_button', _('Log out of Joplin Server'), this.logoutJoplinServerSaml_);
	addSettingButton('logout_joplin_server_saml_button', _('Logout'), this.logoutJoplinServerSaml_);

	Alert.alert(_('Synchronization'), _('You are now logged into your organization account.'));
	Alert.alert(_('Synchronisation'), _('You are now logged into your organisation account.'));

		// @param relayState The relay state to use for any subsequent login requests.
		// @returns A ServiceProvider object.

All: Add SAML support #11865

Are you sure you want to change the base?

All: Add SAML support #11865

Conversation

ttcchhmm commented Feb 20, 2025

Server

Clients

Log-in flow

Testing and development

github-actions bot commented Feb 20, 2025 • edited Loading

ttcchhmm commented Feb 20, 2025

laurent22 commented Feb 20, 2025

ttcchhmm commented Feb 21, 2025

laurent22 commented Feb 21, 2025

ttcchhmm commented Feb 21, 2025

ttcchhmm commented Mar 26, 2025

laurent22 commented Mar 26, 2025

ttcchhmm commented Mar 27, 2025

laurent22 commented Mar 27, 2025

laurent22 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ttcchhmm commented Apr 10, 2025

laurent22 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ttcchhmm commented Apr 25, 2025

github-actions bot commented Feb 20, 2025 •

edited

Loading