diff --git a/app/assets/javascripts/modules/namespaces/pages/index.vue b/app/assets/javascripts/modules/namespaces/pages/index.vue index 320ef23ef..8e594e259 100644 --- a/app/assets/javascripts/modules/namespaces/pages/index.vue +++ b/app/assets/javascripts/modules/namespaces/pages/index.vue @@ -1,5 +1,7 @@ @@ -54,6 +58,9 @@ canCreateNamespace: { type: Boolean, }, + accessibleTeamsIds: { + type: Array, + }, }, components: { @@ -65,31 +72,50 @@ data() { return { state: NamespacesStore.state, - normalNamespaces: [], - specialNamespaces: [], + namespaces: [], }; }, + computed: { + otherNamespaces() { + // eslint-disable-next-line + return this.namespaces.filter((n) => { + return !n.global && + n.id !== this.userNamespaceId && + this.accessibleTeamsIds.indexOf(n.team_id) === -1; + }); + }, + + normalNamespaces() { + // eslint-disable-next-line + return this.namespaces.filter((n) => { + return !n.global && + n.id !== this.userNamespaceId && + this.accessibleTeamsIds.indexOf(n.team_id) !== -1; + }); + }, + + specialNamespaces() { + return this.namespaces.filter(n => n.global || n.id === this.userNamespaceId); + }, + }, + methods: { onCreate(namespace) { - const currentNamespaces = this.normalNamespaces; + const currentNamespaces = this.namespaces; const namespaces = [ ...currentNamespaces, namespace, ]; - set(this, 'normalNamespaces', namespaces); + set(this, 'namespaces', namespaces); }, loadData() { NamespacesService.all().then((response) => { const namespaces = response.data; - const normal = namespaces.filter(n => !n.global && n.id !== this.userNamespaceId); - const special = namespaces.filter(n => n.global || n.id === this.userNamespaceId); - - set(this, 'normalNamespaces', normal); - set(this, 'specialNamespaces', special); + set(this, 'namespaces', namespaces); set(this.state, 'isLoading', false); }); }, diff --git a/app/controllers/admin/namespaces_controller.rb b/app/controllers/admin/namespaces_controller.rb deleted file mode 100644 index cdab2bb87..000000000 --- a/app/controllers/admin/namespaces_controller.rb +++ /dev/null @@ -1,11 +0,0 @@ -# frozen_string_literal: true - -class Admin::NamespacesController < Admin::BaseController - def index - @special_namespaces = Namespace.where(global: true) - @namespaces = Namespace.not_portus - .where(global: false) - .order(created_at: :asc) - .page(params[:page]) - end -end diff --git a/app/controllers/namespaces_controller.rb b/app/controllers/namespaces_controller.rb index 0c8f20d08..754029fb0 100644 --- a/app/controllers/namespaces_controller.rb +++ b/app/controllers/namespaces_controller.rb @@ -56,9 +56,14 @@ def update # GET /namespace/typeahead/%QUERY def typeahead - @query = params[:query] - valid_teams = TeamUser.get_valid_team_ids(current_user.id) - matches = Team.search_from_query(valid_teams, "#{@query}%").pluck(:name) + valid_teams_ids = if current_user.admin? + Team.all_non_special.pluck(:id) + else + TeamUser.get_valid_team_ids(current_user.id) + end + + query = "#{params[:query]}%" + matches = Team.search_from_query(valid_teams_ids, query).pluck(:name) matches = matches.map { |team| { name: ActionController::Base.helpers.sanitize(team) } } respond_to do |format| format.json { render json: matches.to_json } diff --git a/app/controllers/teams_controller.rb b/app/controllers/teams_controller.rb index 464abe8a0..8ff2cc001 100644 --- a/app/controllers/teams_controller.rb +++ b/app/controllers/teams_controller.rb @@ -9,7 +9,7 @@ class TeamsController < ApplicationController # GET /teams def index - @teams = policy_scope(Team).page(params[:page]) + @teams = policy_scope(Team) @teams_serialized = API::Entities::Teams.represent( @teams, current_user: current_user, diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 209c26a00..5b05093e6 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -29,10 +29,6 @@ class Namespace < ActiveRecord::Base attributes :name, :description end - scope :special_for, lambda { |user| - where("global = ? OR namespaces.id = ?", true, user.namespace_id) - } - # This regexp is extracted from the reference package of Docker Distribution # and it matches a valid namespace name. NAME_REGEXP = /\A[a-z0-9]+(?:[._\\-][a-z0-9]+)*\Z/ diff --git a/app/policies/namespace_policy.rb b/app/policies/namespace_policy.rb index 0a646c97e..501e5fd2d 100644 --- a/app/policies/namespace_policy.rb +++ b/app/policies/namespace_policy.rb @@ -82,17 +82,23 @@ def initialize(user, scope) end def resolve - scope - .joins(team: [:team_users]) - .where( - "(namespaces.visibility = :public OR namespaces.visibility = :protected " \ - "OR team_users.user_id = :user_id) AND " \ - "namespaces.global = :global AND namespaces.id != :namespace_id", - public: Namespace.visibilities[:visibility_public], - protected: Namespace.visibilities[:visibility_protected], - user_id: user.id, global: false, namespace_id: user.namespace_id - ) - .distinct + if user.admin? + global = scope.where(global: true) + normal = scope.not_portus + .where(global: false) + .order(created_at: :asc) + global + normal + else + scope + .joins(team: [:team_users]) + .where( + "namespaces.visibility = :public OR namespaces.visibility = :protected " \ + "OR team_users.user_id = :user_id", + public: Namespace.visibilities[:visibility_public], + protected: Namespace.visibilities[:visibility_protected], + user_id: user.id + ) + end end end diff --git a/app/views/admin/dashboard/index.html.slim b/app/views/admin/dashboard/index.html.slim index 82f7b9a9c..bc341bde3 100644 --- a/app/views/admin/dashboard/index.html.slim +++ b/app/views/admin/dashboard/index.html.slim @@ -47,7 +47,7 @@ .col-md-3.col-xs-12.namespaces .panel-overview .panel-heading - = link_to 'Namespaces', admin_namespaces_path + = link_to 'Namespaces', namespaces_path .panel-body .row .col-xs-3.col-md-4 diff --git a/app/views/namespaces/index.html.erb b/app/views/namespaces/index.html.erb index 498e31764..99198ca42 100644 --- a/app/views/namespaces/index.html.erb +++ b/app/views/namespaces/index.html.erb @@ -1,6 +1,7 @@ diff --git a/config/routes/admin.rb b/config/routes/admin.rb index c84b8905d..f7ce69874 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -4,7 +4,6 @@ resources :activities, only: [:index] resources :dashboard, only: [:index] resources :registries, except: %i[show destroy] - resources :namespaces, only: [:index] resources :users do put "toggle_admin", on: :member end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index b33a7b80e..48cd7a66b 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -2,14 +2,12 @@ require "portus/auth_from_token" require "api/helpers/errors" -require "api/helpers/namespaces" module API module Helpers include ::Portus::AuthFromToken include Errors - include Namespaces # On success it will fill the @user instance variable with the currently # authenticated user for the API. Otherwise it will raise: diff --git a/lib/api/helpers/namespaces.rb b/lib/api/helpers/namespaces.rb deleted file mode 100644 index 071b2cec7..000000000 --- a/lib/api/helpers/namespaces.rb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -module API - module Helpers - # Helpers for namespaces - module Namespaces - # Returns an aggregate of the accessible namespaces for the current user. - def accessible_namespaces - special = Namespace.special_for(current_user).order(created_at: :asc) - normal = policy_scope(Namespace).order(created_at: :asc) - special + normal - end - end - end -end diff --git a/lib/api/v1/namespaces.rb b/lib/api/v1/namespaces.rb index 7058aa4c5..ab84f8b5e 100644 --- a/lib/api/v1/namespaces.rb +++ b/lib/api/v1/namespaces.rb @@ -11,11 +11,12 @@ class Namespaces < Grape::API authorization!(force_admin: false) end - helpers ::API::Helpers::Namespaces - desc "Returns a list of namespaces", tags: ["namespaces"], - detail: "This will expose all accessible namespaces", + detail: "This will expose all accessible namespaces by the user via either team + membership or visibility. Keep in mind that if the user is an admin, this will + return all the global, personal and other namespaces created by all the + users.", is_array: true, entity: API::Entities::Namespaces, failure: [ @@ -24,7 +25,7 @@ class Namespaces < Grape::API ] get do - present accessible_namespaces, + present policy_scope(Namespace), with: API::Entities::Namespaces, current_user: current_user, type: current_type diff --git a/spec/api/grape_api/v1/namespaces_spec.rb b/spec/api/grape_api/v1/namespaces_spec.rb index 52b3623b7..fef99edcf 100644 --- a/spec/api/grape_api/v1/namespaces_spec.rb +++ b/spec/api/grape_api/v1/namespaces_spec.rb @@ -36,22 +36,29 @@ end context "GET /api/v1/namespaces" do - it "returns an empty list" do - get "/api/v1/namespaces", nil, @admin_header + let!(:registry) { create(:registry) } - namespaces = JSON.parse(response.body) - expect(response).to have_http_status(:success) - expect(namespaces.length).to eq(0) + context "as admin" do + it "returns list of accessible namespaces" do + get "/api/v1/namespaces", nil, @admin_header + + # global + personal + weird ones that I have no idea where it comes from + # so, magic number :( + namespaces = JSON.parse(response.body) + expect(response).to have_http_status(:success) + expect(namespaces.length).to eq(7) + end end - it "returns list of accessible namespaces" do - # global + personal + below - create(:namespace, visibility: public_visibility, team: team) - get "/api/v1/namespaces", nil, @admin_header + context "as regular user" do + it "returns list of accessible namespaces" do + get "/api/v1/namespaces", nil, @owner_header - namespaces = JSON.parse(response.body) - expect(response).to have_http_status(:success) - expect(namespaces.length).to eq(3) + # only personal one + namespaces = JSON.parse(response.body) + expect(response).to have_http_status(:success) + expect(namespaces.length).to eq(1) + end end end diff --git a/spec/controllers/admin/namespaces_controller_spec.rb b/spec/controllers/admin/namespaces_controller_spec.rb deleted file mode 100644 index 03b439e28..000000000 --- a/spec/controllers/admin/namespaces_controller_spec.rb +++ /dev/null @@ -1,78 +0,0 @@ -# frozen_string_literal: true - -require "rails_helper" - -RSpec.describe Admin::NamespacesController, type: :controller do - let(:admin) { create(:admin) } - let(:user) { create(:user) } - - context "as admin user" do - before do - create(:registry) - sign_in admin - end - - describe "GET #index" do - let!(:portus) { create(:admin, username: "portus") } - - context "pagination" do - it "paginates namespaces" do - get :index - expect(assigns(:namespaces)).to respond_to(:total_pages) - end - - it "paginates correctly in multiple pages" do - t = create(:team, name: "randomteam") - r = Registry.get - 33.times { |_| create(:namespace, team: t, registry: r) } - - get :index - ns = assigns(:namespaces) - - get :index, page: 2 - ns += assigns(:namespaces) - - # The global namespace and the one for the portus user. - ns = Namespace.all - ns - expect(ns.count).to eq 2 - end - end - - it "returns http success" do - get :index - expect(response).to have_http_status(:success) - end - - it "does not contain the portus namespace" do - get :index - namespaces = assigns(:namespaces) - expect(namespaces.count).to eq 2 - - # Global & portus namespaces. - expect(Namespace.all.count).to eq 4 - end - end - end - - context "not logged into portus" do - describe "GET #index" do - it "redirects to login page" do - get :index - expect(response).to redirect_to(new_user_session_path) - end - end - end - - context "as normal user" do - before do - sign_in user - end - - describe "GET #index" do - it "blocks access" do - get :index - expect(response.status).to eq(401) - end - end - end -end diff --git a/spec/controllers/namespaces_controller_spec.rb b/spec/controllers/namespaces_controller_spec.rb index aa37886b7..4e8cf64c2 100644 --- a/spec/controllers/namespaces_controller_spec.rb +++ b/spec/controllers/namespaces_controller_spec.rb @@ -310,34 +310,52 @@ describe "typeahead" do render_views - it "does allow to search for valid teams by owner" do - testing_team = create(:team, name: "testing", owners: [owner]) - sign_in owner - get :typeahead, query: "test", format: :json - expect(response.status).to eq(200) - teamnames = JSON.parse(response.body) - expect(teamnames.length).to eq(1) - expect(teamnames[0]["name"]).to eq(testing_team.name) - end + context "when admin" do + it "does allow to search for all the valid teams" do + create(:team, name: "testing", owners: [admin]) + create(:team, name: "testing2", owners: [owner]) - it "does not allow to search by viewers" do - create(:team, name: "testing", owners: [owner], viewers: [viewer]) - sign_in viewer - get :typeahead, query: "test", format: :json - expect(response.status).to eq(200) - teamnames = JSON.parse(response.body) - expect(teamnames.length).to eq(0) + sign_in admin + get :typeahead, query: "test", format: :json + + expect(response.status).to eq(200) + result = JSON.parse(response.body) + expect(result.length).to eq(2) + expect(result[0]["name"]).to eq("testing") + expect(result[1]["name"]).to eq("testing2") + end end - it "prevents XSS attacks" do - create(:team, name: "", owners: [owner]) + context "when regular user" do + it "does allow to search for valid teams by owner" do + testing_team = create(:team, name: "testing", owners: [owner]) + sign_in owner + get :typeahead, query: "test", format: :json + expect(response.status).to eq(200) + teamnames = JSON.parse(response.body) + expect(teamnames.length).to eq(1) + expect(teamnames[0]["name"]).to eq(testing_team.name) + end - sign_in owner - get :typeahead, query: "<", format: :json - expect(response.status).to eq(200) - teamnames = JSON.parse(response.body) + it "does not allow to search by viewers" do + create(:team, name: "testing", owners: [owner], viewers: [viewer]) + sign_in viewer + get :typeahead, query: "test", format: :json + expect(response.status).to eq(200) + teamnames = JSON.parse(response.body) + expect(teamnames.length).to eq(0) + end - expect(teamnames[0]["name"]).to eq("alert(1)") + it "prevents XSS attacks" do + create(:team, name: "", owners: [owner]) + + sign_in owner + get :typeahead, query: "<", format: :json + expect(response.status).to eq(200) + teamnames = JSON.parse(response.body) + + expect(teamnames[0]["name"]).to eq("alert(1)") + end end end diff --git a/spec/controllers/teams_controller_spec.rb b/spec/controllers/teams_controller_spec.rb index 12aab5e28..633d548ea 100644 --- a/spec/controllers/teams_controller_spec.rb +++ b/spec/controllers/teams_controller_spec.rb @@ -88,11 +88,6 @@ end describe "GET #index" do - it "paginates teams" do - get :index - expect(assigns(:teams)).to respond_to(:total_pages) - end - it "returns the informations about the teams the user is associated with" do # another team the user has nothing to do with create(:team) diff --git a/spec/features/namespaces_spec.rb b/spec/features/namespaces_spec.rb index 4d18d43b3..20ce2f263 100644 --- a/spec/features/namespaces_spec.rb +++ b/spec/features/namespaces_spec.rb @@ -189,19 +189,19 @@ it "URL is updated when namespaces column is sorted", js: true do visit namespaces_path - expect(page).to have_css(".namespaces-panel:last-of-type th:nth-child(4)") + expect(page).to have_css(".member-namespaces-panel th:nth-child(4)") # sort asc & created_at - find(".namespaces-panel:last-of-type th:nth-child(4)").click + find(".member-namespaces-panel th:nth-child(4)").click - expect(page).to have_css(".namespaces-panel th:nth-child(4) .fa-sort-amount-asc") + expect(page).to have_css(".member-namespaces-panel th:nth-child(4) .fa-sort-amount-asc") path = namespaces_path(ns_sort_asc: true, ns_sort_by: "created_at") expect(page).to have_current_path(path) # sort desc & created_at - find(".namespaces-panel:last-of-type th:nth-child(4)").click + find(".member-namespaces-panel th:nth-child(4)").click - expect(page).to have_css(".namespaces-panel th:nth-child(4) .fa-sort-amount-desc") + expect(page).to have_css(".member-namespaces-panel th:nth-child(4) .fa-sort-amount-desc") path = namespaces_path(ns_sort_asc: false, ns_sort_by: "created_at") expect(page).to have_current_path(path) end @@ -225,18 +225,19 @@ visit namespaces_path - expect(page).to have_css(".namespaces-panel:last-of-type .pagination li:nth-child(3)") + expect(page).to have_content("Created at") + expect(page).to have_css(".member-namespaces-panel .pagination li:nth-child(3)") # page 2 - find(".namespaces-panel:last-of-type .pagination li:nth-child(3) a").click + find(".member-namespaces-panel .pagination li:nth-child(3) a").click - expect(page).to have_css(".namespaces-panel:last-of-type .pagination li.active:nth-child(3)") + expect(page).to have_css(".member-namespaces-panel .pagination li.active:nth-child(3)") expect(page).to have_current_path(namespaces_path(ns_page: 2)) # page 1 - find(".namespaces-panel:last-of-type .pagination li:nth-child(2) a").click + find(".member-namespaces-panel .pagination li:nth-child(2) a").click - expect(page).to have_css(".namespaces-panel:last-of-type .pagination li.active:nth-child(2)") + expect(page).to have_css(".member-namespaces-panel .pagination li.active:nth-child(2)") expect(page).to have_current_path(namespaces_path(ns_page: 1)) end end diff --git a/spec/policies/namespace_policy_spec.rb b/spec/policies/namespace_policy_spec.rb index 751f3255d..337ca0ffe 100644 --- a/spec/policies/namespace_policy_spec.rb +++ b/spec/policies/namespace_policy_spec.rb @@ -6,6 +6,7 @@ subject { described_class } let!(:registry) { create(:registry) } + let(:admin) { create(:admin) } let(:user) { create(:user) } let(:owner) { create(:user) } let(:viewer) { create(:user) } @@ -267,35 +268,61 @@ end it "shows namespaces controlled by teams the user is member of" do - expected = team.namespaces - expect(Pundit.policy_scope(viewer, Namespace).to_a).to match_array(expected) + team.namespaces.each do |n| + expect(Pundit.policy_scope(owner, Namespace).to_a).to include(n) + expect(Pundit.policy_scope(contributor, Namespace).to_a).to include(n) + expect(Pundit.policy_scope(viewer, Namespace).to_a).to include(n) + end + end - expect(Pundit.policy_scope(user, Namespace).to_a).to be_empty + it "shows namespaces for admin even if not member" do + team.namespaces.each do |n| + expect(Pundit.policy_scope(admin, Namespace).to_a).to include(n) + end end - it "always shows public namespaces" do - n = create(:namespace, visibility: :visibility_public) - create(:team, namespaces: [n], owners: [owner]) - expect(Pundit.policy_scope(user, Namespace).to_a).to match_array([n]) + it "does't show namespaces for regular user if not member" do + team.namespaces.each do |n| + expect(Pundit.policy_scope(user, Namespace).to_a).not_to include(n) + end end - it "never shows public or personal namespaces" do - expect(Namespace.find_by(name: user.username)).not_to be_nil - create(:namespace, global: true, visibility: :visibility_public) - expect(Pundit.policy_scope(user, Namespace).to_a).to be_empty + it "shows global namespaces to everyone" do + global = Namespace.where(global: true) + + global.each do |n| + expect(Pundit.policy_scope(admin, Namespace).to_a).to include(n) + expect(Pundit.policy_scope(owner, Namespace).to_a).to include(n) + expect(Pundit.policy_scope(contributor, Namespace).to_a).to include(n) + expect(Pundit.policy_scope(viewer, Namespace).to_a).to include(n) + expect(Pundit.policy_scope(user, Namespace).to_a).to include(n) + end end - it "does not show duplicates" do - # Namespaces controlled by the team that are also public are listed twice - expected = team.namespaces - expected.first.update(visibility: :visibility_public) - expect(Pundit.policy_scope(viewer, Namespace).to_a).to match_array(expected) + it "shows public namespaces to everyone" do + n = create(:namespace, visibility: :visibility_public) + create(:team, namespaces: [n], owners: [owner]) + + expect(Pundit.policy_scope(admin, Namespace).to_a).to include(n) + expect(Pundit.policy_scope(owner, Namespace).to_a).to include(n) + expect(Pundit.policy_scope(contributor, Namespace).to_a).to include(n) + expect(Pundit.policy_scope(viewer, Namespace).to_a).to include(n) + expect(Pundit.policy_scope(user, Namespace).to_a).to include(n) end - it "shows protected namespaces" do + it "shows protected namespaces to everyone" do n = create(:namespace, visibility: :visibility_protected) create(:team, namespaces: [n], owners: [owner]) - expect(Pundit.policy_scope(user, Namespace)).to match_array([n]) + + expect(Pundit.policy_scope(admin, Namespace).to_a).to include(n) + expect(Pundit.policy_scope(owner, Namespace).to_a).to include(n) + expect(Pundit.policy_scope(contributor, Namespace).to_a).to include(n) + expect(Pundit.policy_scope(viewer, Namespace).to_a).to include(n) + expect(Pundit.policy_scope(user, Namespace).to_a).to include(n) + end + + it "shows personal namespaces for specific user" do + expect(Pundit.policy_scope(user, Namespace).to_a).to include(user.namespace) end end end