Fix EncryptedValue handling in lookup adapter forms (7.0)

changelog/unreleased/pr-25244.toml

@@ -0,0 +1,4 @@
+type = "fixed"
+message = "Fix deserialization error when editing GreyNoise Quick IP Lookup data adapter."
+
+pulls = ["25244"]

graylog2-server/src/main/java/org/graylog/integrations/dataadapters/GreyNoiseQuickIPDataAdapter.java

@@ -21,6 +21,7 @@
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonInclude;
 import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.annotation.JsonTypeName;
 import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
 import com.google.auto.value.AutoValue;
@@ -46,8 +47,8 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import jakarta.annotation.Nonnull;
 import jakarta.inject.Inject;
-
 import jakarta.validation.constraints.NotEmpty;
 
 import java.io.IOException;
@@ -236,10 +237,27 @@ public static abstract class Config implements LookupDataAdapterConfiguration {
         @NotEmpty
         public abstract EncryptedValue apiToken();
 
+        public abstract Builder toBuilder();
+
         public static Builder builder() {
             return new AutoValue_GreyNoiseQuickIPDataAdapter_Config.Builder();
         }
 
+        @Override
+        @JsonIgnore
+        public LookupDataAdapterConfiguration prepareConfigUpdate(@Nonnull LookupDataAdapterConfiguration newConfig) {
+            final Config newGreyNoiseConfig = (Config) newConfig;
+            EncryptedValue newApiToken = newGreyNoiseConfig.apiToken();
+
+            if (newApiToken.isKeepValue()) {
+                newApiToken = apiToken();
+            } else if (newApiToken.isDeleteValue()) {
+                newApiToken = EncryptedValue.createUnset();
+            }
+
+            return newGreyNoiseConfig.toBuilder().apiToken(newApiToken).build();
+        }
+
         @AutoValue.Builder
         public abstract static class Builder {
             @JsonCreator

graylog2-server/src/main/java/org/graylog2/plugin/lookup/LookupDataAdapterConfiguration.java

@@ -22,6 +22,8 @@
 import com.google.common.collect.Multimap;
 import org.graylog2.lookup.adapters.LookupDataAdapterValidationContext;
 
+import jakarta.annotation.Nonnull;
+
 import java.util.Optional;
 
 @JsonTypeInfo(
@@ -70,4 +72,22 @@ default Optional<Multimap<String, String>> validate(LookupDataAdapterValidationC
     default boolean isCloudCompatible() {
         return true;
     }
+
+    /**
+     * Prepares the config for an update by merging encrypted values from the existing config.
+     * <p>
+     * When a config contains {@link org.graylog2.security.encryption.EncryptedValue} fields,
+     * the client sends {@code {"keep_value": true}} to indicate the existing value should be preserved.
+     * Implementations must override this method to replace such sentinel values with the actual
+     * encrypted values from the existing (this) config.
+     * <p>
+     * This method is called on the <b>existing</b> config with the <b>new</b> config as argument.
+     *
+     * @param newConfig the incoming config from the update request
+     * @return the merged config with encrypted values properly resolved
+     */
+    @JsonIgnore
+    default LookupDataAdapterConfiguration prepareConfigUpdate(@Nonnull LookupDataAdapterConfiguration newConfig) {
+        return newConfig;
+    }
 }

graylog2-server/src/main/java/org/graylog2/rest/resources/system/lookup/LookupTableResource.java

@@ -69,6 +69,7 @@
 import org.graylog2.lookup.dto.LookupTableDto;
 import org.graylog2.plugin.lookup.LookupCache;
 import org.graylog2.plugin.lookup.LookupDataAdapter;
+import org.graylog2.plugin.lookup.LookupDataAdapterConfiguration;
 import org.graylog2.plugin.lookup.LookupPreview;
 import org.graylog2.plugin.lookup.LookupResult;
 import org.graylog2.plugin.rest.ValidationResult;
@@ -628,7 +629,15 @@ public DataAdapterApi updateAdapter(@ApiParam(name = "idOrName") @PathParam("idO
                                         @Valid @ApiParam DataAdapterApi toUpdate) {
         checkLookupAdapterId(idOrName, toUpdate);
         checkPermission(RestPermissions.LOOKUP_TABLES_EDIT, toUpdate.id());
-        DataAdapterDto saved = dbDataAdapterService.saveAndPostEvent(toUpdate.toDto());
+
+        final DataAdapterDto existingDto = dbDataAdapterService.get(idOrName)
+                .orElseThrow(() -> new NotFoundException("Data adapter <" + idOrName + "> not found."));
+
+        final LookupDataAdapterConfiguration mergedConfig =
+                existingDto.config().prepareConfigUpdate(toUpdate.config());
+        final DataAdapterDto dtoToSave = toUpdate.toDto().toBuilder().config(mergedConfig).build();
+
+        DataAdapterDto saved = dbDataAdapterService.saveAndPostEvent(dtoToSave);
 
         return DataAdapterApi.fromDto(saved);
     }

graylog2-server/src/test/java/org/graylog/integrations/dataadapters/GreyNoiseQuickIPDataAdapterConfigTest.java

@@ -0,0 +1,107 @@
+/*
+ * Copyright (C) 2020 Graylog, Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the Server Side Public License, version 1,
+ * as published by MongoDB, Inc.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * Server Side Public License for more details.
+ *
+ * You should have received a copy of the Server Side Public License
+ * along with this program. If not, see
+ * <http://www.mongodb.com/licensing/server-side-public-license>.
+ */
+package org.graylog.integrations.dataadapters;
+
+import org.graylog2.plugin.lookup.LookupDataAdapterConfiguration;
+import org.graylog2.security.encryption.EncryptedValue;
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+class GreyNoiseQuickIPDataAdapterConfigTest {
+
+    private static final EncryptedValue EXISTING_TOKEN = EncryptedValue.builder()
+            .value("encrypted-secret")
+            .salt("test-salt")
+            .isKeepValue(false)
+            .isDeleteValue(false)
+            .build();
+
+    private GreyNoiseQuickIPDataAdapter.Config existingConfig() {
+        return GreyNoiseQuickIPDataAdapter.Config.builder()
+                .type(GreyNoiseQuickIPDataAdapter.NAME)
+                .apiToken(EXISTING_TOKEN)
+                .build();
+    }
+
+    @Test
+    void prepareConfigUpdatePreservesTokenWhenKeepValue() {
+        final GreyNoiseQuickIPDataAdapter.Config existing = existingConfig();
+        final GreyNoiseQuickIPDataAdapter.Config incoming = GreyNoiseQuickIPDataAdapter.Config.builder()
+                .type(GreyNoiseQuickIPDataAdapter.NAME)
+                .apiToken(EncryptedValue.createWithKeepValue())
+                .build();
+
+        final LookupDataAdapterConfiguration result = existing.prepareConfigUpdate(incoming);
+
+        assertThat(result).isInstanceOf(GreyNoiseQuickIPDataAdapter.Config.class);
+        final GreyNoiseQuickIPDataAdapter.Config resultConfig = (GreyNoiseQuickIPDataAdapter.Config) result;
+        assertThat(resultConfig.apiToken()).isEqualTo(EXISTING_TOKEN);
+        assertThat(resultConfig.apiToken().isSet()).isTrue();
+    }
+
+    @Test
+    void prepareConfigUpdateClearsTokenWhenDeleteValue() {
+        final GreyNoiseQuickIPDataAdapter.Config existing = existingConfig();
+        final GreyNoiseQuickIPDataAdapter.Config incoming = GreyNoiseQuickIPDataAdapter.Config.builder()
+                .type(GreyNoiseQuickIPDataAdapter.NAME)
+                .apiToken(EncryptedValue.createWithDeleteValue())
+                .build();
+
+        final LookupDataAdapterConfiguration result = existing.prepareConfigUpdate(incoming);
+
+        final GreyNoiseQuickIPDataAdapter.Config resultConfig = (GreyNoiseQuickIPDataAdapter.Config) result;
+        assertThat(resultConfig.apiToken().isSet()).isFalse();
+        assertThat(resultConfig.apiToken().isKeepValue()).isFalse();
+        assertThat(resultConfig.apiToken().isDeleteValue()).isFalse();
+    }
+
+    @Test
+    void prepareConfigUpdateUsesNewTokenWhenSetValue() {
+        final GreyNoiseQuickIPDataAdapter.Config existing = existingConfig();
+        final EncryptedValue newToken = EncryptedValue.builder()
+                .value("new-encrypted-value")
+                .salt("new-salt")
+                .isKeepValue(false)
+                .isDeleteValue(false)
+                .build();
+        final GreyNoiseQuickIPDataAdapter.Config incoming = GreyNoiseQuickIPDataAdapter.Config.builder()
+                .type(GreyNoiseQuickIPDataAdapter.NAME)
+                .apiToken(newToken)
+                .build();
+
+        final LookupDataAdapterConfiguration result = existing.prepareConfigUpdate(incoming);
+
+        final GreyNoiseQuickIPDataAdapter.Config resultConfig = (GreyNoiseQuickIPDataAdapter.Config) result;
+        assertThat(resultConfig.apiToken()).isEqualTo(newToken);
+    }
+
+    @Test
+    void defaultPrepareConfigUpdateReturnsNewConfig() {
+        // Verify the default interface method just passes through
+        final GreyNoiseQuickIPDataAdapter.Config config = existingConfig();
+        final GreyNoiseQuickIPDataAdapter.Config incoming = GreyNoiseQuickIPDataAdapter.Config.builder()
+                .type(GreyNoiseQuickIPDataAdapter.NAME)
+                .apiToken(EncryptedValue.createUnset())
+                .build();
+
+        // The override should still work, but let's verify the result is the incoming config when a new value is set
+        final LookupDataAdapterConfiguration result = config.prepareConfigUpdate(incoming);
+        final GreyNoiseQuickIPDataAdapter.Config resultConfig = (GreyNoiseQuickIPDataAdapter.Config) result;
+        assertThat(resultConfig.apiToken()).isEqualTo(EncryptedValue.createUnset());
+    }
+}

graylog2-web-interface/src/components/lookup-tables/adapter-form/AdapterForm.tsx

@@ -14,7 +14,7 @@
  * along with this program. If not, see
  * <http://www.mongodb.com/licensing/server-side-public-license>.
  */
-import React from 'react';
+import React, { useMemo } from 'react';
 import styled from 'styled-components';
 import { Formik, Form } from 'formik';
 
@@ -79,10 +79,18 @@ const DataAdapterForm = ({ type, title, saved, onCancel, create = false, dataAda
   const { updateAdapter, updatingAdapter } = useUpdateAdapter();
 
   const adapterPlugins = usePluginEntities('lookupTableAdapters');
-  const plugin = React.useMemo(() => adapterPlugins.find((p) => p.type === type), [adapterPlugins, type]);
+  const plugin = adapterPlugins.find((p) => p.type === type);
 
-  const DocComponent = React.useMemo(() => plugin?.documentationComponent, [plugin]);
-  const pluginDisplayName = React.useMemo(() => plugin?.displayName || type, [plugin, type]);
+  const DocComponent = plugin?.documentationComponent;
+  const pluginDisplayName = plugin?.displayName || type;
+
+  const initialValues = useMemo(() => {
+    if (plugin?.prepareConfig && dataAdapter?.config) {
+      return { ...dataAdapter, config: plugin.prepareConfig(dataAdapter.config) };
+    }
+
+    return dataAdapter;
+  }, [dataAdapter, plugin]);
 
   const handleSubmit = async (values: LookupTableAdapter) => {
     const promise = create ? createAdapter(values) : updateAdapter(values);
@@ -100,16 +108,13 @@ const DataAdapterForm = ({ type, title, saved, onCancel, create = false, dataAda
     });
   };
 
-  const canModify = React.useMemo(
-    () => create || (!loadingScopePermissions && scopePermissions?.is_mutable),
-    [create, loadingScopePermissions, scopePermissions?.is_mutable],
-  );
+  const canModify = create || (!loadingScopePermissions && scopePermissions?.is_mutable);
 
   return (
     <RowContainer style={{ flexGrow: 1 }} $withDocs={!!DocComponent}>
       <Col style={{ flexGrow: 1, height: '100%' }}>
         <Title title={title} typeName={pluginDisplayName} create={create} />
-        <Formik initialValues={dataAdapter} onSubmit={handleSubmit} validateOnBlur={false} enableReinitialize>
+        <Formik initialValues={initialValues} onSubmit={handleSubmit} validateOnBlur={false} enableReinitialize>
           {({ isSubmitting, isValid }) => (
             <FlexForm className="form form-horizontal">
               <Row $gap="xl" style={{ flexGrow: 1 }}>

graylog2-web-interface/src/components/lookup-tables/adapter-form/AdapterTypeSelect.tsx

@@ -63,28 +63,16 @@ function AdapterTypeSelect({ adapterConfigType, onAdapterChange }: Props) {
     return [];
   }, [types, fetchingDataAdapterTypes, adapterPlugins]);
 
-  const _getCorrectUserpasswd = (config: LookupTableAdapter['config']) => {
-    if (config.user_passwd.is_set) return { is_set: true, keep_value: true };
-
-    return { set_value: '' };
-  };
-
   const handleTypeSelect = React.useCallback(
     (adapterType: string) => {
       const defaultConfig = { ...types[adapterType].default_config };
-      const isLDAP = defaultConfig.type === 'LDAP';
-
-      const configWithPassword = {
-        ...defaultConfig,
-        ...(isLDAP ? { user_passwd: _getCorrectUserpasswd(defaultConfig) } : {}),
-      };
 
       onAdapterChange({
         id: null,
         title: '',
         name: '',
         description: '',
-        config: configWithPassword,
+        config: defaultConfig,
       });
     },
     [onAdapterChange, types],

graylog2-web-interface/src/components/lookup-tables/types.ts

@@ -44,6 +44,7 @@ export interface DataAdapterPluginType {
   formComponent?: any;
   summaryComponent?: any;
   documentationComponent?: any;
+  prepareConfig?: (config: Record<string, unknown>) => Record<string, unknown>;
 }
 
 declare module 'graylog-web-plugin/plugin' {

graylog2-web-interface/src/integrations/bindings.jsx

@@ -32,6 +32,7 @@ import TeamsNotificationSummary from './event-notifications/event-notification-t
 import GreyNoiseAdapterFieldSet from './dataadapters/GreyNoiseAdapterFieldSet';
 import GreyNoiseAdapterSummary from './dataadapters/GreyNoiseAdapterSummary';
 import GreyNoiseAdapterDocumentation from './dataadapters/GreyNoiseAdapterDocumentation';
+import prepareGreyNoiseConfig from './dataadapters/prepareGreyNoiseConfig';
 import TeamsNotificationV2Form from './event-notifications/event-notification-types/TeamsNotificationV2Form';
 import TeamsNotificationV2Summary from './event-notifications/event-notification-types/TeamsNotificationV2Summary';
 import TeamsNotificationV2Details from './event-notifications/event-notification-details/TeamsNotificationV2Details';
@@ -86,6 +87,7 @@ const bindings = {
       formComponent: GreyNoiseAdapterFieldSet,
       summaryComponent: GreyNoiseAdapterSummary,
       documentationComponent: GreyNoiseAdapterDocumentation,
+      prepareConfig: prepareGreyNoiseConfig,
     },
   ],
 };

graylog2-web-interface/src/integrations/dataadapters/GreyNoiseAdapterFieldSet.tsx

@@ -14,44 +14,33 @@
  * along with this program. If not, see
  * <http://www.mongodb.com/licensing/server-side-public-license>.
  */
-import React, { useCallback, useEffect, useRef, useState } from 'react';
+import React, { useState } from 'react';
 
 import { Button, Input } from 'components/bootstrap';
 import type { ValidationState } from 'components/common/types';
+import type { EncryptedFieldValue } from 'components/configurationforms/types';
 
 type GreyNoiseAdapterFieldSetProps = {
   config: {
     api_token: any;
   };
-  updateConfig: (...args: any[]) => void;
+  setFieldValue: (...args: any[]) => void;
   validationState: (...args: any[]) => ValidationState;
   validationMessage: (...args: any[]) => string;
 };
 
 const GreyNoiseAdapterFieldSet = ({
   config,
-  updateConfig,
+  setFieldValue,
   validationMessage,
   validationState,
 }: GreyNoiseAdapterFieldSetProps) => {
-  const isCreate = useRef(!config.api_token?.is_set);
-  const [showResetPasswordButton, setShowResetPasswordButton] = useState(config.api_token?.is_set === true);
+  const [isCreate] = useState(() => !config.api_token?.keep_value);
+  const [showResetPasswordButton, setShowResetPasswordButton] = useState(!!config.api_token?.keep_value);
 
-  const setUserPassword = useCallback(
-    (nextUserPassword) => {
-      updateConfig({ ...config, api_token: nextUserPassword });
-    },
-    [updateConfig, config],
-  );
-
-  useEffect(() => {
-    // Set a default value on `api_token` that the server can deserialize
-    if (config.api_token?.is_set !== undefined) {
-      // Keeping value is only helpful when editing, but since setting '' as value throws an error during
-      // validation, this at least avoids users seeing validation errors constantly.
-      setUserPassword({ keep_value: true });
-    }
-  }, [setUserPassword, config.api_token]);
+  const setUserPassword = (nextUserPassword: EncryptedFieldValue<string>) => {
+    setFieldValue('config.api_token', nextUserPassword);
+  };
 
   const handleUserPasswordChange = ({ target }) => {
     const typedPassword = target.value;
@@ -89,7 +78,7 @@ const GreyNoiseAdapterFieldSet = ({
           name="api_token"
           label="API Token"
           buttonAfter={
-            !isCreate.current ? (
+            !isCreate ? (
               <Button type="button" onClick={toggleUserPasswordReset}>
                 Undo Reset
               </Button>