fix: incorporate review comments

laurent22 · Dec 18, 2024 · caa8e92 · caa8e92
1 parent e09a9b1
commit caa8e92
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 13 deletions.
diff --git a/packages/server/src/config.ts b/packages/server/src/config.ts
@@ -125,7 +125,7 @@ function ldapConfigFromEnv(env: EnvVariables): LdapConfig[] {
 			baseDN: env.LDAP_1_BASE_DN,
 			bindDN: env.LDAP_1_BIND_DN,
 			bindPW: env.LDAP_1_BIND_PW,
-			sslCaFile: env.LDAP_1_SSL_CA_FILE,
+			tlsCaFile: env.LDAP_1_TLS_CA_FILE,
 		});
 	}
 
@@ -139,7 +139,7 @@ function ldapConfigFromEnv(env: EnvVariables): LdapConfig[] {
 			baseDN: env.LDAP_2_BASE_DN,
 			bindDN: env.LDAP_2_BIND_DN,
 			bindPW: env.LDAP_2_BIND_PW,
-			sslCaFile: env.LDAP_2_SSL_CA_FILE,
+			tlsCaFile: env.LDAP_2_TLS_CA_FILE,
 		});
 	}
 	return ldapConfig;

diff --git a/packages/server/src/env.ts b/packages/server/src/env.ts
@@ -137,7 +137,7 @@ const defaultEnvValues: EnvVariables = {
 	LDAP_1_BASE_DN: '',
 	LDAP_1_BIND_DN: '', // used for user search - leave empty if ldap server allows anonymous bind
 	LDAP_1_BIND_PW: '', // used for user search - leave empty if ldap server allows anonymous bind
-	LDAP_1_SSL_CA_FILE: '', // used for self-signed certificate with ldaps - leave empty if using ldap or server uses CA-issued certificate
+	LDAP_1_TLS_CA_FILE: '', // used for self-signed certificate with ldaps - leave empty if using ldap or server uses CA-issued certificate
 
 	LDAP_2_ENABLED: false,
 	LDAP_2_USER_AUTO_CREATION: true, // if set to true, users will be created on the fly after ldap authentication
@@ -147,7 +147,7 @@ const defaultEnvValues: EnvVariables = {
 	LDAP_2_BASE_DN: '',
 	LDAP_2_BIND_DN: '', // used for user search - leave empty if ldap server allows anonymous bind
 	LDAP_2_BIND_PW: '', // used for user search - leave empty if ldap server allows anonymous bind
-	LDAP_2_SSL_CA_FILE: '', // used for self-signed certificate with ldaps - leave empty if using ldap or server uses CA-issued certificate
+	LDAP_2_TLS_CA_FILE: '', // used for self-signed certificate with ldaps - leave empty if using ldap or server uses CA-issued certificate
 
 };
 
@@ -230,7 +230,7 @@ export interface EnvVariables {
 	LDAP_1_BASE_DN: string;
 	LDAP_1_BIND_DN: string;
 	LDAP_1_BIND_PW: string;
-	LDAP_1_SSL_CA_FILE: string;
+	LDAP_1_TLS_CA_FILE: string;
 
 	LDAP_2_ENABLED: boolean;
 	LDAP_2_USER_AUTO_CREATION: boolean;
@@ -240,7 +240,7 @@ export interface EnvVariables {
 	LDAP_2_BASE_DN: string;
 	LDAP_2_BIND_DN: string;
 	LDAP_2_BIND_PW: string;
-	LDAP_2_SSL_CA_FILE: string;
+	LDAP_2_TLS_CA_FILE: string;
 }
 
 const parseBoolean = (s: string): boolean => {

diff --git a/packages/server/src/routes/api/sessions.test.ts b/packages/server/src/routes/api/sessions.test.ts
@@ -86,6 +86,7 @@ describe('api/sessions', () => {
 			baseDN: '',
 			bindDN: '',
 			bindPW: '',
+			tlsCaFile: '',
 		};
 
 		{
@@ -123,6 +124,7 @@ describe('api/sessions', () => {
 			baseDN: '',
 			bindDN: '',
 			bindPW: '',
+			tlsCaFile: '',
 		};
 
 		const context = await postSession(user.email, password);
@@ -151,6 +153,7 @@ describe('api/sessions', () => {
 			baseDN: '',
 			bindDN: '',
 			bindPW: '',
+			tlsCaFile: '',
 		};
 
 		(ldapLogin as jest.Mock).mockResolvedValue(user);
@@ -179,6 +182,7 @@ describe('api/sessions', () => {
 			baseDN: '',
 			bindDN: '',
 			bindPW: '',
+			tlsCaFile: '',
 		};
 
 		(ldapLogin as jest.Mock).mockImplementationOnce(() => {
@@ -203,6 +207,7 @@ describe('api/sessions', () => {
 			baseDN: '',
 			bindDN: '',
 			bindPW: '',
+			tlsCaFile: '',
 		};
 
 		(ldapLogin as jest.Mock).mockImplementationOnce(() => {

diff --git a/packages/server/src/utils/ldapLogin.ts b/packages/server/src/utils/ldapLogin.ts
@@ -3,7 +3,7 @@ import { User } from '../services/database/types';
 import Logger from '@joplin/utils/Logger';
 import { LdapConfig } from './types';
 import { ErrorForbidden } from './errors';
-import { readFileSync } from 'node:fs';
+import { readFile } from 'fs/promises';
 
 const logger = Logger.create('LDAP');
 
@@ -17,7 +17,7 @@ export default async function ldapLogin(email: string, password: string, user: U
 	const baseDN = config.baseDN;
 	const bindDN = config.bindDN;
 	const bindPW = config.bindPW;
-	const sslCaFile = config.sslCaFile;
+	const tlsCaFile = config.tlsCaFile;
 
 	logger.info(`Starting authentication with Server ${host}`);
 
@@ -29,12 +29,10 @@ export default async function ldapLogin(email: string, password: string, user: U
 		let searchResults;
 
 		let tlsOptions;
-		if (sslCaFile.length !== 0) {
+		if (tlsCaFile.length !== 0) {
 			tlsOptions = {
-				ca: [readFileSync(sslCaFile)],
+				ca: [await readFile(tlsCaFile)],
 			};
-		} else {
-			null;
 		}
 
 		const client = new Client({

diff --git a/packages/server/src/utils/types.ts b/packages/server/src/utils/types.ts
@@ -141,7 +141,7 @@ export interface LdapConfig {
 	baseDN: string;
 	bindDN: string;
 	bindPW: string;
-	sslCaFile?: string;
+	tlsCaFile: string;
 }
 
 export interface Config extends EnvVariables {