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

Fix RBAC for User and enable tagging for Tenants #17061

Conversation

lpichler
Copy link
Contributor

@lpichler lpichler commented Feb 28, 2018

There are two things in this PR:

1) Fix RBAC for User:

before fix:
Restricted user was able to see all groups and all users.
after fix:
Restricted user can see only his groups and users from his groups

2) Enable tagging for Tenants

Tenants are taggable but this was not included in RBAC.

How it is related to chargeback BZ?

in filter tab, there are list of tenants and users(owners) (select box Show costs by). And these lists will be visible after UI fix for this BZ stated below.

Links

@miq-bot assign @gtanzillo
@miq-bot add_label bug, rbac

@miq-bot add_label gaprindashvili/yes

@lpichler lpichler force-pushed the add_tenant_to_rbac_and_fix_user_and_miq_group branch from ebb5b7f to e8f44c7 Compare February 28, 2018 12:55
@miq-bot
Copy link
Member

miq-bot commented Feb 28, 2018

Checked commits lpichler/manageiq@8194595~...e8f44c7 with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0
5 files checked, 0 offenses detected
Everything looks fine. 🍪

@gtanzillo gtanzillo added this to the Sprint 81 Ending Mar 12, 2018 milestone Mar 7, 2018
@gtanzillo gtanzillo merged commit d31d9d7 into ManageIQ:master Mar 7, 2018
@lpichler lpichler deleted the add_tenant_to_rbac_and_fix_user_and_miq_group branch March 7, 2018 14:47
@himdel
Copy link
Contributor

himdel commented Mar 8, 2018

This PR breaks UI-classic travis:

$ be rspec ./spec/presenters/tree_builder_ops_rbac_spec.rb  ./spec/helpers/application_helper/views_shared_spec.rb ./spec/controllers/ops_controller_spec.rb spec/controllers/ops_controller/ops_rbac_spec.rb

Failures:

  1) OpsController::MiqRegion displays the access object count for the current tenant
     Failure/Error: expect(controller.instance_variable_get(:@groups_count)).to eq(2)
     
       expected: 2
            got: 1
     
       (compared using ==)
     # ./spec/controllers/ops_controller/ops_rbac_spec.rb:599:in `block (3 levels) in <top (required)>'

  2) ApplicationHelper#ownership_user_options a tenant user lists users in that tenant
     Failure/Error: expect(subject.values(&:id).map(&:to_i)).to match_array(ids)
     
       expected collection contained:  [91000000000317, 91000000000318]
       actual collection contained:    [91000000000317]
       the missing elements were:      [91000000000318]
     # ./spec/helpers/application_helper/views_shared_spec.rb:38:in `block (4 levels) in <top (required)>'

  3) OpsController x_button x_button actions rbac group edit
     Failure/Error: expect(response.status).to eq(200)
     
       expected: 200
            got: 500
     
       (compared using ==)
     # ./spec/controllers/ops_controller_spec.rb:36:in `block (4 levels) in <top (required)>'

  4) OpsController rbac_user_edit can add a user w/ group
     Failure/Error: expect(controller).to receive(:replace_right_cell)

(..blob..)
           expected: 1 time with any arguments
           received: 0 times with any arguments
     # ./spec/controllers/ops_controller_spec.rb:88:in `block (3 levels) in <top (required)>'

  5) TreeBuilderOpsRbac#x_get_tree_custom_kids is listing all users
     Failure/Error: expect(objects).to match_array(expected_objects)
     
       expected collection contained:  [#<User id: 91000000000348, name: "Test User 163", email: nil, icon: nil, created_on: "2018-03-08 10:...ame: nil, last_name: nil, password_digest: "$2a$10$FTbGT/y/PQ1HvoOoc1FcyuuTtHzfop/uG/mcEAJLYpz...">]
       actual collection contained:    [#<User id: 91000000000348, name: "Test User 163", email: nil, icon: nil, created_on: "2018-03-08 10:...ame: nil, last_name: nil, password_digest: "$2a$10$FTbGT/y/PQ1HvoOoc1FcyuuTtHzfop/uG/mcEAJLYpz...">]
       the missing elements were:      [#<User id: 91000000000349, name: "Test User 164", email: nil, icon: nil, created_on: "2018-03-08 10:...ame: nil, last_name: nil, password_digest: "$2a$10$FTbGT/y/PQ1HvoOoc1FcyuuTtHzfop/uG/mcEAJLYpz...">]
     # ./spec/presenters/tree_builder_ops_rbac_spec.rb:60:in `x_get_tree_custom_kids_for_and_expect_objects'
     # ./spec/presenters/tree_builder_ops_rbac_spec.rb:64:in `block (3 levels) in <top (required)>'

  6) TreeBuilderOpsRbac#x_get_tree_custom_kids is listing all groups
     Failure/Error: expect(objects).to match_array(expected_objects)
     
       expected collection contained:  [#<MiqGroup id: 91000000001819, description: "Test Group 0000000000154", group_type: "user", sequence..."2018-03-08 10:49:46", updated_on: "2018-03-08 10:49:46", settings: nil, tenant_id: 91000000001173>]
       actual collection contained:    [#<MiqGroup id: 91000000001819, description: "Test Group 0000000000154", group_type: "user", sequence..."2018-03-08 10:49:46", updated_on: "2018-03-08 10:49:46", settings: nil, tenant_id: 91000000001173>]
       the missing elements were:      [#<MiqGroup id: 91000000001820, description: "Test Group 0000000000155", group_type: "user", sequence..."2018-03-08 10:49:46", updated_on: "2018-03-08 10:49:46", settings: nil, tenant_id: 91000000001173>]
     # ./spec/presenters/tree_builder_ops_rbac_spec.rb:60:in `x_get_tree_custom_kids_for_and_expect_objects'
     # ./spec/presenters/tree_builder_ops_rbac_spec.rb:68:in `block (3 levels) in <top (required)>'

Finished in 1 minute 16.03 seconds (files took 7.07 seconds to load)
156 examples, 6 failures

EDIT: looks like this is being addressed in ManageIQ/manageiq-ui-classic#3533 :)

simaishi pushed a commit that referenced this pull request Mar 9, 2018
@simaishi
Copy link
Contributor

simaishi commented Mar 9, 2018

Gaprindashvili backport details:

$ git log -1
commit 37471e6278ae5e05568fb6902b64f965ab316fac
Author: Gregg Tanzillo <[email protected]>
Date:   Wed Mar 7 09:29:48 2018 -0500

    Merge pull request #17061 from lpichler/add_tenant_to_rbac_and_fix_user_and_miq_group
    
    Fix RBAC for User and enable tagging for Tenants
    (cherry picked from commit d31d9d76e40d1824f11d6d97902d24251e3c0207)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1553776
    https://bugzilla.redhat.com/show_bug.cgi?id=1553779

@simaishi
Copy link
Contributor

simaishi commented Apr 9, 2018

@lpichler @gtanzillo Can this PR and ManageIQ/manageiq-ui-classic#3494 be fine/yes? The BZ is marked as 5.8.4 blocker.

@gtanzillo
Copy link
Member

@simaishi I think it can be backported to fine but I'll wait for @lpichler to confirm

@lpichler
Copy link
Contributor Author

lpichler commented Apr 9, 2018

@simaishi yes it can be 👍(hopefully without conflict) but together with ManageIQ/manageiq-ui-classic#3533 (fix for tests in UI repo caused by this PR)

@simaishi
Copy link
Contributor

@lpichler There is a conflict in filterer_spec.rb as context 'with tags' doesn't exist in Fine branch. Should I add it along with before section?

@lpichler
Copy link
Contributor Author

lpichler commented Apr 11, 2018

@simaishi I will create FINE PRs all these (I found to other one in API)

Fix for broken specs:
ManageIQ/manageiq-api#340 (conflict)
ManageIQ/manageiq-ui-classic#3533

Fix for BZ(s):
#17061 (this) (conflict)
ManageIQ/manageiq-ui-classic#3494 (conflict)

@simaishi
Copy link
Contributor

Backported to Fine via #17292

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