Merge branch 'djadmin-fix-auth-placeholders' into 'master'

Improve UX for redacted fields in DAST Profiles

See merge request gitlab-org/gitlab!58930

ee/app/assets/javascripts/security_configuration/dast_site_profiles_form/components/dast_site_auth_section.vue

@@ -60,11 +60,6 @@ export default {
       isSensitiveFieldRequired: !isEditMode,
     };
   },
-  computed: {
-    passwordFieldPlaceholder() {
-      return this.isEditMode ? '••••••••' : '';
-    },
-  },
   watch: {
     form: { handler: 'emitUpdate', immediate: true, deep: true },
   },
@@ -128,7 +123,6 @@ export default {
               autocomplete="off"
               name="password"
               type="password"
-              :placeholder="passwordFieldPlaceholder"
               :required="isSensitiveFieldRequired"
               :state="form.fields.password.state"
             />

ee/app/assets/javascripts/security_configuration/dast_site_profiles_form/components/dast_site_profile_form.vue

@@ -21,6 +21,8 @@ import {
   MAX_CHAR_LIMIT_EXCLUDED_URLS,
   MAX_CHAR_LIMIT_REQUEST_HEADERS,
   EXCLUDED_URLS_SEPARATOR,
+  REDACTED_PASSWORD,
+  REDACTED_REQUEST_HEADERS,
 } from '../constants';
 import dastSiteProfileCreateMutation from '../graphql/dast_site_profile_create.mutation.graphql';
 import dastSiteProfileUpdateMutation from '../graphql/dast_site_profile_update.mutation.graphql';
@@ -61,7 +63,8 @@ export default {
     },
   },
   data() {
-    const { name = '', targetUrl = '', excludedUrls = [], auth = {} } = this.siteProfile || {};
+    const { name = '', targetUrl = '', excludedUrls = [], requestHeaders = '', auth = {} } =
+      this.siteProfile || {};
 
     const form = {
       state: false,
@@ -75,7 +78,7 @@ export default {
           skipValidation: true,
         }),
         requestHeaders: initFormField({
-          value: '',
+          value: requestHeaders || '',
           required: false,
           skipValidation: true,
         }),
@@ -131,10 +134,8 @@ export default {
           tooltip: s__(
             'DastProfiles|Request header names and values. Headers are added to every request made by DAST.',
           ),
-          placeholder: this.hasRequestHeaders
-            ? __('[Redacted]')
-            : // eslint-disable-next-line @gitlab/require-i18n-strings
-              'Cache-control: no-cache, User-Agent: DAST/1.0',
+          // eslint-disable-next-line @gitlab/require-i18n-strings
+          placeholder: 'Cache-control: no-cache, User-Agent: DAST/1.0',
         },
       };
     },
@@ -149,6 +150,14 @@ export default {
         .split(EXCLUDED_URLS_SEPARATOR)
         .map((url) => url.trim());
     },
+    serializedAuthFields() {
+      const authFields = serializeFormObject(this.authSection.fields);
+      // not to send password value if unchanged
+      if (authFields.password === REDACTED_PASSWORD) {
+        delete authFields.password;
+      }
+      return authFields;
+    },
   },
   methods: {
     onSubmit() {
@@ -166,7 +175,9 @@ export default {
       this.hideErrors();
       const { errorMessage } = this.i18n;
 
-      const { profileName, targetUrl, ...additionalFields } = serializeFormObject(this.form.fields);
+      const { profileName, targetUrl, requestHeaders, excludedUrls } = serializeFormObject(
+        this.form.fields,
+      );
 
       const variables = {
         input: {
@@ -175,11 +186,13 @@ export default {
           profileName,
           targetUrl,
           ...(this.glFeatures.securityDastSiteProfilesAdditionalFields && {
-            ...additionalFields,
-            auth: serializeFormObject(this.authSection.fields),
-            ...(additionalFields.excludedUrls && {
+            auth: this.serializedAuthFields,
+            ...(excludedUrls && {
               excludedUrls: this.parsedExcludedUrls,
             }),
+            ...(requestHeaders !== REDACTED_REQUEST_HEADERS && {
+              requestHeaders,
+            }),
           }),
         },
       };

ee/app/assets/javascripts/security_configuration/dast_site_profiles_form/constants.js

 export const MAX_CHAR_LIMIT_EXCLUDED_URLS = 2048;
 export const MAX_CHAR_LIMIT_REQUEST_HEADERS = 2048;
 export const EXCLUDED_URLS_SEPARATOR = ',';
+export const REDACTED_PASSWORD = '••••••••';
+export const REDACTED_REQUEST_HEADERS = '••••••••';

ee/app/presenters/dast/site_profile_presenter.rb

@@ -3,7 +3,7 @@
 module Dast
   class SiteProfilePresenter < Gitlab::View::Presenter::Delegated
     REDACTED_PASSWORD = '••••••••'
-    REDACTED_REQUEST_HEADERS = '[Redacted]'
+    REDACTED_REQUEST_HEADERS = '••••••••'
 
     presents :site_profile
 

ee/spec/frontend/security_configuration/dast_site_profiles_form/components/dast_site_profile_form_spec.js

@@ -186,30 +186,34 @@ describe('DastSiteProfileForm', () => {
       expect(findRequestHeadersInput().attributes('maxlength')).toBe('2048');
     });
 
-    describe('should have correct placeholders', () => {
-      const defaultPlaceholder = 'Cache-control: no-cache, User-Agent: DAST/1.0';
-
+    describe('request-headers and password fields renders correctly', () => {
       it('when creating a new profile', async () => {
-        expect(findRequestHeadersInput().attributes('placeholder')).toBe(defaultPlaceholder);
+        expect(findRequestHeadersInput().attributes('placeholder')).toBe(
+          'Cache-control: no-cache, User-Agent: DAST/1.0',
+        );
+
+        expect(findRequestHeadersInput().element.value).toBe('');
+        expect(findByNameAttribute('password').exists()).toBe(false);
       });
 
-      it('when updating an existing profile with no request headers set', () => {
+      it('when updating an existing profile', () => {
         createFullComponent({
           propsData: {
-            siteProfile: { ...siteProfileOne, requestHeaders: '' },
+            siteProfile: siteProfileOne,
           },
         });
-        expect(findRequestHeadersInput().attributes('placeholder')).toBe(defaultPlaceholder);
+        expect(findRequestHeadersInput().element.value).toBe(siteProfileOne.requestHeaders);
+        expect(findByNameAttribute('password').element.value).toBe(siteProfileOne.auth.password);
       });
 
-      it('when updating an existing profile', () => {
+      it('when updating an existing profile with no request-header & password', () => {
         createFullComponent({
           propsData: {
-            siteProfile: siteProfileOne,
+            siteProfile: { ...siteProfileOne, requestHeaders: null, auth: { enabled: true } },
           },
         });
-        expect(findRequestHeadersInput().attributes('placeholder')).toBe('[Redacted]');
-        expect(findByNameAttribute('password').attributes('placeholder')).toBe('••••••••');
+        expect(findRequestHeadersInput().element.value).toBe('');
+        expect(findByNameAttribute('password').element.value).toBe('');
       });
     });
   });

ee/spec/graphql/types/dast_site_profile_type_spec.rb

@@ -99,7 +99,7 @@ RSpec.describe GitlabSchema.types['DastSiteProfile'] do
         it 'is redacted' do
           create(:dast_site_profile_secret_variable, dast_site_profile: object, key: Dast::SiteProfileSecretVariable::REQUEST_HEADERS)
 
-          expect(resolve_field(:request_headers, object, current_user: user)).to eq('[Redacted]')
+          expect(resolve_field(:request_headers, object, current_user: user)).to eq('••••••••')
         end
       end
     end