Server: Fix SAML routes to prevent cookie issues on redirect (#13557)

packages/server/src/routes/api/login.ts

@@ -12,82 +12,86 @@ export const router = new Router(RouteType.Api);
 
 router.public = true;
 
-// Redirect the user to the Identity Provider login page, if they somehow get to this URL directly.
-router.get('api/saml', async (_path: SubPath, _ctx: AppContext) => {
-	if (!config().saml.enabled) throw new ErrorForbidden('SAML not enabled');
-	return await generateRedirectHtml();
-});
+export const setupRoutes = (router: Router) => {
+	// Redirect the user to the Identity Provider login page, if they somehow get to this URL directly.
+	router.get('api/saml', async (_path: SubPath, _ctx: AppContext) => {
+		if (!config().saml.enabled) throw new ErrorForbidden('SAML not enabled');
+		return await generateRedirectHtml();
+	});
 
-// Called when a user successfully authenticated with the Identity Provider, and was redirected to Joplin.
-router.post('api/saml', async (_path: SubPath, ctx: AppContext) => {
-	if (!config().saml.enabled) throw new ErrorForbidden('SAML not enabled');
+	// Called when a user successfully authenticated with the Identity Provider, and was redirected to Joplin.
+	router.post('api/saml', async (_path: SubPath, ctx: AppContext) => {
+		if (!config().saml.enabled) throw new ErrorForbidden('SAML not enabled');
 
-	// Load SAML configuration
-	const [serviceProvider, identityProvider] = await Promise.all([
-		getServiceProvider(),
-		getIdentityProvider(),
-	]);
+		// Load SAML configuration
+		const [serviceProvider, identityProvider] = await Promise.all([
+			getServiceProvider(),
+			getIdentityProvider(),
+		]);
 
-	// Parse the login response
-	const fields = await bodyFields<SamlPostResponse>(ctx.req);
+		// Parse the login response
+		const fields = await bodyFields<SamlPostResponse>(ctx.req);
 
-	const result = await serviceProvider.parseLoginResponse(identityProvider, 'post', { body: fields });
+		const result = await serviceProvider.parseLoginResponse(identityProvider, 'post', { body: fields });
 
-	// Extract attributes from the SAML response
-	const email = result.extract.attributes['email'];
-	const displayName = result.extract.attributes['displayName'];
+		// Extract attributes from the SAML response
+		const email = result.extract.attributes['email'];
+		const displayName = result.extract.attributes['displayName'];
 
-	// Load the user
-	const user = await ctx.joplin.models.user().ssoLogin(email, displayName);
-	if (!user) throw new ErrorForbidden(`Could not login using email "${email}" and displayName "${displayName}"`);
+		// Load the user
+		const user = await ctx.joplin.models.user().ssoLogin(email, displayName);
+		if (!user) throw new ErrorForbidden(`Could not login using email "${email}" and displayName "${displayName}"`);
 
-	if (fields.RelayState) {
-		switch (fields.RelayState) {
-		case 'web-login': { // If the user wanted to load a page from Joplin Server, we set the cookie for this session
-			const session = await ctx.joplin.models.session().createUserSession(user.id);
-			cookieSet(ctx, 'sessionId', session.id);
+		if (fields.RelayState) {
+			switch (fields.RelayState) {
+			case 'web-login': { // If the user wanted to load a page from Joplin Server, we set the cookie for this session
+				const session = await ctx.joplin.models.session().createUserSession(user.id);
+				cookieSet(ctx, 'sessionId', session.id);
 
-			return redirect(ctx, `${config().baseUrl}/home`);
-		}
+				return redirect(ctx, `${config().baseUrl}/home`);
+			}
 
-		case 'app-login': { // If the user came from a client, we display the authentication code
+			case 'app-login': { // If the user came from a client, we display the authentication code
+				await ctx.joplin.models.user().generateSsoCode(user);
+
+				const view = defaultView('displaySsoCode', 'Login');
+
+				view.content = {
+					ssoCode: user.sso_auth_code.replace(/\B(?=(\d{3})+(?!\d))/g, '-'), // Split the code into blocks of three digits each
+					organizationName: config().saml.enabled && config().saml.organizationDisplayName ? config().saml.organizationDisplayName : undefined,
+				};
+
+				return view;
+			}
+			}
+		} else { // Otherwise, just return the authentication code
 			await ctx.joplin.models.user().generateSsoCode(user);
 
-			const view = defaultView('displaySsoCode', 'Login');
+			return { code: user.sso_auth_code };
+		}
+	});
 
-			view.content = {
-				ssoCode: user.sso_auth_code.replace(/\B(?=(\d{3})+(?!\d))/g, '-'), // Split the code into blocks of three digits each
-				organizationName: config().saml.enabled && config().saml.organizationDisplayName ? config().saml.organizationDisplayName : undefined,
+	router.get('api/login_with_code/:id', async (path: SubPath, ctx: AppContext) => {
+		const code = path.id;
+		if (!code) {
+			throw new ErrorBadRequest();
+		}
+
+		const user = await ctx.joplin.models.user().authCodeLogin(code);
+
+		if (user) {
+			const session = await ctx.joplin.models.session().createUserSession(user.id);
+
+			return {
+				id: session.id,
+				user_id: session.user_id,
 			};
-
-			return view;
+		} else { // Invalid auth code
+			throw new ErrorBadRequest();
 		}
-		}
-	} else { // Otherwise, just return the authentication code
-		await ctx.joplin.models.user().generateSsoCode(user);
+	});
+};
 
-		return { code: user.sso_auth_code };
-	}
-});
-
-router.get('api/login_with_code/:id', async (path: SubPath, ctx: AppContext) => {
-	const code = path.id;
-	if (!code) {
-		throw new ErrorBadRequest();
-	}
-
-	const user = await ctx.joplin.models.user().authCodeLogin(code);
-
-	if (user) {
-		const session = await ctx.joplin.models.session().createUserSession(user.id);
-
-		return {
-			id: session.id,
-			user_id: session.user_id,
-		};
-	} else { // Invalid auth code
-		throw new ErrorBadRequest();
-	}
-});
+setupRoutes(router);
 
 export default router;

packages/server/src/routes/index/saml.ts

@@ -0,0 +1,20 @@
+// The SAML routes, which are browser-based, were incorrectly set as API routes, and that cause
+// cookie issues since api.example.com is attempting to set cookies for example.com. We can't just
+// remove the /api/saml routes because some organisations already use them in a setup where they
+// don't have a separate domain for the API (and in this case cookies work). Instead, we create this
+// wrapper that duplicates the routes of /api/saml and make them available under /saml.
+//
+// Unfortunately it means that a non-API route will be available under /api which is confusing but
+// the best way to maintain backward compatibility.
+
+import Router from '../../utils/Router';
+import { RouteType } from '../../utils/types';
+import { setupRoutes } from '../api/login';
+
+export const router = new Router(RouteType.Web);
+
+router.public = true;
+
+setupRoutes(router);
+
+export default router;

packages/server/src/routes/routes.ts

@@ -36,6 +36,7 @@ import indexStripe from './index/stripe';
 import indexTerms from './index/terms';
 import indexUpgrade from './index/upgrade';
 import indexUsers from './index/users';
+import indexSaml from './index/saml';
 
 import defaultRoute from './default';
 
@@ -47,7 +48,7 @@ const routes: Routers = {
 	'api/items': apiItems,
 	'api/locks': apiLocks,
 	'api/ping': apiPing,
-	'api/saml': apiLogin,
+	// 'api/saml': apiLogin,
 	'api/login_with_code': apiLogin,
 	'api/sessions': apiSessions,
 	'api/share_users': apiShareUsers,
@@ -77,6 +78,7 @@ const routes: Routers = {
 	'terms': indexTerms,
 	'upgrade': indexUpgrade,
 	'users': indexUsers,
+	'api/saml': indexSaml,
 
 	'': defaultRoute,
 };