diff options
51 files changed, 623 insertions, 195 deletions
diff --git a/CHANGELOG b/CHANGELOG index 638a4f1d3fe..72e91a1dac1 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -4,6 +4,7 @@ v 8.6.0 (unreleased) - Bump gitlab_git to 9.0.3 (Stan Hu) - Support Golang subpackage fetching (Stan Hu) - Bump Capybara gem to 2.6.2 (Stan Hu) + - New branch button appears on issues where applicable - Contributions to forked projects are included in calendar - Improve the formatting for the user page bio (Connor Shea) - Removed the default password from the initial admin account created during @@ -41,6 +42,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 - Continue parameters are checked to ensure redirection goes to the same instance - User deletion is now done in the background so the request can not time out diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index 180926b3b97..bc03c2180be 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -8,20 +8,20 @@ /** COMMON CLASSES **/ .prepend-top-0 { margin-top: 0; } .prepend-top-5 { margin-top: 5px; } -.prepend-top-10 { margin-top:10px } +.prepend-top-10 { margin-top: 10px } .prepend-top-default { margin-top: $gl-padding !important; } -.prepend-top-20 { margin-top:20px } -.prepend-left-10 { margin-left:10px } +.prepend-top-20 { margin-top: 20px } +.prepend-left-10 { margin-left: 10px } .prepend-left-default { margin-left: $gl-padding; } -.prepend-left-20 { margin-left:20px } +.prepend-left-20 { margin-left: 20px } .append-right-5 { margin-right: 5px } -.append-right-10 { margin-right:10px } +.append-right-10 { margin-right: 10px } .append-right-default { margin-right: $gl-padding; } -.append-right-20 { margin-right:20px } -.append-bottom-0 { margin-bottom:0 } -.append-bottom-10 { margin-bottom:10px } -.append-bottom-15 { margin-bottom:15px } -.append-bottom-20 { margin-bottom:20px } +.append-right-20 { margin-right: 20px } +.append-bottom-0 { margin-bottom: 0 } +.append-bottom-10 { margin-bottom: 10px } +.append-bottom-15 { margin-bottom: 15px } +.append-bottom-20 { margin-bottom: 20px } .append-bottom-default { margin-bottom: $gl-padding; } .inline { display: inline-block } .center { text-align: center } @@ -134,10 +134,10 @@ p.time { // Fix issue with notes & lists creating a bunch of bottom borders. li.note { - img { max-width:100% } + img { max-width: 100% } .note-title { li { - border-bottom:none !important; + border-bottom: none !important; } } } diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 5e3546bc6ff..211ead7319d 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -27,7 +27,7 @@ $gl-gray: #5a5a5a; $gl-padding: 16px; $gl-btn-padding: 10px; $gl-vert-padding: 6px; -$gl-padding-top:10px; +$gl-padding-top: 10px; $gl-avatar-size: 40px; $secondary-text: #7f8fa4; $error-exclamation-point: #e62958; diff --git a/app/assets/stylesheets/pages/appearances.scss b/app/assets/stylesheets/pages/appearances.scss index e2070f17c3b..878f44116ba 100644 --- a/app/assets/stylesheets/pages/appearances.scss +++ b/app/assets/stylesheets/pages/appearances.scss @@ -4,7 +4,7 @@ } .appearance-light-logo-preview { - background-color: $background-color; + background-color: $background-color; max-width: 72px; padding: 10px; margin-bottom: 10px; diff --git a/app/assets/stylesheets/pages/dashboard.scss b/app/assets/stylesheets/pages/dashboard.scss index 88639399148..cf7567513ec 100644 --- a/app/assets/stylesheets/pages/dashboard.scss +++ b/app/assets/stylesheets/pages/dashboard.scss @@ -11,15 +11,15 @@ } .dashboard-search-filter { - padding:5px; + padding: 5px; .search-text-input { - float:left; + float: left; @extend .col-md-2; } .btn { margin-left: 5px; - float:left; + float: left; } } diff --git a/app/assets/stylesheets/pages/events.scss b/app/assets/stylesheets/pages/events.scss index e7da0a2f689..b39a9abf40f 100644 --- a/app/assets/stylesheets/pages/events.scss +++ b/app/assets/stylesheets/pages/events.scss @@ -94,7 +94,7 @@ } } - &:last-child { border:none } + &:last-child { border: none } .event_commits { li { @@ -138,7 +138,7 @@ @include str-truncated(100%); padding: 5px 0; font-size: 13px; - float:left; + float: left; margin-right: -150px; padding-right: 150px; line-height: 20px; diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index faa2ebfda78..c975ca0ce43 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -59,7 +59,7 @@ .issuable-sidebar { .block { @include clearfix; - padding: $gl-padding 0; + padding: $gl-padding 0; border-bottom: 1px solid $border-gray-light; // This prevents the mess when resizing the sidebar // of elements repositioning themselves.. diff --git a/app/assets/stylesheets/pages/issues.scss b/app/assets/stylesheets/pages/issues.scss index 73718ff511a..7ac4bc468d6 100644 --- a/app/assets/stylesheets/pages/issues.scss +++ b/app/assets/stylesheets/pages/issues.scss @@ -49,7 +49,7 @@ form.edit-issue { margin: 0; } -.merge-requests-title { +.merge-requests-title, .related-branches-title { font-size: 16px; font-weight: 600; } diff --git a/app/assets/stylesheets/pages/login.scss b/app/assets/stylesheets/pages/login.scss index d9c47881265..bc41f7d306f 100644 --- a/app/assets/stylesheets/pages/login.scss +++ b/app/assets/stylesheets/pages/login.scss @@ -28,7 +28,7 @@ img { max-width: 100%; - margin-bottom: 30px; + margin-bottom: 30px; } a { @@ -85,7 +85,7 @@ &.middle { border-top: 0; - margin-bottom:0; + margin-bottom: 0; @include border-radius(0); } diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 969c79a9be9..d408853cc80 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -3,9 +3,9 @@ */ @-webkit-keyframes targe3-note { - from { background:#fffff0; } - 50% { background:#ffffd3; } - to { background:#fffff0; } + from { background: #fffff0; } + 50% { background: #ffffd3; } + to { background: #fffff0; } } ul.notes { @@ -93,12 +93,12 @@ ul.notes { .discussion { overflow: hidden; display: block; - position:relative; + position: relative; } .note { display: block; - position:relative; + position: relative; .note-body { overflow: auto; @@ -108,6 +108,13 @@ ul.notes { word-wrap: break-word; @include md-typography; + // On diffs code should wrap nicely and not overflow + pre { + code { + white-space: pre-wrap; + } + } + // Reset ul style types since we're nested inside a ul already & > ul { list-style-type: disc; diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index 3fe2c9a3346..6c600c99d51 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -286,11 +286,11 @@ table.table.protected-branches-list tr.no-border { padding-bottom: 4px; ul.nav { - display:inline-block; + display: inline-block; } .nav li { - display:inline; + display: inline; } .nav > li > a { @@ -303,11 +303,11 @@ table.table.protected-branches-list tr.no-border { } li { - display:inline; + display: inline; } a { - float:left; + float: left; font-size: 17px; } diff --git a/app/assets/stylesheets/pages/tree.scss b/app/assets/stylesheets/pages/tree.scss index ef63b010600..73c7c9f687c 100644 --- a/app/assets/stylesheets/pages/tree.scss +++ b/app/assets/stylesheets/pages/tree.scss @@ -46,7 +46,7 @@ img { position: relative; - top:-1px; + top: -1px; } } diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 3063d299b1a..9abf08d0e19 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/controllers/projects/badges_controller.rb b/app/controllers/projects/badges_controller.rb index dc9c96df003..6ff47c4033a 100644 --- a/app/controllers/projects/badges_controller.rb +++ b/app/controllers/projects/badges_controller.rb @@ -1,5 +1,5 @@ class Projects::BadgesController < Projects::ApplicationController - before_action :set_no_cache + before_action :no_cache_headers def build respond_to do |format| @@ -10,15 +10,4 @@ class Projects::BadgesController < Projects::ApplicationController end end end - - private - - def set_no_cache - expires_now - - # Add some deprecated headers for older agents - # - response.headers['Pragma'] = 'no-cache' - response.headers['Expires'] = 'Fri, 01 Jan 1990 00:00:00 GMT' - end end diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb index 4db3b3bf23d..43ea717cbd2 100644 --- a/app/controllers/projects/branches_controller.rb +++ b/app/controllers/projects/branches_controller.rb @@ -9,7 +9,7 @@ class Projects::BranchesController < Projects::ApplicationController @sort = params[:sort] || 'name' @branches = @repository.branches_sorted_by(@sort) @branches = Kaminari.paginate_array(@branches).page(params[:page]).per(PER_PAGE) - + @max_commits = @branches.reduce(0) do |memo, branch| diverging_commit_counts = repository.diverging_commit_counts(branch) [memo, diverging_commit_counts[:behind], diverging_commit_counts[:ahead]].max @@ -23,11 +23,15 @@ class Projects::BranchesController < Projects::ApplicationController def create branch_name = sanitize(strip_tags(params[:branch_name])) branch_name = Addressable::URI.unescape(branch_name) - ref = sanitize(strip_tags(params[:ref])) - ref = Addressable::URI.unescape(ref) + result = CreateBranchService.new(project, current_user). execute(branch_name, ref) + if params[:issue_iid] + issue = @project.issues.find_by(iid: params[:issue_iid]) + SystemNoteService.new_issue_branch(issue, @project, current_user, branch_name) if issue + end + if result[:status] == :success @branch = result[:branch] redirect_to namespace_project_tree_path(@project.namespace, @project, @@ -49,4 +53,15 @@ class Projects::BranchesController < Projects::ApplicationController format.js { render status: status[:return_code] } end end + + private + + def ref + if params[:ref] + ref_escaped = sanitize(strip_tags(params[:ref])) + Addressable::URI.unescape(ref_escaped) + else + @project.default_branch + end + end end diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index b0a03ee45cc..aa7a178dcf4 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -65,6 +65,7 @@ class Projects::IssuesController < Projects::ApplicationController @notes = @issue.notes.nonawards.with_associations.fresh @noteable = @issue @merge_requests = @issue.referenced_merge_requests(current_user) + @related_branches = @issue.related_branches - @merge_requests.map(&:source_branch) respond_with(@issue) end diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb index 2c55f088594..3a5fc5b5907 100644 --- a/app/finders/projects_finder.rb +++ b/app/finders/projects_finder.rb @@ -40,25 +40,26 @@ class ProjectsFinder private def group_projects(current_user, group) - if current_user - [ - group_projects_for_user(current_user, group), - group.projects.public_and_internal_only, - group.shared_projects.visible_to_user(current_user) - ] + return [group.projects.public_only] unless current_user + + user_group_projects = [ + group_projects_for_user(current_user, group), + group.shared_projects.visible_to_user(current_user) + ] + if current_user.external? + user_group_projects << group.projects.public_only else - [group.projects.public_only] + user_group_projects << group.projects.public_and_internal_only end end def all_projects(current_user) - if current_user - [ - current_user.authorized_projects, - public_and_internal_projects - ] + return [public_projects] 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/issue.rb b/app/models/issue.rb index 5f58c0508fd..2447f860c5a 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -87,11 +87,21 @@ class Issue < ActiveRecord::Base end def referenced_merge_requests(current_user = nil) - Gitlab::ReferenceExtractor.lazily do - [self, *notes].flat_map do |note| - note.all_references(current_user).merge_requests - end - end.sort_by(&:iid) + @referenced_merge_requests ||= {} + @referenced_merge_requests[current_user] ||= begin + Gitlab::ReferenceExtractor.lazily do + [self, *notes].flat_map do |note| + note.all_references(current_user).merge_requests + end + end.sort_by(&:iid).uniq + end + end + + def related_branches + return [] if self.project.empty_repo? + self.project.repository.branch_names.select do |branch| + branch =~ /\A#{iid}-(?!\d+-stable)/i + end end # Reset issue events cache @@ -120,4 +130,15 @@ class Issue < ActiveRecord::Base note.all_references(current_user).merge_requests end.uniq.select { |mr| mr.open? && mr.closes_issue?(self) } end + + def to_branch_name + "#{iid}-#{title.parameterize}" + end + + def can_be_worked_on?(current_user) + !self.closed? && + !self.project.forked? && + self.related_branches.empty? && + self.closed_by_merge_requests(current_user).empty? + end end diff --git a/app/models/project.rb b/app/models/project.rb index 89a55a510cd..ab4913e99a8 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -254,12 +254,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/models/user.rb b/app/models/user.rb index 68b242888aa..c011af03591 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 @@ -171,6 +173,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 @@ -218,6 +221,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)') } @@ -273,6 +277,8 @@ class User < ActiveRecord::Base self.with_two_factor when 'wop' self.without_projects + when 'external' + self.external else self.active end @@ -841,4 +847,11 @@ 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_group = false + self.projects_limit = 0 + end end diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index 954746a39a5..fa34753c4fd 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -47,6 +47,21 @@ module MergeRequests merge_request.title = merge_request.source_branch.titleize.humanize end + # When your branch name starts with an iid followed by a dash this pattern will + # be interpreted as the use wants to close that issue on this project + # Pattern example: 112-fix-mep-mep + # Will lead to appending `Closes #112` to the description + if match = merge_request.source_branch.match(/\A(\d+)-/) + iid = match[1] + closes_issue = "Closes ##{iid}" + + if merge_request.description.present? + merge_request.description << closes_issue.prepend("\n") + else + merge_request.description = closes_issue + end + end + merge_request end diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index 58a861ee08e..f09b77c4a57 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -207,6 +207,18 @@ class SystemNoteService create_note(noteable: noteable, project: project, author: author, note: body) end + # Called when a branch is created from the 'new branch' button on a issue + # Example note text: + # + # "Started branch `201-issue-branch-button`" + def self.new_issue_branch(issue, project, author, branch) + h = Gitlab::Application.routes.url_helpers + link = h.namespace_project_compare_url(project.namespace, project, from: project.default_branch, to: branch) + + body = "Started branch [`#{branch}`](#{link})" + create_note(noteable: issue, project: project, author: author, note: body) + end + # Called when a Mentionable references a Noteable # # noteable - Noteable object being referenced diff --git a/app/views/admin/users/_form.html.haml b/app/views/admin/users/_form.html.haml index e18dd9bc905..d2527ede995 100644 --- a/app/views/admin/users/_form.html.haml +++ b/app/views/admin/users/_form.html.haml @@ -58,9 +58,15 @@ = 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 cannot see internal or private projects unless access is explicitly granted. Also, external users cannot create projects or groups. + %fieldset %legend Profile .form-group diff --git a/app/views/admin/users/index.html.haml b/app/views/admin/users/index.html.haml index b6b1168bd37..0ee8dc962b9 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 @@ -70,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/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 @@ -48,6 +48,10 @@ Disabled %li + %span.light External User: + %strong + = @user.external? ? "Yes" : "No" + %li %span.light Can create groups: %strong = @user.can_create_group ? "Yes" : "No" diff --git a/app/views/dashboard/projects/_zero_authorized_projects.html.haml b/app/views/dashboard/projects/_zero_authorized_projects.html.haml index c3efa7727b1..d54c7cad7be 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? diff --git a/app/views/projects/issues/_merge_requests.html.haml b/app/views/projects/issues/_merge_requests.html.haml index d9868ad1f0a..d6b38b327ff 100644 --- a/app/views/projects/issues/_merge_requests.html.haml +++ b/app/views/projects/issues/_merge_requests.html.haml @@ -1,4 +1,4 @@ --if @merge_requests.any? +- if @merge_requests.any? %h2.merge-requests-title = pluralize(@merge_requests.count, 'Related Merge Request') %ul.unstyled-list diff --git a/app/views/projects/issues/_new_branch.html.haml b/app/views/projects/issues/_new_branch.html.haml new file mode 100644 index 00000000000..e66e4669d48 --- /dev/null +++ b/app/views/projects/issues/_new_branch.html.haml @@ -0,0 +1,5 @@ +- if current_user && can?(current_user, :push_code, @project) && @issue.can_be_worked_on?(current_user) + .pull-right + = link_to namespace_project_branches_path(@project.namespace, @project, branch_name: @issue.to_branch_name, issue_iid: @issue.iid), method: :post, class: 'btn', title: @issue.to_branch_name do + = icon('code-fork') + New Branch diff --git a/app/views/projects/issues/_related_branches.html.haml b/app/views/projects/issues/_related_branches.html.haml new file mode 100644 index 00000000000..b10cd03515f --- /dev/null +++ b/app/views/projects/issues/_related_branches.html.haml @@ -0,0 +1,15 @@ +- if @related_branches.any? + %h2.related-branches-title + = pluralize(@related_branches.count, 'Related Branch') + %ul.unstyled-list + - @related_branches.each do |branch| + %li + - sha = @project.repository.find_branch(branch).target + - ci_commit = @project.ci_commit(sha) if sha + - if ci_commit + %span.related-branch-ci-status + = render_ci_status(ci_commit) + %span.related-branch-info + %strong + = link_to namespace_project_compare_path(@project.namespace, @project, from: @project.default_branch, to: branch), class: "label-branch" do + = branch diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index 0242276cd84..1e8308277cc 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -70,8 +70,10 @@ .merge-requests = render 'merge_requests' + = render 'related_branches' .content-block.content-block-small + = render 'new_branch' = render 'votes/votes_block', votable: @issue .row diff --git a/app/workers/post_receive.rb b/app/workers/post_receive.rb index 14d7813412e..3cc232ef1ae 100644 --- a/app/workers/post_receive.rb +++ b/app/workers/post_receive.rb @@ -1,6 +1,5 @@ class PostReceive include Sidekiq::Worker - include Gitlab::Identifier sidekiq_options queue: :post_receive @@ -11,51 +10,44 @@ class PostReceive log("Check gitlab.yml config for correct gitlab_shell.repos_path variable. \"#{Gitlab.config.gitlab_shell.repos_path}\" does not match \"#{repo_path}\"") end - repo_path.gsub!(/\.git\z/, "") - repo_path.gsub!(/\A\//, "") + post_received = Gitlab::GitPostReceive.new(repo_path, identifier, changes) - project = Project.find_with_namespace(repo_path) - - if project.nil? + if post_received.project.nil? log("Triggered hook for non-existing project with full path \"#{repo_path} \"") return false end - changes = Base64.decode64(changes) unless changes.include?(" ") - changes = utf8_encode_changes(changes) - changes = changes.lines + if post_received.wiki? + # Nothing defined here yet. + elsif post_received.regular_project? + process_project_changes(post_received) + else + log("Triggered hook for unidentifiable repository type with full path \"#{repo_path} \"") + false + end + end - changes.each do |change| + def process_project_changes(post_received) + post_received.changes.each do |change| oldrev, newrev, ref = change.strip.split(' ') - @user ||= identify(identifier, project, newrev) + @user ||= post_received.identify(newrev) unless @user - log("Triggered hook for non-existing user \"#{identifier} \"") + log("Triggered hook for non-existing user \"#{post_received.identifier} \"") return false end if Gitlab::Git.tag_ref?(ref) - GitTagPushService.new.execute(project, @user, oldrev, newrev, ref) + GitTagPushService.new.execute(post_received.project, @user, oldrev, newrev, ref) else - GitPushService.new(project, @user, oldrev: oldrev, newrev: newrev, ref: ref).execute + GitPushService.new(post_received.project, @user, oldrev: oldrev, newrev: newrev, ref: ref).execute end end end - def utf8_encode_changes(changes) - changes = changes.dup - - changes.force_encoding("UTF-8") - return changes if changes.valid_encoding? - - # Convert non-UTF-8 branch/tag names to UTF-8 so they can be dumped as JSON. - detection = CharlockHolmes::EncodingDetector.detect(changes) - return changes unless detection && detection[:encoding] - - CharlockHolmes::Converter.convert(changes, detection[:encoding], 'UTF-8') - end - + private + def log(message) Gitlab::GitLogger.error("POST-RECEIVE: #{message}") end 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 2c27b228864..2f075677b30 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -940,6 +940,7 @@ ActiveRecord::Schema.define(version: 20160316123110) 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/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..383e7c76ab0 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 @@ -219,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, @@ -560,7 +562,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..3d375e47c8e 100644 --- a/doc/permissions/permissions.md +++ b/doc/permissions/permissions.md @@ -71,3 +71,24 @@ 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 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. diff --git a/features/steps/project/badges/build.rb b/features/steps/project/badges/build.rb index 47540f356e9..66a48a176e5 100644 --- a/features/steps/project/badges/build.rb +++ b/features/steps/project/badges/build.rb @@ -21,7 +21,7 @@ class Spinach::Features::ProjectBadgesBuild < Spinach::FeatureSteps end step 'I should see a badge that has not been cached' do - expect(page.response_headers).to include('Cache-Control' => 'no-cache') + expect(page.response_headers['Cache-Control']).to include 'no-cache' end def expect_badge(status) diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 9805e53624e..71197205f34 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 diff --git a/lib/api/users.rb b/lib/api/users.rb index fd2128bd179..13ab17c6904 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 - Flags the user as external - 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 @@ -107,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 - Flags the user as external - 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/lib/gitlab/git_post_receive.rb b/lib/gitlab/git_post_receive.rb new file mode 100644 index 00000000000..a088e19d1e7 --- /dev/null +++ b/lib/gitlab/git_post_receive.rb @@ -0,0 +1,60 @@ +module Gitlab + class GitPostReceive + include Gitlab::Identifier + attr_reader :repo_path, :identifier, :changes, :project + + def initialize(repo_path, identifier, changes) + repo_path.gsub!(/\.git\z/, '') + repo_path.gsub!(/\A\//, '') + + @repo_path = repo_path + @identifier = identifier + @changes = deserialize_changes(changes) + + retrieve_project_and_type + end + + def wiki? + @type == :wiki + end + + def regular_project? + @type == :project + end + + def identify(revision) + super(identifier, project, revision) + end + + private + + def retrieve_project_and_type + @type = :project + @project = Project.find_with_namespace(@repo_path) + + if @repo_path.end_with?('.wiki') && !@project + @type = :wiki + @project = Project.find_with_namespace(@repo_path.gsub(/\.wiki\z/, '')) + end + end + + def deserialize_changes(changes) + changes = Base64.decode64(changes) unless changes.include?(' ') + changes = utf8_encode_changes(changes) + changes.lines + end + + def utf8_encode_changes(changes) + changes = changes.dup + + changes.force_encoding('UTF-8') + return changes if changes.valid_encoding? + + # Convert non-UTF-8 branch/tag names to UTF-8 so they can be dumped as JSON. + detection = CharlockHolmes::EncodingDetector.detect(changes) + return changes unless detection && detection[:encoding] + + CharlockHolmes::Converter.convert(changes, detection[:encoding], 'UTF-8') + end + end +end diff --git a/spec/controllers/projects/branches_controller_spec.rb b/spec/controllers/projects/branches_controller_spec.rb index 8e06d4bdc77..98ae424ed7c 100644 --- a/spec/controllers/projects/branches_controller_spec.rb +++ b/spec/controllers/projects/branches_controller_spec.rb @@ -17,49 +17,79 @@ describe Projects::BranchesController do describe "POST create" do render_views - before do - post :create, - namespace_id: project.namespace.to_param, - project_id: project.to_param, - branch_name: branch, - ref: ref - end + context "on creation of a new branch" do + before do + post :create, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + branch_name: branch, + ref: ref + end - context "valid branch name, valid source" do - let(:branch) { "merge_branch" } - let(:ref) { "master" } - it 'redirects' do - expect(subject). - to redirect_to("/#{project.path_with_namespace}/tree/merge_branch") + context "valid branch name, valid source" do + let(:branch) { "merge_branch" } + let(:ref) { "master" } + it 'redirects' do + expect(subject). + to redirect_to("/#{project.path_with_namespace}/tree/merge_branch") + end + end + + context "invalid branch name, valid ref" do + let(:branch) { "<script>alert('merge');</script>" } + let(:ref) { "master" } + it 'redirects' do + expect(subject). + to redirect_to("/#{project.path_with_namespace}/tree/alert('merge');") + end + end + + context "valid branch name, invalid ref" do + let(:branch) { "merge_branch" } + let(:ref) { "<script>alert('ref');</script>" } + it { is_expected.to render_template('new') } + end + + context "invalid branch name, invalid ref" do + let(:branch) { "<script>alert('merge');</script>" } + let(:ref) { "<script>alert('ref');</script>" } + it { is_expected.to render_template('new') } + end + + context "valid branch name with encoded slashes" do + let(:branch) { "feature%2Ftest" } + let(:ref) { "<script>alert('ref');</script>" } + it { is_expected.to render_template('new') } + it { project.repository.branch_names.include?('feature/test') } end end - context "invalid branch name, valid ref" do - let(:branch) { "<script>alert('merge');</script>" } - let(:ref) { "master" } + describe "created from the new branch button on issues" do + let(:branch) { "1-feature-branch" } + let!(:issue) { create(:issue, project: project) } + + it 'redirects' do + post :create, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + branch_name: branch, + issue_iid: issue.iid + expect(subject). - to redirect_to("/#{project.path_with_namespace}/tree/alert('merge');") + to redirect_to("/#{project.path_with_namespace}/tree/1-feature-branch") end - end - context "valid branch name, invalid ref" do - let(:branch) { "merge_branch" } - let(:ref) { "<script>alert('ref');</script>" } - it { is_expected.to render_template('new') } - end + it 'posts a system note' do + expect(SystemNoteService).to receive(:new_issue_branch).with(issue, project, user, "1-feature-branch") - context "invalid branch name, invalid ref" do - let(:branch) { "<script>alert('merge');</script>" } - let(:ref) { "<script>alert('ref');</script>" } - it { is_expected.to render_template('new') } - end + post :create, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + branch_name: branch, + issue_iid: issue.iid + end - context "valid branch name with encoded slashes" do - let(:branch) { "feature%2Ftest" } - let(:ref) { "<script>alert('ref');</script>" } - it { is_expected.to render_template('new') } - it { project.repository.branch_names.include?('feature/test')} end end diff --git a/spec/features/issues/new_branch_button_spec.rb b/spec/features/issues/new_branch_button_spec.rb new file mode 100644 index 00000000000..1f3bd915f48 --- /dev/null +++ b/spec/features/issues/new_branch_button_spec.rb @@ -0,0 +1,49 @@ +require 'rails_helper' + +feature 'Start new branch from an issue', feature: true do + let!(:project) { create(:project) } + let!(:issue) { create(:issue, project: project) } + let!(:user) { create(:user)} + + context "for team members" do + before do + project.team << [user, :master] + login_as(user) + end + + it 'shown the new branch button', js: false do + visit namespace_project_issue_path(project.namespace, project, issue) + + expect(page).to have_link "New Branch" + end + + context "when there is a referenced merge request" do + let(:note) do + create(:note, :on_issue, :system, project: project, + note: "mentioned in !#{referenced_mr.iid}") + end + let(:referenced_mr) do + create(:merge_request, :simple, source_project: project, target_project: project, + description: "Fixes ##{issue.iid}") + end + + before do + issue.notes << note + + visit namespace_project_issue_path(project.namespace, project, issue) + end + + it "hides the new branch button", js: true do + expect(page).not_to have_link "New Branch" + expect(page).to have_content /1 Related Merge Request/ + end + end + end + + context "for visiters" do + it 'no button is shown', js: false do + visit namespace_project_issue_path(project.namespace, project, issue) + expect(page).not_to have_link "New Branch" + end + end +end diff --git a/spec/features/security/project/internal_access_spec.rb b/spec/features/security/project/internal_access_spec.rb index 57563add74c..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] @@ -34,6 +36,8 @@ 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_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -45,6 +49,8 @@ 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_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -56,6 +62,8 @@ 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_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -67,6 +75,8 @@ 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_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -78,6 +88,8 @@ 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_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -89,22 +101,23 @@ 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_allowed_for external_team_member } 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_allowed_for external_team_member } + it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/edit" do @@ -115,6 +128,8 @@ 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_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -126,6 +141,8 @@ 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_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -137,6 +154,8 @@ 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_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -149,6 +168,8 @@ 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_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -160,6 +181,8 @@ 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_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -171,6 +194,8 @@ 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_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -182,6 +207,8 @@ 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_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -193,6 +220,8 @@ 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_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -209,6 +238,8 @@ 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_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -225,6 +256,8 @@ 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_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -236,6 +269,8 @@ 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_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 a1e111c6cab..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] @@ -34,6 +36,8 @@ 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_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -45,6 +49,8 @@ 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_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -56,6 +62,8 @@ 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_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -67,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 @@ -78,6 +87,8 @@ 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_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -89,22 +100,23 @@ 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_allowed_for external_team_member } 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_allowed_for external_team_member } + it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/edit" do @@ -115,6 +127,8 @@ 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_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -126,6 +140,8 @@ 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_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -137,6 +153,8 @@ 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_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -149,6 +167,8 @@ 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_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -160,6 +180,8 @@ 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_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -171,6 +193,8 @@ 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_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -187,6 +211,8 @@ 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_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -203,6 +229,8 @@ 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_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -214,6 +242,8 @@ 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_allowed_for external_team_member } 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/issue_spec.rb b/spec/models/issue_spec.rb index 7f44ca2f7db..2ccdec1eeff 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -130,6 +130,15 @@ describe Issue, models: true do end end + describe '#related_branches' do + it "should " do + allow(subject.project.repository).to receive(:branch_names). + and_return(["mpempe", "#{subject.iid}mepmep", subject.to_branch_name]) + + expect(subject.related_branches).to eq [subject.to_branch_name] + end + end + it_behaves_like 'an editable mentionable' do subject { create(:issue) } @@ -140,4 +149,12 @@ describe Issue, models: true do it_behaves_like 'a Taskable' do let(:subject) { create :issue } end + + describe "#to_branch_name" do + let(:issue) { build(:issue, title: 'a' * 30) } + + it "starts with the issue iid" do + expect(issue.to_branch_name).to match /\A#{issue.iid}-a+\z/ + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 6290ab3ebec..0ab7fd88ce6 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -180,6 +180,20 @@ 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.projects_limit).to be 0 + end + end end describe '#confirm' do @@ -404,6 +418,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..679227bf881 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', @@ -262,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) diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 5dcc39f5fdc..8e6292014d4 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -280,6 +280,18 @@ describe SystemNoteService, services: true do end end + describe '.new_issue_branch' do + subject { described_class.new_issue_branch(noteable, project, author, "1-mepmep") } + + it_behaves_like 'a system note' + + context 'when a branch is created from the new branch button' do + it 'sets the note text' do + expect(subject.note).to match /\AStarted branch [`1-mepmep`]/ + end + end + end + describe '.cross_reference' do subject { described_class.cross_reference(noteable, mentioner, author) } 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 |