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

Add ownership for MiqRequest in RBAC #17208

Merged

Conversation

lpichler
Copy link
Contributor

@lpichler lpichler commented Mar 27, 2018

Self service and limited self service users will see only his or his group's users miq_requests after this fix.

For ownership of MiqRequests to users we are using requester_id.

Links

https://bugzilla.redhat.com/show_bug.cgi?id=1545395

@miq-bot assign @gtanzillo

@miq-bot miq-bot added the wip label Mar 27, 2018
@lpichler lpichler force-pushed the restrict_miq_request_by_users_group branch from 2f8885a to 8918547 Compare March 27, 2018 15:55
@miq-bot
Copy link
Member

miq-bot commented Mar 27, 2018

Checked commit lpichler@8918547 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🏆

from get_self_service_objects to
self_service_ownership_scope
¬(A∨B∨C)=¬A∧¬B∧¬C

return nil if miq_group.nil? || !miq_group.self_service? || !(klass < OwnershipMixin)

to

return unless !(miq_group.nil? || !miq_group.self_service? || !(klass < OwnershipMixin))

to

return unless !miq_group.nil? && !!miq_group.self_service? && !!(klass < OwnershipMixin)

to

return unless miq_group.present? && miq_group.self_service? && klass < OwnershipMixin

to(add to method)

return nil unless self_service_ownership_scope?(miq_group, klass)
@lpichler lpichler force-pushed the restrict_miq_request_by_users_group branch from 8918547 to 97324fc Compare March 28, 2018 12:28
@lpichler lpichler changed the title [WIP] Restrict MiqRequest by user's group Add ownership for MiqRequest in RBAC Mar 28, 2018
@gtanzillo gtanzillo self-assigned this Mar 28, 2018
@gtanzillo gtanzillo merged commit 0969759 into ManageIQ:master Mar 28, 2018
@lpichler lpichler deleted the restrict_miq_request_by_users_group branch March 28, 2018 13:58
@simaishi
Copy link
Contributor

simaishi commented Apr 2, 2018

@lpichler @gtanzillo Can this be fine/yes?

simaishi pushed a commit that referenced this pull request Apr 2, 2018
@simaishi
Copy link
Contributor

simaishi commented Apr 2, 2018

Gaprindashvili backport details:

$ git log -1
commit a1ec51830e537bb4d29099051675d7ab006d1d46
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Wed Mar 28 09:28:38 2018 -0400

    Merge pull request #17208 from lpichler/restrict_miq_request_by_users_group
    
    Add ownership for MiqRequest in RBAC
    (cherry picked from commit 09697591a48eb2d1d994ec57d09a842844c0cab7)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1562777

@gtanzillo
Copy link
Member

@simaishi I'm ok with fine/yes but, since @lpichler did the work, I want to make sure he's comfortable with it befioe I set the label.

@lpichler
Copy link
Contributor Author

lpichler commented Apr 3, 2018

@simaishi No, there are conflicts, I will create a FINE PR.

lpichler pushed a commit to lpichler/manageiq that referenced this pull request Apr 3, 2018
…by_users_group

Add ownership for MiqRequest in RBAC
@simaishi
Copy link
Contributor

Backported to Fine via #17245

yrudman added a commit to yrudman/manageiq that referenced this pull request May 23, 2018
@agrare agrare added this to the Sprint 83 Ending Apr 9, 2018 milestone Jun 4, 2018
d-m-u pushed a commit to d-m-u/manageiq that referenced this pull request Jun 6, 2018
…by_users_group

Add ownership for MiqRequest in RBAC
jrafanie added a commit to jrafanie/manageiq that referenced this pull request Jun 29, 2018
MiqRequest was changed to allow ownership for self service and limited
self-service users in ManageIQ ManageIQ#17208, BZ: 1545395

This caused a problem if you had tag filters assign to a user's group
undefined method `find_tags_by_grouping'.  This was fixed in
ManageIQ ManageIQ#17466, BZ: 1576129, and shipped with:

Fine: https://bugzilla.redhat.com/show_bug.cgi?id=1583711
Gaprindindashvili: https://bugzilla.redhat.com/show_bug.cgi?id=1583710

Unfortunately, this second fix to add taggable caused a new bug: users in
groups having tag filters could not see their own requests.

This commit changes MiqRequest to no longer be taggable, since it's not
even taggable in the UI and instead, we add MiqRequest to a list of
models that are RBAC'able but not taggable so we don't try to filter
MiqRequest based on a user's group tag filters.

Credit goes to github user LorkScorguar who reported this issue and
provided lots of diagnostics to help us fix this properly.

For gaprindashvili and fine:
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1596738

For hammer:
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1576129
jrafanie added a commit to jrafanie/manageiq that referenced this pull request Jun 29, 2018
MiqRequest was changed to allow ownership for self service and limited
self-service users in ManageIQ ManageIQ#17208, BZ #1545395

This caused a problem if you had tag filters assign to a user's group
undefined method `find_tags_by_grouping'.  This was fixed in
ManageIQ ManageIQ#17466, BZ #1576129, and shipped with:

Fine: BZ #1583711
Gaprindindashvili: BZ #1583710

Unfortunately, this second fix to add taggable caused a new bug: users in
groups having tag filters could not see their own requests.

This commit changes MiqRequest to no longer be taggable, since it's not
even taggable in the UI and instead, we add MiqRequest to a list of
models that are RBAC'able but not taggable so we don't try to filter
MiqRequest based on a user's group tag filters.

Credit goes to github user LorkScorguar who reported this issue and
provided lots of diagnostics to help us fix this properly.

For gaprindashvili and fine:
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1596738

For hammer:
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1576129
jrafanie added a commit to jrafanie/manageiq that referenced this pull request Jun 29, 2018
MiqRequest was changed to allow ownership for self service and limited
self-service users in ManageIQ ManageIQ#17208, BZ #1545395

This caused a problem if you had tag filters assign to a user's group
undefined method `find_tags_by_grouping'.  This was fixed in
ManageIQ ManageIQ#17466, BZ #1576129, and shipped with:

Fine: BZ #1583711
Gaprindindashvili: BZ #1583710

Unfortunately, this second fix to add taggable caused a new bug: users in
groups having tag filters could not see their own requests.

This commit changes MiqRequest to no longer be taggable, since it's not
even taggable in the UI and instead, we add MiqRequest to a list of
models that are RBAC'able but not taggable so we don't try to filter
MiqRequest based on a user's group tag filters.

Credit goes to github user LorkScorguar who reported this issue and
provided lots of diagnostics to help us fix this properly.

To test this, simply assign managed filters to a user's group, such as
/managed/environments/production, create a request for that user and
try to see that user's request.  They couldn't see it if they received
the intermediate fix, ManageIQ#17466, or if they didn't receive that fix, they'd
receive the `find_tags_by_grouping` error shown above.

For gaprindashvili and fine:
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1596738

For hammer:
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1576129
jrafanie added a commit to jrafanie/manageiq that referenced this pull request Jun 29, 2018
MiqRequest was changed to allow ownership for self service and limited
self-service users in ManageIQ ManageIQ#17208, BZ #1545395

This caused a problem if you had tag filters assign to a user's group
undefined method `find_tags_by_grouping'.  This was fixed in
ManageIQ ManageIQ#17466, BZ #1576129, and shipped with:

Fine: BZ #1583711
Gaprindindashvili: BZ #1583710

Unfortunately, this second fix to add taggable caused a new bug: users in
groups having tag filters could not see their own requests.

This commit changes MiqRequest to no longer be taggable, since it's not
even taggable in the UI and instead, we add MiqRequest to a list of
models that are RBAC'able but not taggable so we don't try to filter
MiqRequest based on a user's group tag filters.

Credit goes to github user LorkScorguar who reported this issue and
provided lots of diagnostics to help us fix this properly.

To test this, simply assign managed filters to a user's group, such as
/managed/environments/production, create a request for that user and
try to see that user's request.  They couldn't see it if they received
the intermediate fix, ManageIQ#17466, or if they didn't receive that fix, they'd
receive the `find_tags_by_grouping` error shown above.

For gaprindashvili and fine:
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1596738

For hammer:
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1576129
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.

5 participants