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
create additional role Sidecar Manager, reviewed existing permissions #15380
create additional role Sidecar Manager, reviewed existing permissions #15380
Conversation
reason: `sidecars:read` gives too many privileges to the system user. It is necessary that the sidecar user cannot read configurations of all sidecars
sidecars:read
to Sidecar System role…admin-privileges-should-it
.../main/java/org/graylog/plugins/sidecar/migrations/V20230502164900_AddSidecarManagerRole.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some minor things and one thing for the Sidecar Reader
role. Currently, this user can click on the edit and update buttons, which results in an error page. I think it would improve the UX if someone from the frontend team would hide these buttons for the reader role. I don't know if we should make these changes in this PR or create an issue for that.
changelog/unreleased/pr-15380.toml
Outdated
@@ -0,0 +1,6 @@ | |||
type = "a" | |||
message = "Add Sidecar Manager role to read and manage sidecars." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we should also mention here the role of the Sidecar Reader
.
import javax.inject.Inject; | ||
import java.time.ZonedDateTime; | ||
|
||
public class V20230502164900_AddSidecarManagerRole extends Migration { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe V20230502164900_AddSidecarManagerAndReaderRole
?
created #15844 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM as well 👍
@kroepke @moesterheld @AntonEbel can this be backported to 5.0? There's a HubSpot issue open for another customer with this same issue (HS-1821909158) who's on Graylog 5.0.10. |
…#15380) * fix: add `sidecars:read` to Sidecar System role * feat: add SIDECARS_READ restriction to `/sidecars/configurations` endpoint * fix: remove `sidecars:read` from Sidecar System role reason: `sidecars:read` gives too many privileges to the system user. It is necessary that the sidecar user cannot read configurations of all sidecars * feat: add Sidecar Manager role * changelog * fix: add license header * fix: add license header * add reader role and add permissions to manager * add collector permissions for manager * change changelog, rename migration --------- Co-authored-by: Anton Ebel <anton.ebel@graylog.com> (cherry picked from commit 346ee6d)
…#15380) * fix: add `sidecars:read` to Sidecar System role * feat: add SIDECARS_READ restriction to `/sidecars/configurations` endpoint * fix: remove `sidecars:read` from Sidecar System role reason: `sidecars:read` gives too many privileges to the system user. It is necessary that the sidecar user cannot read configurations of all sidecars * feat: add Sidecar Manager role * changelog * fix: add license header * fix: add license header * add reader role and add permissions to manager * add collector permissions for manager * change changelog, rename migration --------- Co-authored-by: Anton Ebel <anton.ebel@graylog.com> (cherry picked from commit 346ee6d)
Co-authored-by: Anton Ebel <anton.ebel@graylog.com> Co-authored-by: Matthias Oesterheld <33032967+moesterheld@users.noreply.github.com> fix timestamp of sidecar roles migration (#16477)
Co-authored-by: Anton Ebel <anton.ebel@graylog.com> Co-authored-by: Matthias Oesterheld <33032967+moesterheld@users.noreply.github.com> fix timestamp of sidecar roles migration (#16477)
Description
reviewed permissions of Sidecar System user, made
sidecars/configurations
unavailable to the user since it is only used from the UI, not the sidecar.added Sidecar Manager role with permissions to manage and read sidecars
Motivation and Context
closes #12044
Types of changes
Checklist: