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

Adding Volume Snapshot (CloudVolumeSnapshot) to RBAC #19356

Merged
merged 2 commits into from
Oct 11, 2019

Conversation

jdeubel
Copy link
Member

@jdeubel jdeubel commented Oct 2, 2019

This enables RBAC checking on volume snapshots. Previously RBAC was not filtering on this class and users with managed filters (tags) were able to see all ever instance of a volume snapshot.

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

@jdeubel jdeubel force-pushed the add-cloud_volume_snapshot-to-rbac branch from 4d27db6 to 0a1b41f Compare October 3, 2019 18:46
@gtanzillo gtanzillo requested a review from lpichler October 3, 2019 20:52
@gtanzillo gtanzillo self-assigned this Oct 3, 2019
@lpichler
Copy link
Contributor

lpichler commented Oct 4, 2019

@jdeubel looks good 👍 I tried to test it also with specs:

      context 'searching for instances of CloudVolumeSnapshot' do
        let!(:csv) { FactoryBot.create_list(:cloud_volume_snapshot, 2).first }

        before do
          csv.tag_with('/managed/environment/prod', :ns => '*')
        end

        it 'lists only tagged CloudVolumeSnapshot' do
          results = described_class.search(:class => CloudVolumeSnapshot, :user => user).first
          expect(results).to match_array [csv]
        end

        it 'lists only all CloudVolumeSnapshot' do
          results = described_class.search(:class => CloudVolumeSnapshot, :user => admin_user).first
          expect(results).to match_array CloudVolumeSnapshot.all
        end
      end

so If you can, can you add it to ./spec/lib/rbac/filterer_spec.rb for example on line 214 ?
if not I can add it in follow-up PR.

thanks!

@miq-bot
Copy link
Member

miq-bot commented Oct 10, 2019

Checked commits jdeubel/manageiq@0a1b41f~...385f100 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🏆

Copy link
Contributor

@lpichler lpichler left a comment

Choose a reason for hiding this comment

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

thanks! 👍

@gtanzillo
Copy link
Member

Thanks for the help on this @lpichler

@gtanzillo gtanzillo added this to the Sprint 122 Ending Oct 14, 2019 milestone Oct 11, 2019
@gtanzillo gtanzillo merged commit 68a06a3 into ManageIQ:master Oct 11, 2019
simaishi pushed a commit that referenced this pull request Nov 18, 2019
Adding Volume Snapshot (CloudVolumeSnapshot) to RBAC

(cherry picked from commit 68a06a3)

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

Hammer backport details:

$ git log -1
commit 079f7d367655970e2ab5deca54f549e1d98d2842
Author: Gregg Tanzillo <[email protected]>
Date:   Fri Oct 11 09:33:15 2019 -0400

    Merge pull request #19356 from jdeubel/add-cloud_volume_snapshot-to-rbac
    
    Adding Volume Snapshot (CloudVolumeSnapshot) to RBAC
    
    (cherry picked from commit 68a06a3d9ed7ba8f2a122450657e2e1636053d36)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1773630

simaishi pushed a commit that referenced this pull request Dec 16, 2019
Adding Volume Snapshot (CloudVolumeSnapshot) to RBAC

(cherry picked from commit 68a06a3)

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

Ivanchuk backport details:

$ git log -1
commit 693361c08dd4ed255b0a163472c15cee05cd0016
Author: Gregg Tanzillo <[email protected]>
Date:   Fri Oct 11 09:33:15 2019 -0400

    Merge pull request #19356 from jdeubel/add-cloud_volume_snapshot-to-rbac

    Adding Volume Snapshot (CloudVolumeSnapshot) to RBAC

    (cherry picked from commit 68a06a3d9ed7ba8f2a122450657e2e1636053d36)

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

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