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

Remove Request taggable and prevent tag filtering #17656

Conversation

jrafanie
Copy link
Member

@jrafanie jrafanie commented Jun 29, 2018

MiqRequest was changed to allow ownership for self service and limited
self-service users in 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 #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, #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
Copy link
Member Author

Thank you @LorkScorguar for reporting this issue and providing the detailed information needed to fix it.

@jrafanie
Copy link
Member Author

Added fine/yes and gap/yes since #17466 was backported to both.

Copy link
Member

@gtanzillo gtanzillo left a comment

Choose a reason for hiding this comment

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

Looks good 👍

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 jrafanie force-pushed the remove_miq_request_taggable_and_prevent_tag_filtering_on_it branch from 36a25f2 to f8c0553 Compare June 29, 2018 16:02
@miq-bot
Copy link
Member

miq-bot commented Jun 29, 2018

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

@gtanzillo gtanzillo added this to the Sprint 89 Ending Jul 2, 2018 milestone Jun 29, 2018
@gtanzillo gtanzillo merged commit b856925 into ManageIQ:master Jun 29, 2018
@jrafanie jrafanie deleted the remove_miq_request_taggable_and_prevent_tag_filtering_on_it branch June 29, 2018 16:49
simaishi pushed a commit that referenced this pull request Jul 2, 2018
…nd_prevent_tag_filtering_on_it

Remove Request taggable and prevent tag filtering
(cherry picked from commit b856925)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1597321
@simaishi
Copy link
Contributor

simaishi commented Jul 2, 2018

Gaprindashvili backport details:

$ git log -1
commit afd03d748807cec70df2e7eb10e599499e0ea5d9
Author: Gregg Tanzillo <[email protected]>
Date:   Fri Jun 29 12:46:07 2018 -0400

    Merge pull request #17656 from jrafanie/remove_miq_request_taggable_and_prevent_tag_filtering_on_it
    
    Remove Request taggable and prevent tag filtering
    (cherry picked from commit b856925614bc8130afaa37accf885b9e94b6d8a6)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1597321

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.

4 participants