Fixes #37418 - Fixes an issue that caused hidden Ansible variables to…

… be shown in plain text on the Host-Details page - Add "hiddenValue" to GraphQL query hostVariableOverrides.gql - Replace plain text secret with masked value - Adds a parameter "redact_secrets" to AnsibleInventoriesController#show_inventory - Change frontend code to use newly added "redact_secrets" parameter - Add a new "to_hash_with_secrets_redacted" method to InventoryCreator
theforeman · May 8, 2024 · d468a18 · d468a18
1 parent 8c6be4f
commit d468a18
Show file tree

Hide file tree

Showing 8 changed files with 64 additions and 13 deletions.
diff --git a/app/controllers/api/v2/ansible_inventories_controller.rb b/app/controllers/api/v2/ansible_inventories_controller.rb
@@ -99,7 +99,7 @@ def schedule_params
 
       def show_inventory(ids_key, condition_key)
         ids = params.fetch(ids_key, []).uniq
-        render :json => ForemanAnsible::InventoryCreator.new(Host.where(condition_key => ids)).to_hash.to_json
+        render :json => ForemanAnsible::InventoryCreator.new(Host.where(condition_key => ids)).to_hash_with_secrets_redacted.to_json
       end
     end
   end

diff --git a/app/services/foreman_ansible/ansible_info.rb b/app/services/foreman_ansible/ansible_info.rb
@@ -1,16 +1,18 @@
 module ForemanAnsible
   class AnsibleInfo < ::HostInfo::Provider
-    def host_info
-      { 'parameters' => ansible_params }
+    def host_info(redact_secrets = false)
+      { 'parameters' => ansible_params(redact_secrets) }
     end
 
-    def ansible_params
+    def ansible_params(redact_secrets = false)
       variables = AnsibleVariable.where(:ansible_role_id => host.all_ansible_roles.pluck(:id), :override => true)
       values = variables.values_hash(host)
 
       variables.each_with_object({}) do |var, memo|
         value = values[var]
-        memo[var.key] = value unless value.nil?
+        unless value.nil?
+          memo[var.key] = redact_secrets && var.hidden_value? ? var.hidden_value : value
+        end
         memo
       end
     end

diff --git a/app/services/foreman_ansible/inventory_creator.rb b/app/services/foreman_ansible/inventory_creator.rb
@@ -19,28 +19,32 @@ def initialize(hosts, template_invocation = nil)
     # more advanced cases). Therefore we have only the 'all' group
     # with all hosts.
     def to_hash
+      to_hash_with_secrets_redacted(false)
+    end
+
+    def to_hash_with_secrets_redacted(redact_secrets = true)
       hosts = @hosts.map(&:name)
 
       { 'all' => { 'hosts' => hosts,
                    'vars' => template_inputs(@template_invocation) },
-        '_meta' => { 'hostvars' => hosts_vars } }
+        '_meta' => { 'hostvars' => hosts_vars(redact_secrets) } }
     end
 
-    def hosts_vars
+    def hosts_vars(redact_secrets = false)
       hosts.reduce({}) do |hash, host|
         hash.update(
-          host.name => host_vars(host)
+          host.name => host_vars(host, redact_secrets)
         )
       end
     end
 
-    def host_vars(host)
+    def host_vars(host, redact_secrets = false)
       {
         'foreman' => reduced_host_info(host).fetch('parameters', {}),
         'foreman_ansible_roles' => host_roles(host)
       }.merge(connection_params(host)).
         merge(host_params(host)).
-        merge(ansible_params(host))
+        merge(ansible_params(host, redact_secrets))
     end
 
     def connection_params(host)
@@ -62,8 +66,8 @@ def host_roles(host)
       host.all_ansible_roles.map(&:name)
     end
 
-    def ansible_params(host)
-      ForemanAnsible::AnsibleInfo.new(host).ansible_params
+    def ansible_params(host, redact_secrets = false)
+      ForemanAnsible::AnsibleInfo.new(host).ansible_params(redact_secrets)
     end
 
     def reduced_host_info(host)

diff --git a/webpack/components/AnsibleHostDetail/components/AnsibleHostInventory/index.js b/webpack/components/AnsibleHostDetail/components/AnsibleHostInventory/index.js
@@ -9,7 +9,10 @@ import AnsibleHostInventory from './AnsibleHostInventory';
 import ErrorState from '../../../ErrorState';
 
 const WrappedAnsibleHostInventory = ({ hostId }) => {
-  const params = useMemo(() => ({ params: { host_ids: [hostId] } }), [hostId]);
+  const params = useMemo(
+    () => ({ params: { host_ids: [hostId], redact_secrets: true } }),
+    [hostId]
+  );
 
   const url = hostId && foremanUrl('/ansible/api/ansible_inventories/hosts');
   const { response: inventory, status } = useAPI('get', url, params);

diff --git a/...ibleHostDetail/components/AnsibleVariableOverrides/AnsibleVariableOverridesTableHelper.js b/...ibleHostDetail/components/AnsibleVariableOverrides/AnsibleVariableOverridesTableHelper.js
@@ -1,4 +1,5 @@
 import React from 'react';
+import { TextInput } from '@patternfly/react-core';
 import { TimesIcon, CheckIcon } from '@patternfly/react-icons';
 import { sprintf, translate as __ } from 'foremanReact/common/I18n';
 
@@ -22,6 +23,21 @@ export const formatValue = variable => {
     ? variable.currentValue.value
     : variable.defaultValue;
 
+  if (variable.hiddenValue) {
+    return (
+      <TextInput
+        value="Not the secrets you're looking for..."
+        type="password"
+        aria-label="hidden ansible variable"
+        isDisabled
+        style={{
+          '--pf-c-form-control--BackgroundColor': 'white',
+          color: 'black',
+        }}
+      />
+    );
+  }
+
   switch (variable.parameterType) {
     case 'boolean':
       return value ? <CheckIcon /> : <TimesIcon />;

diff --git a/...tDetail/components/AnsibleVariableOverrides/__test__/AnsibleVariableOverrides.fixtures.js b/...tDetail/components/AnsibleVariableOverrides/__test__/AnsibleVariableOverrides.fixtures.js
@@ -36,6 +36,7 @@ const withFqdnOverride = canEdit => ({
   validatorType: '',
   validatorRule: null,
   required: false,
+  hiddenValue: false,
   lookupValues: {
     nodes: [
       {
@@ -70,6 +71,7 @@ const withDomainOverride = canEdit => ({
   validatorType: '',
   validatorRule: null,
   required: false,
+  hiddenValue: false,
   lookupValues: {
     nodes: [],
   },
@@ -142,6 +144,7 @@ export const mocks = [
                 validatorType: 'list',
                 validatorRule: 'a,b,c',
                 required: true,
+                hiddenValue: false,
                 lookupValues: {
                   nodes: [
                     {
@@ -170,6 +173,7 @@ export const mocks = [
                 validatorType: '',
                 validatorRule: null,
                 required: false,
+                hiddenValue: false,
                 lookupValues: {
                   nodes: [],
                 },
@@ -190,6 +194,7 @@ export const mocks = [
                 validatorType: '',
                 validatorRule: null,
                 required: false,
+                hiddenValue: false,
                 lookupValues: {
                   nodes: [],
                 },
@@ -215,6 +220,7 @@ export const mocks = [
                 validatorType: '',
                 validatorRule: null,
                 required: false,
+                hiddenValue: true,
                 lookupValues: {
                   nodes: [],
                 },
@@ -240,6 +246,7 @@ export const mocks = [
                 validatorType: '',
                 validatorRule: null,
                 required: false,
+                hiddenValue: false,
                 lookupValues: {
                   nodes: [],
                 },
@@ -260,6 +267,7 @@ export const mocks = [
                 validatorType: '',
                 validatorRule: null,
                 required: false,
+                hiddenValue: true,
                 lookupValues: {
                   nodes: [],
                 },
@@ -282,6 +290,7 @@ export const mocks = [
                 validatorType: '',
                 validatorRule: null,
                 required: false,
+                hiddenValue: true,
                 lookupValues: {
                   nodes: [],
                 },

diff --git a/...eHostDetail/components/AnsibleVariableOverrides/__test__/AnsibleVariableOverrides.test.js b/...eHostDetail/components/AnsibleVariableOverrides/__test__/AnsibleVariableOverrides.test.js
@@ -70,4 +70,20 @@ describe('AnsibleVariableOverrides', () => {
     const actions = screen.queryAllByRole('button', { name: 'Actions' });
     expect(actions).toHaveLength(0);
   });
+  it('should hide hidden values', async () => {
+    const { container } = render(
+      <TestComponent
+        hostId={hostId}
+        mocks={mocks}
+        hostAttrs={hostAttrs}
+        history={historyMock}
+      />
+    );
+    await waitFor(tick);
+    expect(screen.getByText('ellipse')).toBeInTheDocument();
+    expect(screen.getByText('sun')).toBeInTheDocument();
+    expect(screen.getByText('moon')).toBeInTheDocument();
+    // number of hidden variables + 1 for pagination input
+    expect(container.getElementsByTagName('input')).toHaveLength(3 + 1);
+  });
 });
diff --git a/webpack/graphql/queries/hostVariableOverrides.gql b/webpack/graphql/queries/hostVariableOverrides.gql
@@ -18,6 +18,7 @@ query($id: String!, $match: String, $first: Int, $last: Int) {
         validatorType
         validatorRule
         required
+        hiddenValue
         lookupValues(match: $match) {
           nodes {
             id