diff --git a/app/controllers/namespaces_controller.rb b/app/controllers/namespaces_controller.rb index a7c3911c9..23bd115e9 100644 --- a/app/controllers/namespaces_controller.rb +++ b/app/controllers/namespaces_controller.rb @@ -2,7 +2,7 @@ class NamespacesController < ApplicationController include ChangeNameDescription respond_to :html, :js - before_action :set_namespace, only: [:toggle_public, :show, :update] + before_action :set_namespace, only: [:change_visibility, :show, :update] before_action :check_team, only: [:create] before_action :check_role, only: [:create] @@ -77,17 +77,18 @@ def typeahead end end - # PATCH/PUT /namespace/1/toggle_public - def toggle_public + # PATCH/PUT /namespace/1/change_visibility + def change_visibility authorize @namespace - @namespace.update_attributes(public: !(@namespace.public?)) - if @namespace.public? - @namespace.create_activity :public, owner: current_user - else - @namespace.create_activity :private, owner: current_user - end - render template: "namespaces/toggle_public", locals: { namespace: @namespace } + # Update the visibility if needed + return if params[:visibility] == @namespace.visibility + + return unless @namespace.update_attributes(visibility: params[:visibility]) + @namespace.create_activity :change_visibility, + owner: current_user, + parameters: { visibility: @namespace.visibility } + render template: "namespaces/change_visibility", locals: { namespace: @namespace } end private @@ -98,9 +99,10 @@ def fetch_namespace ns = params.require(:namespace).permit(:namespace, :description) @namespace = Namespace.new( - team: @team, - name: ns["namespace"], - registry: Registry.get + team: @team, + name: ns["namespace"], + visibility: Namespace.visibilities[:visibility_private], + registry: Registry.get ) @namespace.description = ns["description"] if ns["description"] @namespace diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 765cc4b2e..5122f0608 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -7,10 +7,10 @@ # created_at :datetime not null # updated_at :datetime not null # team_id :integer -# public :boolean default("0") # registry_id :integer not null # global :boolean default("0") # description :text(65535) +# visibility :integer # # Indexes # @@ -40,13 +40,25 @@ class Namespace < ActiveRecord::Base belongs_to :registry belongs_to :team - validates :public, inclusion: { in: [true] }, if: :global? + enum visibility: [:visibility_private, :visibility_protected, :visibility_public] + + validate :global_namespace_cannot_be_private validates :name, presence: true, uniqueness: { scope: "registry_id" }, length: { maximum: MAX_NAME_LENGTH }, namespace: true + # global_namespace_cannot_be_private adds an error and returns false if the + # visibility of the global namespace is set to private. Otherwise, it returns + # true. This function is used to validate the visibility. + def global_namespace_cannot_be_private + if global? && visibility_private? + errors.add(:visibility, "global namespace cannot be private") + return false + end + true + end # From the given repository name that can be prefix by the name of the # namespace, returns two values: # 1. The namespace where the given repository belongs to. diff --git a/app/models/registry.rb b/app/models/registry.rb index ad0309ad4..d14d4d0eb 100644 --- a/app/models/registry.rb +++ b/app/models/registry.rb @@ -196,7 +196,7 @@ def create_namespaces! Namespace.create!( name: "portus_global_namespace_#{count}", registry: self, - public: true, + visibility: Namespace.visibilities[:visibility_public], global: true, description: "The global namespace for the registry #{Registry.name}.", team: team) diff --git a/app/models/user.rb b/app/models/user.rb index 0ca97c084..c76f4e107 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -110,6 +110,7 @@ def create_personal_namespace! namespace = Namespace.find_or_create_by!( team: team, name: namespace_name, + visibility: Namespace.visibilities[:visibility_private], description: default_description, registry: Registry.get # TODO: fix once we handle more registries ) diff --git a/app/policies/comment_policy.rb b/app/policies/comment_policy.rb index 90a3203d3..0ea4dfdb1 100644 --- a/app/policies/comment_policy.rb +++ b/app/policies/comment_policy.rb @@ -8,10 +8,11 @@ def initialize(user, comment) # Allows the admin to write comments # Allows all members of a team to comment on their repos - # Allows all users to comment on repos under a public namespace + # Allows all users to comment on repos under a protected or public namespace def create? @user.admin? || - @comment.repository.namespace.public? || + @comment.repository.namespace.visibility_public? || + @comment.repository.namespace.visibility_protected? || @comment.repository.namespace.team.team_users.where(user_id: @user.id).any? end diff --git a/app/policies/namespace_policy.rb b/app/policies/namespace_policy.rb index 12cca1931..ad851222b 100644 --- a/app/policies/namespace_policy.rb +++ b/app/policies/namespace_policy.rb @@ -8,11 +8,15 @@ def initialize(user, namespace) def pull? # Even non-logged in users can pull from a public namespace. - return true if namespace.public? + return true if namespace.visibility_public? # From now on, all the others require to be logged in. raise Pundit::NotAuthorizedError, "must be logged in" unless user + # Logged-in users can pull from a protected namespace even if they are + # not part of the team. + return true if namespace.visibility_protected? + # All the members of the team have READ access or anyone if # the namespace is public # Everybody can pull from the global namespace @@ -40,9 +44,10 @@ def index? alias_method :create?, :push? alias_method :update?, :push? - def toggle_public? + def change_visibility? raise Pundit::NotAuthorizedError, "must be logged in" unless user - !namespace.global? && (user.admin? || namespace.team.owners.exists?(user.id)) + (namespace.global? && user.admin?) || \ + (!namespace.global? && (user.admin? || namespace.team.owners.exists?(user.id))) end # Only owners and admins can change the team ownership. @@ -63,9 +68,10 @@ def resolve scope .joins(team: [:team_users]) .where( - "(namespaces.public = :public OR team_users.user_id = :user_id) AND " \ + "(namespaces.visibility = :visibility OR team_users.user_id = :user_id) AND " \ "namespaces.global = :global AND namespaces.name != :username", - public: true, user_id: user.id, global: false, username: user.username) + visibility: Namespace.visibilities[:visibility_public], + user_id: user.id, global: false, username: user.username) .distinct end end diff --git a/app/policies/public_activity/activity_policy.rb b/app/policies/public_activity/activity_policy.rb index 33b877911..465919926 100644 --- a/app/policies/public_activity/activity_policy.rb +++ b/app/policies/public_activity/activity_policy.rb @@ -23,8 +23,9 @@ def resolve "INNER JOIN team_users ON teams.id = team_users.team_id") .where("activities.trackable_type = ? AND " \ "(team_users.user_id = ? OR " \ - "((activities.key = ? OR activities.key = ?) AND namespaces.public = ?))", - "Namespace", user.id, "namespace.public", "namespace.private", true) + "( activities.key = ? AND namespaces.visibility = ?))", + "Namespace", user.id, "namespace.change_visibility", + Namespace.visibilities[:visibility_public]) # Show tag events for repositories inside of namespaces controller by # a team the user is part of, or tags part of public namespaces. @@ -34,8 +35,8 @@ def resolve "INNER JOIN teams ON namespaces.team_id = teams.id " \ "INNER JOIN team_users ON teams.id = team_users.team_id") .where("activities.trackable_type = ? AND " \ - "(team_users.user_id = ? OR namespaces.public = ?)", - "Repository", user.id, true) + "(team_users.user_id = ? OR namespaces.visibility = ?)", + "Repository", user.id, Namespace.visibilities[:visibility_public]) # Show application tokens activities related only to the current user application_token_activities = @scope diff --git a/app/policies/repository_policy.rb b/app/policies/repository_policy.rb index a1294a4bf..479b7541b 100644 --- a/app/policies/repository_policy.rb +++ b/app/policies/repository_policy.rb @@ -8,7 +8,8 @@ def initialize(user, repository) def show? @user.admin? || - @repository.namespace.public? || + @repository.namespace.visibility_public? || + @repository.namespace.visibility_protected? || @repository.namespace.team.users.exists?(user.id) end @@ -28,9 +29,10 @@ def resolve # the repository belongs to the current_user @scope .joins(namespace: { team: :users }) - .where("namespaces.public = :namespace_public OR " \ + .where("namespaces.visibility = :namespace_visibility OR " \ "users.id = :user_id", - namespace_public: true, user_id: @user.id) + namespace_visibility: Namespace.visibilities[:visibility_public], + user_id: @user.id) .distinct end end diff --git a/app/policies/webhook_policy.rb b/app/policies/webhook_policy.rb index 185a707b8..97f40740a 100644 --- a/app/policies/webhook_policy.rb +++ b/app/policies/webhook_policy.rb @@ -42,9 +42,12 @@ def resolve namespaces = Namespace .joins(team: [:team_users]) .where( - "(namespaces.public = :public OR team_users.user_id = :user_id) AND " \ + "(namespaces.visibility = :public OR namespaces.visibility = :protected OR "\ + "team_users.user_id = :user_id) AND " \ "namespaces.global = :global AND namespaces.name != :username", - public: true, user_id: user.id, global: false, username: user.username) + public: Namespace.visibilities[:visibility_public], + protected: Namespace.visibilities[:visibility_protected], user_id: user.id, + global: false, username: user.username) .pluck(:id) scope.includes(:headers, :deliveries).where(namespace: namespaces) diff --git a/app/views/admin/namespaces/index.html.slim b/app/views/admin/namespaces/index.html.slim index 70c164c90..85922bf5b 100644 --- a/app/views/admin/namespaces/index.html.slim +++ b/app/views/admin/namespaces/index.html.slim @@ -16,7 +16,7 @@ th Repositories th Webhooks th Created At - th Public + th Visibility tbody - @special_namespaces.each do |namespace| = render partial: 'namespaces/namespace', locals: {namespace: namespace} @@ -39,7 +39,7 @@ th Repositories th Webhooks th Created At - th Public + th Visibility tbody - @namespaces.each do |namespace| = render partial: 'namespaces/namespace', locals: {namespace: namespace} diff --git a/app/views/namespaces/_namespace.html.slim b/app/views/namespaces/_namespace.html.slim index ea332b974..71806f6b4 100644 --- a/app/views/namespaces/_namespace.html.slim +++ b/app/views/namespaces/_namespace.html.slim @@ -7,17 +7,38 @@ tr id="namespace_#{namespace.id}" td= namespace.webhooks.count td= time_tag namespace.created_at td - - if can_manage_namespace?(namespace) && !namespace.global - a.btn.btn-default[data-remote="true" + div.btn-group.pull-right + - if !namespace.global? + a.btn[ + id="private" + class=(namespace.visibility_private? ? "btn-primary" : "btn-default") + class=("disabled" if !can_manage_namespace?(namespace)) + title=(!namespace.global? ? "Team members can pull images from this namespace" : "The global namespace cannot be private") + data-remote="true" data-method="put" rel="nofollow" - href=url_for(toggle_public_namespace_path(namespace)) + href=url_for(change_visibility_namespace_path(namespace, visibility: "visibility_private")) ] - - if namespace.public? - i.fa.fa-lg class="fa-toggle-on" - - else - i.fa.fa-lg class="fa-toggle-off" title="Click so anyone can pull from this namespace" - - elsif namespace.public? - i.fa.fa-lg class="fa-toggle-on" title="Anyone can pull images from this namespace" - - else - i.fa.fa-lg class="fa-toggle-off" + i.fa.fa-lock + a.btn[ + id="protected" + class=(namespace.visibility_protected? ? "btn-primary" : "btn-default") + class=("disabled" if !can_manage_namespace?(namespace)) + title="Logged-in users can pull images from this namespace" + data-remote="true" + data-method="put" + rel="nofollow" + href=url_for(change_visibility_namespace_path(namespace, visibility: "visibility_protected")) + ] + i.fa.fa-users + a.btn[ + id="public" + class=(namespace.visibility_public? ? "btn-primary" : "btn-default") + class=("disabled" if !can_manage_namespace?(namespace)) + title="Anyone can pull images from this namespace" + data-remote="true" + data-method="put" + rel="nofollow" + href=url_for(change_visibility_namespace_path(namespace, visibility: "visibility_public")) + ] + i.fa.fa.fa-globe diff --git a/app/views/namespaces/change_visibility.js.erb b/app/views/namespaces/change_visibility.js.erb new file mode 100644 index 000000000..4bb789a5a --- /dev/null +++ b/app/views/namespaces/change_visibility.js.erb @@ -0,0 +1,39 @@ +var ns = $('#namespace_<%= namespace.id %> td a.btn'); + +<% if namespace.global? %> + <% if namespace.visibility_public? %> + $(ns[0]).addClass("btn-default") + $(ns[0]).removeClass("btn-primary") + $(ns[1]).addClass("btn-primary") + $(ns[1]).removeClass("btn-default") + <% elsif namespace.visibility_protected? %> + $(ns[0]).addClass("btn-primary") + $(ns[0]).removeClass("btn-default") + $(ns[1]).addClass("btn-default") + $(ns[1]).removeClass("btn-primary") + <% end %> +<% else %> + <% if namespace.visibility_public? %> + $(ns[0]).addClass("btn-default") + $(ns[0]).removeClass("btn-primary") + $(ns[1]).addClass("btn-default") + $(ns[1]).removeClass("btn-primary") + $(ns[2]).addClass("btn-primary") + $(ns[2]).removeClass("btn-default") + <% elsif namespace.visibility_protected? %> + $(ns[0]).addClass("btn-default") + $(ns[0]).removeClass("btn-primary") + $(ns[1]).addClass("btn-primary") + $(ns[1]).removeClass("btn-default") + $(ns[2]).addClass("btn-default") + $(ns[2]).removeClass("btn-primary") + <% else %> + // private + $(ns[0]).addClass("btn-primary") + $(ns[0]).removeClass("btn-default") + $(ns[1]).addClass("btn-default") + $(ns[1]).removeClass("btn-primary") + $(ns[2]).addClass("btn-default") + $(ns[2]).removeClass("btn-primary") + <% end %> +<% end %> diff --git a/app/views/namespaces/index.html.slim b/app/views/namespaces/index.html.slim index 4ec0c3a04..1ccda765f 100644 --- a/app/views/namespaces/index.html.slim +++ b/app/views/namespaces/index.html.slim @@ -24,7 +24,7 @@ th Repositories th Webhooks th Created - th Public + th Visibility tbody - @special_namespaces.each do |namespace| = render partial: 'namespaces/namespace', locals: {namespace: namespace} @@ -76,7 +76,7 @@ th Repositories th Webhooks th Created At - th Public + th Visibility tbody#accessible-namespaces - @namespaces.each do |namespace| = render partial: 'namespaces/namespace', locals: {namespace: namespace} diff --git a/app/views/namespaces/toggle_public.js.erb b/app/views/namespaces/toggle_public.js.erb deleted file mode 100644 index 88b5a5adf..000000000 --- a/app/views/namespaces/toggle_public.js.erb +++ /dev/null @@ -1,13 +0,0 @@ -var ns = $('#namespace_<%= namespace.id %> td a i'); - -<% if namespace.public? %> - ns.removeAttr("title"); - ns.addClass("fa-toggle-on"); - ns.removeClass("fa-toggle-off"); -<% else %> - ns.prop("title", "Click so anyone can pull from this namespace"); - ns.addClass("fa-toggle-off"); - ns.removeClass("fa-toggle-on"); -<% end %> -$('#float-alert p').html("Namespace '<%= namespace.name %>' is now <%= namespace.public? ? 'public' : 'private' %>"); -$('#float-alert').fadeIn(setTimeOutAlertDelay()); \ No newline at end of file diff --git a/app/views/public_activity/namespace/_change_visibility.csv.slim b/app/views/public_activity/namespace/_change_visibility.csv.slim new file mode 100644 index 000000000..fdcc5b672 --- /dev/null +++ b/app/views/public_activity/namespace/_change_visibility.csv.slim @@ -0,0 +1 @@ += CSV.generate_line(['namespace', activity.trackable.name, 'change visibility', '-', activity.owner.display_username, activity.created_at, "is #{activity.parameters[:visibility].sub('visibility_', '')}"]) diff --git a/app/views/public_activity/namespace/_change_visibility.html.slim b/app/views/public_activity/namespace/_change_visibility.html.slim new file mode 100644 index 000000000..b514a5c18 --- /dev/null +++ b/app/views/public_activity/namespace/_change_visibility.html.slim @@ -0,0 +1,15 @@ +li + .activitie-container + .activity-type.change-namespace-visibility + i.fa.fa-unlock + .user-image + = user_image_tag(activity.owner.email) + .description + h6 + strong + = "#{activity.owner.display_username} set the " + = link_to activity.trackable.name, activity.trackable + = " namespace as #{activity.parameters[:visibility].sub('visibility_', '')}" + small + i.fa.fa-clock-o + = activity_time_tag activity.created_at diff --git a/app/views/teams/show.html.slim b/app/views/teams/show.html.slim index 5d7b822ef..8795c3888 100644 --- a/app/views/teams/show.html.slim +++ b/app/views/teams/show.html.slim @@ -149,7 +149,7 @@ .col-sm-6 h5 small - a[data-placement="right" data-toggle="popover" data-container=".panel-heading" data-content='

A team can own one or more namespaces. By default all the namespaces can be accessed only by the members of the team.

It is possible to add read only (pull) access to all Portus users by toggling the "public" flag.

' data-original-title="What's this?" tabindex="0" data-html="true"] + a[data-placement="right" data-toggle="popover" data-container=".panel-heading" data-content='

A team can own one or more namespaces. By default all the namespaces can be accessed only by the members of the team.

It is possible to add read only (pull) access to logged-in users or all Portus users by changing the visibility to "protected" or "public" respectively.

' data-original-title="What's this?" tabindex="0" data-html="true"] i.fa.fa-info-circle ' Namespaces owned .col-sm-6.text-right @@ -173,7 +173,7 @@ th Repositories th Webhooks th Created At - th Public + th Visibility tbody#namespaces - @team_namespaces.each do |namespace| = render(namespace) diff --git a/config/routes.rb b/config/routes.rb index bab1f3d42..39eaf4630 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -9,7 +9,7 @@ resources :team_users, only: [:create, :destroy, :update] resources :namespaces, only: [:create, :index, :show, :update] do - put "toggle_public", on: :member + put "change_visibility", on: :member resources :webhooks do resources :headers, only: [:create, :destroy], controller: :webhook_headers resources :deliveries, only: [:update], controller: :webhook_deliveries diff --git a/db/migrate/20160614114318_add_visibility_to_namespace.rb b/db/migrate/20160614114318_add_visibility_to_namespace.rb new file mode 100644 index 000000000..f5da95d21 --- /dev/null +++ b/db/migrate/20160614114318_add_visibility_to_namespace.rb @@ -0,0 +1,9 @@ +class AddVisibilityToNamespace < ActiveRecord::Migration + def up + add_column :namespaces, :visibility, :integer + end + + def down + remove_column :namespaces, :visibility, :integer + end +end diff --git a/db/migrate/20160614121943_move_public_to_visibility_on_namespace.rb b/db/migrate/20160614121943_move_public_to_visibility_on_namespace.rb new file mode 100644 index 000000000..09fba40bd --- /dev/null +++ b/db/migrate/20160614121943_move_public_to_visibility_on_namespace.rb @@ -0,0 +1,31 @@ +class MovePublicToVisibilityOnNamespace < ActiveRecord::Migration + def up + Namespace.all.each do |namespace| + namespace.visibility = namespace.public? ? :visibility_public : :visibility_private + namespace.save! + end + PublicActivity::Activity.where("activities.key = ? OR activities.key = ?", + "private", "public").each do |activity| + activity.parameters = { visibility: "visibility_#{activity.key}" } + activity.key = "namespace.change_visibility" + activity.save! + end + end + + def down + Namespace.all.each do |namespace| + namespace.public = namespace.visibility_public? ? true : false + namespace.save! + end + PublicActivity::Activity.where(key: "change_visibility").each do |activity| + if activity.parameters[:visibility] == 'visibility_private' + activity.key = "namespace.private" + else + # protected namespace are made public as well + activity.key = "namespace.public" + end + activity.parameters = {} + activity.save! + end + end +end diff --git a/db/migrate/20160614122012_remove_public_from_namespace.rb b/db/migrate/20160614122012_remove_public_from_namespace.rb new file mode 100644 index 000000000..17fe811c2 --- /dev/null +++ b/db/migrate/20160614122012_remove_public_from_namespace.rb @@ -0,0 +1,9 @@ +class RemovePublicFromNamespace < ActiveRecord::Migration + def up + remove_column :namespaces, :public, :boolean + end + + def down + add_column :namespaces, :public, :boolean + end +end diff --git a/db/schema.rb b/db/schema.rb index 920efecff..ac9ff13f6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160531151718) do +ActiveRecord::Schema.define(version: 20160614122012) do create_table "activities", force: :cascade do |t| t.integer "trackable_id", limit: 4 @@ -66,10 +66,10 @@ t.datetime "created_at", null: false t.datetime "updated_at", null: false t.integer "team_id", limit: 4 - t.boolean "public", default: false t.integer "registry_id", limit: 4, null: false t.boolean "global", default: false t.text "description", limit: 65535 + t.integer "visibility", limit: 4 end add_index "namespaces", ["name", "registry_id"], name: "index_namespaces_on_name_and_registry_id", unique: true, using: :btree diff --git a/spec/controllers/admin/activities_controller_spec.rb b/spec/controllers/admin/activities_controller_spec.rb index 56419cf1d..7b8e54bbb 100644 --- a/spec/controllers/admin/activities_controller_spec.rb +++ b/spec/controllers/admin/activities_controller_spec.rb @@ -31,8 +31,12 @@ let(:activity_owner) { create(:user, username: "castiel") } let(:registry) { create(:registry, hostname: "registry.test.lan") } let(:global_namespace) do - create(:namespace, name: "globalnamespace", registry: registry, global: true, - team: global_team, public: true) + create(:namespace, + name: "globalnamespace", + registry: registry, + global: true, + team: global_team, + visibility: "visibility_public") end let(:namespace) { create(:namespace, name: "patched_images", registry: registry, team: team) } let(:team) { create(:team, name: "qa", owners: [user]) } @@ -65,12 +69,18 @@ create(:activity_namespace_create, trackable_id: namespace.id, owner_id: activity_owner.id) - create(:activity_namespace_public, + create(:activity_namespace_change_visibility, trackable_id: namespace.id, - owner_id: activity_owner.id) - create(:activity_namespace_private, + owner_id: activity_owner.id, + parameters: { visibility: "visibility_public" }) + create(:activity_namespace_change_visibility, trackable_id: namespace.id, - owner_id: activity_owner.id) + owner_id: activity_owner.id, + parameters: { visibility: "visibility_protected" }) + create(:activity_namespace_change_visibility, + trackable_id: namespace.id, + owner_id: activity_owner.id, + parameters: { visibility: "visibility_private" }) create(:activity_repository_push, trackable_id: tag.repository.id, recipient_id: tag.id, @@ -95,8 +105,9 @@ team,qa,remove member,dean,castiel,2015-01-01 00:00:00 UTC,role viewer team,qa,change member role,dean,castiel,2015-01-01 00:00:00 UTC,from viewer to contributor namespace,patched_images,create,-,castiel,2015-01-01 00:00:00 UTC,owned by team qa -namespace,patched_images,make public,-,castiel,2015-01-01 00:00:00 UTC,- -namespace,patched_images,make private,-,castiel,2015-01-01 00:00:00 UTC,- +namespace,patched_images,change visibility,-,castiel,2015-01-01 00:00:00 UTC,is public +namespace,patched_images,change visibility,-,castiel,2015-01-01 00:00:00 UTC,is protected +namespace,patched_images,change visibility,-,castiel,2015-01-01 00:00:00 UTC,is private repository,patched_images/sles12:1.0.0,push tag,-,castiel,2015-01-01 00:00:00 UTC,- repository,registry.test.lan/sles11sp3:1.0.0,push tag,-,castiel,2015-01-01 00:00:00 UTC,- CSV diff --git a/spec/controllers/comments_controller_spec.rb b/spec/controllers/comments_controller_spec.rb index 6892d0ddc..2117f3088 100644 --- a/spec/controllers/comments_controller_spec.rb +++ b/spec/controllers/comments_controller_spec.rb @@ -5,9 +5,23 @@ let(:owner) { create(:user) } let(:user) { create(:user) } let!(:team) { create(:team, owners: [owner]) } - let!(:public_namespace) { create(:namespace, public: 1, team: team) } + let!(:public_namespace) do + create(:namespace, + visibility: Namespace.visibilities[:visibility_public], + team: team) + end let!(:visible_repository) { create(:repository, namespace: public_namespace) } - let!(:private_namespace) { create(:namespace, public: 0, team: team) } + let!(:protected_namespace) do + create(:namespace, + visibility: Namespace.visibilities[:visibility_protected], + team: team) + end + let!(:protected_repository) { create(:repository, namespace: protected_namespace) } + let!(:private_namespace) do + create(:namespace, + visibility: Namespace.visibilities[:visibility_private], + team: team) + end let!(:invisible_repository) { create(:repository, namespace: private_namespace) } let(:comment) { create(:comment, author: owner) } let!(:commented_repository) do @@ -40,6 +54,12 @@ expect(response.status).to eq 200 end + it "allows logged-in users to write comments for a repository under a protected namespace" do + sign_in user + post :create, repository_id: protected_repository.id, comment: valid_attributes, format: :js + expect(response.status).to eq 200 + end + it "does allow the admin to write comments for every repository" do sign_in admin post :create, repository_id: invisible_repository.id, comment: valid_attributes, format: :js diff --git a/spec/controllers/namespaces_controller_spec.rb b/spec/controllers/namespaces_controller_spec.rb index 2161062df..e6b896f93 100644 --- a/spec/controllers/namespaces_controller_spec.rb +++ b/spec/controllers/namespaces_controller_spec.rb @@ -65,19 +65,25 @@ end end - describe "PUT #toggle_public" do - it "allows the owner of the team to change the public attribute" do + describe "PUT #change_visibility" do + it "allows the owner of the team to change the visibility attribute" do sign_in owner - put :toggle_public, id: namespace.id, format: :js + put :change_visibility, + id: namespace.id, + visibility: "visibility_public", + format: :js namespace.reload - expect(namespace).to be_public + expect(namespace.visibility).to eq("visibility_public") expect(response.status).to eq 200 end it "blocks users that are not part of the team" do sign_in create(:user) - put :toggle_public, id: namespace.id, format: :js + put :change_visibility, + id: namespace.id, + visibility: "visibility_public", + format: :js expect(response.status).to eq 401 end @@ -296,27 +302,52 @@ end it "tracks set namespace private" do - namespace.update_attributes(public: true) + namespace.update_attributes(visibility: Namespace.visibilities[:visibilty_public]) expect do - put :toggle_public, id: namespace.id, format: :js + put :change_visibility, + id: namespace.id, + visibility: "visibility_private", + format: :js end.to change(PublicActivity::Activity, :count).by(1) activity = PublicActivity::Activity.last - expect(activity.key).to eq("namespace.private") + expect(activity.key).to eq("namespace.change_visibility") + expect(activity.parameters[:visibility]).to eq("visibility_private") + expect(activity.owner).to eq(owner) + expect(activity.trackable).to eq(namespace) + end + + it "tracks set namespace protected" do + namespace.update_attributes(visibility: Namespace.visibilities[:visibilty_public]) + + expect do + put :change_visibility, + id: namespace.id, + visibility: "visibility_protected", + format: :js + end.to change(PublicActivity::Activity, :count).by(1) + + activity = PublicActivity::Activity.last + expect(activity.key).to eq("namespace.change_visibility") + expect(activity.parameters[:visibility]).to eq("visibility_protected") expect(activity.owner).to eq(owner) expect(activity.trackable).to eq(namespace) end it "tracks set namespace public" do - namespace.update_attributes(public: false) + namespace.update_attributes(visibility: Namespace.visibilities[:visibility_private]) expect do - put :toggle_public, id: namespace.id, format: :js + put :change_visibility, + id: namespace.id, + visibility: "visibility_public", + format: :js end.to change(PublicActivity::Activity, :count).by(1) activity = PublicActivity::Activity.last - expect(activity.key).to eq("namespace.public") + expect(activity.key).to eq("namespace.change_visibility") + expect(activity.parameters[:visibility]).to eq("visibility_public") expect(activity.owner).to eq(owner) expect(activity.trackable).to eq(namespace) end diff --git a/spec/controllers/repositories_controller_spec.rb b/spec/controllers/repositories_controller_spec.rb index fcdcf31d6..f52effbba 100644 --- a/spec/controllers/repositories_controller_spec.rb +++ b/spec/controllers/repositories_controller_spec.rb @@ -3,9 +3,17 @@ describe RepositoriesController, type: :controller do let(:valid_session) { {} } let(:user) { create(:user) } - let!(:public_namespace) { create(:namespace, public: 1, team: create(:team)) } + let!(:public_namespace) do + create(:namespace, + visibility: Namespace.visibilities[:visibility_public], + team: create(:team)) + end let!(:visible_repository) { create(:repository, namespace: public_namespace) } - let!(:private_namespace) { create(:namespace, public: 0, team: create(:team)) } + let!(:private_namespace) do + create(:namespace, + visibility: Namespace.visibilities[:visibility_private], + team: create(:team)) + end let!(:invisible_repository) { create(:repository, namespace: private_namespace) } before :each do diff --git a/spec/factories/activities.rb b/spec/factories/activities.rb index 866ef36a1..3fca988c5 100644 --- a/spec/factories/activities.rb +++ b/spec/factories/activities.rb @@ -33,15 +33,9 @@ trackable_type "Namespace" end - factory :activity_namespace_public, class: PublicActivity::Activity do + factory :activity_namespace_change_visibility, class: PublicActivity::Activity do owner_type "User" - key "namespace.public" - trackable_type "Namespace" - end - - factory :activity_namespace_private, class: PublicActivity::Activity do - owner_type "User" - key "namespace.private" + key "namespace.change_visibility" trackable_type "Namespace" end diff --git a/spec/factories/namespaces.rb b/spec/factories/namespaces.rb index 8f220b0bc..8ee72fbfa 100644 --- a/spec/factories/namespaces.rb +++ b/spec/factories/namespaces.rb @@ -7,10 +7,10 @@ # created_at :datetime not null # updated_at :datetime not null # team_id :integer -# public :boolean default("0") # registry_id :integer not null # global :boolean default("0") # description :text(65535) +# visibility :integer # # Indexes # @@ -26,6 +26,7 @@ "namespace#{n}" end + visibility Namespace.visibilities[:visibility_private] registry { association(:registry) } end end diff --git a/spec/features/dashboard_spec.rb b/spec/features/dashboard_spec.rb index c7e51fdcc..4d96d8c00 100644 --- a/spec/features/dashboard_spec.rb +++ b/spec/features/dashboard_spec.rb @@ -13,8 +13,19 @@ let!(:another_user) { create(:admin) } let!(:another_team) { create(:team, owners: [another_user]) } - let!(:public_namespace) { create(:namespace, team: another_team, public: true) } + let!(:another_team2) { create(:team, owners: [another_user]) } + let!(:public_namespace) do + create(:namespace, + team: another_team, + visibility: Namespace.visibilities[:visibility_public]) + end let!(:public_repository) { create(:repository, namespace: public_namespace) } + let!(:protected_namespace) do + create(:namespace, + team: another_team2, + visibility: Namespace.visibilities[:visibility_protected]) + end + let!(:protected_repository) { create(:repository, namespace: protected_namespace) } before do login_as user, scope: :user @@ -27,6 +38,7 @@ expect(page).to have_content("#{namespace.name}/#{repository.name}") expect(page).to have_content("#{namespace.name}/#{starred_repo.name}") expect(page).to have_content("#{public_namespace.name}/#{public_repository.name}") + expect(page).to have_content("#{protected_namespace.name}/#{protected_repository.name}") end scenario "Show personal repositories", js: true do @@ -38,6 +50,7 @@ expect(page).not_to have_content("#{namespace.name}/#{starred_repo.name}") expect(page).not_to have_content("#{namespace.name}/#{repository.name}") expect(page).not_to have_content("#{public_namespace.name}/#{public_repository.name}") + expect(page).not_to have_content("#{protected_namespace.name}/#{protected_repository.name}") end scenario "Show personal repositories", js: true do @@ -49,6 +62,7 @@ expect(page).not_to have_content("#{personal_namespace.name}/#{personal_repository.name}") expect(page).not_to have_content("#{namespace.name}/#{repository.name}") expect(page).not_to have_content("#{public_namespace.name}/#{public_repository.name}") + expect(page).not_to have_content("#{protected_namespace.name}/#{protected_repository.name}") end end diff --git a/spec/features/namespaces_spec.rb b/spec/features/namespaces_spec.rb index 34c88ee42..d73d7c6dd 100644 --- a/spec/features/namespaces_spec.rb +++ b/spec/features/namespaces_spec.rb @@ -108,22 +108,26 @@ expect(page).to_not have_css("#add_namespace_btn i.fa-minus-circle") end - scenario "The namespace can be toggled public/private", js: true do + scenario "The namespace visibility can be changed", js: true do visit namespaces_path id = namespace.id - expect(namespace.public?).to be false - expect(page).to have_css("#namespace_#{id} .fa-toggle-off") + expect(namespace.visibility_private?).to be true + expect(page).to have_css("#namespace_#{id} a#private.btn-primary") - find("#namespace_#{id} .btn").click + find("#namespace_#{id} a#protected.btn").click wait_for_ajax - expect(page).to have_css("#namespace_#{id} .fa-toggle-on") + expect(page).to have_css("#namespace_#{id} a#protected.btn-primary") namespace = Namespace.find(id) - expect(namespace.public?).to be true + expect(namespace.visibility_protected?).to be true - wait_for_effect_on("#float-alert") - expect(page).to have_content("Namespace '#{namespace.name}' is now public") + find("#namespace_#{id} a#public.btn").click + wait_for_ajax + + expect(page).to have_css("#namespace_#{id} a#public.btn-primary") + namespace = Namespace.find(id) + expect(namespace.visibility_public?).to be true end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 0f99ea273..756c08cc4 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -7,10 +7,10 @@ # created_at :datetime not null # updated_at :datetime not null # team_id :integer -# public :boolean default("0") # registry_id :integer not null # global :boolean default("0") # description :text(65535) +# visibility :integer # # Indexes # @@ -66,10 +66,16 @@ end context "global namespace" do - it "must be public" do - namespace = create(:namespace, global: true, public: true) - namespace.public = false + it "cannot be private" do + namespace = create( + :namespace, + global: true, + visibility: Namespace.visibilities[:visibility_public]) + namespace.visibility = Namespace.visibilities[:visibility_private] expect(namespace.save).to be false + + namespace.visibility = Namespace.visibilities[:visibility_protected] + expect(namespace.save).to be true end end @@ -87,7 +93,11 @@ context "global namespace" do it "returns the name of the namespace" do - global_namespace = create(:namespace, global: true, public: true, registry: registry) + global_namespace = create( + :namespace, + global: true, + visibility: Namespace.visibilities[:visibility_public], + registry: registry) expect(global_namespace.clean_name).to eq(registry.hostname) end end diff --git a/spec/policies/namespace_policy_spec.rb b/spec/policies/namespace_policy_spec.rb index 5e07a68cc..549ce1c98 100644 --- a/spec/policies/namespace_policy_spec.rb +++ b/spec/policies/namespace_policy_spec.rb @@ -44,7 +44,7 @@ end it "allows access to any user if the namespace is public" do - namespace.public = true + namespace.visibility = :visibility_public expect(subject).to permit(user, namespace) end @@ -63,9 +63,24 @@ end it "allows access to a non-logged user if the namespace is public" do - namespace.public = true + namespace.visibility = :visibility_public expect(subject).to permit(nil, namespace) end + + it "disallows access to a non-logged-in user if the namespace is protected" do + namespace.visibility = :visibility_protected + expect do + subject.new(nil, namespace).pull? + end.to raise_error(Pundit::NotAuthorizedError, /must be logged in/) + end + + it "allows access to any logged-in user if the namespace is protected" do + namespace.visibility = :visibility_protected + expect(subject).to permit(user, namespace) + expect(subject).to permit(viewer, namespace) + expect(subject).to permit(owner, namespace) + expect(subject).to permit(@admin, namespace) + end end permissions :push? do @@ -130,7 +145,7 @@ end end - permissions :toggle_public? do + permissions :change_visibility? do it "allows admin to change it" do expect(subject).to permit(@admin, namespace) end @@ -149,15 +164,19 @@ it "disallows access to user who is not logged in" do expect do - subject.new(nil, namespace).toggle_public? + subject.new(nil, namespace).change_visibility? end.to raise_error(Pundit::NotAuthorizedError, /must be logged in/) end context "global namespace" do - let(:namespace) { create(:namespace, global: true, public: true) } + let(:namespace) { create(:namespace, global: true, visibility: :visibility_public) } + + it "allows access to admin" do + expect(subject).to permit(@admin, namespace) + end - it "disallow access to everybody" do - expect(subject).to_not permit(@admin, namespace) + it "disallows access to everyone normal users" do + expect(subject).to_not permit(user, namespace) end end end @@ -176,7 +195,7 @@ end it "always shows public namespaces" do - n = create(:namespace, public: true) + n = create(:namespace, visibility: :visibility_public) create(:team, namespaces: [n], owners: [owner]) expect(Pundit.policy_scope(create(:user), Namespace).to_a).to match_array([n]) end @@ -184,14 +203,14 @@ it "never shows public or personal namespaces" do user = create(:user) expect(Namespace.find_by(name: user.username)).not_to be_nil - create(:namespace, global: true, public: true) + create(:namespace, global: true, visibility: :visibility_public) expect(Pundit.policy_scope(create(:user), Namespace).to_a).to be_empty 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_attributes(public: true) + expected.first.update_attributes(visibility: :visibility_public) expect(Pundit.policy_scope(viewer, Namespace).to_a).to match_array(expected) end end diff --git a/spec/policies/public_activity/activity_policy_spec.rb b/spec/policies/public_activity/activity_policy_spec.rb index cc329b5af..d3eeb1f86 100644 --- a/spec/policies/public_activity/activity_policy_spec.rb +++ b/spec/policies/public_activity/activity_policy_spec.rb @@ -34,27 +34,26 @@ it "returns pertinent namespace events" do namespace2 = create(:namespace, - registry: registry, - team: create(:team, - owners: [another_user]), - public: true) + registry: registry, + team: create(:team, owners: [another_user]), + visibility: Namespace.visibilities[:visibility_public]) activities = [ create(:activity_namespace_create, trackable_id: namespace.id, owner_id: activity_owner.id), - create(:activity_namespace_public, + create(:activity_namespace_change_visibility, trackable_id: namespace.id, owner_id: activity_owner.id), - create(:activity_namespace_private, + create(:activity_namespace_change_visibility, trackable_id: namespace.id, owner_id: activity_owner.id), # all the public/private events are shown, even the ones # involving namespaces the user does not control - create(:activity_namespace_public, + create(:activity_namespace_change_visibility, trackable_id: namespace2.id, owner_id: activity_owner.id), - create(:activity_namespace_private, + create(:activity_namespace_change_visibility, trackable_id: namespace2.id, owner_id: activity_owner.id) ] @@ -74,9 +73,9 @@ private_tag = create(:tag, repository: create(:repository, namespace: namespace2)) public_namespace = create(:namespace, - registry: registry, - public: true, - team: create(:team, + registry: registry, + visibility: Namespace.visibilities[:visibility_public], + team: create(:team, owners: [another_user], namespaces: [namespace2])) public_tag = create(:tag, repository: create(:repository, namespace: public_namespace)) diff --git a/spec/policies/repository_policy_spec.rb b/spec/policies/repository_policy_spec.rb index 7c75ac475..34534fe26 100644 --- a/spec/policies/repository_policy_spec.rb +++ b/spec/policies/repository_policy_spec.rb @@ -12,9 +12,20 @@ permissions :show? do before :each do - public_namespace = create(:namespace, team: team2, public: true, registry: registry) + public_namespace = create( + :namespace, + team: team2, + visibility: Namespace.visibilities[:visibility_public], + registry: registry) @public_repository = create(:repository, namespace: public_namespace) + protected_namespace = create( + :namespace, + team: team2, + visibility: Namespace.visibilities[:visibility_protected], + registry: registry) + @protected_repository = create(:repository, namespace: protected_namespace) + private_namespace = create(:namespace, team: team2, registry: registry) @private_repository = create(:repository, namespace: private_namespace) end @@ -31,6 +42,10 @@ expect(subject).to permit(user, @public_repository) end + it "grants access if the namespace is protected" do + expect(subject).to permit(user, @protected_repository) + end + it "grants access if the repository belongs to a namespace of a team member" do user3 = create(:user) TeamUser.create(team: team2, user: user3, role: TeamUser.roles["viewer"]) @@ -44,7 +59,11 @@ describe "scope" do before :each do - public_namespace = create(:namespace, team: team2, public: true, registry: registry) + public_namespace = create( + :namespace, + team: team2, + visibility: Namespace.visibilities[:visibility_public], + registry: registry) @public_repository = create(:repository, namespace: public_namespace) private_namespace = create(:namespace, team: team2, registry: registry)