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

Mask password fields of inputs returned by the REST API. #5432

Merged
merged 8 commits into from Jan 8, 2019

Conversation

Projects
None yet
3 participants
@dennisoelkers
Copy link
Member

dennisoelkers commented Dec 20, 2018

Description

Motivation and Context

Before this change, input details returned by the REST API would contain
all configuration fields without any modification. This implies that
password fields are also contained using their original value, showing
configured password for inputs in clear text.

For users which are not allowed to edit an input, this change now iterates over configuration fields checking for the presence of password fields and replace their content with <password set> instead of the original value if they are not empty.

For users which possess the required permission to edit an input, the original value is returned as it is used to set the value of the password field and would be used for updating the input if left unchanged.

This is a middle ground between a quick fix improving the situation (not everybody who is able to view inputs can also see passwords) and a proper fix that would require more work and attention to consistency, which will happen in a future version.

Refs #5408 and partially fixes it.

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@dennisoelkers dennisoelkers added this to the 3.0.0 milestone Dec 20, 2018

@dennisoelkers dennisoelkers force-pushed the mask-password-fields-of-inputs branch 2 times, most recently from 110f476 to 36fba21 Dec 21, 2018

@kmerz kmerz self-assigned this Jan 2, 2019

@kmerz
Copy link
Member

kmerz left a comment

This code does not deal with the problem when the password is displayed in the GUI for edit.
The GUI uses the value <password set>to display *****. When a user edits the input the value is also used for the input field. When the user changes something else (like the name of the input) the value <password set> is send to the server and used as new value for the password.

@dennisoelkers dennisoelkers force-pushed the mask-password-fields-of-inputs branch from 36fba21 to a299818 Jan 7, 2019

dennisoelkers added some commits Dec 20, 2018

Mask password fields of inputs returned by the REST API.
Before this change, input details returned by the REST API would contain
all configuration fields without any modification. This implies that
password fields are also contained using their original value, showing
configured password for inputs in clear text.

This change now iterates over configuration fields checking for the
presence of password fields and replace their content with `<password
set>` instead of the original value if they are not empty.

Fixes #5408.
Do not mask passwords in input config for users with edit permission.
If a user contains the required permission to edit an input, passwords
in the input's config are not masked. This is prevented so the input
edit dialog still functions in the same way as before.

@dennisoelkers dennisoelkers force-pushed the mask-password-fields-of-inputs branch from a299818 to 3c61da3 Jan 7, 2019

@mpfz0r

mpfz0r approved these changes Jan 7, 2019

Copy link
Member

mpfz0r left a comment

Looks good and I couldn't find any issues while testing.

@dennisoelkers

This comment has been minimized.

Copy link
Member Author

dennisoelkers commented Jan 8, 2019

@kmerz: Can you also check again and merge if you are happy?

@kmerz

kmerz approved these changes Jan 8, 2019

Copy link
Member

kmerz left a comment

Estoy feliz! 👍 🎉

@mpfz0r mpfz0r merged commit a562a33 into master Jan 8, 2019

3 of 5 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details
ci-web-linter Jenkins build graylog-pr-linter-check 3188 has succeeded
Details
graylog-project/pr Jenkins build graylog-project-pr-snapshot 2657 has succeeded
Details
license/cla Contributor License Agreement is signed.
Details

@mpfz0r mpfz0r referenced this pull request Jan 8, 2019

Closed

Mask password fields of inputs #5464

4 of 9 tasks complete

mpfz0r added a commit that referenced this pull request Jan 8, 2019

Mask password fields of inputs returned by the REST API. (#5432)
* Mask password fields of inputs returned by the REST API.

Before this change, input details returned by the REST API would contain
all configuration fields without any modification. This implies that
password fields are also contained using their original value, showing
configured password for inputs in clear text.

This change now iterates over configuration fields checking for the
presence of password fields and replace their content with `<password
set>` instead of the original value if they are not empty.

Fixes #5408.

* Adding test for actual resource method, including license headers.

* Adding test for complete input list retrievel.

* Adding guard clause for null parameters.

* Using locales for toLowerCase.

* Handling null values in map.

* Do not mask passwords in input config for users with edit permission.

If a user contains the required permission to edit an input, passwords
in the input's config are not masked. This is prevented so the input
edit dialog still functions in the same way as before.

* Adding/adapting tests.

(Cherry-Pick from a562a33)

kmerz added a commit that referenced this pull request Jan 8, 2019

Mask password fields of inputs 3.0 (#5467)
* Mask password fields of inputs returned by the REST API. (#5432)

* Mask password fields of inputs returned by the REST API.

Before this change, input details returned by the REST API would contain
all configuration fields without any modification. This implies that
password fields are also contained using their original value, showing
configured password for inputs in clear text.

This change now iterates over configuration fields checking for the
presence of password fields and replace their content with `<password
set>` instead of the original value if they are not empty.

Fixes #5408.

* Adding test for actual resource method, including license headers.

* Adding test for complete input list retrievel.

* Adding guard clause for null parameters.

* Using locales for toLowerCase.

* Handling null values in map.

* Do not mask passwords in input config for users with edit permission.

If a user contains the required permission to edit an input, passwords
in the input's config are not masked. This is prevented so the input
edit dialog still functions in the same way as before.

* Adding/adapting tests.

(Cherry-Pick from a562a33)

* Fixing import.

bernd added a commit that referenced this pull request Feb 28, 2019

Mask password fields of inputs returned by the REST API. (#5432)
* Mask password fields of inputs returned by the REST API.

Before this change, input details returned by the REST API would contain
all configuration fields without any modification. This implies that
password fields are also contained using their original value, showing
configured password for inputs in clear text.

This change now iterates over configuration fields checking for the
presence of password fields and replace their content with `<password
set>` instead of the original value if they are not empty.

Fixes #5408.

* Adding test for actual resource method, including license headers.

* Adding test for complete input list retrievel.

* Adding guard clause for null parameters.

* Using locales for toLowerCase.

* Handling null values in map.

* Do not mask passwords in input config for users with edit permission.

If a user contains the required permission to edit an input, passwords
in the input's config are not masked. This is prevented so the input
edit dialog still functions in the same way as before.

* Adding/adapting tests.

(cherry picked from commit a562a33)

bernd added a commit that referenced this pull request Feb 28, 2019

Mask password fields of inputs returned by the REST API. (#5432)
* Mask password fields of inputs returned by the REST API.

Before this change, input details returned by the REST API would contain
all configuration fields without any modification. This implies that
password fields are also contained using their original value, showing
configured password for inputs in clear text.

This change now iterates over configuration fields checking for the
presence of password fields and replace their content with `<password
set>` instead of the original value if they are not empty.

Fixes #5408.

* Adding test for actual resource method, including license headers.

* Adding test for complete input list retrievel.

* Adding guard clause for null parameters.

* Using locales for toLowerCase.

* Handling null values in map.

* Do not mask passwords in input config for users with edit permission.

If a user contains the required permission to edit an input, passwords
in the input's config are not masked. This is prevented so the input
edit dialog still functions in the same way as before.

* Adding/adapting tests.

(cherry picked from commit a562a33)

edmundoa added a commit that referenced this pull request Feb 28, 2019

Mask password fields of inputs returned by the REST API. (#5432) (#5734)
* Mask password fields of inputs returned by the REST API. (#5432)

* Mask password fields of inputs returned by the REST API.

Before this change, input details returned by the REST API would contain
all configuration fields without any modification. This implies that
password fields are also contained using their original value, showing
configured password for inputs in clear text.

This change now iterates over configuration fields checking for the
presence of password fields and replace their content with `<password
set>` instead of the original value if they are not empty.

Fixes #5408.

* Adding test for actual resource method, including license headers.

* Adding test for complete input list retrievel.

* Adding guard clause for null parameters.

* Using locales for toLowerCase.

* Handling null values in map.

* Do not mask passwords in input config for users with edit permission.

If a user contains the required permission to edit an input, passwords
in the input's config are not masked. This is prevented so the input
edit dialog still functions in the same way as before.

* Adding/adapting tests.

(cherry picked from commit a562a33)

* Fixing import. (#5465)


(cherry picked from commit a9a1df0)

edmundoa added a commit that referenced this pull request Feb 28, 2019

Mask password fields of inputs returned by the REST API. (#5432) (#5733)
* Mask password fields of inputs returned by the REST API. (#5432)

* Mask password fields of inputs returned by the REST API.

Before this change, input details returned by the REST API would contain
all configuration fields without any modification. This implies that
password fields are also contained using their original value, showing
configured password for inputs in clear text.

This change now iterates over configuration fields checking for the
presence of password fields and replace their content with `<password
set>` instead of the original value if they are not empty.

Fixes #5408.

* Adding test for actual resource method, including license headers.

* Adding test for complete input list retrievel.

* Adding guard clause for null parameters.

* Using locales for toLowerCase.

* Handling null values in map.

* Do not mask passwords in input config for users with edit permission.

If a user contains the required permission to edit an input, passwords
in the input's config are not masked. This is prevented so the input
edit dialog still functions in the same way as before.

* Adding/adapting tests.

(cherry picked from commit a562a33)

* Fixing import. (#5465)


(cherry picked from commit a9a1df0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.