Fixes #27536: Allow removing optional password fields in connection edit form

openmetadata-service/src/main/java/org/openmetadata/service/secrets/DBSecretsManager.java

@@ -13,6 +13,7 @@
 
 package org.openmetadata.service.secrets;
 
+import java.util.Objects;
 import org.openmetadata.schema.security.secrets.SecretsManagerProvider;
 
 public class DBSecretsManager extends SecretsManager {
@@ -33,6 +34,9 @@ public static DBSecretsManager getInstance(
 
   @Override
   protected String storeValue(String fieldName, String value, String secretId, boolean store) {
+    if (Objects.isNull(value) || value.isEmpty()) {
+      return null;
+    }
     return value;
   }
 

openmetadata-service/src/main/java/org/openmetadata/service/secrets/ExternalSecretsManager.java

@@ -33,6 +33,15 @@ protected ExternalSecretsManager(
   @Override
   protected String storeValue(String fieldName, String value, String secretId, boolean store) {
     String fieldSecretId = buildSecretId(false, secretId, fieldName.toLowerCase(Locale.ROOT));
+    if (Objects.isNull(value) || value.isEmpty()) {
+      if (store) {
+        try {
+          deleteSecretInternal(fieldSecretId);
+        } catch (Exception e) {
+        }
+      }
+      return null;
+    }
     // check if value does not start with 'config:' only String can have password annotation
     if (Boolean.FALSE.equals(isSecret(value))) {
       if (store) {

openmetadata-service/src/main/java/org/openmetadata/service/secrets/SecretsManager.java

@@ -414,9 +414,11 @@ private Object encryptPasswordFields(Object toEncryptObject, String secretId, bo
                     // set new value
                     ReflectionUtil.setValueInMethod(
                         toEncryptObject,
-                        Fernet.isTokenized(newFieldValue)
-                            ? newFieldValue
-                            : store ? fernet.encrypt(newFieldValue) : newFieldValue,
+                        isNull(newFieldValue)
+                            ? null
+                            : Fernet.isTokenized(newFieldValue)
+                                ? newFieldValue
+                                : store ? fernet.encrypt(newFieldValue) : newFieldValue,
                         toSet);
                   }
                 });

openmetadata-service/src/test/java/org/openmetadata/service/secrets/DBSecretsManagerTest.java

@@ -51,7 +51,9 @@ static void setUp() {
                 "openmetadata", "prefix", List.of("key:value", "key2:value2"), null));
     Fernet fernet = Mockito.mock(Fernet.class);
     lenient().when(fernet.decrypt(anyString())).thenReturn(DECRYPTED_VALUE);
-    lenient().when(fernet.decryptIfApplies(anyString())).thenReturn(DECRYPTED_VALUE);
+    lenient()
+        .when(fernet.decryptIfApplies(anyString()))
+        .thenAnswer(invocation -> invocation.getArgument(0));
     lenient().when(fernet.encrypt(anyString())).thenReturn(ENCRYPTED_VALUE);
     secretsManager.setFernet(fernet);
   }
@@ -89,6 +91,21 @@ void testDecryptServiceConnectionWithoutPassword() {
     assertNotSame(connection, actualConfig);
   }
 
+  @Test
+  void testEncryptDatabaseConnectionWithEmptyPasswordRemovesValue() {
+    MysqlConnection connection =
+        new MysqlConnection().withAuthType(new basicAuth().withPassword(""));
+    Object actualConfig =
+        secretsManager.encryptServiceConnectionConfig(
+            connection, Mysql.value(), "test-empty-password", ServiceType.DATABASE);
+
+    assertEquals(
+        null,
+        JsonUtils.convertValue(((MysqlConnection) actualConfig).getAuthType(), basicAuth.class)
+            .getPassword());
+    assertNotSame(connection, actualConfig);
+  }
+
   @Test
   void testReturnsExpectedSecretManagerProvider() {
     assertEquals(SecretsManagerProvider.DB, secretsManager.getSecretsManagerProvider());

openmetadata-service/src/test/java/org/openmetadata/service/secrets/ExternalSecretsManagerTest.java

@@ -1,7 +1,6 @@
 package org.openmetadata.service.secrets;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.openmetadata.schema.api.services.CreateDatabaseService.DatabaseServiceType.Mysql;
@@ -236,10 +235,9 @@ void testEncryptDatabaseConnectionWithEmptyPassword() {
                 mysqlConnection, Mysql.value(), "test-empty-password", ServiceType.DATABASE);
 
     assertNotNull(actualConnection, "Encryption should succeed even with empty password");
-    assertFalse(
-        JsonUtils.convertValue(actualConnection.getAuthType(), basicAuth.class)
-            .getPassword()
-            .isEmpty(),
-        "Empty password should be converted to non-empty encrypted value");
+    assertEquals(
+        null,
+        JsonUtils.convertValue(actualConnection.getAuthType(), basicAuth.class).getPassword(),
+        "Empty password should be treated as explicit secret removal");
   }
 }

openmetadata-service/src/test/java/org/openmetadata/service/secrets/KubernetesSecretsManagerTest.java

@@ -261,27 +261,17 @@ void testBuildSecretIdHandlesSpecialCharacters() throws ApiException {
   }
 
   @Test
-  void testEmptySecretValueShouldBeStoredAsNullString() throws ApiException {
-    ArgumentCaptor<V1Secret> secretCaptor = ArgumentCaptor.forClass(V1Secret.class);
-    when(mockApiClient.readNamespacedSecret(anyString(), eq(NAMESPACE))).thenReturn(readRequest);
-    when(readRequest.execute()).thenThrow(new ApiException(404, "Not Found"));
-
-    when(mockApiClient.createNamespacedSecret(eq(NAMESPACE), any(V1Secret.class)))
-        .thenReturn(createRequest);
-    when(createRequest.execute()).thenReturn(new V1Secret());
-
-    secretsManager.storeValue("field", "", SECRET_ID, true);
+  void testEmptySecretValueShouldDeleteSecret() throws ApiException {
+    when(mockApiClient.deleteNamespacedSecret("openmetadata-database-myservice-field", NAMESPACE))
+        .thenReturn(deleteRequest);
+    when(deleteRequest.execute()).thenReturn(new V1Status());
 
-    verify(mockApiClient).createNamespacedSecret(eq(NAMESPACE), secretCaptor.capture());
-    verify(createRequest).execute();
+    String result = secretsManager.storeValue("field", "", SECRET_ID, true);
 
-    V1Secret createdSecret = secretCaptor.getValue();
-    Map<String, byte[]> data = createdSecret.getData();
-    assert data != null;
-    assertEquals(
-        ExternalSecretsManager.NULL_SECRET_STRING,
-        new String(data.get("value"), StandardCharsets.UTF_8),
-        "Empty string should be stored as 'null' to prevent secrets manager rejection");
+    assertNull(result);
+    verify(mockApiClient)
+        .deleteNamespacedSecret("openmetadata-database-myservice-field", NAMESPACE);
+    verify(deleteRequest).execute();
   }
 
   @Test

openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/ServiceForm.spec.ts

@@ -553,6 +553,70 @@ test.describe(
           page.locator(String.raw`#root\/schemaRegistryTopicSuffixName`)
         ).toHaveValue('');
       });
+
+      test('should clear optional saslPassword via remove button and persist the empty value', async ({
+        page,
+      }) => {
+        await visitServiceDetailsPage(
+          page,
+          {
+            name: kafkaService.entity.name,
+            type: SERVICE_TYPE.Messaging,
+          },
+          false,
+          false
+        );
+
+        await page.getByRole('tab', { name: 'Connection' }).click();
+        await page.getByTestId('edit-connection-button').click();
+        await waitForAllLoadersToDisappear(page);
+
+        const removePasswordButton = page.getByTestId(
+          'password-remove-btn-root/saslPassword'
+        );
+
+        await expect(removePasswordButton).toBeVisible();
+        await removePasswordButton.click();
+
+        const passwordInput = page.getByTestId(
+          'password-input-widget-root/saslPassword'
+        );
+
+        await expect(passwordInput).toBeVisible();
+        await expect(passwordInput).toHaveValue('');
+
+        const patchResponse = page.waitForResponse(
+          (response) =>
+            response.url().includes('/api/v1/services/messagingServices') &&
+            response.request().method() === 'PATCH'
+        );
+
+        await page.getByTestId('submit-btn').click();
+        await page.getByTestId('submit-btn').click();
+
+        const patch = await patchResponse;
+        const patchBody = patch.request().postDataJSON() as Array<{
+          op: string;
+          path: string;
+          value?: unknown;
+        }>;
+
+        const passwordOp = patchBody.find(
+          (op) => op.path === '/connection/config/saslPassword'
+        );
+
+        expect(passwordOp).toBeDefined();
+        expect(passwordOp?.value).toBe('');
+
+        await waitForAllLoadersToDisappear(page);
+
+        await page.getByTestId('edit-connection-button').click();
+        await waitForAllLoadersToDisappear(page);
+
+        await expect(
+          page.getByTestId('password-input-widget-root/saslPassword')
+        ).toHaveValue('');
+      });
     });
   }
 );