Skip to content

Fix CommandACL bug that prevents messages to be seen#763

Merged
pstorz merged 5 commits intobareos:masterfrom
alaaeddineelamri:dev/alaaeddineelamri/master/acl-messages-command
Apr 15, 2021
Merged

Fix CommandACL bug that prevents messages to be seen#763
pstorz merged 5 commits intobareos:masterfrom
alaaeddineelamri:dev/alaaeddineelamri/master/acl-messages-command

Conversation

@alaaeddineelamri
Copy link
Contributor

@alaaeddineelamri alaaeddineelamri commented Mar 12, 2021

Thank you for contributing to the Bareos Project!

Description

When using the bconsole in the restricted mode, messages could not be showed to the user with the messages command even though messages were available. This PR aims to fix that issue.

Please check

  • Short description and the purpose of this PR is present above this paragraph
  • Your name is present in the AUTHORS file (optional)

If you have any questions or problems, please give a comment in the PR.

Helpful documentation and best practices

Checklist for the reviewer of the PR (will be processed by the Bareos team)

General
  • PR name is meaningful
  • Purpose of the PR is understood
  • Separate commit for this PR in the CHANGELOG.md, PR number referenced is same
  • Commit descriptions are understandable and well formatted
Source code quality
  • Source code changes are understandable
  • check-sources --since-merge does not report any problems

@alaaeddineelamri alaaeddineelamri force-pushed the dev/alaaeddineelamri/master/acl-messages-command branch from 0bb13b8 to aefcce6 Compare March 12, 2021 12:18
@pstorz pstorz self-requested a review March 15, 2021 11:29
@pstorz pstorz force-pushed the dev/alaaeddineelamri/master/acl-messages-command branch from aefcce6 to 99abeb5 Compare March 15, 2021 12:09
@pstorz pstorz self-assigned this Mar 15, 2021
@pstorz pstorz marked this pull request as ready for review March 15, 2021 12:09
Copy link
Member

@pstorz pstorz left a comment

Choose a reason for hiding this comment

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

Good work

@pstorz pstorz force-pushed the dev/alaaeddineelamri/master/acl-messages-command branch 2 times, most recently from e025c74 to 7d6ebab Compare March 23, 2021 15:35
@pstorz pstorz changed the title messages command bug in restricted mode Fix CommandACL Bug that prevents messages to be seen Mar 23, 2021
@pstorz pstorz changed the title Fix CommandACL Bug that prevents messages to be seen Fix CommandACL bug that prevents messages to be seen Mar 23, 2021
@pstorz pstorz requested a review from arogge March 23, 2021 15:46
@pstorz pstorz force-pushed the dev/alaaeddineelamri/master/acl-messages-command branch from 7d6ebab to 9c87f96 Compare March 23, 2021 16:02
Copy link
Member

@arogge arogge left a comment

Choose a reason for hiding this comment

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

There are some misleading descriptions and commented out directives in the test configurations. I know you just copied them, but it should still be improved.

The ACL for running the messages command is now fixed. However, if I understand it correctly, you will still see the line "you have messages" when there are messages in the buffer even if you don't have access to the messages command.
This should probably also be addressed.

Description = "Restricted console used by tray-monitor to get the status of the director."
Password = "@dir_password@"
CommandACL = status, .status
#JobACL = *all*
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#JobACL = *all*

Description = "Restricted console used by tray-monitor to get the status of the director."
Password = "@dir_password@"
CommandACL = status, .status, configure
#JobACL = *all*
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#JobACL = *all*

Comment on lines +5 to +6
#CommandACL = status, .status
#JobACL = *all*
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#CommandACL = status, .status
#JobACL = *all*

@@ -0,0 +1,7 @@
Console {
Name = bareos-acl-none
Description = "Restricted console used by tray-monitor to get the status of the director."
Copy link
Member

Choose a reason for hiding this comment

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

The description is misleading, as there is no ACL restriction here. Either update the description or remove it.

@@ -0,0 +1,7 @@
Console {
Name = bareos-acl-status-conf
Description = "Restricted console used by tray-monitor to get the status of the director."
Copy link
Member

Choose a reason for hiding this comment

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

The description is misleading, as there is no ACL restriction here. Either update the description or remove it.

@@ -0,0 +1,7 @@
Console {
Name = bareos-acl-status
Description = "Restricted console used by tray-monitor to get the status of the director."
Copy link
Member

Choose a reason for hiding this comment

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

The description is misleading, as there is no ACL restriction here. Either update the description or remove it.

@alaaeddineelamri alaaeddineelamri force-pushed the dev/alaaeddineelamri/master/acl-messages-command branch 5 times, most recently from 03f10cf to c5453d5 Compare March 30, 2021 16:46
@alaaeddineelamri alaaeddineelamri force-pushed the dev/alaaeddineelamri/master/acl-messages-command branch 3 times, most recently from 646d18a to 336baa8 Compare April 10, 2021 14:45
pstorz and others added 5 commits April 13, 2021 08:33
@alaaeddineelamri alaaeddineelamri force-pushed the dev/alaaeddineelamri/master/acl-messages-command branch from 336baa8 to 0d0180b Compare April 13, 2021 07:33
@pstorz pstorz merged commit c7aa25e into bareos:master Apr 15, 2021
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