Do not change current_group for super admin user when executing Rbac#lookup_user_group #17347
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…lookup_user_group Example when updating user.current_group in group's look-up is bad: if widget set-up for different group than during content generation (triggered manually from UI) the last group will become current group for super user and this may throw unexpectd errors ( like failing ApplicationController.assert_privileges(widget_refresh) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1564986
|
@yrudman should this still be WIP? just double checking. |
yrudman
force-pushed
the
do-not-changed-user-current-group-when-do-rback-search
branch
from
1baf52a
to
911af7e
Compare
yrudman
changed the title
|
@gtanzillo It looks strange when we change |
|
@miq-bot add_label blocker |
|
@yrudman if this can be backported, can you add the gaprindashvili/yes and fine/yes labels. |
bdunne
reviewed
Apr 30, 2018
lib/rbac/filterer.rb
Outdated
| @@ -573,16 +573,18 @@ def lookup_user_group(user, userid, miq_group, miq_group_id) | |||
| miq_group_id ||= miq_group.try!(:id) | |||
| return [user, user.current_group] if user && user.current_group_id.to_s == miq_group_id.to_s | |||
|
|
|||
| found_group = user.try(:current_group) | |||
There was a problem hiding this comment.
Maybe it belongs in the missing else before line 583 instead? Then it can be found_group = if... and drop all of the variable setting below.
There was a problem hiding this comment.
@bdunne 👍 it looks much cleaner
|
Checked commits yrudman/manageiq@d7a37e8~...c798fc5 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
simaishi
pushed a commit
that referenced
this pull request
May 1, 2018
…oup-when-do-rback-search Do not change current_group for super admin user when executing Rbac#lookup_user_group (cherry picked from commit 2d81fce) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1573539
|
Gaprindashvili backport details: |
simaishi
pushed a commit
that referenced
this pull request
May 1, 2018
…oup-when-do-rback-search Do not change current_group for super admin user when executing Rbac#lookup_user_group (cherry picked from commit 2d81fce) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1573540
|
Fine backport details: |
d-m-u
pushed a commit
to d-m-u/manageiq
that referenced
this pull request
Jun 6, 2018
…rrent-group-when-do-rback-search Do not change current_group for super admin user when executing Rbac#lookup_user_group (cherry picked from commit 2d81fce) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1573540
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changing
user.current_userfor superadmin and assigning group this admin does not belong-to may trigger not expected errors,like below error raised for super admin in attempt to generate widget's content:
caused by: 55a6a33
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1564986
@miq-bot add-label bug, core, rbac