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

Docs: Update writer role with least required privileges #13849

Merged
merged 11 commits into from
Oct 14, 2019

Conversation

cwurm
Copy link
Contributor

@cwurm cwurm commented Sep 30, 2019

Updates the writer role documentation based on #13847 and #13848. Also corrects some mistakes.

  1. Changes read from to the correct write to (Beats does not read from indices).
  2. Setting setup.template.enabled to false is no longer necessary after Use less restrictive API to check if template exists #13847.
  3. Setting setup.ilm.overwrite to false is unnecessary if setup.ilm.check_exists is already false (even today).
  4. Adds a note about only monitor and create_doc being always necessary, explicitly calling out the most secure configuration (following Use less restrictive API to check if template exists #13847 and Do not check for alias when setup.ilm.check_exists is false #13848).
  5. Correct what monitor is for: It's for checking things like cluster version and license, not "sending monitor info".
  6. Replaces manage_pipeline with the read-only cluster:admin/ingest/pipeline/get. Unfortunately, there is no read-only cluster role for pipelines, so it requires this privilege. But better than the very permissive manage_pipeline that allows changing any pipeline.
  7. Changes index to the more restrictive, append-only create_doc (introduced in Add 'create_doc' index privilege elasticsearch#45806).

This is one of three PRs to reduce the Beats privileges required in code and documentation:

  1. Use less restrictive API to check if template exists (Use less restrictive API to check if template exists #13847)
  2. Do not check for alias when setup.ilm.check_exists is false (Do not check for alias when setup.ilm.check_exists is false #13848)
  3. Docs: Update writer role with least required privileges (this PR)

Relates: #10241

Copy link
Contributor

@dedemorton dedemorton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating the docs. Found a few minor places in the diff. I also found some other global things that I think we should change, but I'll add that as as separate comment for discussion.

libbeat/docs/security/users.asciidoc Show resolved Hide resolved
libbeat/docs/security/users.asciidoc Outdated Show resolved Hide resolved
libbeat/docs/security/users.asciidoc Outdated Show resolved Hide resolved
libbeat/docs/security/users.asciidoc Outdated Show resolved Hide resolved
libbeat/docs/security/users.asciidoc Outdated Show resolved Hide resolved
libbeat/docs/security/users.asciidoc Outdated Show resolved Hide resolved
libbeat/docs/security/users.asciidoc Outdated Show resolved Hide resolved
libbeat/docs/security/users.asciidoc Outdated Show resolved Hide resolved
@dedemorton
Copy link
Contributor

Note that I checked the changes against 7.4.0 running on cloud (didn't pull down the latest ES snapshot because I didn't think it would make a difference).

There are a couple things I noticed that are probably my fault.

  • Originally, I indicated which privileges were cluster vs index. I'm not sure how that got lost, but I think it's potentially confusing because we do have some privileges that are available as cluster and index privileges. Do you think users will be confused by this?
  • When you're in the UI, it's odd to jump back and forth between cluster and index privileges. We should at least list all the cluster privileges first followed by the index privileges. our current doc build swallows most table formatting, or I'd do something interesting to make this clear.

You can fix these issues or punt them to me, but I'm not sure when I'll have time to open up this topic again.

@urso urso removed their request for review October 7, 2019 12:20
@cwurm
Copy link
Contributor Author

cwurm commented Oct 8, 2019

I've changed from create to the append-only create_doc now that it's merged into ES (elastic/elasticsearch#45806). To use it, we need #13936 - I've tested that it works then.

Copy link
Contributor

@dedemorton dedemorton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving doc changes with the caveat that I have not tested the create_doc privilege.

Thanks for adding the type col. Makes it a lot easier to follow the docs when setting up privileges in Kibana!

@cwurm cwurm merged commit f20aee7 into elastic:master Oct 14, 2019
@cwurm cwurm deleted the writer_docs branch October 14, 2019 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants