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

Require privilege network providers and cloud tenants for new cloud network, cloud subnet, network router form #3165

Conversation

lpichler
Copy link
Contributor

@lpichler lpichler commented Jan 4, 2018

imagine that you are the user without allowed section network providers.
then when you go to Network -> Networks|Subnets|Network Routers-> Add new and select any network provider, it will end with error(there is video in BZ)

so I am adding the assert for this feature and I am adding the method role_allows - it check if user has feature related to Network Providers(ems_network_show_list)

@miq-bot assign @himdel
@miq-bot add_label bug, blocker, gaprindashvili/yes

Links

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

@lpichler lpichler force-pushed the require_ems_network_show_list_product_feature_for_cloud_network_new_form branch from c5b734f to 13cca64 Compare January 5, 2018 08:31
@lpichler lpichler changed the title Require allowed network providers for new cloud network form Require allowed network providers for new cloud network, cloud subnet, network router form Jan 5, 2018
@lpichler lpichler force-pushed the require_ems_network_show_list_product_feature_for_cloud_network_new_form branch from 13cca64 to a28f4ab Compare January 5, 2018 08:35
@lpichler lpichler changed the title Require allowed network providers for new cloud network, cloud subnet, network router form [WIP] Require allowed network providers for new cloud network, cloud subnet, network router form Jan 5, 2018
@miq-bot miq-bot added the wip label Jan 5, 2018
@lpichler lpichler changed the title [WIP] Require allowed network providers for new cloud network, cloud subnet, network router form [WIP] Require privilege network providers and cloud tenants for new cloud network, cloud subnet, network router form Jan 5, 2018
@lpichler lpichler changed the title [WIP] Require privilege network providers and cloud tenants for new cloud network, cloud subnet, network router form Require privilege network providers and cloud tenants for new cloud network, cloud subnet, network router form Jan 5, 2018
@miq-bot miq-bot removed the wip label Jan 5, 2018
@lpichler
Copy link
Contributor Author

lpichler commented Jan 5, 2018

Last 2 commits are extending this and adding also privilege for Cloud Tenants as requirement (according this different but related BZ https://bugzilla.redhat.com/show_bug.cgi?id=1520651 )

@lpichler
Copy link
Contributor Author

lpichler commented Jan 5, 2018

cc @martinpovolny

lpichler added a commit to lpichler/manageiq-ui-classic that referenced this pull request Jan 8, 2018
in new cloud volume form

if permission for list of cloud tenant(cloud_tenant_show_list) is missing

Fixes  ManageIQ#3165
@himdel himdel closed this Jan 8, 2018
@himdel himdel reopened this Jan 8, 2018
lpichler added a commit to lpichler/manageiq-ui-classic that referenced this pull request Jan 8, 2018
in new cloud volume form

if permission for list of cloud tenant(cloud_tenant_show_list) is missing

Fixes  ManageIQ#3165
@himdel
Copy link
Contributor

himdel commented Jan 8, 2018

Test failures look relevant ;)

@lpichler
Copy link
Contributor Author

lpichler commented Jan 8, 2018

ce267948f18a43cf36cefb929c739220cfb8a253b6f41a7c24158586f8de5090

@lpichler lpichler force-pushed the require_ems_network_show_list_product_feature_for_cloud_network_new_form branch from d9ae86a to 1c63c15 Compare January 8, 2018 16:44
@lpichler
Copy link
Contributor Author

lpichler commented Jan 8, 2018

should be fixed 👍

@lpichler lpichler closed this Jan 9, 2018
@lpichler lpichler reopened this Jan 9, 2018
@mzazrivec mzazrivec closed this in e37c911 Jan 10, 2018
@mzazrivec mzazrivec reopened this Jan 10, 2018
@lpichler lpichler force-pushed the require_ems_network_show_list_product_feature_for_cloud_network_new_form branch 2 times, most recently from a65a000 to 031656f Compare January 10, 2018 13:20
@lpichler lpichler reopened this Jan 10, 2018
@h-kataria
Copy link
Contributor

@lpichler @himdel i think with role_allows check in place https://github.com/ManageIQ/manageiq-ui-classic/pull/3165/files#diff-fd8f9ab59428266a5f699efcb9933979R7 we shouldn't need those additional assert_priviliges calls

@lpichler
Copy link
Contributor Author

@h-kataria I guess it is beneficial to have these checks in the controller as well for cases if somebody would perform the controller action but not from the toolbar button, but manually.

@lpichler lpichler force-pushed the require_ems_network_show_list_product_feature_for_cloud_network_new_form branch from 8ac0f43 to 52f1f9d Compare January 11, 2018 11:13
@himdel
Copy link
Contributor

himdel commented Jan 11, 2018

@h-kataria That wouldn't stop users from being able to open the screen manually.. But then again, if they do, it just won't work for them.. Shouldn't be a security issue, agreed :)

Then again, it makes it clearer what is really needed by that screen..

@lpichler lpichler force-pushed the require_ems_network_show_list_product_feature_for_cloud_network_new_form branch from 52f1f9d to a7f3c5c Compare January 11, 2018 13:03
@miq-bot
Copy link
Member

miq-bot commented Jan 11, 2018

Checked commits lpichler/manageiq-ui-classic@994be6f~...a7f3c5c with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0
9 files checked, 9 offenses detected

spec/controllers/cloud_network_controller_spec.rb

spec/controllers/cloud_subnet_controller_spec.rb

spec/controllers/network_router_controller_spec.rb

@himdel
Copy link
Contributor

himdel commented Jan 11, 2018

(restarted travis after #3227, hoping for green)

@lpichler
Copy link
Contributor Author

@himdel it is green 👍

@himdel himdel added this to the Sprint 77 Ending Jan 15, 2018 milestone Jan 12, 2018
@himdel himdel merged commit f244a4e into ManageIQ:master Jan 12, 2018
simaishi pushed a commit that referenced this pull request Jan 12, 2018
…product_feature_for_cloud_network_new_form

Require privilege network providers and cloud tenants for new cloud network, cloud subnet, network router form
(cherry picked from commit f244a4e)

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

Gaprindashvili backport details:

$ git log -1
commit 0fc57bc4336c884e4f040eb28c13c12da4f0d513
Author: Martin Hradil <[email protected]>
Date:   Fri Jan 12 16:06:08 2018 +0100

    Merge pull request #3165 from lpichler/require_ems_network_show_list_product_feature_for_cloud_network_new_form
    
    Require privilege network providers and cloud tenants for new cloud network, cloud subnet, network router form
    (cherry picked from commit f244a4e3e6ddc878e38eee2c9701664cea7c1021)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1534057

@lpichler lpichler deleted the require_ems_network_show_list_product_feature_for_cloud_network_new_form branch January 15, 2018 12:46
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.

7 participants