From c9e82f790f83bd3c2258df339124499c4d20aded Mon Sep 17 00:00:00 2001 From: timalces Date: Thu, 22 Feb 2024 13:01:08 +0000 Subject: [PATCH 01/11] only team admins can create clusters --- app/controllers/cluster_types_controller.rb | 2 +- app/models/ability.rb | 11 ++-- app/views/cluster_types/_card.html.erb | 2 +- app/views/cluster_types/index.html.erb | 4 +- config/navigation.rb | 4 +- spec/models/ability_spec.rb | 70 +++++++++++++++++++++ 6 files changed, 83 insertions(+), 10 deletions(-) create mode 100644 spec/models/ability_spec.rb diff --git a/app/controllers/cluster_types_controller.rb b/app/controllers/cluster_types_controller.rb index 372039747..e13eb1b5e 100644 --- a/app/controllers/cluster_types_controller.rb +++ b/app/controllers/cluster_types_controller.rb @@ -8,7 +8,7 @@ def index result = SyncAllClusterTypesJob.perform_now(@cloud_service_config, use_cache) flash.now.alert = result.error_message unless result.success? end - @valid_teams = current_user.teams.meets_cluster_credit_requirement + @valid_teams = current_user.teams_where_admin.meets_cluster_credit_requirement @unavailable_teams = current_user.teams.where.not(id: @valid_teams.pluck(:id)) @all_teams = current_user.teams.reorder(:name) @team = Team.find(params[:team_id]) if params[:team_id] diff --git a/app/models/ability.rb b/app/models/ability.rb index 4ae518784..e7701c29f 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -38,14 +38,17 @@ def non_root_abilities can :read, InteractiveRackView can :read, Template - can :manage, Chassis, location: {rack: {team_id: @user.team_ids }} - can :manage, Device, chassis: {location: {rack: {team_id: @user.team_ids }}} - can :manage, HwRack, team_id: @user.team_ids + can :read, Chassis, location: {rack: {team_id: @user.team_ids }} + can :read, Device, chassis: {location: {rack: {team_id: @user.team_ids }}} + can :read, HwRack, team_id: @user.team_ids + can :manage, Chassis, location: {rack: {team_id: @user.teams_where_admin.pluck(:id) }} + can :manage, Device, chassis: {location: {rack: {team_id: @user.teams_where_admin.pluck(:id) }}} + can :manage, HwRack, team_id: @user.teams_where_admin.pluck(:id) can :manage, RackviewPreset, user: @user can :read, ClusterType - can :create, Cluster, team_id: @user.team_ids + can :create, Cluster, team_id: @user.teams_where_admin.pluck(:id) can :read, KeyPair, user: @user can :create, KeyPair, user: @user diff --git a/app/views/cluster_types/_card.html.erb b/app/views/cluster_types/_card.html.erb index bcd260019..caa1f9200 100644 --- a/app/views/cluster_types/_card.html.erb +++ b/app/views/cluster_types/_card.html.erb @@ -26,7 +26,7 @@ <% title = if !current_user.can?(:create, Cluster) "You do not have permission to create a cluster" elsif !available_teams - "Unable to create a cluster - you must belong to a team with at least #{Rails.application.config.cluster_credit_requirement} credits to create a cluster" + "Unable to create a cluster - you must be admin for a team with at least #{Rails.application.config.cluster_credit_requirement} credits to create a cluster" else "Please select a team" end diff --git a/app/views/cluster_types/index.html.erb b/app/views/cluster_types/index.html.erb index af45e7bea..abd4e61f9 100644 --- a/app/views/cluster_types/index.html.erb +++ b/app/views/cluster_types/index.html.erb @@ -35,12 +35,12 @@
- Must have at least <%= Rails.application.config.cluster_credit_requirement %> credits + You must be a team admin and the team have at least <%= Rails.application.config.cluster_credit_requirement %> credits
<% else %>

- You must belong to a team with at least <%= Rails.application.config.cluster_credit_requirement %> credits to create a cluster. + You must be admin for a team with at least <%= Rails.application.config.cluster_credit_requirement %> credits to create a cluster.

<% end %> diff --git a/config/navigation.rb b/config/navigation.rb index ced066203..749ded6f1 100644 --- a/config/navigation.rb +++ b/config/navigation.rb @@ -27,9 +27,9 @@ if current_user.can?(:read, ClusterType) html_options = {} - if !current_user.teams.meets_cluster_credit_requirement.exists? + if !current_user.teams_where_admin.meets_cluster_credit_requirement.exists? html_options[:class] = "limited-action-icon" - html_options[:title] = "You must belong to a team with at least #{Rails.application.config.cluster_credit_requirement} credits to create a cluster" + html_options[:title] = "You must be admin for a team with at least #{Rails.application.config.cluster_credit_requirement} credits to create a cluster" end primary.item :cluster_types, 'Launch cluster', url_helpers.cluster_types_path, diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb new file mode 100644 index 000000000..7b74f6b2b --- /dev/null +++ b/spec/models/ability_spec.rb @@ -0,0 +1,70 @@ +require 'rails_helper' + +RSpec.describe Ability, type: :model do + let!(:user) { create(:user) } + + describe "#enough_credits_to_create_cluster?" do + it "is false if zero" do + user.credits = 0 + expect(Ability.new(user).enough_credits_to_create_cluster?).to eq false + end + + context 'user has team' do + + shared_examples 'is admin for another team' do + let!(:another_team_role) { create(:team_role, user: user, team: another_team, role: "admin") } + + it 'it is true if at least one team above or equal to requirement' do + team.credits = 0 + team.save! + Rails.application.config.cluster_credit_requirement = 10 + another_team.credits = 10 + another_team.save! + expect(Ability.new(user).enough_credits_to_create_cluster?).to eq true + another_team.credits = 11 + another_team.save! + expect(Ability.new(user).enough_credits_to_create_cluster?).to eq true + end + end + + context 'user is a member' do + let!(:team_role) { create(:team_role, user: user, team: team, role: "member") } + + it 'is false' do + expect(Ability.new(user).enough_credits_to_create_cluster?).to eq false + end + + include_examples 'is admin for another team' + end + + context 'user is admin' do + let!(:team_role) { create(:team_role, user: user, team: team, role: "admin") } + + it "is false if team has no credits" do + team.credits = 0 + team.save! + expect(Ability.new(user).enough_credits_to_create_cluster?).to eq false + end + + it "is false if below set requirement" do + Rails.application.config.cluster_credit_requirement = 10 + team.credits = 9 + team.save! + expect(Ability.new(user).enough_credits_to_create_cluster?).to eq false + end + + it "is true if above or equal to requirement" do + Rails.application.config.cluster_credit_requirement = 10 + team.credits = 10 + team.save! + expect(Ability.new(user).enough_credits_to_create_cluster?).to eq true + team.credits = 11 + team.save! + expect(Ability.new(user).enough_credits_to_create_cluster?).to eq true + end + + include_examples 'is admin for another team' + end + end + end +end From f14de9e29351373f44cac843180eaf7d0094302c Mon Sep 17 00:00:00 2001 From: timalces Date: Fri, 23 Feb 2024 18:14:41 +0000 Subject: [PATCH 02/11] added role based permissions to irv actions menu --- .../irv/_interactive_canvas_view.scss | 4 ++ app/controllers/api/v1/users_controller.rb | 48 ++++++++----------- app/javascript/canvas/common/util/RBAC.js | 32 ++++--------- app/javascript/canvas/irv/view/Chassis.js | 4 ++ app/javascript/canvas/irv/view/ContextMenu.js | 4 +- app/javascript/canvas/irv/view/Rack.js | 1 - app/javascript/canvas/irv/view/RackObject.js | 4 ++ app/javascript/canvas/irv/view/RackSpace.js | 7 ++- .../canvas/irv/view/RackSpaceDragHandler.js | 3 +- app/jobs/broadcast_rack_change_job.rb | 9 ++-- app/models/interactive_rack_view.rb | 14 +++++- app/presenters/api/v1/rack_presenter.rb | 2 +- app/services/irv/hw_rack_services/index.rb | 3 +- app/views/api/v1/irv/racks/index.rabl | 2 +- app/views/api/v1/irv/racks/show.rabl | 7 +++ .../_configuration.json | 32 ++++++++----- config/routes.rb | 2 +- 17 files changed, 99 insertions(+), 79 deletions(-) diff --git a/app/assets/stylesheets/irv/_interactive_canvas_view.scss b/app/assets/stylesheets/irv/_interactive_canvas_view.scss index 9f0df6974..4ad0c4752 100644 --- a/app/assets/stylesheets/irv/_interactive_canvas_view.scss +++ b/app/assets/stylesheets/irv/_interactive_canvas_view.scss @@ -157,6 +157,10 @@ -moz-box-shadow: 3px 3px 12px 3px rgba(0,0,0,0.3); -webkit-box-shadow: 3px 3px 12px 3px rgba(0,0,0,0.3); box-shadow: 3px 3px 12px 3px rgba(0,0,0,0.3); + + .menu-spacer:last-child { + display: none; /* Hide spacer if no subsequent options */ + } } .disabled_context_menu_item diff --git a/app/controllers/api/v1/users_controller.rb b/app/controllers/api/v1/users_controller.rb index da85c5701..20687aeb2 100644 --- a/app/controllers/api/v1/users_controller.rb +++ b/app/controllers/api/v1/users_controller.rb @@ -1,5 +1,5 @@ class Api::V1::UsersController < Api::V1::ApplicationController - load_and_authorize_resource :user, :class => User, except: [:current, :can_i?] + load_and_authorize_resource :user, :class => User, except: [:current, :permissions] def index @users = @users.map {|user| Api::V1::UserPresenter.new(user)} @@ -38,36 +38,26 @@ def destroy end # - # GET /api/v1/users/can_i + # GET /api/v1/users/permissions # - # Endpoint for cancan check - this just passes the "can" request on to the - # cancan ability checker - used to check yourself and your own abilities. + # Endpoint for specifying what permissions each team role/ being root provides. + # This is based on the assumption that such permissions are based purely + # on team role for the given object (or being root). # - def can_i? - # On the permissions params, this action should receive a structure of the - # following form - # - # { - # "permissions" => { - # "manage" => {"0" => "HwRack", "1" => "Device"}, - # "read" => {"0" => "Device"}, - # "move" => {"0" => "Device"}, - # } - # } - - result = {} - params[:permissions].each do |rbac_action,rbac_resources| - result[rbac_action] = {} - rbac_resources.each do |_, rbac_resource| - if rbac_resource == "all" - result[rbac_action][rbac_resource] = current_user.can?(rbac_action.to_sym, :all) - elsif rbac_resource.safe_constantize - result[rbac_action][rbac_resource] = current_user.can?(rbac_action.to_sym, rbac_resource.safe_constantize) - else - result[rbac_action][rbac_resource] = false - end - end - end + def permissions + admins = %w(superAdmin admin) + all = admins + ["member"] + result = { + manage: { + racks: admins, devices: admins, chassis: admins + }, + move: { + racks: [], devices: admins, chassis: admins + }, + view: { + racks: all, devices: all, chassis: all + } + } render json: result end diff --git a/app/javascript/canvas/common/util/RBAC.js b/app/javascript/canvas/common/util/RBAC.js index 58ab80a7b..aeb656986 100644 --- a/app/javascript/canvas/common/util/RBAC.js +++ b/app/javascript/canvas/common/util/RBAC.js @@ -3,12 +3,10 @@ import Events from 'canvas/common/util/Events'; // RBAC = Rule Based Access Control // -// RBAC queries the api action /-/api/v1/users/can_i with a -// specific set of permissions (getPermissionsToQuery) on construction time. +// RBAC queries the api action /-/api/v1/users/permissions on construction time. // Then, via the function can_i, the results obtained from the api call are queried. -// This class is shared between the DCRV and DCPV. class RBAC { - static PATH = '/api/v1/users/can_i'; + static PATH = '/api/v1/users/permissions'; constructor({onSuccess}) { this.onSuccessCallback = onSuccess; @@ -18,7 +16,6 @@ class RBAC { headers : {'X-CSRF-Token': $$('meta[name="csrf-token"]')[0].getAttribute('content')}, url : RBAC.PATH, method : 'get', - data : this.getPermissionsToQuery(), onSuccess : this.permisionsReceived }).send(); } @@ -29,34 +26,23 @@ class RBAC { } permisionsReceived(permissions) { - this.debug("recevied permissions"); + this.debug("received permissions"); this.permissions = permissions; if (this.onSuccessCallback) { this.onSuccessCallback() } } - getPermissionsToQuery() { - return { - permissions: - { - manage: ["HwRack", "Device", "Chassis"], - move: ["Device", "Chassis"], - view: ["all"] - } - }; + can_i(action, resource, teamRole) { + return this.permissions[action][resource].includes(teamRole); } - can_i(action, resource) { - return this.permissions[action][resource] === true; + can_i_move_device(device) { + return this.can_i("move", "devices", device.teamRole) || this.can_i("move", "chassis", device.teamRole); } - can_i_move_devices() { - return this.can_i("move", "Device") || this.can_i("move", "Chassis"); - } - - can_i_manage_devices() { - return this.can_i("manage", "Device") || this.can_i("manage", "Chassis"); + can_i_manage_device(device) { + return this.can_i("manage", "devices", device.teamRole) || this.can_i("manage", "chassis", device.teamRole); } debug(...msg) { diff --git a/app/javascript/canvas/irv/view/Chassis.js b/app/javascript/canvas/irv/view/Chassis.js index 6d7fe0a9d..09c8e49c3 100644 --- a/app/javascript/canvas/irv/view/Chassis.js +++ b/app/javascript/canvas/irv/view/Chassis.js @@ -151,6 +151,10 @@ class Chassis extends RackObject { } } + teamRole() { + + } + setCoordsBasedOnUStart() { const x = (this.parent().x + (this.parent().width / 2)) - (this.width/2); const y = (RackObject.U_PX_HEIGHT * (this.parent().uHeight - this.uStart() - this.uHeight)) + this.parent().chassisOffsetY + this.parent().y; diff --git a/app/javascript/canvas/irv/view/ContextMenu.js b/app/javascript/canvas/irv/view/ContextMenu.js index 52eab0b66..940cb3b3b 100644 --- a/app/javascript/canvas/irv/view/ContextMenu.js +++ b/app/javascript/canvas/irv/view/ContextMenu.js @@ -112,8 +112,8 @@ class ContextMenu { for (var option of Array.from(total_options)) { // If the option has the attribute RBAC defined, then query the @model.RBAC object // to see if such permission has been granted. Otherwise, continue to the next option. - if (option.RBAC != null) { - if (!this.model.RBAC.can_i(option.RBAC.action,option.RBAC.resource)) { continue; } + if (option.rbacAction != null) { + if (!this.model.RBAC.can_i(option.rbacAction, option_set, device.teamRole)) { continue; } } if (option.availableToBuildStatuses !== undefined && option.availableToBuildStatuses.indexOf(device.buildStatus) === -1) { diff --git a/app/javascript/canvas/irv/view/Rack.js b/app/javascript/canvas/irv/view/Rack.js index 87029b320..c958f0dff 100644 --- a/app/javascript/canvas/irv/view/Rack.js +++ b/app/javascript/canvas/irv/view/Rack.js @@ -123,7 +123,6 @@ class Rack extends RackObject { this.selected = false; this.visible = true; this.assets = []; - this.addImageLink(); const len = def.chassis.length; diff --git a/app/javascript/canvas/irv/view/RackObject.js b/app/javascript/canvas/irv/view/RackObject.js index 8ad8ef1bc..604aefc90 100644 --- a/app/javascript/canvas/irv/view/RackObject.js +++ b/app/javascript/canvas/irv/view/RackObject.js @@ -57,6 +57,10 @@ class RackObject extends RackSpaceObject { this.parent = ko.observable(this.parent); this.parent.subscribe(this.setLayers); + // Based on the assumption that items in a rack share the same role. + // This will need to change if we have more specific permissions e.g. for individual devices + this.teamRole = def.teamRole || this.parent().teamRole; + this.setLayers(); this.availableSpaces = []; diff --git a/app/javascript/canvas/irv/view/RackSpace.js b/app/javascript/canvas/irv/view/RackSpace.js index 8a8190b57..1702796df 100644 --- a/app/javascript/canvas/irv/view/RackSpace.js +++ b/app/javascript/canvas/irv/view/RackSpace.js @@ -1013,7 +1013,7 @@ class RackSpace { evHalfFlipped(img_id) { const show_u_labels = this.scale >= RackSpace.U_LBL_SCALE_CUTOFF; const show_name_label = this.scale >= RackSpace.NAME_LBL_SCALE_CUTOFF; - const show_owner_label = show_name_label && this.model.RBAC.can_i("view", "all"); + const show_owner_label = show_name_label; // redraw the rack in the (hidden) rack layer, since the rack image in the fx layer is a slice of the rack layer it will automatically // reflect the changes @@ -1376,14 +1376,13 @@ class RackSpace { this.infoGfx.setScale(this.targetScale); this.alertGfx.setScale(this.targetScale); - // decide wether to show rack labels + // decide whether to show rack labels const show_name_label = this.targetScale >= RackSpace.NAME_LBL_SCALE_CUTOFF; const show_u_labels = this.targetScale >= RackSpace.U_LBL_SCALE_CUTOFF; - const show_owner_label = show_name_label && this.model.RBAC.can_i("view", "all"); if (this.model.showingRacks()) { for (var rack of Array.from(this.racks)) { - if (rack instanceof Rack) { rack.showOwnerLabel(show_owner_label); } + if (rack instanceof Rack) { rack.showOwnerLabel(show_name_label); } rack.showNameLabel(show_name_label); if (rack instanceof Rack) { rack.showULabels(show_u_labels, this.targetScale); } diff --git a/app/javascript/canvas/irv/view/RackSpaceDragHandler.js b/app/javascript/canvas/irv/view/RackSpaceDragHandler.js index 12ff266f0..1ffa2c016 100644 --- a/app/javascript/canvas/irv/view/RackSpaceDragHandler.js +++ b/app/javascript/canvas/irv/view/RackSpaceDragHandler.js @@ -31,6 +31,7 @@ class RackSpaceDragHandler { if (this.draggee == null || !this.canIMoveThisItem()) { // If not over a device or we're not permitted to move it abort. + this.draggee = null; return; } @@ -111,7 +112,7 @@ class RackSpaceDragHandler { } doIHavePermissionToMoveOrDrag() { - return ((this.draggee instanceof Chassis || this.draggee instanceof Machine) && this.model.RBAC.can_i_move_devices()); + return ((this.draggee instanceof Chassis || this.draggee instanceof Machine) && this.model.RBAC.can_i_move_device(this.draggee)); } // public method, updates dragging of a draggee box or device diff --git a/app/jobs/broadcast_rack_change_job.rb b/app/jobs/broadcast_rack_change_job.rb index d4149726b..a0a035c13 100644 --- a/app/jobs/broadcast_rack_change_job.rb +++ b/app/jobs/broadcast_rack_change_job.rb @@ -7,13 +7,16 @@ def perform(rack_id, team_id, action) else msg = rack_content(rack_id, action) end - user_ids = TeamRole.where(team_id: team_id).pluck(:user_id) - User.where(root: true).or(User.where(id: user_ids)).each do |user| + user_roles = TeamRole.where(team_id: team_id) + role_mapping = user_roles.pluck(:user_id, :role).to_h + User.where(root: true).or(User.where(id: role_mapping.keys)).each do |user| + role = user.root? ? "superAdmin" : role_mapping[user.id] + msg[:rack][:teamRole] = role InteractiveRackViewChannel.broadcast_to(user, msg) end end def rack_content(rack_id, action) - { action: action, rack: Irv::HwRackServices::Show.call(rack_id) } + { action: action, rack: Irv::HwRackServices::Show.call(rack_id) } end end diff --git a/app/models/interactive_rack_view.rb b/app/models/interactive_rack_view.rb index 661e268a0..079c32604 100644 --- a/app/models/interactive_rack_view.rb +++ b/app/models/interactive_rack_view.rb @@ -64,6 +64,16 @@ def rack_ids(racks, user) end end + def role_query(user) + return unless user + + if user.root + "( SELECT 'superAdmin' as \"teamRole\" ) as \"teamRole\"," + else + "( SELECT TR.role AS \"teamRole\" FROM team_roles TR WHERE TR.team_id = R.team_id AND TR.user_id = '#{user.id.to_s}' LIMIT 1) AS \"teamRole\"," + end + end + def generate_sql(racks, user) ids = rack_ids(racks, user) sanitized_ids = ids.map { |id| "'#{ApplicationRecord.sanitize_sql(id)}'" }.join(',') @@ -74,6 +84,7 @@ def generate_sql(racks, user) SELECT racks.id AS id, racks.name AS name, racks.u_height AS u_height, racks.status AS status, ROUND(racks.cost, 2) AS cost, racks.template_id AS template_id, racks.team_id AS team_id FROM racks JOIN teams as teams ON racks.team_id = teams.id + JOIN team_roles as team_roles ON racks.team_id = team_roles.team_id ORDER BY LOWER(teams.name) , SUBSTRING("racks"."name" FROM E'^(.*?)(\\\\d+)?$') , LPAD(SUBSTRING( "racks"."name" FROM E'(\\\\d+)$'), 30, '0') ASC @@ -87,6 +98,7 @@ def generate_sql(racks, user) R.u_height AS "uHeight" , R.status AS "buildStatus" , cast(R.cost as money) AS "cost", + #{role_query(user)} ( SELECT id FROM sorted_racks OFFSET (SELECT row_num FROM (SELECT id,row_number() OVER () AS row_num FROM sorted_racks) t WHERE id=R.id) LIMIT 1) AS "nextRackId"), ( SELECT XmlElement( name "owner", XmlAttributes (O.id, O.name)) FROM teams O WHERE O.id = R.team_id LIMIT 1 @@ -155,7 +167,7 @@ def generate_sql(racks, user) ) ) ) - ) FROM sorted_racks R + ) FROM sorted_racks R SQL ret + condition diff --git a/app/presenters/api/v1/rack_presenter.rb b/app/presenters/api/v1/rack_presenter.rb index 043720bbd..ccf11a639 100644 --- a/app/presenters/api/v1/rack_presenter.rb +++ b/app/presenters/api/v1/rack_presenter.rb @@ -8,7 +8,7 @@ class RackPresenter < Presenter # Be selective about what attributes and methods we expose. delegate :id, :name, :u_height, :metadata, :status, :template, :rack_start_u, :rack_end_u, - :network_details, :creation_output, :order_id, to: :o + :network_details, :creation_output, :order_id, :team_id, to: :o def devices @devices ||= o.devices.occupying_rack_u.map {|d| Api::V1::DevicePresenter.new(d) } diff --git a/app/services/irv/hw_rack_services/index.rb b/app/services/irv/hw_rack_services/index.rb index 1ee723ca1..7488bb991 100644 --- a/app/services/irv/hw_rack_services/index.rb +++ b/app/services/irv/hw_rack_services/index.rb @@ -28,11 +28,12 @@ def call @racks = @user.root? ? HwRack.all : @user.racks @racks = @racks.where(id: @rack_ids) if @rack_ids&.any? @racks = @racks.map { |rack| Api::V1::RackPresenter.new(rack) } - renderer = Rabl::Renderer.new('api/v1/irv/racks/index', @racks, view_path: 'app/views', format: 'hash') + renderer = Rabl::Renderer.new('api/v1/irv/racks/index', @racks, view_path: 'app/views', format: 'hash', locals: { user: @user} ) {Racks: {Rack: renderer.render}} else # The fast and awkward to understand method. irv_rack_structure = Crack::XML.parse(InteractiveRackView.get_structure(@rack_ids, @user)) + Rails.logger.info(irv_rack_structure) fix_structure(irv_rack_structure) irv_rack_structure end diff --git a/app/views/api/v1/irv/racks/index.rabl b/app/views/api/v1/irv/racks/index.rabl index b3d951503..c70e39c0e 100644 --- a/app/views/api/v1/irv/racks/index.rabl +++ b/app/views/api/v1/irv/racks/index.rabl @@ -1,4 +1,4 @@ collection @racks node do |rack| - partial('api/v1/irv/racks/show', :object => rack) + partial('api/v1/irv/racks/show', object: rack, locals: { user: locals[:user] }) end diff --git a/app/views/api/v1/irv/racks/show.rabl b/app/views/api/v1/irv/racks/show.rabl index 1449b86c0..c33d7d0ce 100644 --- a/app/views/api/v1/irv/racks/show.rabl +++ b/app/views/api/v1/irv/racks/show.rabl @@ -2,6 +2,13 @@ object @rack attributes :id, :name attribute :currency_cost => :cost attributes u_height: :uHeight, status: :buildStatus +node(:team_role) do |rack| + if locals[:user].root + "superAdmin" + else + locals[:user].team_roles.where(team_id: rack.team_id).pluck(:role).first + end +end child :team, root: 'owner' do extends 'api/v1/teams/show' diff --git a/app/views/interactive_rack_views/_configuration.json b/app/views/interactive_rack_views/_configuration.json index 16bf6716b..6fd97845a 100644 --- a/app/views/interactive_rack_views/_configuration.json +++ b/app/views/interactive_rack_views/_configuration.json @@ -362,7 +362,7 @@ "*COMMENT*" : "e.g. for a rack and chassis options will also be displayed for a device", "verbose" : false, "urlInternalPrefix" : "internal::", - "spacer" : "", + "spacer" : "", "aspectMap": { @@ -404,7 +404,8 @@ }, { "caption" : "View details", - "url" : "/racks/[[rack_id]]" + "url" : "/racks/[[rack_id]]", + "rbacAction" : "view" }, { "caption": "[[spacer]]" @@ -412,7 +413,8 @@ { "caption" : "Destroy", "url" : "internal::statusChangeRequest,destroy,racks,[[rack_id]],[[rack_name]]", - "availableToBuildStatuses": ["STOPPED", "ACTIVE", "FAILED"] + "availableToBuildStatuses": ["STOPPED", "ACTIVE", "FAILED"], + "rbacAction" : "manage" } ], @@ -423,7 +425,8 @@ }, { "caption" : "Focus", - "url" : "internal::focusOn,chassis,[[chassis_id]]" + "url" : "internal::focusOn,chassis,[[chassis_id]]", + "rbacAction" : "view" } ], @@ -434,11 +437,13 @@ }, { "caption" : "Focus", - "url" : "internal::focusOn,devices,[[device_id]]" + "url" : "internal::focusOn,devices,[[device_id]]", + "rbacAction" : "view" }, { "caption" : "View details", - "url" : "/devices/[[device_id]]" + "url" : "/devices/[[device_id]]", + "rbacAction" : "view" }, { "caption": "[[spacer]]" @@ -446,27 +451,32 @@ { "caption" : "Switch off", "url" : "internal::statusChangeRequest,off,devices,[[device_id]],[[device_name]]", - "availableToBuildStatuses": ["ACTIVE"] + "availableToBuildStatuses": ["ACTIVE"], + "rbacAction" : "manage" }, { "caption" : "Switch on", "url" : "internal::statusChangeRequest,on,devices,[[device_id]],[[device_name]]", - "availableToBuildStatuses": ["STOPPED"] + "availableToBuildStatuses": ["STOPPED"], + "rbacAction" : "manage" }, { "caption" : "Suspend", "url" : "internal::statusChangeRequest,suspend,devices,[[device_id]],[[device_name]]", - "availableToBuildStatuses": ["ACTIVE"] + "availableToBuildStatuses": ["ACTIVE"], + "rbacAction" : "manage" }, { "caption" : "Resume", "url" : "internal::statusChangeRequest,resume,devices,[[device_id]],[[device_name]]", - "availableToBuildStatuses": ["SUSPENDED"] + "availableToBuildStatuses": ["SUSPENDED"], + "rbacAction" : "manage" }, { "caption" : "Destroy", "url" : "internal::statusChangeRequest,destroy,devices,[[device_id]],[[device_name]]", - "availableToBuildStatuses": ["STOPPED", "ACTIVE", "SUSPENDED", "FAILED"] + "availableToBuildStatuses": ["STOPPED", "ACTIVE", "SUSPENDED", "FAILED"], + "rbacAction" : "manage" } ], diff --git a/config/routes.rb b/config/routes.rb index 06189d7bf..cededee59 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -126,7 +126,7 @@ resources :users, only: [:index, :update, :destroy] do collection do # Endpoint for checking user abilities. - get :can_i, action: :can_i?, as: :ability_check + get :permissions, action: :permissions, as: :permissions # Endpoint for getting the currently signed in user. get :current end From 36f34d7bed5cf597fef228b01f45a3550a1fefc1 Mon Sep 17 00:00:00 2001 From: timalces Date: Wed, 6 Mar 2024 13:59:54 +0000 Subject: [PATCH 03/11] removed unwanted join in racks query --- app/models/interactive_rack_view.rb | 3 +- spec/models/ability_spec.rb | 70 ----------------------------- 2 files changed, 1 insertion(+), 72 deletions(-) delete mode 100644 spec/models/ability_spec.rb diff --git a/app/models/interactive_rack_view.rb b/app/models/interactive_rack_view.rb index 079c32604..2062ba528 100644 --- a/app/models/interactive_rack_view.rb +++ b/app/models/interactive_rack_view.rb @@ -67,7 +67,7 @@ def rack_ids(racks, user) def role_query(user) return unless user - if user.root + if user.root? "( SELECT 'superAdmin' as \"teamRole\" ) as \"teamRole\"," else "( SELECT TR.role AS \"teamRole\" FROM team_roles TR WHERE TR.team_id = R.team_id AND TR.user_id = '#{user.id.to_s}' LIMIT 1) AS \"teamRole\"," @@ -84,7 +84,6 @@ def generate_sql(racks, user) SELECT racks.id AS id, racks.name AS name, racks.u_height AS u_height, racks.status AS status, ROUND(racks.cost, 2) AS cost, racks.template_id AS template_id, racks.team_id AS team_id FROM racks JOIN teams as teams ON racks.team_id = teams.id - JOIN team_roles as team_roles ON racks.team_id = team_roles.team_id ORDER BY LOWER(teams.name) , SUBSTRING("racks"."name" FROM E'^(.*?)(\\\\d+)?$') , LPAD(SUBSTRING( "racks"."name" FROM E'(\\\\d+)$'), 30, '0') ASC diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb deleted file mode 100644 index 7b74f6b2b..000000000 --- a/spec/models/ability_spec.rb +++ /dev/null @@ -1,70 +0,0 @@ -require 'rails_helper' - -RSpec.describe Ability, type: :model do - let!(:user) { create(:user) } - - describe "#enough_credits_to_create_cluster?" do - it "is false if zero" do - user.credits = 0 - expect(Ability.new(user).enough_credits_to_create_cluster?).to eq false - end - - context 'user has team' do - - shared_examples 'is admin for another team' do - let!(:another_team_role) { create(:team_role, user: user, team: another_team, role: "admin") } - - it 'it is true if at least one team above or equal to requirement' do - team.credits = 0 - team.save! - Rails.application.config.cluster_credit_requirement = 10 - another_team.credits = 10 - another_team.save! - expect(Ability.new(user).enough_credits_to_create_cluster?).to eq true - another_team.credits = 11 - another_team.save! - expect(Ability.new(user).enough_credits_to_create_cluster?).to eq true - end - end - - context 'user is a member' do - let!(:team_role) { create(:team_role, user: user, team: team, role: "member") } - - it 'is false' do - expect(Ability.new(user).enough_credits_to_create_cluster?).to eq false - end - - include_examples 'is admin for another team' - end - - context 'user is admin' do - let!(:team_role) { create(:team_role, user: user, team: team, role: "admin") } - - it "is false if team has no credits" do - team.credits = 0 - team.save! - expect(Ability.new(user).enough_credits_to_create_cluster?).to eq false - end - - it "is false if below set requirement" do - Rails.application.config.cluster_credit_requirement = 10 - team.credits = 9 - team.save! - expect(Ability.new(user).enough_credits_to_create_cluster?).to eq false - end - - it "is true if above or equal to requirement" do - Rails.application.config.cluster_credit_requirement = 10 - team.credits = 10 - team.save! - expect(Ability.new(user).enough_credits_to_create_cluster?).to eq true - team.credits = 11 - team.save! - expect(Ability.new(user).enough_credits_to_create_cluster?).to eq true - end - - include_examples 'is admin for another team' - end - end - end -end From b43894588741da857a7181dd0afec62e3b95f72d Mon Sep 17 00:00:00 2001 From: timalces Date: Wed, 6 Mar 2024 18:09:08 +0000 Subject: [PATCH 04/11] logic fixes and updated tests --- app/javascript/canvas/irv/view/Chassis.js | 4 -- app/jobs/broadcast_rack_change_job.rb | 6 ++- app/models/user.rb | 2 +- app/services/irv/hw_rack_services/index.rb | 1 - app/views/api/v1/irv/racks/show.rabl | 2 +- app/views/api/v1/racks/index.rabl | 2 +- spec/jobs/broadcast_rack_change_job_spec.rb | 3 +- spec/models/user_spec.rb | 44 +++++++++++++++++++ .../api/v1/devices_controller_spec.rb | 17 +++++-- .../api/v1/irv/racks_controller_spec.rb | 1 + spec/requests/api/v1/racks_controller_spec.rb | 14 +++++- 11 files changed, 79 insertions(+), 17 deletions(-) diff --git a/app/javascript/canvas/irv/view/Chassis.js b/app/javascript/canvas/irv/view/Chassis.js index 09c8e49c3..6d7fe0a9d 100644 --- a/app/javascript/canvas/irv/view/Chassis.js +++ b/app/javascript/canvas/irv/view/Chassis.js @@ -151,10 +151,6 @@ class Chassis extends RackObject { } } - teamRole() { - - } - setCoordsBasedOnUStart() { const x = (this.parent().x + (this.parent().width / 2)) - (this.width/2); const y = (RackObject.U_PX_HEIGHT * (this.parent().uHeight - this.uStart() - this.uHeight)) + this.parent().chassisOffsetY + this.parent().y; diff --git a/app/jobs/broadcast_rack_change_job.rb b/app/jobs/broadcast_rack_change_job.rb index a0a035c13..19da6342e 100644 --- a/app/jobs/broadcast_rack_change_job.rb +++ b/app/jobs/broadcast_rack_change_job.rb @@ -10,8 +10,10 @@ def perform(rack_id, team_id, action) user_roles = TeamRole.where(team_id: team_id) role_mapping = user_roles.pluck(:user_id, :role).to_h User.where(root: true).or(User.where(id: role_mapping.keys)).each do |user| - role = user.root? ? "superAdmin" : role_mapping[user.id] - msg[:rack][:teamRole] = role + unless action == "deleted" + role = user.root? ? "superAdmin" : role_mapping[user.id] + msg[:rack][:teamRole] = role + end InteractiveRackViewChannel.broadcast_to(user, msg) end end diff --git a/app/models/user.rb b/app/models/user.rb index b27877319..291585099 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -126,6 +126,6 @@ def mark_as_pending_deletion end def teams_where_admin - @teams_where_admin ||= teams.where(team_roles: { role: 'admin' }) + @teams_where_admin ||= teams.where(team_roles: { role: 'admin'}) end end diff --git a/app/services/irv/hw_rack_services/index.rb b/app/services/irv/hw_rack_services/index.rb index 7488bb991..753cf96c5 100644 --- a/app/services/irv/hw_rack_services/index.rb +++ b/app/services/irv/hw_rack_services/index.rb @@ -33,7 +33,6 @@ def call else # The fast and awkward to understand method. irv_rack_structure = Crack::XML.parse(InteractiveRackView.get_structure(@rack_ids, @user)) - Rails.logger.info(irv_rack_structure) fix_structure(irv_rack_structure) irv_rack_structure end diff --git a/app/views/api/v1/irv/racks/show.rabl b/app/views/api/v1/irv/racks/show.rabl index c33d7d0ce..6f3b84a37 100644 --- a/app/views/api/v1/irv/racks/show.rabl +++ b/app/views/api/v1/irv/racks/show.rabl @@ -2,7 +2,7 @@ object @rack attributes :id, :name attribute :currency_cost => :cost attributes u_height: :uHeight, status: :buildStatus -node(:team_role) do |rack| +node(:teamRole) do |rack| if locals[:user].root "superAdmin" else diff --git a/app/views/api/v1/racks/index.rabl b/app/views/api/v1/racks/index.rabl index cc843bfab..2963035d7 100644 --- a/app/views/api/v1/racks/index.rabl +++ b/app/views/api/v1/racks/index.rabl @@ -1,4 +1,4 @@ object @racks node do |rack| - partial('api/v1/racks/show', :object => rack) + partial('api/v1/racks/show', object: rack ) end diff --git a/spec/jobs/broadcast_rack_change_job_spec.rb b/spec/jobs/broadcast_rack_change_job_spec.rb index 1140aa7ab..260adff9a 100644 --- a/spec/jobs/broadcast_rack_change_job_spec.rb +++ b/spec/jobs/broadcast_rack_change_job_spec.rb @@ -3,7 +3,7 @@ RSpec.describe BroadcastRackChangeJob, type: :job do let(:user) { create(:user) } let(:team) { create(:team) } - let!(:team_role) { create(:team_role, team: team, user: user) } + let!(:team_role) { create(:team_role, team: team, user: user, role: "member") } let(:template) { create(:template, :rack_template) } let(:device_template) { create(:template, :device_template) } let!(:rack) { create(:rack, team: team, template: template) } @@ -33,6 +33,7 @@ expect(rack_data["id"]).to eq rack.id.to_s expect(rack_data["name"]).to eq rack.name expect(rack_data["cost"]).to eq "$0.00" + expect(rack_data["teamRole"]).to eq "member" } expect { subject }.to have_broadcasted_to(user).from_channel(InteractiveRackViewChannel).with(nil, &expected) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index c8b57a57a..4ddd0c270 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -74,4 +74,48 @@ end end end + + describe 'teams where admin' do + let!(:team) { create(:team) } + + context "no team roles" do + it "returns empty" do + expect(user.teams_where_admin).to eq [] + end + end + + context "with member role" do + let!(:role) { create(:team_role, user: user, team: team, role: "member") } + + it "returns empty" do + expect(user.teams_where_admin).to eq [] + end + + context "team has other users with roles" do + let!(:other_users_role) { create(:team_role, team: team, role: "member") } + let!(:another_users_role) { create(:team_role, team: team, role: "admin") } + + it "returns empty" do + expect(user.teams_where_admin).to eq [] + end + end + end + + context "with admin role" do + let!(:role) { create(:team_role, user: user, team: team, role: "admin") } + + it "returns team" do + expect(user.teams_where_admin).to eq [team] + end + + context "with roles in other teams" do + let!(:other_role) { create(:team_role, user: user, role: "member") } + let!(:another_role) { create(:team_role, user: user, role: "admin") } + + it "returns all teams where admin" do + expect(user.teams_where_admin.sort).to eq [team, another_role.team].sort + end + end + end + end end diff --git a/spec/requests/api/v1/devices_controller_spec.rb b/spec/requests/api/v1/devices_controller_spec.rb index 069bcc309..37c7db0f6 100644 --- a/spec/requests/api/v1/devices_controller_spec.rb +++ b/spec/requests/api/v1/devices_controller_spec.rb @@ -32,7 +32,7 @@ include_examples "unauthorised JSON response" end - context "when logged in as admin" do + context "when logged in as super admin" do include_context "Logged in as admin" context "when there are no racks" do @@ -117,7 +117,7 @@ end end - context "when logged in as admin" do + context "when logged in as super admin" do include_context "Logged in as admin" include_examples "successful JSON response" @@ -444,18 +444,27 @@ def send_request context "when logged in as device's rack team member" do include_context "Logged in as non-admin" - let!(:team_role) { create(:team_role, team: device.rack.team, user: authenticated_user) } + include_examples "forbidden JSON response" do + let(:request_method) { :patch } + let!(:team_role) { create(:team_role, team: device.rack.team, user: authenticated_user, role: "member") } + end + end + + context "when logged in as device's rack team admin" do + include_context "Logged in as non-admin" + let!(:team_role) { create(:team_role, team: device.rack.team, user: authenticated_user, role: "admin") } include_examples "authorized user updating device" end context "when logged in as another user" do include_context "Logged in as non-admin" include_examples "forbidden JSON response" do + let(:request_method) { :patch } let!(:team_role) { create(:team_role, user: authenticated_user) } end end - context "when logged in as admin" do + context "when logged in as super admin" do include_context "Logged in as admin" include_examples "authorized user updating device" end diff --git a/spec/requests/api/v1/irv/racks_controller_spec.rb b/spec/requests/api/v1/irv/racks_controller_spec.rb index 880af91a7..f782397f8 100644 --- a/spec/requests/api/v1/irv/racks_controller_spec.rb +++ b/spec/requests/api/v1/irv/racks_controller_spec.rb @@ -50,6 +50,7 @@ expect(parsed_rack["name"]).to eq rack.name expect(parsed_rack["uHeight"].to_i).to eq rack.u_height expect(parsed_rack["cost"]).to eq "$9.99" + expect(parsed_rack["teamRole"]).to eq "superAdmin" end it "includes the rack's template" do diff --git a/spec/requests/api/v1/racks_controller_spec.rb b/spec/requests/api/v1/racks_controller_spec.rb index e389daf58..954bd4c1b 100644 --- a/spec/requests/api/v1/racks_controller_spec.rb +++ b/spec/requests/api/v1/racks_controller_spec.rb @@ -363,17 +363,27 @@ def send_request include_examples "unauthorised JSON response" end - context "when logged in as member of rack's team" do + context "when logged in as admin of rack's team" do include_context "Logged in as non-admin" - let!(:team_role) { create(:team_role, user: authenticated_user, team: rack.team) } + let!(:team_role) { create(:team_role, user: authenticated_user, team: rack.team, role: "admin") } include_examples "authorized user updating rack" do let(:can_update_order_id) { false } end end + context "when logged in as member of rack's team" do + include_context "Logged in as non-admin" + let!(:team_role) { create(:team_role, user: authenticated_user, team: rack.team, role: "member") } + include_examples "forbidden JSON response" do + let(:request_method) { :patch } + let!(:team_role) { create(:team_role, user: authenticated_user) } + end + end + context "when logged in as another user" do include_context "Logged in as non-admin" include_examples "forbidden JSON response" do + let(:request_method) { :patch } let!(:team_role) { create(:team_role, user: authenticated_user) } end end From 8ffe7d6ca44b958e9962693584ea252f89fd199f Mon Sep 17 00:00:00 2001 From: timalces Date: Wed, 6 Mar 2024 18:33:10 +0000 Subject: [PATCH 05/11] added edge case handling --- app/controllers/team_roles_controller.rb | 2 +- app/javascript/canvas/irv/view/RackObject.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/team_roles_controller.rb b/app/controllers/team_roles_controller.rb index bffc20380..94d8dc0f9 100644 --- a/app/controllers/team_roles_controller.rb +++ b/app/controllers/team_roles_controller.rb @@ -94,7 +94,7 @@ def update if result.success? flash[:info] = "Successfully updated team role" - redirect_to team_team_roles_path(@team_role.team, @team_role) + redirect_to @team_role.user == current_user ? teams_path : team_team_roles_path(@team_role.team) else flash[:alert] = "Unable to update team role" render action: :edit diff --git a/app/javascript/canvas/irv/view/RackObject.js b/app/javascript/canvas/irv/view/RackObject.js index 604aefc90..36e711f32 100644 --- a/app/javascript/canvas/irv/view/RackObject.js +++ b/app/javascript/canvas/irv/view/RackObject.js @@ -59,7 +59,7 @@ class RackObject extends RackSpaceObject { // Based on the assumption that items in a rack share the same role. // This will need to change if we have more specific permissions e.g. for individual devices - this.teamRole = def.teamRole || this.parent().teamRole; + this.teamRole = def.teamRole || this.parent()?.teamRole || "unknown"; this.setLayers(); From 05788ff4e7572e0e3e5b8f9c5b00c80c47599e14 Mon Sep 17 00:00:00 2001 From: timalces Date: Thu, 7 Mar 2024 12:21:49 +0000 Subject: [PATCH 06/11] added team role change broadcast tests --- app/models/user.rb | 2 +- app/presenters/api/v1/rack_presenter.rb | 2 +- app/views/api/v1/racks/index.rabl | 2 +- spec/models/team_role_spec.rb | 60 ++++++++++++++++++++++++- spec/rails_helper.rb | 2 +- 5 files changed, 63 insertions(+), 5 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 291585099..b27877319 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -126,6 +126,6 @@ def mark_as_pending_deletion end def teams_where_admin - @teams_where_admin ||= teams.where(team_roles: { role: 'admin'}) + @teams_where_admin ||= teams.where(team_roles: { role: 'admin' }) end end diff --git a/app/presenters/api/v1/rack_presenter.rb b/app/presenters/api/v1/rack_presenter.rb index ccf11a639..043720bbd 100644 --- a/app/presenters/api/v1/rack_presenter.rb +++ b/app/presenters/api/v1/rack_presenter.rb @@ -8,7 +8,7 @@ class RackPresenter < Presenter # Be selective about what attributes and methods we expose. delegate :id, :name, :u_height, :metadata, :status, :template, :rack_start_u, :rack_end_u, - :network_details, :creation_output, :order_id, :team_id, to: :o + :network_details, :creation_output, :order_id, to: :o def devices @devices ||= o.devices.occupying_rack_u.map {|d| Api::V1::DevicePresenter.new(d) } diff --git a/app/views/api/v1/racks/index.rabl b/app/views/api/v1/racks/index.rabl index 2963035d7..6c7d8b05d 100644 --- a/app/views/api/v1/racks/index.rabl +++ b/app/views/api/v1/racks/index.rabl @@ -1,4 +1,4 @@ object @racks node do |rack| - partial('api/v1/racks/show', object: rack ) + partial('api/v1/racks/show', object: rack) end diff --git a/spec/models/team_role_spec.rb b/spec/models/team_role_spec.rb index 65d66f62f..fd212c6cd 100644 --- a/spec/models/team_role_spec.rb +++ b/spec/models/team_role_spec.rb @@ -2,7 +2,7 @@ RSpec.describe TeamRole, type: :model do subject { team_role } - let!(:team_role) { create(:team_role) } + let(:team_role) { create(:team_role, role: "member", team: team) } let(:team) { create(:team) } let(:user) { create(:user) } @@ -66,5 +66,63 @@ expect(subject).to have_error(:role, :inclusion) end end + + it "must be a unique user team combination" do + new_role = build(:team_role, team: subject.team, user: subject.user) + expect(new_role).to have_error(:user_id, :taken) + new_role.team = create(:team) + expect(new_role).to be_valid + end + end + + describe "broadcast changes" do + let!(:template) { create(:template, :rack_template) } + let!(:rack) { create(:rack, team: team, template: template) } + let(:user) { team_role.user } + + shared_examples 'rack details' do + it 'broadcasts rack details' do + expect { subject }.to have_broadcasted_to(user).from_channel(InteractiveRackViewChannel).with { |data| + expect(data["action"]).to eq "latest_full_data" + rack_data = data["Racks"]["Rack"][0] + expect(rack_data.present?).to be true + expect(rack_data["owner"]["id"]).to eq rack.team.id.to_s + expect(rack_data["template"]["name"]).to eq rack.template.name + expect(rack_data["id"]).to eq rack.id.to_s + expect(rack_data["name"]).to eq rack.name + expect(rack_data["cost"]).to eq "$0.00" + expect(rack_data["teamRole"]).to eq team_role.role + } + end + end + + context 'created' do + let!(:user) { create(:user) } + let(:team_role) { build(:team_role, team: team, role: "member", user: user) } + + subject { team_role.save! } + + include_examples 'rack details' + end + + context 'updated' do + subject do + team_role.role = "admin" + team_role.save! + end + + include_examples 'rack details' + end + + context 'deleted' do + subject { team_role.destroy! } + + it "broadcasts rack data, without the team's rack" do + expect { subject }.to have_broadcasted_to(user).from_channel(InteractiveRackViewChannel).with { |data| + expect(data["action"]).to eq "latest_full_data" + expect(data["Racks"]["Rack"]).to eq [] + } + end + end end end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 7258008df..4e9763812 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -39,7 +39,7 @@ end RSpec.configure do |config| # Remove this line if you're not using ActiveRecord or ActiveRecord fixtures - config.fixture_path = "#{::Rails.root}/spec/fixtures" + config.fixture_paths = ["#{::Rails.root}/spec/fixtures"] # If you're not using ActiveRecord, or you'd prefer not to run each of your # examples within a transaction, remove the following line or assign false From 5130d7a79c65dc67a20028d24def08b87e3037f7 Mon Sep 17 00:00:00 2001 From: timalces Date: Tue, 12 Mar 2024 10:36:49 +0000 Subject: [PATCH 07/11] allow members to see rack devices list --- app/models/ability.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/ability.rb b/app/models/ability.rb index e7701c29f..20a8f2cc3 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -40,7 +40,7 @@ def non_root_abilities can :read, Template can :read, Chassis, location: {rack: {team_id: @user.team_ids }} can :read, Device, chassis: {location: {rack: {team_id: @user.team_ids }}} - can :read, HwRack, team_id: @user.team_ids + can [:read, :devices], HwRack, team_id: @user.team_ids can :manage, Chassis, location: {rack: {team_id: @user.teams_where_admin.pluck(:id) }} can :manage, Device, chassis: {location: {rack: {team_id: @user.teams_where_admin.pluck(:id) }}} can :manage, HwRack, team_id: @user.teams_where_admin.pluck(:id) From 05a77ec0f104b169a3ca697ee7f95370125f7152 Mon Sep 17 00:00:00 2001 From: timalces Date: Tue, 12 Mar 2024 10:38:33 +0000 Subject: [PATCH 08/11] fix rack devices action links --- app/views/racks/devices.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/racks/devices.html.erb b/app/views/racks/devices.html.erb index 055df40ef..fd6b20669 100644 --- a/app/views/racks/devices.html.erb +++ b/app/views/racks/devices.html.erb @@ -16,7 +16,7 @@ t.attribute_column :cost, sortable: true t.actions_column do |actions, device| - actions.add title: 'View details', path: device_path(device) + actions.add text: 'View details', path: device_path(device) end end end From 9261a1303d29917d456003f033236df4a24829ca Mon Sep 17 00:00:00 2001 From: timalces Date: Tue, 12 Mar 2024 10:46:54 +0000 Subject: [PATCH 09/11] improved wording --- app/views/cluster_types/index.html.erb | 2 +- config/navigation.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/cluster_types/index.html.erb b/app/views/cluster_types/index.html.erb index abd4e61f9..0985593b4 100644 --- a/app/views/cluster_types/index.html.erb +++ b/app/views/cluster_types/index.html.erb @@ -40,7 +40,7 @@ <% else %>

- You must be admin for a team with at least <%= Rails.application.config.cluster_credit_requirement %> credits to create a cluster. + You must be an admin for a team with at least <%= Rails.application.config.cluster_credit_requirement %> credits to create a cluster.

<% end %> diff --git a/config/navigation.rb b/config/navigation.rb index 749ded6f1..7e40ebc83 100644 --- a/config/navigation.rb +++ b/config/navigation.rb @@ -29,7 +29,7 @@ html_options = {} if !current_user.teams_where_admin.meets_cluster_credit_requirement.exists? html_options[:class] = "limited-action-icon" - html_options[:title] = "You must be admin for a team with at least #{Rails.application.config.cluster_credit_requirement} credits to create a cluster" + html_options[:title] = "You must be an admin for a team with at least #{Rails.application.config.cluster_credit_requirement} credits to create a cluster" end primary.item :cluster_types, 'Launch cluster', url_helpers.cluster_types_path, From 01a7c079e1a8b5ed2b5fa528cce61a84deab799c Mon Sep 17 00:00:00 2001 From: timalces Date: Tue, 12 Mar 2024 11:13:29 +0000 Subject: [PATCH 10/11] improved tests --- spec/factories/users.rb | 21 +++++++++++++++++++++ spec/jobs/broadcast_rack_change_job_spec.rb | 3 +-- spec/models/device_spec.rb | 3 +-- spec/models/hw_rack_spec.rb | 3 +-- spec/models/team_role_spec.rb | 2 +- spec/system/invoices/resource_table_spec.rb | 3 +-- 6 files changed, 26 insertions(+), 9 deletions(-) diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 345a8cbec..a99b9c903 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -31,4 +31,25 @@ create(:team_role, team: rack.team, user: user) end end + + trait :with_team_role do + transient do + role { 'member' } + team { create(:team) } + end + + after(:create) do |user, evaluator| + user.team_roles.create!(role: evaluator.role, team: evaluator.team) + end + end + + trait :as_team_member do + with_team_role + role { 'member' } + end + + trait :as_team_admin do + with_team_role + role { 'admin' } + end end diff --git a/spec/jobs/broadcast_rack_change_job_spec.rb b/spec/jobs/broadcast_rack_change_job_spec.rb index 260adff9a..4b52fc5c4 100644 --- a/spec/jobs/broadcast_rack_change_job_spec.rb +++ b/spec/jobs/broadcast_rack_change_job_spec.rb @@ -1,9 +1,8 @@ require 'rails_helper' RSpec.describe BroadcastRackChangeJob, type: :job do - let(:user) { create(:user) } + let(:user) { create(:user, :as_team_member, team: team) } let(:team) { create(:team) } - let!(:team_role) { create(:team_role, team: team, user: user, role: "member") } let(:template) { create(:template, :rack_template) } let(:device_template) { create(:template, :device_template) } let!(:rack) { create(:rack, team: team, template: template) } diff --git a/spec/models/device_spec.rb b/spec/models/device_spec.rb index f17e987af..9c41def82 100644 --- a/spec/models/device_spec.rb +++ b/spec/models/device_spec.rb @@ -7,8 +7,7 @@ let(:chassis) { create(:chassis, location: location, template: device_template) } let(:location) { create(:location, rack: rack) } let!(:rack) { create(:rack, template: rack_template) } - let(:user) { create(:user) } - let!(:team_role) { create(:team_role, team: rack.team, user: user) } + let(:user) { create(:user, :as_team_member, team: rack.team) } let(:device_template) { create(:template, :device_template) } describe 'validations' do diff --git a/spec/models/hw_rack_spec.rb b/spec/models/hw_rack_spec.rb index 1be517ad9..a7b84e997 100644 --- a/spec/models/hw_rack_spec.rb +++ b/spec/models/hw_rack_spec.rb @@ -149,8 +149,7 @@ end describe "broadcast changes" do - let!(:user) { create(:user) } - let!(:team_role) { create(:team_role, user: user, team: team) } + let!(:user) { create(:user, :as_team_member, team: team) } shared_examples 'rack details' do it 'broadcasts rack details' do diff --git a/spec/models/team_role_spec.rb b/spec/models/team_role_spec.rb index fd212c6cd..eaf356371 100644 --- a/spec/models/team_role_spec.rb +++ b/spec/models/team_role_spec.rb @@ -37,7 +37,7 @@ it "must be a unique user team combination" do new_role = build(:team_role, team: subject.team, user: subject.user) expect(new_role).to have_error(:user_id, :taken) - new_role.team = team + new_role.team = create(:team) expect(new_role).to be_valid end diff --git a/spec/system/invoices/resource_table_spec.rb b/spec/system/invoices/resource_table_spec.rb index 7175d6224..c2132e84d 100644 --- a/spec/system/invoices/resource_table_spec.rb +++ b/spec/system/invoices/resource_table_spec.rb @@ -2,9 +2,8 @@ RSpec.describe "invoices index page table", type: :system do let(:user_password) { 'user-password' } - let!(:user) { create(:user, :with_openstack_account, password: user_password) } + let!(:user) { create(:user, :with_openstack_account, :as_team_admin, password: user_password, team: team) } let!(:team) { create(:team, :with_openstack_details) } - let!(:team_role) { create(:team_role, user: user, team: team, role: "admin") } let(:items_per_page) { 20 } before(:each) do From c88f5ff7852ca9658b23324e6b790fc878e48dd9 Mon Sep 17 00:00:00 2001 From: timalces Date: Tue, 12 Mar 2024 11:32:25 +0000 Subject: [PATCH 11/11] always show rack owner label if showing name label in irv --- app/javascript/canvas/irv/view/Rack.js | 4 ++-- app/javascript/canvas/irv/view/RackSpace.js | 6 ++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/app/javascript/canvas/irv/view/Rack.js b/app/javascript/canvas/irv/view/Rack.js index c958f0dff..ceded8a73 100644 --- a/app/javascript/canvas/irv/view/Rack.js +++ b/app/javascript/canvas/irv/view/Rack.js @@ -306,14 +306,14 @@ class Rack extends RackObject { })(); } - draw(show_u_labels, show_name_label, show_owner_label) { + draw(show_u_labels, show_name_label) { Profiler.begin(Profiler.DEBUG, this.draw); // clear for (var asset of Array.from(this.assets)) { RackObject.RACK_GFX.remove(asset); } this.assets = []; // add labels as necessary - this.showOwnerLabel(show_owner_label); + this.showOwnerLabel(show_name_label); this.showNameLabel(show_name_label); this.showULabels(show_u_labels); diff --git a/app/javascript/canvas/irv/view/RackSpace.js b/app/javascript/canvas/irv/view/RackSpace.js index 1702796df..c08fe6f60 100644 --- a/app/javascript/canvas/irv/view/RackSpace.js +++ b/app/javascript/canvas/irv/view/RackSpace.js @@ -539,9 +539,8 @@ class RackSpace { const show_u_labels = this.scale >= RackSpace.U_LBL_SCALE_CUTOFF; const show_name_label = this.scale >= RackSpace.NAME_LBL_SCALE_CUTOFF; - const show_owner_label = show_name_label; if (this.model.showingRacks()) { - for (var rack of Array.from(this.racks)) { rack.draw(show_u_labels, show_name_label, show_owner_label); } + for (var rack of Array.from(this.racks)) { rack.draw(show_u_labels, show_name_label); } this.updateRackImage(); } if (this.model.showHoldingArea()) { @@ -1013,11 +1012,10 @@ class RackSpace { evHalfFlipped(img_id) { const show_u_labels = this.scale >= RackSpace.U_LBL_SCALE_CUTOFF; const show_name_label = this.scale >= RackSpace.NAME_LBL_SCALE_CUTOFF; - const show_owner_label = show_name_label; // redraw the rack in the (hidden) rack layer, since the rack image in the fx layer is a slice of the rack layer it will automatically // reflect the changes - this.rackLookup[img_id].draw(show_u_labels, show_name_label, show_owner_label); + this.rackLookup[img_id].draw(show_u_labels, show_name_label); const x = this.fx.getAttribute(img_id, 'x'); const width = this.fx.getAttribute(img_id, 'sliceWidth');