Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #37418 - Fixes an issue that caused hidden Ansible variables to be shown in plain text on the Host-Details page #717

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Thorben-D
Copy link
Contributor

Redmine Issue #37418 and reproducer

Variables marked as hidden were shown in plain text under Variables and Inventory on a host's details page.
This PR fixes that by masking the values in question in the UI.
Values are still shown in plain text when editing, as this requires the same permissions, edit_ansible_variables, as

Configure > Ansible > Variables.

It should be noted, that hidden variables are NOT considered secrets. The point of hidden is to only hide the values of the respective variables in the UI. The Foreman documentation clearly reflects this fact under point 6.

Changes:

  • Add "hiddenValue" to GraphQL query hostVariableOverrides.gql
  • Replace plain text secret with masked value
  • Add 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

Requires #716

image
image

@nofaralfasi
Copy link
Contributor

I'm getting the following error when navigating to the Variables tab:

RuntimeError (Failed to implement AnsibleVariable.hiddenValue, tried:
- `Types::OverridenAnsibleVariable#hidden_value`, which did not exist
- `Presenters::OverridenAnsibleVariablePresenter#hidden_value`, which did not exist
- Looking up hash key `:hidden_value` or `"hidden_value"` on `#<Presenters::OverridenAnsibleVariablePresenter:0x00007f28c28ced48>`, but it wasn't a Hash
To implement this field, define one of the methods above (and check for typos)

@Thorben-D
Copy link
Contributor Author

@nofaralfasi I think that is because you still have the broken GQL scheme... Did you make sure the content of #716 is present on your branch?

@nofaralfasi
Copy link
Contributor

@nofaralfasi I think that is because you still have the broken GQL scheme... Did you make sure the content of #716 is present on your branch?

You are right, I missed that part.
Now the Variables tab is shown properly. However, the default value of the hidden variable is still visible through the GraphQL response.
It's a little more complicated to find it, but still very possible.

Also, it's not possible to edit the variable value from the Variables tab. I see it's not related to the changes here, but we need to take care of it as well.

@Thorben-D
Copy link
Contributor Author

Thorben-D commented May 21, 2024

Glad you got it sorted.
Yes, the value may indeed still be extracted from the query response. I suppose we could check if the user has the edit_ansible_variables permission and require it for the value to be queried.

I tried to reproduce the issue you faced with editing the value, but without success.
The default value is not changed by this action, but the value for this particular host is. For me, this is then shown under:
Configure > Ansible > Variables > Variable > Specify Matchers

@nofaralfasi
Copy link
Contributor

Glad you got it sorted. Yes, the value may indeed still be extracted from the query response. I suppose we could check if the user has the edit_ansible_variables permission and require it for the value to be queried.

Exactly. That should be the correct implementation.

I tried to reproduce the issue you faced with editing the value, but without success. The default value is not changed by this action, but the value for this particular host is. For me, this is then shown under: Configure > Ansible > Variables > Variable > Specify Matchers

I apologize for the confusion, it was a problem on my setup. I'll be more careful next time.

@Thorben-D
Copy link
Contributor Author

Great, I'll implement that then!
No worries about the mix-up, thanks for having a look!

… 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
- Hide hidden values in GQL response by if edit_ansible_variables not granted
@Thorben-D Thorben-D force-pushed the OR-4732_hidden_ansible_variable_not_hidden_in_host_details branch from d468a18 to 0792fb6 Compare May 22, 2024 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants