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

Join RBAC check for MiqUserRole, User and MiqGroup to one if branch #14901

Conversation

lpichler
Copy link
Contributor

@lpichler lpichler commented Apr 26, 2017

RBAC check for MiqUserRole, User, MiqGroup models - these checks are related so
I think they should be together. This could be helpful to implement the other condition for MiqGroup and Group described in #14903

Depends on(reason of WIP):
#14898
Continue in #14903 (blocker)
Starting in commit Join User and Group rbac check to one if branch

@miq-bot add_label wip, fine/yes, refactoring

@miq-bot assign @gtanzillo
cc @kbrock

@lpichler lpichler force-pushed the join_rbac_for_role_user_and_group_to_one_if_branch branch from 06b3f17 to e9b79de Compare May 2, 2017 12:33
@lpichler lpichler changed the title [WIP] Join RBAC check for MiqUserRole, User and MiqGroup to one if branch Join RBAC check for MiqUserRole, User and MiqGroup to one if branch May 2, 2017
@miq-bot miq-bot removed the wip label May 2, 2017
@miq-bot
Copy link
Member

miq-bot commented May 2, 2017

Checked commits lpichler/manageiq@998f45c~...e9b79de with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks fine. 🍪

elsif klass == MiqGroup && miq_group.try!(:self_service?)
# Self Service users searching for groups only see their group
scope.where(:id => miq_group.id)
elsif [User, MiqGroup].include?(klass) && (miq_group || user).try!(:self_service?)
Copy link
Member

Choose a reason for hiding this comment

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

Interestingly, we can do this. Since the miq_group getting here should be always equal to user.current_group (if both supplied). And the self_service? method get's routed to the current_group.

Net result is the same. Even though, I keep scratching my chin at this. :)

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.

👍 LGTM

@gtanzillo gtanzillo added this to the Sprint 60 Ending May 8, 2017 milestone May 2, 2017
@gtanzillo gtanzillo merged commit b8adc25 into ManageIQ:master May 2, 2017
@isimluk
Copy link
Member

isimluk commented May 2, 2017

Hehe. Was gonna merge this also :-D and the button was gone!

@lpichler lpichler deleted the join_rbac_for_role_user_and_group_to_one_if_branch branch May 2, 2017 14:00
@lpichler
Copy link
Contributor Author

lpichler commented May 2, 2017

@miq-bot add_label fine/yes

simaishi pushed a commit that referenced this pull request May 2, 2017
…roup_to_one_if_branch

Join RBAC check for MiqUserRole, User and MiqGroup to one if branch
(cherry picked from commit b8adc25)

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

simaishi commented May 2, 2017

Fine backport details:

$ git log -1
commit bb3be292fc0d30afbbfb38cf1e78c8b8e9660a9d
Author: Gregg Tanzillo <[email protected]>
Date:   Tue May 2 09:14:56 2017 -0400

    Merge pull request #14901 from lpichler/join_rbac_for_role_user_and_group_to_one_if_branch
    
    Join RBAC check for MiqUserRole, User and MiqGroup to one if branch
    (cherry picked from commit b8adc2579749cf3ee7ea096f660ae7cc61961e86)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1447372

@kbrock
Copy link
Member

kbrock commented May 4, 2017

thanks @lpichler - sorry, late to the party

@mfeifer
Copy link
Contributor

mfeifer commented May 5, 2017

@miq-bot add_label bug

@miq-bot miq-bot added the bug label May 5, 2017
@lpichler
Copy link
Contributor Author

@miq-bot add_label euwe/yes

@lpichler
Copy link
Contributor Author

lpichler commented Jun 20, 2017

all related PRs:
#14898
#14901
#14903

simaishi pushed a commit that referenced this pull request Jun 20, 2017
…roup_to_one_if_branch

Join RBAC check for MiqUserRole, User and MiqGroup to one if branch
(cherry picked from commit b8adc25)

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

Euwe backport details:

$ git log -1
commit 904386f381cc447ac8acb6c90a446257d646a329
Author: Gregg Tanzillo <[email protected]>
Date:   Tue May 2 09:14:56 2017 -0400

    Merge pull request #14901 from lpichler/join_rbac_for_role_user_and_group_to_one_if_branch
    
    Join RBAC check for MiqUserRole, User and MiqGroup to one if branch
    (cherry picked from commit b8adc2579749cf3ee7ea096f660ae7cc61961e86)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1460979

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.

8 participants