refactor: update file upload settings validation and handling for optional fields

apps/app/src/server/routes/apiv3/app-settings/file-upload-setting.integ.ts

@@ -32,9 +32,8 @@ describe('file-upload-setting route', () => {
   let app: express.Application;
   let crowiMock: Crowi;
 
-  beforeAll(async() => {
-
-    // Initialize configManager
+  beforeEach(async() => {
+    // Initialize configManager for each test
     const s2sMessagingServiceMock = mock<S2sMessagingService>();
     configManager.setS2sMessagingService(s2sMessagingServiceMock);
     await configManager.loadConfigs();
@@ -84,4 +83,39 @@ describe('file-upload-setting route', () => {
     expect(response.body.responseParams.fileUploadType).toBe('local');
     expect(crowiMock.setUpFileUpload).toHaveBeenCalledWith(true);
   });
+
+  it('should preserve existing s3SecretAccessKey when not included in request', async() => {
+    // Arrange: Set up existing secret in DB
+    const existingSecret = 'existing-secret-key-12345';
+    await configManager.updateConfigs({
+      'app:fileUploadType': 'aws',
+      'aws:s3SecretAccessKey': existingSecret,
+      'aws:s3Region': 'us-west-2',
+      'aws:s3Bucket': 'existing-bucket',
+    });
+    await configManager.loadConfigs(); // Reload configs after update
+
+    // Verify the secret was set
+    const secretBeforeUpdate = configManager.getConfig('aws:s3SecretAccessKey');
+    expect(secretBeforeUpdate).toBe(existingSecret);
+
+    // Act: Update AWS settings without including s3SecretAccessKey
+    const response = await request(app)
+      .put('/')
+      .send({
+        fileUploadType: 'aws',
+        s3Region: 'us-east-1',
+        s3Bucket: 'test-bucket',
+        s3ReferenceFileWithRelayMode: false,
+      })
+      .expect(200);
+
+    // Reload configs to get latest values
+    await configManager.loadConfigs();
+
+    // Assert: Secret should still exist in DB
+    const secretAfterUpdate = configManager.getConfig('aws:s3SecretAccessKey');
+    expect(secretAfterUpdate).toBe(existingSecret);
+    expect(response.body.responseParams.fileUploadType).toBe('aws');
+  });
 });

apps/app/src/server/routes/apiv3/app-settings/file-upload-setting.ts

@@ -51,14 +51,14 @@ type ResponseParams = BaseResponseParams | GcsResponseParams | AwsResponseParams
 const validator = {
   fileUploadSetting: [
     body('fileUploadType').isIn(['aws', 'gcs', 'local', 'gridfs', 'azure']),
-    body('gcsApiKeyJsonPath').trim(),
-    body('gcsBucket').trim(),
-    body('gcsUploadNamespace').trim(),
+    body('gcsApiKeyJsonPath').optional(),
+    body('gcsBucket').optional(),
+    body('gcsUploadNamespace').optional(),
     body('gcsReferenceFileWithRelayMode').if(value => value != null).isBoolean(),
-    body('s3Bucket').trim(),
+    body('s3Bucket').optional(),
     body('s3Region')
-      .trim()
-      .if(value => value !== '')
+      .optional()
+      .if(value => value !== '' && value != null)
       .custom(async(value) => {
         const { t } = await getTranslation();
         if (!/^[a-z]+-[a-z]+-\d+$/.test(value)) {
@@ -67,8 +67,8 @@ const validator = {
         return true;
       }),
     body('s3CustomEndpoint')
-      .trim()
-      .if(value => value !== '')
+      .optional()
+      .if(value => value !== '' && value != null)
       .custom(async(value) => {
         const { t } = await getTranslation();
         if (!/^(https?:\/\/[^/]+|)$/.test(value)) {
@@ -76,14 +76,14 @@ const validator = {
         }
         return true;
       }),
-    body('s3AccessKeyId').trim().if(value => value !== '').matches(/^[\da-zA-Z]+$/),
-    body('s3SecretAccessKey').trim(),
+    body('s3AccessKeyId').optional().if(value => value !== '' && value != null).matches(/^[\da-zA-Z]+$/),
+    body('s3SecretAccessKey').optional(),
     body('s3ReferenceFileWithRelayMode').if(value => value != null).isBoolean(),
-    body('azureTenantId').trim(),
-    body('azureClientId').trim(),
-    body('azureClientSecret').trim(),
-    body('azureStorageAccountName').trim(),
-    body('azureStorageStorageName').trim(),
+    body('azureTenantId').optional(),
+    body('azureClientId').optional(),
+    body('azureClientSecret').optional(),
+    body('azureStorageAccountName').optional(),
+    body('azureStorageStorageName').optional(),
     body('azureReferenceFileWithRelayMode').if(value => value != null).isBoolean(),
   ],
 };
@@ -163,15 +163,36 @@ module.exports = (crowi) => {
             'aws:referenceFileWithRelayMode': req.body.s3ReferenceFileWithRelayMode,
           },
           { skipPubsub: true });
+
+          // Update optional non-secret fields (can be removed if undefined)
           await configManager.updateConfigs({
             'aws:s3CustomEndpoint': toNonBlankStringOrUndefined(req.body.s3CustomEndpoint),
-            'aws:s3AccessKeyId': toNonBlankStringOrUndefined(req.body.s3AccessKeyId),
-            'aws:s3SecretAccessKey': toNonBlankStringOrUndefined(req.body.s3SecretAccessKey),
           },
           {
             skipPubsub: true,
             removeIfUndefined: true,
           });
+
+          // Update secret fields only if explicitly provided in request
+          if (req.body.s3AccessKeyId !== undefined) {
+            await configManager.updateConfigs({
+              'aws:s3AccessKeyId': toNonBlankStringOrUndefined(req.body.s3AccessKeyId),
+            },
+            {
+              skipPubsub: true,
+              removeIfUndefined: true,
+            });
+          }
+
+          if (req.body.s3SecretAccessKey !== undefined) {
+            await configManager.updateConfigs({
+              'aws:s3SecretAccessKey': toNonBlankStringOrUndefined(req.body.s3SecretAccessKey),
+            },
+            {
+              skipPubsub: true,
+              removeIfUndefined: true,
+            });
+          }
         }
         catch (err) {
           const msg = `Error occurred in updating AWS S3 settings: ${err.message}`;
@@ -187,12 +208,21 @@ module.exports = (crowi) => {
             'gcs:referenceFileWithRelayMode': req.body.gcsReferenceFileWithRelayMode,
           },
           { skipPubsub: true });
+
+          // Update optional non-secret fields (can be removed if undefined)
           await configManager.updateConfigs({
-            'gcs:apiKeyJsonPath': toNonBlankStringOrUndefined(req.body.gcsApiKeyJsonPath),
             'gcs:bucket': toNonBlankStringOrUndefined(req.body.gcsBucket),
             'gcs:uploadNamespace': toNonBlankStringOrUndefined(req.body.gcsUploadNamespace),
           },
           { skipPubsub: true, removeIfUndefined: true });
+
+          // Update secret fields only if explicitly provided in request
+          if (req.body.gcsApiKeyJsonPath !== undefined) {
+            await configManager.updateConfigs({
+              'gcs:apiKeyJsonPath': toNonBlankStringOrUndefined(req.body.gcsApiKeyJsonPath),
+            },
+            { skipPubsub: true, removeIfUndefined: true });
+          }
         }
         catch (err) {
           const msg = `Error occurred in updating GCS settings: ${err.message}`;
@@ -208,13 +238,21 @@ module.exports = (crowi) => {
             'azure:referenceFileWithRelayMode': req.body.azureReferenceFileWithRelayMode,
           },
           { skipPubsub: true });
+
+          // Update optional non-secret fields (can be removed if undefined)
           await configManager.updateConfigs({
             'azure:tenantId': toNonBlankStringOrUndefined(req.body.azureTenantId),
             'azure:clientId': toNonBlankStringOrUndefined(req.body.azureClientId),
-            'azure:clientSecret': toNonBlankStringOrUndefined(req.body.azureClientSecret),
             'azure:storageAccountName': toNonBlankStringOrUndefined(req.body.azureStorageAccountName),
             'azure:storageContainerName': toNonBlankStringOrUndefined(req.body.azureStorageContainerName),
           }, { skipPubsub: true, removeIfUndefined: true });
+
+          // Update secret fields only if explicitly provided in request
+          if (req.body.azureClientSecret !== undefined) {
+            await configManager.updateConfigs({
+              'azure:clientSecret': toNonBlankStringOrUndefined(req.body.azureClientSecret),
+            }, { skipPubsub: true, removeIfUndefined: true });
+          }
         }
         catch (err) {
           const msg = `Error occurred in updating Azure settings: ${err.message}`;