Merge branch 'philipcunningham-return-additional-config-in-graphql-query-225406' into 'master'

Return additional site profile config from GraphQL

See merge request gitlab-org/gitlab!57441

ee/app/graphql/resolvers/dast_site_profile_resolver.rb

@@ -2,6 +2,8 @@
 
 module Resolvers
   class DastSiteProfileResolver < BaseResolver
+    include LooksAhead
+
     alias_method :project, :object
 
     type Types::DastSiteProfileType.connection_type, null: true
@@ -12,7 +14,20 @@ module Resolvers
                description: "ID of the site profile."
     end
 
-    def resolve(**args)
+    def resolve_with_lookahead(**args)
+      apply_lookahead(find_dast_site_profiles(args))
+    end
+
+    private
+
+    def preloads
+      {
+        request_headers: [:secret_variables],
+        auth: [:secret_variables]
+      }
+    end
+
+    def find_dast_site_profiles(args)
       if args[:id]
         # TODO: remove this coercion when the compatibility layer is removed
         # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883

ee/app/graphql/types/dast/site_profile_auth_type.rb

@@ -3,6 +3,8 @@
 module Types
   module Dast
     class SiteProfileAuthType < BaseObject
+      REDACTED_PASSWORD = '••••••••'
+
       graphql_name 'DastSiteProfileAuth'
       description 'Input type for DastSiteProfile authentication'
 
@@ -39,7 +41,9 @@ module Types
             description: 'Redacted password to authenticate with on the target website.'
 
       def password
-        nil
+        return unless object.secret_variables.any? { |variable| variable.key == ::Dast::SiteProfileSecretVariable::PASSWORD }
+
+        REDACTED_PASSWORD
       end
     end
   end

ee/app/graphql/types/dast_site_profile_type.rb

@@ -2,6 +2,8 @@
 
 module Types
   class DastSiteProfileType < BaseObject
+    REDACTED_REQUEST_HEADERS = '[Redacted]'
+
     graphql_name 'DastSiteProfile'
     description 'Represents a DAST Site Profile'
 
@@ -68,7 +70,10 @@ module Types
     end
 
     def request_headers
-      nil
+      return unless Feature.enabled?(:security_dast_site_profiles_additional_fields, object.project, default_enabled: :yaml)
+      return unless object.secret_variables.any? { |variable| variable.key == ::Dast::SiteProfileSecretVariable::REQUEST_HEADERS }
+
+      REDACTED_REQUEST_HEADERS
     end
 
     def normalized_target_url

ee/app/models/dast/site_profile_secret_variable.rb

@@ -2,6 +2,9 @@
 
 module Dast
   class SiteProfileSecretVariable < ApplicationRecord
+    REQUEST_HEADERS = 'DAST_REQUEST_HEADERS_BASE64'
+    PASSWORD = 'DAST_PASSWORD_BASE64'
+
     include Ci::HasVariable
     include Ci::Maskable
 

ee/spec/graphql/types/dast/site_profile_auth_type_spec.rb

@@ -2,9 +2,66 @@
 
 require 'spec_helper'
 
-RSpec.describe Types::Dast::SiteProfileAuthType do
+RSpec.describe GitlabSchema.types['DastSiteProfileAuth'] do
+  include GraphqlHelpers
+
+  let_it_be(:project) { create(:project) }
+  let_it_be(:user) { create(:user, developer_projects: [project]) }
+  let_it_be(:object, reload: true) { create(:dast_site_profile, project: project) }
+  let_it_be(:fields) { %i[enabled url usernameField passwordField username password] }
+
+  before do
+    stub_licensed_features(security_on_demand_scans: true)
+  end
+
   specify { expect(described_class.graphql_name).to eq('DastSiteProfileAuth') }
   specify { expect(described_class).to require_graphql_authorizations(:read_on_demand_scans) }
 
-  it { expect(described_class).to have_graphql_fields(%w[enabled url usernameField passwordField username password]) }
+  it { expect(described_class).to have_graphql_fields(fields) }
+
+  describe 'enabled field' do
+    it 'is auth_enabled' do
+      expect(resolve_field(:enabled, object, current_user: user)).to eq(object.auth_enabled)
+    end
+  end
+
+  describe 'url field' do
+    it 'is auth_url' do
+      expect(resolve_field(:url, object, current_user: user)).to eq(object.auth_url)
+    end
+  end
+
+  describe 'usernameField field' do
+    it 'is auth_username_field' do
+      expect(resolve_field(:username_field, object, current_user: user)).to eq(object.auth_username_field)
+    end
+  end
+
+  describe 'passwordField field' do
+    it 'is auth_password_field' do
+      expect(resolve_field(:password_field, object, current_user: user)).to eq(object.auth_password_field)
+    end
+  end
+
+  describe 'username field' do
+    it 'is auth_username' do
+      expect(resolve_field(:username, object, current_user: user)).to eq(object.auth_username)
+    end
+  end
+
+  describe 'password field' do
+    context 'when there is no associated secret variable' do
+      it 'is nil' do
+        expect(resolve_field(:password, object, current_user: user)).to be_nil
+      end
+    end
+
+    context 'when there an associated secret variable' do
+      it 'is redacted' do
+        create(:dast_site_profile_secret_variable, dast_site_profile: object, key: Dast::SiteProfileSecretVariable::PASSWORD)
+
+        expect(resolve_field(:password, object, current_user: user)).to eq('••••••••')
+      end
+    end
+  end
 end

ee/spec/graphql/types/dast_site_profile_type_spec.rb

@@ -5,23 +5,11 @@ require 'spec_helper'
 RSpec.describe GitlabSchema.types['DastSiteProfile'] do
   include GraphqlHelpers
 
-  let_it_be(:dast_site_profile) { create(:dast_site_profile) }
-  let_it_be(:project) { dast_site_profile.project }
-  let_it_be(:user) { create(:user) }
+  let_it_be(:project) { create(:project) }
+  let_it_be(:user) { create(:user, developer_projects: [project]) }
+  let_it_be(:object, reload: true) { create(:dast_site_profile, project: project) }
   let_it_be(:fields) { %i[id profileName targetUrl editPath excludedUrls requestHeaders validationStatus userPermissions normalizedTargetUrl auth referencedInSecurityPolicies] }
 
-  subject do
-    GitlabSchema.execute(
-      query,
-      context: {
-        current_user: user
-      },
-      variables: {
-        fullPath: project.full_path
-      }
-    ).as_json
-  end
-
   before do
     stub_licensed_features(security_on_demand_scans: true)
   end
@@ -31,117 +19,109 @@ RSpec.describe GitlabSchema.types['DastSiteProfile'] do
   specify { expect(described_class).to expose_permissions_using(Types::PermissionTypes::DastSiteProfile) }
 
   it { expect(described_class).to have_graphql_fields(fields) }
+  it { expect(described_class).to have_graphql_field(:referenced_in_security_policies, calls_gitaly?: true, complexity: 10) }
 
-  describe 'dast_site_profiles' do
-    before do
-      project.add_developer(user)
+  describe 'id field' do
+    it 'is the global id' do
+      expect(resolve_field(:id, object, current_user: user)).to eq(object.to_global_id)
     end
+  end
 
-    let(:query) do
-      %(
-        query project($fullPath: ID!) {
-          project(fullPath: $fullPath) {
-            dastSiteProfiles(first: 1) {
-              nodes { #{all_graphql_fields_for('DastSiteProfile')} }
-            }
-          }
-        }
-      )
+  describe 'profileName field' do
+    it 'is the name' do
+      expect(resolve_field(:profile_name, object, current_user: user)).to eq(object.name)
     end
+  end
 
-    let(:first_dast_site_profile) do
-      subject.dig('data', 'project', 'dastSiteProfiles', 'nodes', 0)
+  describe 'targetUrl field' do
+    it 'is the url of the associated dast_site' do
+      expect(resolve_field(:target_url, object, current_user: user)).to eq(object.dast_site.url)
     end
+  end
 
-    describe 'id field' do
-      it 'is a global id' do
-        expect(first_dast_site_profile['id']).to eq(dast_site_profile.to_global_id.to_s)
-      end
+  describe 'editPath field' do
+    it 'is the relative path to edit the dast_site_profile' do
+      path = "/#{project.full_path}/-/security/configuration/dast_scans/dast_site_profiles/#{object.id}/edit"
+
+      expect(resolve_field(:edit_path, object, current_user: user)).to eq(path)
     end
+  end
+
+  describe 'auth field' do
+    context 'when the feature flag is disabled' do
+      it 'is nil' do
+        stub_feature_flags(security_dast_site_profiles_additional_fields: false)
 
-    describe 'profile_name field' do
-      it 'is the name' do
-        expect(first_dast_site_profile['profileName']).to eq(dast_site_profile.name)
+        expect(resolve_field(:auth, object, current_user: user)).to be_nil
       end
     end
 
-    describe 'target_url field' do
-      it 'is the url of the associated dast_site' do
-        expect(first_dast_site_profile['targetUrl']).to eq(dast_site_profile.dast_site.url)
+    context 'when the feature flag is enabled' do
+      it 'is the dast_site_profile' do
+        expect(resolve_field(:auth, object, current_user: user)).to eq(object)
       end
     end
+  end
 
-    describe 'edit_path field' do
-      it 'is the relative path to edit the dast_site_profile' do
-        path = "/#{project.full_path}/-/security/configuration/dast_scans/dast_site_profiles/#{dast_site_profile.id}/edit"
+  describe 'excludedUrls field' do
+    context 'when the feature flag is disabled' do
+      it 'is nil' do
+        stub_feature_flags(security_dast_site_profiles_additional_fields: false)
 
-        expect(first_dast_site_profile['editPath']).to eq(path)
+        expect(resolve_field(:excluded_urls, object, current_user: user)).to be_nil
       end
     end
 
-    describe 'excludedUrls field' do
-      context 'when the feature flag is disabled' do
-        it 'is nil' do
-          stub_feature_flags(security_dast_site_profiles_additional_fields: false)
-
-          expect(first_dast_site_profile['excludedUrls']).to eq(nil)
-        end
+    context 'when the feature flag is enabled' do
+      it 'is the excluded urls' do
+        expect(resolve_field(:excluded_urls, object, current_user: user)).to eq(object.excluded_urls)
       end
+    end
+  end
 
-      context 'when the feature flag is enabled' do
-        it 'is the excluded urls' do
-          expect(first_dast_site_profile['excludedUrls']).to eq(dast_site_profile.excluded_urls)
-        end
+  describe 'requestHeaders field' do
+    context 'when the feature flag is disabled' do
+      it 'is nil' do
+        stub_feature_flags(security_dast_site_profiles_additional_fields: false)
+
+        expect(resolve_field(:request_headers, object, current_user: user)).to be_nil
       end
     end
 
-    describe 'auth field' do
-      context 'when the feature flag is disabled' do
+    context 'when the feature flag is enabled' do
+      context 'when there is no associated secret variable' do
         it 'is nil' do
-          stub_feature_flags(security_dast_site_profiles_additional_fields: false)
-
-          expect(first_dast_site_profile['auth']).to eq(nil)
+          expect(resolve_field(:request_headers, object, current_user: user)).to be_nil
         end
       end
 
-      context 'when the feature flag is enabled' do
-        it 'includes the correct values' do
-          auth = first_dast_site_profile['auth']
-
-          expect(auth).to include(
-            'enabled' => false,
-            'url' => dast_site_profile.auth_url,
-            'usernameField' => dast_site_profile.auth_username_field,
-            'passwordField' => dast_site_profile.auth_password_field,
-            'username' => dast_site_profile.auth_username,
-            'password' => nil
-          )
+      context 'when there an associated secret variable' 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]')
         end
       end
     end
+  end
 
-    describe 'validation_status field' do
-      it 'is the validation status' do
-        expect(first_dast_site_profile['validationStatus']).to eq('NONE')
-      end
+  describe 'validation_status field' do
+    it 'is the validation status' do
+      expect(resolve_field(:validation_status, object, current_user: user)).to eq('none')
     end
+  end
 
-    describe 'normalized_target_url field' do
-      it 'is the normalized url of the associated dast_site' do
-        normalized_url = DastSiteValidation.get_normalized_url_base(dast_site_profile.dast_site.url)
+  describe 'normalizedTargetUrl field' do
+    it 'is the normalized url of the associated dast_site' do
+      normalized_url = DastSiteValidation.get_normalized_url_base(object.dast_site.url)
 
-        expect(first_dast_site_profile['normalizedTargetUrl']).to eq(normalized_url)
-      end
+      expect(resolve_field(:normalized_target_url, object, current_user: user)).to eq(normalized_url)
     end
+  end
 
-    context 'when there are no dast_site_profiles' do
-      let(:project) { create(:project) }
-
-      it 'has no nodes' do
-        nodes = subject.dig('data', 'project', 'dastSiteProfiles', 'nodes')
-
-        expect(nodes).to be_empty
-      end
+  describe 'referencedInSecurityPolicies field' do
+    it 'is the policies' do
+      expect(resolve_field(:referenced_in_security_policies, object, current_user: user)).to eq(object.referenced_in_security_policies)
     end
   end
 end

ee/spec/models/dast_site_profile_spec.rb

@@ -122,65 +122,67 @@ RSpec.describe DastSiteProfile, type: :model do
     end
   end
 
-  describe '#destroy!' do
-    context 'when the associated dast_site has no dast_site_profiles' do
-      it 'is also destroyed' do
-        subject.destroy!
+  describe 'instance methods' do
+    describe '#destroy!' do
+      context 'when the associated dast_site has no dast_site_profiles' do
+        it 'is also destroyed' do
+          subject.destroy!
 
-        expect { subject.dast_site.reload }.to raise_error(ActiveRecord::RecordNotFound)
+          expect { subject.dast_site.reload }.to raise_error(ActiveRecord::RecordNotFound)
+        end
       end
-    end
 
-    context 'when the associated dast_site has dast_site_profiles' do
-      it 'is not destroyed' do
-        create(:dast_site_profile, dast_site: subject.dast_site, project: subject.project)
+      context 'when the associated dast_site has dast_site_profiles' do
+        it 'is not destroyed' do
+          create(:dast_site_profile, dast_site: subject.dast_site, project: subject.project)
 
-        subject.destroy!
+          subject.destroy!
 
-        expect { subject.dast_site.reload }.not_to raise_error
+          expect { subject.dast_site.reload }.not_to raise_error
+        end
       end
     end
-  end
 
-  describe '#status' do
-    context 'when dast_site_validation association does not exist' do
-      it 'is none', :aggregate_failures do
-        subject.dast_site.update!(dast_site_validation_id: nil)
+    describe '#status' do
+      context 'when dast_site_validation association does not exist' do
+        it 'is none', :aggregate_failures do
+          subject.dast_site.update!(dast_site_validation_id: nil)
 
-        expect(subject.dast_site_validation).to be_nil
-        expect(subject.status).to eq('none')
+          expect(subject.dast_site_validation).to be_nil
+          expect(subject.status).to eq('none')
+        end
       end
-    end
 
-    context 'when dast_site_validation association does exist' do
-      it 'is dast_site_validation#state' do
-        expect(subject.status).to eq(subject.dast_site_validation.state)
+      context 'when dast_site_validation association does exist' do
+        it 'is dast_site_validation#state' do
+          expect(subject.status).to eq(subject.dast_site_validation.state)
+        end
       end
     end
-  end
 
-  describe '#referenced_in_security_policies' do
-    context 'there is no security_orchestration_policy_configuration assigned to project' do
-      it 'returns empty array' do
-        expect(subject.referenced_in_security_policies).to eq([])
+    describe '#referenced_in_security_policies' do
+      context 'there is no security_orchestration_policy_configuration assigned to project' do
+        it 'returns empty array' do
+          expect(subject.referenced_in_security_policies).to eq([])
+        end
       end
-    end
 
-    context 'there is security_orchestration_policy_configuration assigned to project' do
-      let(:security_orchestration_policy_configuration) { instance_double(Security::OrchestrationPolicyConfiguration, present?: true, active_policy_names_with_dast_site_profile: ['Policy Name']) }
+      context 'there is security_orchestration_policy_configuration assigned to project' do
+        let(:security_orchestration_policy_configuration) { instance_double(Security::OrchestrationPolicyConfiguration, present?: true, active_policy_names_with_dast_site_profile: ['Policy Name']) }
 
-      before do
-        allow(subject.project).to receive(:security_orchestration_policy_configuration).and_return(security_orchestration_policy_configuration)
-      end
+        before do
+          allow(subject.project).to receive(:security_orchestration_policy_configuration).and_return(security_orchestration_policy_configuration)
+        end
 
-      it 'calls security_orchestration_policy_configuration.active_policy_names_with_dast_site_profile with profile name' do
-        expect(security_orchestration_policy_configuration).to receive(:active_policy_names_with_dast_site_profile).with(subject.name)
+        it 'calls security_orchestration_policy_configuration.active_policy_names_with_dast_site_profile with profile name' do
+          expect(security_orchestration_policy_configuration).to receive(:active_policy_names_with_dast_site_profile).with(subject.name)
 
-        subject.referenced_in_security_policies
-      end
+          subject.referenced_in_security_policies
+        end
 
-      it 'returns the referenced policy name' do
-        expect(subject.referenced_in_security_policies).to eq(['Policy Name'])
+        it 'returns the referenced policy name' do
+          expect(subject.referenced_in_security_policies).to eq(['Policy Name'])
+        end
       end
     end
   end

ee/spec/requests/api/graphql/project/dast_site_profiles_spec.rb

@@ -70,7 +70,7 @@ RSpec.describe 'Query.project(fullPath).dastSiteProfiles' do
       expect(first_dast_site_profile_response['id']).to eq(dast_site_profile.to_global_id.to_s)
     end
 
-    it 'eager loads the dast site and dast site validation' do
+    it 'avoids N+1 queries' do
       control = ActiveRecord::QueryRecorder.new do
         post_graphql(
           query,