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

Don't render partial page content when missing permissions #4416

Merged
merged 5 commits into from Dec 14, 2017
Merged

Conversation

@kroepke
Copy link
Member

@kroepke kroepke commented Dec 13, 2017

Hides various content when the user does not have the correct permission to avoid accidentally leaking information in the future.

Added a new authentication:read permission to disambiguate the clusterconfigentry:read check

fix #4407

kroepke added 4 commits Dec 13, 2017
disallow reading authentication provider config if the user doesn't have the necessary permission

#4407
actual data loading is prevented by the server, too
also hide create/upload buttons if the user doesn't have the necessary permissions

the backend already disallowed any actions
…ission

the backend already checks the necessary permissions before returning data
@kroepke kroepke added this to the 2.4.0 milestone Dec 13, 2017
@ghost ghost assigned kroepke Dec 13, 2017
@kroepke
Copy link
Member Author

@kroepke kroepke commented Dec 13, 2017

Against master, should be backported to 2.4 for next release.

@kroepke
Copy link
Member Author

@kroepke kroepke commented Dec 13, 2017

linter errors are due to old codebase

@bernd bernd requested a review from edmundoa Dec 13, 2017
@bernd
Copy link
Member

@bernd bernd commented Dec 13, 2017

@edmundoa Can you please review this? 😃

@kroepke kroepke changed the title Issue 4407 Don't render partial page content when missing permissions Dec 13, 2017
Copy link
Member

@edmundoa edmundoa left a comment

Other than those inline comments, this looks good to me.


const GrokPatternsPage = React.createClass({
render() {
return (
<DocumentTitle title="Grok patterns">
<GrokPatterns />
<IfPermitted permissions="inputs:read">
<GrokPatterns />

This comment has been minimized.

@edmundoa

edmundoa Dec 13, 2017
Member

Shouldn't we leave the permissions check to the component that renders the entity? With your changes, we are already checking in GrokPatterns for the right permissions, so I guess this other check can be avoided.

This comment has been minimized.

@kroepke

kroepke Dec 14, 2017
Author Member

True!

dataRowFormatter={this._patternFormatter}
filterLabel="Filter patterns"
filterKeys={filterKeys} />
<IfPermitted permissions="inputs:read">

This comment has been minimized.

@edmundoa

edmundoa Dec 13, 2017
Member

This DataTable component also renders delete and edit buttons for each grok pattern. They don't seem to work, but it feels "wrong". I guess we should either disable (or remove) those buttons, or not render the table unless the user can edit it.

… (only affected rendering)

don't check permissions twice: once on the page and once in the component
@kroepke
Copy link
Member Author

@kroepke kroepke commented Dec 14, 2017

@edmundoa Fixed, I also cleaned up some more obvious eslint warnings.

Copy link
Member

@edmundoa edmundoa left a comment

LGTM 👍

@edmundoa edmundoa merged commit ea4df1d into master Dec 14, 2017
4 of 5 checks passed
4 of 5 checks passed
@garybot2
ci-web-linter Jenkins build graylog-pr-linter-check 2130 has failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@garybot2
graylog-project/pr Jenkins build graylog-project-pr-snapshot 813 has succeeded
Details
license/cla Contributor License Agreement is signed.
Details
@ghost ghost removed the ready-for-review label Dec 14, 2017
@edmundoa edmundoa deleted the issue-4407 branch Dec 14, 2017
edmundoa added a commit that referenced this pull request Dec 14, 2017
* add authentication:read permission

disallow reading authentication provider config if the user doesn't have the necessary permission

#4407

* do not render authentication page content if permissions are missing

actual data loading is prevented by the server, too

* disallow access to grokpatterns page

also hide create/upload buttons if the user doesn't have the necessary permissions

the backend already disallowed any actions

* hide logger page content if the user does not have the necessary permission

the backend already checks the necessary permissions before returning data

* don't display edit buttons when the user lacks permission to use them (only affected rendering)

don't check permissions twice: once on the page and once in the component

(cherry picked from commit ea4df1d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants