From 42fcd3881fcece5c9bd4b720460d6cade573b151 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Thu, 10 Mar 2016 22:08:11 +0100 Subject: External Users The user has the rights of a public user execpt it can never create a project, group, or team. Also it cant view internal projects. --- app/controllers/admin/users_controller.rb | 2 +- app/finders/projects_finder.rb | 11 +++--- app/models/ability.rb | 34 +++++++++--------- app/models/user.rb | 12 +++++++ app/views/admin/users/_form.html.haml | 5 +++ app/views/admin/users/show.html.haml | 4 +++ .../20160310185910_add_external_flag_to_users.rb | 5 +++ db/schema.rb | 3 +- lib/api/users.rb | 5 +-- .../security/project/internal_access_spec.rb | 37 +++++++++++++------ .../security/project/private_access_spec.rb | 34 ++++++++++++------ .../security/project/public_access_spec.rb | 41 ++++++++++++++++------ spec/models/user_spec.rb | 16 +++++++++ spec/requests/api/users_spec.rb | 20 +++++++++++ spec/support/matchers/access_matchers.rb | 2 ++ 15 files changed, 171 insertions(+), 60 deletions(-) create mode 100644 db/migrate/20160310185910_add_external_flag_to_users.rb diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 87f4fb455b8..be192964a93 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -150,7 +150,7 @@ class Admin::UsersController < Admin::ApplicationController :email, :remember_me, :bio, :name, :username, :skype, :linkedin, :twitter, :website_url, :color_scheme_id, :theme_id, :force_random_password, :extern_uid, :provider, :password_expires_at, :avatar, :hide_no_ssh_key, :hide_no_password, - :projects_limit, :can_create_group, :admin, :key_id + :projects_limit, :can_create_group, :admin, :key_id, :external ) end diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb index 0e5a8f5ee0f..711d9019a3d 100644 --- a/app/finders/projects_finder.rb +++ b/app/finders/projects_finder.rb @@ -51,13 +51,12 @@ class ProjectsFinder end def all_projects(current_user) - if current_user - [ - current_user.authorized_projects, - public_and_internal_projects - ] + return [Project.public_only] unless current_user + + if current_user.external? + [current_user.authorized_projects, public_projects] else - [Project.public_only] + [current_user.authorized_projects, public_and_internal_projects] end end diff --git a/app/models/ability.rb b/app/models/ability.rb index fe9e0aab717..ccac08b7d3f 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -109,23 +109,10 @@ class Ability key = "/user/#{user.id}/project/#{project.id}" RequestStore.store[key] ||= begin - team = project.team + # Push abilities on the users team role + rules.push(*project_team_rules(project.team, user)) - # Rules based on role in project - if team.master?(user) - rules.push(*project_master_rules) - - elsif team.developer?(user) - rules.push(*project_dev_rules) - - elsif team.reporter?(user) - rules.push(*project_report_rules) - - elsif team.guest?(user) - rules.push(*project_guest_rules) - end - - if project.public? || project.internal? + if project.public? || (project.internal? && !user.external?) rules.push(*public_project_rules) # Allow to read builds for internal projects @@ -148,6 +135,19 @@ class Ability end end + def project_team_rules(team, user) + # Rules based on role in project + if team.master?(user) + project_master_rules + elsif team.developer?(user) + project_dev_rules + elsif team.reporter?(user) + project_report_rules + elsif team.guest?(user) + project_guest_rules + end + end + def public_project_rules @public_project_rules ||= project_guest_rules + [ :download_code, @@ -356,7 +356,7 @@ class Ability ] end - if snippet.public? || snippet.internal? + if snippet.public? || (snippet.internal? && !user.external?) rules << :read_personal_snippet end diff --git a/app/models/user.rb b/app/models/user.rb index 043bc825ade..adff238c60f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -59,6 +59,7 @@ # hide_project_limit :boolean default(FALSE) # unlock_token :string # otp_grace_period_started_at :datetime +# external :boolean default(FALSE) # require 'carrierwave/orm/activerecord' @@ -77,6 +78,7 @@ class User < ActiveRecord::Base add_authentication_token_field :authentication_token default_value_for :admin, false + default_value_for :external, false default_value_for :can_create_group, gitlab_config.default_can_create_group default_value_for :can_create_team, false default_value_for :hide_no_ssh_key, false @@ -179,6 +181,7 @@ class User < ActiveRecord::Base after_update :update_emails_with_primary_email, if: ->(user) { user.email_changed? } before_save :ensure_authentication_token + before_save :ensure_external_user_rights after_save :ensure_namespace_correct after_initialize :set_projects_limit after_create :post_create_hook @@ -848,4 +851,13 @@ class User < ActiveRecord::Base def send_devise_notification(notification, *args) devise_mailer.send(notification, self, *args).deliver_later end + + def ensure_external_user_rights + return unless self.external? + + self.can_create_team = false + self.can_create_group = false + self.projects_limit = 0 + self.hide_project_limit = true + end end diff --git a/app/views/admin/users/_form.html.haml b/app/views/admin/users/_form.html.haml index e18dd9bc905..5506827edc4 100644 --- a/app/views/admin/users/_form.html.haml +++ b/app/views/admin/users/_form.html.haml @@ -61,6 +61,11 @@ .col-sm-10 You cannot remove your own admin rights - else .col-sm-10= f.check_box :admin + + .form-group + = f.label :external, class: 'control-label' + .col-sm-10= f.check_box :external + %fieldset %legend Profile .form-group diff --git a/app/views/admin/users/show.html.haml b/app/views/admin/users/show.html.haml index 2bdbae19588..d37489bebea 100644 --- a/app/views/admin/users/show.html.haml +++ b/app/views/admin/users/show.html.haml @@ -47,6 +47,10 @@ - else Disabled + %li + %span.light External User: + %strong + = @user.external? ? "Yes" : "No" %li %span.light Can create groups: %strong diff --git a/db/migrate/20160310185910_add_external_flag_to_users.rb b/db/migrate/20160310185910_add_external_flag_to_users.rb new file mode 100644 index 00000000000..54937f1eb71 --- /dev/null +++ b/db/migrate/20160310185910_add_external_flag_to_users.rb @@ -0,0 +1,5 @@ +class AddExternalFlagToUsers < ActiveRecord::Migration + def change + add_column :users, :external, :boolean, default: false + end +end diff --git a/db/schema.rb b/db/schema.rb index 3ac6203632d..16e28d7a3f0 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: 20160309140734) do +ActiveRecord::Schema.define(version: 20160310185910) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -929,6 +929,7 @@ ActiveRecord::Schema.define(version: 20160309140734) do t.string "unlock_token" t.datetime "otp_grace_period_started_at" t.boolean "ldap_email", default: false, null: false + t.boolean "external", default: false end add_index "users", ["admin"], name: "index_users_on_admin", using: :btree diff --git a/lib/api/users.rb b/lib/api/users.rb index fd2128bd179..c574f042a66 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -61,19 +61,20 @@ module API # admin - User is admin - true or false (default) # can_create_group - User can create groups - true or false # confirm - Require user confirmation - true (default) or false + # external - Is user an external user - true or false(default) # Example Request: # POST /users post do authenticated_as_admin! required_attributes! [:email, :password, :name, :username] - attrs = attributes_for_keys [:email, :name, :password, :skype, :linkedin, :twitter, :projects_limit, :username, :bio, :can_create_group, :admin, :confirm] + attrs = attributes_for_keys [:email, :name, :password, :skype, :linkedin, :twitter, :projects_limit, :username, :bio, :can_create_group, :admin, :confirm, :external] admin = attrs.delete(:admin) confirm = !(attrs.delete(:confirm) =~ (/(false|f|no|0)$/i)) user = User.build_user(attrs) user.admin = admin unless admin.nil? user.skip_confirmation! unless confirm - identity_attrs = attributes_for_keys [:provider, :extern_uid] + if identity_attrs.any? user.identities.build(identity_attrs) end diff --git a/spec/features/security/project/internal_access_spec.rb b/spec/features/security/project/internal_access_spec.rb index 57563add74c..31bd80e7ccb 100644 --- a/spec/features/security/project/internal_access_spec.rb +++ b/spec/features/security/project/internal_access_spec.rb @@ -34,6 +34,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -45,6 +46,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -56,6 +58,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -67,6 +70,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -78,6 +82,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -89,22 +94,21 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/blob" do - before do - commit = project.repository.commit - path = '.gitignore' - @blob_path = namespace_project_blob_path(project.namespace, project, File.join(commit.id, path)) - end + let(:commit) { project.repository.commit } + subject { namespace_project_blob_path(project.namespace, project, File.join(commit.id, '.gitignore')) } - it { expect(@blob_path).to be_allowed_for master } - it { expect(@blob_path).to be_allowed_for reporter } - it { expect(@blob_path).to be_allowed_for :admin } - it { expect(@blob_path).to be_allowed_for guest } - it { expect(@blob_path).to be_allowed_for :user } - it { expect(@blob_path).to be_denied_for :visitor } + it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for reporter } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for guest } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_denied_for :external } + it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/edit" do @@ -115,6 +119,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -126,6 +131,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -137,6 +143,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -149,6 +156,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -160,6 +168,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -171,6 +180,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -182,6 +192,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -193,6 +204,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -209,6 +221,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -225,6 +238,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -236,6 +250,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end end diff --git a/spec/features/security/project/private_access_spec.rb b/spec/features/security/project/private_access_spec.rb index a1e111c6cab..13ff29c98ae 100644 --- a/spec/features/security/project/private_access_spec.rb +++ b/spec/features/security/project/private_access_spec.rb @@ -34,6 +34,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -45,6 +46,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -56,6 +58,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -78,6 +81,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -89,22 +93,21 @@ describe "Private Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/blob" do - before do - commit = project.repository.commit - path = '.gitignore' - @blob_path = namespace_project_blob_path(project.namespace, project, File.join(commit.id, path)) - end + let(:commit) { project.repository.commit } + subject { namespace_project_blob_path(project.namespace, project, File.join(commit.id, '.gitignore'))} - it { expect(@blob_path).to be_allowed_for master } - it { expect(@blob_path).to be_allowed_for reporter } - it { expect(@blob_path).to be_allowed_for :admin } - it { expect(@blob_path).to be_denied_for guest } - it { expect(@blob_path).to be_denied_for :user } - it { expect(@blob_path).to be_denied_for :visitor } + it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for reporter } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_denied_for guest } + it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } + it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/edit" do @@ -115,6 +118,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -126,6 +130,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -137,6 +142,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -149,6 +155,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -160,6 +167,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -171,6 +179,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -187,6 +196,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -203,6 +213,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -214,6 +225,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end end diff --git a/spec/features/security/project/public_access_spec.rb b/spec/features/security/project/public_access_spec.rb index b98476f854e..4e135076367 100644 --- a/spec/features/security/project/public_access_spec.rb +++ b/spec/features/security/project/public_access_spec.rb @@ -38,6 +38,7 @@ describe "Public Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } + it { is_expected.to be_allowed_for :external } it { is_expected.to be_allowed_for :visitor } end @@ -49,6 +50,7 @@ describe "Public Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } + it { is_expected.to be_allowed_for :external } it { is_expected.to be_allowed_for :visitor } end @@ -60,6 +62,7 @@ describe "Public Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } + it { is_expected.to be_allowed_for :external } it { is_expected.to be_allowed_for :visitor } end @@ -71,6 +74,7 @@ describe "Public Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } + it { is_expected.to be_allowed_for :external } it { is_expected.to be_allowed_for :visitor } end @@ -82,6 +86,7 @@ describe "Public Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } + it { is_expected.to be_allowed_for :external } it { is_expected.to be_allowed_for :visitor } end @@ -93,6 +98,7 @@ describe "Public Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -107,6 +113,7 @@ describe "Public Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } + it { is_expected.to be_allowed_for :external } it { is_expected.to be_allowed_for :visitor } end @@ -118,6 +125,7 @@ describe "Public Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end end @@ -135,6 +143,7 @@ describe "Public Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } + it { is_expected.to be_allowed_for :external } it { is_expected.to be_allowed_for :visitor } end @@ -146,23 +155,22 @@ describe "Public Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end end describe "GET /:project_path/blob" do - before do - commit = project.repository.commit - path = '.gitignore' - @blob_path = namespace_project_blob_path(project.namespace, project, File.join(commit.id, path)) - end + let(:commit) { project.repository.commit } + + subject { namespace_project_blob_path(project.namespace, project, File.join(commit.id, '.gitignore')) } - it { expect(@blob_path).to be_allowed_for master } - it { expect(@blob_path).to be_allowed_for reporter } - it { expect(@blob_path).to be_allowed_for :admin } - it { expect(@blob_path).to be_allowed_for guest } - it { expect(@blob_path).to be_allowed_for :user } - it { expect(@blob_path).to be_allowed_for :visitor } + it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for reporter } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for guest } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_allowed_for :visitor } end describe "GET /:project_path/edit" do @@ -173,6 +181,7 @@ describe "Public Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -184,6 +193,7 @@ describe "Public Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -195,6 +205,7 @@ describe "Public Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } + it { is_expected.to be_allowed_for :external } it { is_expected.to be_allowed_for :visitor } end @@ -207,6 +218,7 @@ describe "Public Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -218,6 +230,7 @@ describe "Public Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } + it { is_expected.to be_allowed_for :external } it { is_expected.to be_allowed_for :visitor } end @@ -229,6 +242,7 @@ describe "Public Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -240,6 +254,7 @@ describe "Public Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } + it { is_expected.to be_allowed_for :external } it { is_expected.to be_allowed_for :visitor } end @@ -251,6 +266,7 @@ describe "Public Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end @@ -267,6 +283,7 @@ describe "Public Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } + it { is_expected.to be_allowed_for :external } it { is_expected.to be_allowed_for :visitor } end @@ -283,6 +300,7 @@ describe "Public Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } + it { is_expected.to be_allowed_for :external } it { is_expected.to be_allowed_for :visitor } end @@ -294,6 +312,7 @@ describe "Public Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 909b6796591..8642654ad33 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -206,6 +206,21 @@ describe User, models: true do it { is_expected.to respond_to(:is_admin?) } it { is_expected.to respond_to(:name) } it { is_expected.to respond_to(:private_token) } + it { is_expected.to respond_to(:external?) } + end + + describe 'before save hook' do + context 'when saving an external user' do + let(:user) { create(:user) } + let(:external_user) { create(:user, external: true) } + + it "sets other properties aswell" do + expect(external_user.can_create_team).to be_falsey + expect(external_user.can_create_group).to be_falsey + expect(external_user.hide_project_limit).to be_truthy + expect(external_user.projects_limit).to be 0 + end + end end describe '#confirm' do @@ -430,6 +445,7 @@ describe User, models: true do expect(user.projects_limit).to eq(Gitlab.config.gitlab.default_projects_limit) expect(user.can_create_group).to eq(Gitlab.config.gitlab.default_can_create_group) expect(user.theme_id).to eq(Gitlab.config.gitlab.default_theme) + expect(user.external).to be_falsey end end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 96e8c8c51f8..5366a7bd06b 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -120,6 +120,26 @@ describe API::API, api: true do expect(response.status).to eq(201) end + it 'creates non-external users by default' do + post api("/users", admin), attributes_for(:user) + expect(response.status).to eq(201) + + user_id = json_response['id'] + new_user = User.find(user_id) + expect(new_user).not_to eq nil + expect(new_user.external).to be_falsy + end + + it 'should allow an external user to be created' do + post api("/users", admin), attributes_for(:user, external: true) + expect(response.status).to eq(201) + + user_id = json_response['id'] + new_user = User.find(user_id) + expect(new_user).not_to eq nil + expect(new_user.external).to be_truthy + end + it "should not create user with invalid email" do post api('/users', admin), email: 'invalid email', diff --git a/spec/support/matchers/access_matchers.rb b/spec/support/matchers/access_matchers.rb index 558e8b1612f..4e007c777e3 100644 --- a/spec/support/matchers/access_matchers.rb +++ b/spec/support/matchers/access_matchers.rb @@ -15,6 +15,8 @@ module AccessMatchers logout when :admin login_as(create(:admin)) + when :external + login_as(create(:user, external: true)) when User login_as(user) else -- cgit v1.2.1 From fc8d64b3a0fc5aeb048b2bce79b2ae93a4fcc798 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Sat, 12 Mar 2016 15:02:38 +0100 Subject: Remove Project#publicish --- app/models/project.rb | 6 ------ app/views/dashboard/projects/_zero_authorized_projects.html.haml | 4 ++-- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 1f18ad78164..51952ba72a2 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -252,12 +252,6 @@ class Project < ActiveRecord::Base where('projects.last_activity_at < ?', 6.months.ago) end - def publicish(user) - visibility_levels = [Project::PUBLIC] - visibility_levels << Project::INTERNAL if user - where(visibility_level: visibility_levels) - end - def with_push joins(:events).where('events.action = ?', Event::PUSHED) end diff --git a/app/views/dashboard/projects/_zero_authorized_projects.html.haml b/app/views/dashboard/projects/_zero_authorized_projects.html.haml index c3efa7727b1..65edc05b793 100644 --- a/app/views/dashboard/projects/_zero_authorized_projects.html.haml +++ b/app/views/dashboard/projects/_zero_authorized_projects.html.haml @@ -1,4 +1,4 @@ -- publicish_project_count = Project.publicish(current_user).count +- publicish_project_count = ProjectsFinder.new.execute(current_user).count %h3.page-title Welcome to GitLab! %p.light Self hosted Git management application. %hr @@ -18,7 +18,7 @@ - if current_user.can_create_project? .link_holder = link_to new_project_path, class: "btn btn-new" do - %i.fa.fa-plus + =icon('plus') New Project - if current_user.can_create_group? -- cgit v1.2.1 From 76eeb316df2f256d0d3c41d97421f709a21a02a8 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Sun, 13 Mar 2016 10:05:32 +0100 Subject: Create an external users tab on Admin user list Also incorporates the review into this, mainly spec changes. --- CHANGELOG | 1 + app/finders/projects_finder.rb | 13 +++++++++---- app/models/user.rb | 5 +++-- app/views/admin/users/_form.html.haml | 3 ++- app/views/admin/users/index.html.haml | 4 ++++ .../security/project/internal_access_spec.rb | 20 ++++++++++++++++++++ .../features/security/project/private_access_spec.rb | 18 ++++++++++++++++++ spec/models/user_spec.rb | 1 - 8 files changed, 57 insertions(+), 8 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 07b187b31a2..f9c6f6e23e9 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -30,6 +30,7 @@ v 8.6.0 (unreleased) - Add main language of a project in the list of projects (Tiago Botelho) - Add ability to show archived projects on dashboard, explore and group pages - Move group activity to separate page + - Create external users which are excluded of internal and private projects unless access was explicitly granted v 8.5.5 - Ensure removing a project removes associated Todo entries diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb index 711d9019a3d..4a6c2fbd71c 100644 --- a/app/finders/projects_finder.rb +++ b/app/finders/projects_finder.rb @@ -40,18 +40,23 @@ class ProjectsFinder private def group_projects(current_user, group) - if current_user + return [group.projects.public_only] unless current_user + + if current_user.external? [ group_projects_for_user(current_user, group), - group.projects.public_and_internal_only + group.projects.public_only ] else - [group.projects.public_only] + [ + group_projects_for_user(current_user, group), + group.projects.public_and_internal_only + ] end end def all_projects(current_user) - return [Project.public_only] unless current_user + return [public_projects] unless current_user if current_user.external? [current_user.authorized_projects, public_projects] diff --git a/app/models/user.rb b/app/models/user.rb index adff238c60f..34f1cfc0be6 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -229,6 +229,7 @@ class User < ActiveRecord::Base # Scopes scope :admins, -> { where(admin: true) } scope :blocked, -> { with_states(:blocked, :ldap_blocked) } + scope :external, -> { where(external: true) } scope :active, -> { with_state(:active) } scope :not_in_project, ->(project) { project.users.present? ? where("id not in (:ids)", ids: project.users.map(&:id) ) : all } scope :without_projects, -> { where('id NOT IN (SELECT DISTINCT(user_id) FROM members)') } @@ -284,6 +285,8 @@ class User < ActiveRecord::Base self.with_two_factor when 'wop' self.without_projects + when 'external' + self.external else self.active end @@ -855,9 +858,7 @@ class User < ActiveRecord::Base def ensure_external_user_rights return unless self.external? - self.can_create_team = false self.can_create_group = false self.projects_limit = 0 - self.hide_project_limit = true end end diff --git a/app/views/admin/users/_form.html.haml b/app/views/admin/users/_form.html.haml index 5506827edc4..b910040a16a 100644 --- a/app/views/admin/users/_form.html.haml +++ b/app/views/admin/users/_form.html.haml @@ -58,13 +58,14 @@ = f.label :admin, class: 'control-label' - if current_user == @user .col-sm-10= f.check_box :admin, disabled: true - .col-sm-10 You cannot remove your own admin rights + .col-sm-10 You cannot remove your own admin rights. - else .col-sm-10= f.check_box :admin .form-group = f.label :external, class: 'control-label' .col-sm-10= f.check_box :external + .col-sm-10 External users can not see internal or private projects unless access is explicitly granted. Also, external user can not create projects or groups. %fieldset %legend Profile diff --git a/app/views/admin/users/index.html.haml b/app/views/admin/users/index.html.haml index b6b1168bd37..4394e181008 100644 --- a/app/views/admin/users/index.html.haml +++ b/app/views/admin/users/index.html.haml @@ -19,6 +19,10 @@ = link_to admin_users_path(filter: 'two_factor_disabled') do 2FA Disabled %small.badge= number_with_delimiter(User.without_two_factor.count) + %li.filter-external{class: "#{'active' if params[:filter] == 'external'}"} + = link_to admin_users_path(filter: 'external') do + External + %small.badge= number_with_delimiter(User.external.count) %li{class: "#{'active' if params[:filter] == "blocked"}"} = link_to admin_users_path(filter: "blocked") do Blocked diff --git a/spec/features/security/project/internal_access_spec.rb b/spec/features/security/project/internal_access_spec.rb index 31bd80e7ccb..f88c591d897 100644 --- a/spec/features/security/project/internal_access_spec.rb +++ b/spec/features/security/project/internal_access_spec.rb @@ -8,10 +8,12 @@ describe "Internal Project Access", feature: true do let(:master) { create(:user) } let(:guest) { create(:user) } let(:reporter) { create(:user) } + let(:external_team_member) { create(:user, external: true) } before do # full access project.team << [master, :master] + project.team << [external_team_member, :master] # readonly project.team << [reporter, :reporter] @@ -35,6 +37,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -47,6 +50,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -59,6 +63,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -71,6 +76,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -83,6 +89,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -95,6 +102,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -108,6 +116,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -120,6 +129,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -132,6 +142,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -144,6 +155,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -157,6 +169,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -169,6 +182,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -181,6 +195,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -193,6 +208,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -205,6 +221,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -222,6 +239,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -239,6 +257,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -251,6 +270,7 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end end diff --git a/spec/features/security/project/private_access_spec.rb b/spec/features/security/project/private_access_spec.rb index 13ff29c98ae..19f287ce7a4 100644 --- a/spec/features/security/project/private_access_spec.rb +++ b/spec/features/security/project/private_access_spec.rb @@ -8,10 +8,12 @@ describe "Private Project Access", feature: true do let(:master) { create(:user) } let(:guest) { create(:user) } let(:reporter) { create(:user) } + let(:external_team_member) { create(:user, external: true) } before do # full access project.team << [master, :master] + project.team << [external_team_member, :master] # readonly project.team << [reporter, :reporter] @@ -35,6 +37,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -47,6 +50,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -59,6 +63,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -70,6 +75,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -82,6 +88,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -94,6 +101,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -107,6 +115,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -119,6 +128,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -131,6 +141,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -143,6 +154,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -156,6 +168,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -168,6 +181,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -180,6 +194,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -197,6 +212,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -214,6 +230,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -226,6 +243,7 @@ describe "Private Project Access", feature: true do it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } + it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 8642654ad33..ca0c84c8632 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -217,7 +217,6 @@ describe User, models: true do it "sets other properties aswell" do expect(external_user.can_create_team).to be_falsey expect(external_user.can_create_group).to be_falsey - expect(external_user.hide_project_limit).to be_truthy expect(external_user.projects_limit).to be 0 end end -- cgit v1.2.1 From ab418e27a9121704d623343417126b0bdae08e79 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Mon, 14 Mar 2016 22:06:23 +0100 Subject: Improve external users feature --- app/finders/projects_finder.rb | 4 ++-- app/views/admin/users/_form.html.haml | 2 +- app/views/admin/users/index.html.haml | 6 ++++-- app/views/dashboard/projects/_zero_authorized_projects.html.haml | 2 +- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb index 70c073f7d5c..3a5fc5b5907 100644 --- a/app/finders/projects_finder.rb +++ b/app/finders/projects_finder.rb @@ -47,9 +47,9 @@ class ProjectsFinder group.shared_projects.visible_to_user(current_user) ] if current_user.external? - user_group_projects.push(group.projects.public_only) + user_group_projects << group.projects.public_only else - user_group_projects.push(group.projects.public_and_internal_only) + user_group_projects << group.projects.public_and_internal_only end end diff --git a/app/views/admin/users/_form.html.haml b/app/views/admin/users/_form.html.haml index b910040a16a..d2527ede995 100644 --- a/app/views/admin/users/_form.html.haml +++ b/app/views/admin/users/_form.html.haml @@ -65,7 +65,7 @@ .form-group = f.label :external, class: 'control-label' .col-sm-10= f.check_box :external - .col-sm-10 External users can not see internal or private projects unless access is explicitly granted. Also, external user can not create projects or groups. + .col-sm-10 External users cannot see internal or private projects unless access is explicitly granted. Also, external users cannot create projects or groups. %fieldset %legend Profile diff --git a/app/views/admin/users/index.html.haml b/app/views/admin/users/index.html.haml index 4394e181008..0ee8dc962b9 100644 --- a/app/views/admin/users/index.html.haml +++ b/app/views/admin/users/index.html.haml @@ -74,12 +74,14 @@ %li .list-item-name - if user.blocked? - %i.fa.fa-lock.cred + = icon("lock", class: "cred") - else - %i.fa.fa-user.cgreen + = icon("user", class: "cgreen") = link_to user.name, [:admin, user] - if user.admin? %strong.cred (Admin) + - if user.external? + %strong.cred (External) - if user == current_user %span.cred It's you! .pull-right diff --git a/app/views/dashboard/projects/_zero_authorized_projects.html.haml b/app/views/dashboard/projects/_zero_authorized_projects.html.haml index 65edc05b793..d54c7cad7be 100644 --- a/app/views/dashboard/projects/_zero_authorized_projects.html.haml +++ b/app/views/dashboard/projects/_zero_authorized_projects.html.haml @@ -18,7 +18,7 @@ - if current_user.can_create_project? .link_holder = link_to new_project_path, class: "btn btn-new" do - =icon('plus') + = icon('plus') New Project - if current_user.can_create_group? -- cgit v1.2.1 From aaf4434b0e24da916d4392aa9cd001cdb8e0c7dc Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Tue, 15 Mar 2016 00:11:20 +0100 Subject: Doc external users feature --- doc/README.md | 2 +- doc/api/users.md | 3 ++- doc/permissions/permissions.md | 17 +++++++++++++++++ lib/api/entities.rb | 1 + 4 files changed, 21 insertions(+), 2 deletions(-) diff --git a/doc/README.md b/doc/README.md index 0ca30e4e0f2..db19c3de8d1 100644 --- a/doc/README.md +++ b/doc/README.md @@ -8,7 +8,7 @@ - [Importing to GitLab](workflow/importing/README.md). - [Markdown](markdown/markdown.md) GitLab's advanced formatting system. - [Migrating from SVN](workflow/importing/migrating_from_svn.md) Convert a SVN repository to Git and GitLab -- [Permissions](permissions/permissions.md) Learn what each role in a project (guest/reporter/developer/master/owner) can do. +- [Permissions](permissions/permissions.md) Learn what each role in a project (external/guest/reporter/developer/master/owner) can do. - [Profile Settings](profile/README.md) - [Project Services](project_services/project_services.md) Integrate a project with external services, such as CI and chat. - [Public access](public_access/public_access.md) Learn how you can allow public and internal access to projects. diff --git a/doc/api/users.md b/doc/api/users.md index 82c57a2fd43..44a29da5ecc 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -194,6 +194,7 @@ Parameters: - `admin` (optional) - User is admin - true or false (default) - `can_create_group` (optional) - User can create groups - true or false - `confirm` (optional) - Require confirmation - true (default) or false +- `external` (optional) - Flags the user as external - true or false(default) ## User modification @@ -560,7 +561,7 @@ Parameters: - `uid` (required) - id of specified user -Will return `200 OK` on success, `404 User Not Found` is user cannot be found or +Will return `200 OK` on success, `404 User Not Found` is user cannot be found or `403 Forbidden` when trying to block an already blocked user by LDAP synchronization. ## Unblock user diff --git a/doc/permissions/permissions.md b/doc/permissions/permissions.md index ac0fd3d1756..2dfd08755ba 100644 --- a/doc/permissions/permissions.md +++ b/doc/permissions/permissions.md @@ -71,3 +71,20 @@ Any user can remove themselves from a group, unless they are the last Owner of t | Create project in group | | | | ✓ | ✓ | | Manage group members | | | | | ✓ | | Remove group | | | | | ✓ | + +## External Users + +In cases where it is desired that a user has access to some internal or private projects, but others +should remain hidden from this user, there is the option of creating `External Users`. +An administrator can flag a user as external through the API or by checking the checkbox on the admin panel. + +In the case of a new user: navigate to the **Admin** area and click the **New User** button. If you would like to +edit a user, go to the user list on the **Admin** area and click the **Edit** button. + +External users can only access projects to which they are explicitly granted access, thus hiding all internal projects. +Access can be granted by adding the users as member to the project or by including this user in a group. External users will, like usual users, receive +a role in the project or group with all the abilities that are mentioned in the table above. + +External users cannot create groups or projects, and have the same access as logged out users in all other cases. This feature may be +useful when for example a contractor is working on a given project and should only access the given project and public +projects. diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 7204dca34ba..4b3ad1443bb 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -31,6 +31,7 @@ module API expose :can_create_group?, as: :can_create_group expose :can_create_project?, as: :can_create_project expose :two_factor_enabled + expose :external end class UserLogin < UserFull -- cgit v1.2.1 From cde7eee73e611ca7f775e179fb07b931c37f54ae Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Thu, 17 Mar 2016 09:24:06 +0200 Subject: Refactor external users docs [ci skip] --- doc/permissions/permissions.md | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/doc/permissions/permissions.md b/doc/permissions/permissions.md index 2dfd08755ba..3d375e47c8e 100644 --- a/doc/permissions/permissions.md +++ b/doc/permissions/permissions.md @@ -74,17 +74,21 @@ Any user can remove themselves from a group, unless they are the last Owner of t ## External Users -In cases where it is desired that a user has access to some internal or private projects, but others -should remain hidden from this user, there is the option of creating `External Users`. -An administrator can flag a user as external through the API or by checking the checkbox on the admin panel. - -In the case of a new user: navigate to the **Admin** area and click the **New User** button. If you would like to -edit a user, go to the user list on the **Admin** area and click the **Edit** button. - -External users can only access projects to which they are explicitly granted access, thus hiding all internal projects. -Access can be granted by adding the users as member to the project or by including this user in a group. External users will, like usual users, receive -a role in the project or group with all the abilities that are mentioned in the table above. - -External users cannot create groups or projects, and have the same access as logged out users in all other cases. This feature may be -useful when for example a contractor is working on a given project and should only access the given project and public -projects. +In cases where it is desired that a user has access only to some internal or +private projects, there is the option of creating **External Users**. This +feature may be useful when for example a contractor is working on a given +project and should only have access to that project. + +External users can only access projects to which they are explicitly granted +access, thus hiding all other internal or private ones from them. Access can be +granted by adding the user as member to the project or group. + +They will, like usual users, receive a role in the project or group with all +the abilities that are mentioned in the table above. They cannot however create +groups or projects, and they have the same access as logged out users in all +other cases. + +An administrator can flag a user as external [through the API](../api/users.md) +or by checking the checkbox on the admin panel. As an administrator, navigate +to **Admin > Users** to create a new user or edit an existing one. There, you +will find the option to flag the user as external. -- cgit v1.2.1 From 51300d9b2a65a43742985847376150229b91c4ca Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Thu, 17 Mar 2016 09:36:00 +0100 Subject: API support for setting External flag on existing users --- lib/api/users.rb | 3 ++- spec/requests/api/users_spec.rb | 7 +++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/api/users.rb b/lib/api/users.rb index c574f042a66..8849fff60af 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -108,12 +108,13 @@ module API # bio - Bio # admin - User is admin - true or false (default) # can_create_group - User can create groups - true or false + # external - Is user an external user - true or false(default) # Example Request: # PUT /users/:id put ":id" do authenticated_as_admin! - attrs = attributes_for_keys [:email, :name, :password, :skype, :linkedin, :twitter, :website_url, :projects_limit, :username, :bio, :can_create_group, :admin] + attrs = attributes_for_keys [:email, :name, :password, :skype, :linkedin, :twitter, :website_url, :projects_limit, :username, :bio, :can_create_group, :admin, :external] user = User.find(params[:id]) not_found!('User') unless user diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 5366a7bd06b..679227bf881 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -282,6 +282,13 @@ describe API::API, api: true do expect(user.reload.admin).to eq(true) end + it "should update external status" do + put api("/users/#{user.id}", admin), { external: true } + expect(response.status).to eq 200 + expect(json_response['external']).to eq(true) + expect(user.reload.external?).to be_truthy + end + it "should not update admin status" do put api("/users/#{admin_user.id}", admin), { can_create_group: false } expect(response.status).to eq(200) -- cgit v1.2.1 From 956e914307029dbfbdb387fd6c0749dd50935fa4 Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Thu, 17 Mar 2016 12:21:12 +0200 Subject: Add missing API docs on external user [ci skip] --- doc/api/users.md | 1 + lib/api/users.rb | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/doc/api/users.md b/doc/api/users.md index 44a29da5ecc..383e7c76ab0 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -220,6 +220,7 @@ Parameters: - `bio` - User's biography - `admin` (optional) - User is admin - true or false (default) - `can_create_group` (optional) - User can create groups - true or false +- `external` (optional) - Flags the user as external - true or false(default) Note, at the moment this method does only return a 404 error, even in cases where a 409 (Conflict) would be more appropriate, diff --git a/lib/api/users.rb b/lib/api/users.rb index 8849fff60af..13ab17c6904 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -61,7 +61,7 @@ module API # admin - User is admin - true or false (default) # can_create_group - User can create groups - true or false # confirm - Require user confirmation - true (default) or false - # external - Is user an external user - true or false(default) + # external - Flags the user as external - true or false(default) # Example Request: # POST /users post do @@ -108,7 +108,7 @@ module API # bio - Bio # admin - User is admin - true or false (default) # can_create_group - User can create groups - true or false - # external - Is user an external user - true or false(default) + # external - Flags the user as external - true or false(default) # Example Request: # PUT /users/:id put ":id" do -- cgit v1.2.1