diff options
84 files changed, 690 insertions, 412 deletions
diff --git a/app/assets/javascripts/shortcuts.js b/app/assets/javascripts/shortcuts.js index 49d980212d6..e3daa8cf949 100644 --- a/app/assets/javascripts/shortcuts.js +++ b/app/assets/javascripts/shortcuts.js @@ -1,6 +1,5 @@ /* eslint-disable func-names, space-before-function-paren, no-var, prefer-rest-params, wrap-iife, quotes, prefer-arrow-callback, consistent-return, object-shorthand, no-unused-vars, one-var, one-var-declaration-per-line, no-else-return, comma-dangle, max-len */ /* global Mousetrap */ -/* global findFileURL */ import Cookies from 'js-cookie'; import findAndFollowLink from './shortcuts_dashboard_navigation'; @@ -20,6 +19,7 @@ import findAndFollowLink from './shortcuts_dashboard_navigation'; const $globalDropdownMenu = $('.global-dropdown-menu'); const $globalDropdownToggle = $('.global-dropdown-toggle'); + const findFileURL = document.body.dataset.findFile; $('.global-dropdown').on('hide.bs.dropdown', () => { $globalDropdownMenu.removeClass('shortcuts'); diff --git a/app/assets/stylesheets/pages/builds.scss b/app/assets/stylesheets/pages/builds.scss index 0393820dee6..b6fc628c02b 100644 --- a/app/assets/stylesheets/pages/builds.scss +++ b/app/assets/stylesheets/pages/builds.scss @@ -244,6 +244,10 @@ } } + .block-last { + padding: 16px 0; + } + .trigger-build-variable { color: $code-color; } diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 1cc060e4de8..c1bc4c0d675 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -113,6 +113,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController :html_emails_enabled, :koding_enabled, :koding_url, + :password_authentication_enabled, :plantuml_enabled, :plantuml_url, :max_artifacts_size, @@ -135,7 +136,6 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController :require_two_factor_authentication, :session_expire_delay, :sign_in_text, - :signin_enabled, :signup_enabled, :sentry_dsn, :sentry_enabled, diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index db7edbd619b..43462b13903 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -170,7 +170,7 @@ class ApplicationController < ActionController::Base end def check_password_expiration - if current_user && current_user.password_expires_at && current_user.password_expires_at < Time.now && !current_user.ldap_user? + if current_user && current_user.password_expires_at && current_user.password_expires_at < Time.now && current_user.allow_password_authentication? return redirect_to new_profile_password_path end end diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb index a8575e037e4..aa8cf630032 100644 --- a/app/controllers/passwords_controller.rb +++ b/app/controllers/passwords_controller.rb @@ -1,6 +1,8 @@ class PasswordsController < Devise::PasswordsController + include Gitlab::CurrentSettings + before_action :resource_from_email, only: [:create] - before_action :prevent_ldap_reset, only: [:create] + before_action :check_password_authentication_available, only: [:create] before_action :throttle_reset, only: [:create] def edit @@ -25,7 +27,7 @@ class PasswordsController < Devise::PasswordsController def update super do |resource| - if resource.valid? && resource.require_password? + if resource.valid? && resource.require_password_creation? resource.update_attribute(:password_automatically_set, false) end end @@ -38,11 +40,11 @@ class PasswordsController < Devise::PasswordsController self.resource = resource_class.find_by_email(email) end - def prevent_ldap_reset - return unless resource && resource.ldap_user? + def check_password_authentication_available + return if current_application_settings.password_authentication_enabled? && (resource.nil? || resource.allow_password_authentication?) redirect_to after_sending_reset_password_instructions_path_for(resource_name), - alert: "Cannot reset password for LDAP user." + alert: "Password authentication is unavailable." end def throttle_reset diff --git a/app/controllers/profiles/passwords_controller.rb b/app/controllers/profiles/passwords_controller.rb index 10145bae0d3..c423761ab24 100644 --- a/app/controllers/profiles/passwords_controller.rb +++ b/app/controllers/profiles/passwords_controller.rb @@ -77,7 +77,7 @@ class Profiles::PasswordsController < Profiles::ApplicationController end def authorize_change_password! - return render_404 if @user.ldap_user? + render_404 unless @user.allow_password_authentication? end def user_params diff --git a/app/controllers/projects/merge_requests/creations_controller.rb b/app/controllers/projects/merge_requests/creations_controller.rb index da058da795e..f35d53896ba 100644 --- a/app/controllers/projects/merge_requests/creations_controller.rb +++ b/app/controllers/projects/merge_requests/creations_controller.rb @@ -107,7 +107,7 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap @target_project = @merge_request.target_project @source_project = @merge_request.source_project - @commits = @merge_request.compare_commits.reverse + @commits = @merge_request.commits @commit = @merge_request.diff_head_commit @note_counts = Note.where(commit_id: @commits.map(&:id)) diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index f39441a281e..e0e72170d1e 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -58,7 +58,7 @@ class SessionsController < Devise::SessionsController user = User.admins.last - return unless user && user.require_password? + return unless user && user.require_password_creation? Users::UpdateService.new(user).execute do |user| @token = user.generate_reset_token diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 45b10f61f33..29b88c60dab 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -1,7 +1,7 @@ module ApplicationSettingsHelper delegate :gravatar_enabled?, :signup_enabled?, - :signin_enabled?, + :password_authentication_enabled?, :akismet_enabled?, :koding_enabled?, to: :current_application_settings diff --git a/app/helpers/button_helper.rb b/app/helpers/button_helper.rb index ba84dbe4a7a..bf9ad95b7c2 100644 --- a/app/helpers/button_helper.rb +++ b/app/helpers/button_helper.rb @@ -50,12 +50,12 @@ module ButtonHelper def http_clone_button(project, placement = 'right', append_link: true) klass = 'http-selector' - klass << ' has-tooltip' if current_user.try(:require_password?) || current_user.try(:require_personal_access_token?) + klass << ' has-tooltip' if current_user.try(:require_password_creation?) || current_user.try(:require_personal_access_token_creation_for_git_auth?) protocol = gitlab_config.protocol.upcase tooltip_title = - if current_user.try(:require_password?) + if current_user.try(:require_password_creation?) _("Set a password on your account to pull or push via %{protocol}.") % { protocol: protocol } else _("Create a personal access token on your account to pull or push via %{protocol}.") % { protocol: protocol } diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 25969adb649..26d11f9ab46 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -214,11 +214,11 @@ module ProjectsHelper def show_no_password_message? cookies[:hide_no_password_message].blank? && !current_user.hide_no_password && - ( current_user.require_password? || current_user.require_personal_access_token? ) + ( current_user.require_password_creation? || current_user.require_personal_access_token_creation_for_git_auth? ) end def link_to_set_password - if current_user.require_password? + if current_user.require_password_creation? link_to s_('SetPasswordToCloneLink|set a password'), edit_profile_password_path else link_to s_('CreateTokenToCloneLink|create a personal access token'), profile_personal_access_tokens_path @@ -518,4 +518,12 @@ module ProjectsHelper current_application_settings.restricted_visibility_levels || [] end + + def find_file_path + return unless @project && !@project.empty_repo? + + ref = @ref || @project.repository.root_ref + + project_find_file_path(@project, ref) + end end diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 98e3906a932..898ce45f60e 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -237,6 +237,7 @@ class ApplicationSetting < ActiveRecord::Base koding_url: nil, max_artifacts_size: Settings.artifacts['max_size'], max_attachment_size: Settings.gitlab['max_attachment_size'], + password_authentication_enabled: Settings.gitlab['password_authentication_enabled'], performance_bar_allowed_group_id: nil, plantuml_enabled: false, plantuml_url: nil, @@ -251,7 +252,6 @@ class ApplicationSetting < ActiveRecord::Base shared_runners_text: nil, sidekiq_throttling_enabled: false, sign_in_text: nil, - signin_enabled: Settings.gitlab['signin_enabled'], signup_enabled: Settings.gitlab['signup_enabled'], terminal_max_session_time: 0, two_factor_grace_period: 48, diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 815c5b43406..e4e7999d0f2 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -32,9 +32,6 @@ class MergeRequest < ActiveRecord::Base after_create :ensure_merge_request_diff, unless: :importing? after_update :reload_diff_if_branch_changed - delegate :commits, :real_size, :commit_shas, :commits_count, - to: :merge_request_diff, prefix: nil - # When this attribute is true some MR validation is ignored # It allows us to close or modify broken merge requests attr_accessor :allow_broken @@ -224,6 +221,36 @@ class MergeRequest < ActiveRecord::Base "#{project.to_reference(from, full: full)}#{reference}" end + def commits + if persisted? + merge_request_diff.commits + elsif compare_commits + compare_commits.reverse + else + [] + end + end + + def commits_count + if persisted? + merge_request_diff.commits_count + elsif compare_commits + compare_commits.size + else + 0 + end + end + + def commit_shas + if persisted? + merge_request_diff.commit_shas + elsif compare_commits + compare_commits.reverse.map(&:sha) + else + [] + end + end + def first_commit merge_request_diff ? merge_request_diff.first_commit : compare_commits.first end @@ -246,9 +273,7 @@ class MergeRequest < ActiveRecord::Base def diff_size # Calling `merge_request_diff.diffs.real_size` will also perform # highlighting, which we don't need here. - return real_size if merge_request_diff - - diffs.real_size + merge_request_diff&.real_size || diffs.real_size end def diff_base_commit diff --git a/app/models/repository.rb b/app/models/repository.rb index 10b429c707e..8663cf5e602 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -123,6 +123,7 @@ class Repository commits end + # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/384 def find_commits_by_message(query, ref = nil, path = nil, limit = 1000, offset = 0) unless exists? && has_visible_content? && query.present? return [] @@ -610,6 +611,7 @@ class Repository commit(sha) end + # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/383 def last_commit_id_for_path(sha, path) key = path.blank? ? "last_commit_id_for_path:#{sha}" : "last_commit_id_for_path:#{sha}:#{Digest::SHA1.hexdigest(path)}" diff --git a/app/models/user.rb b/app/models/user.rb index 2d39b1c1c34..dd1a391773a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -580,16 +580,20 @@ class User < ActiveRecord::Base keys.count == 0 && Gitlab::ProtocolAccess.allowed?('ssh') end - def require_password? - password_automatically_set? && !ldap_user? && current_application_settings.signin_enabled? + def require_password_creation? + password_automatically_set? && allow_password_authentication? end - def require_personal_access_token? - return false if current_application_settings.signin_enabled? || ldap_user? + def require_personal_access_token_creation_for_git_auth? + return false if allow_password_authentication? || ldap_user? PersonalAccessTokensFinder.new(user: self, impersonation: false, state: 'active').execute.none? end + def allow_password_authentication? + !ldap_user? && current_application_settings.password_authentication_enabled? + end + def can_change_username? gitlab_config.username_changing_enabled end diff --git a/app/services/git_operation_service.rb b/app/services/git_operation_service.rb index 43636fde0be..32925e9c1f2 100644 --- a/app/services/git_operation_service.rb +++ b/app/services/git_operation_service.rb @@ -129,6 +129,7 @@ class GitOperationService end end + # Gitaly note: JV: wait with migrating #update_ref until we know how to migrate its call sites. def update_ref(ref, newrev, oldrev) # We use 'git update-ref' because libgit2/rugged currently does not # offer 'compare and swap' ref updates. Without compare-and-swap we can diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index 7f1e13c7989..26f7c1a473a 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -145,9 +145,9 @@ .form-group .col-sm-offset-2.col-sm-10 .checkbox - = f.label :signin_enabled do - = f.check_box :signin_enabled - Sign-in enabled + = f.label :password_authentication_enabled do + = f.check_box :password_authentication_enabled + Password authentication enabled - if omniauth_enabled? && button_based_providers.any? .form-group = f.label :enabled_oauth_sign_in_sources, 'Enabled OAuth sign-in sources', class: 'control-label col-sm-2' diff --git a/app/views/devise/sessions/new.html.haml b/app/views/devise/sessions/new.html.haml index af87129e49e..dd61dcf2a7b 100644 --- a/app/views/devise/sessions/new.html.haml +++ b/app/views/devise/sessions/new.html.haml @@ -6,15 +6,15 @@ - else = render 'devise/shared/tabs_normal' .tab-content - - if signin_enabled? || ldap_enabled? || crowd_enabled? + - if password_authentication_enabled? || ldap_enabled? || crowd_enabled? = render 'devise/shared/signin_box' -# Signup only makes sense if you can also sign-in - - if signin_enabled? && signup_enabled? + - if password_authentication_enabled? && signup_enabled? = render 'devise/shared/signup_box' -# Show a message if none of the mechanisms above are enabled - - if !signin_enabled? && !ldap_enabled? && !(omniauth_enabled? && devise_mapping.omniauthable?) + - if !password_authentication_enabled? && !ldap_enabled? && !(omniauth_enabled? && devise_mapping.omniauthable?) %div No authentication methods configured. diff --git a/app/views/devise/shared/_signin_box.html.haml b/app/views/devise/shared/_signin_box.html.haml index da4769e214e..3b06008febe 100644 --- a/app/views/devise/shared/_signin_box.html.haml +++ b/app/views/devise/shared/_signin_box.html.haml @@ -7,12 +7,12 @@ .login-box.tab-pane{ id: "#{server['provider_name']}", role: 'tabpanel', class: active_when(i.zero? && !crowd_enabled?) } .login-body = render 'devise/sessions/new_ldap', server: server - - if signin_enabled? + - if password_authentication_enabled? .login-box.tab-pane{ id: 'ldap-standard', role: 'tabpanel' } .login-body = render 'devise/sessions/new_base' -- elsif signin_enabled? +- elsif password_authentication_enabled? .login-box.tab-pane.active{ id: 'login-pane', role: 'tabpanel' } .login-body = render 'devise/sessions/new_base' diff --git a/app/views/devise/shared/_tabs_ldap.html.haml b/app/views/devise/shared/_tabs_ldap.html.haml index dd34600490e..6d0243a325d 100644 --- a/app/views/devise/shared/_tabs_ldap.html.haml +++ b/app/views/devise/shared/_tabs_ldap.html.haml @@ -5,9 +5,9 @@ - @ldap_servers.each_with_index do |server, i| %li{ class: active_when(i.zero? && !crowd_enabled?) } = link_to server['label'], "##{server['provider_name']}", 'data-toggle' => 'tab' - - if signin_enabled? + - if password_authentication_enabled? %li = link_to 'Standard', '#ldap-standard', 'data-toggle' => 'tab' - - if signin_enabled? && signup_enabled? + - if password_authentication_enabled? && signup_enabled? %li = link_to 'Register', '#register-pane', 'data-toggle' => 'tab' diff --git a/app/views/devise/shared/_tabs_normal.html.haml b/app/views/devise/shared/_tabs_normal.html.haml index c225d800a98..212856c0676 100644 --- a/app/views/devise/shared/_tabs_normal.html.haml +++ b/app/views/devise/shared/_tabs_normal.html.haml @@ -1,6 +1,6 @@ %ul.nav-links.new-session-tabs.nav-tabs{ role: 'tablist' } %li.active{ role: 'presentation' } %a{ href: '#login-pane', data: { toggle: 'tab' }, role: 'tab' } Sign in - - if signin_enabled? && signup_enabled? + - if password_authentication_enabled? && signup_enabled? %li{ role: 'presentation' } %a{ href: '#register-pane', data: { toggle: 'tab' }, role: 'tab' } Register diff --git a/app/views/layouts/_broadcast.html.haml b/app/views/layouts/_broadcast.html.haml index bcd2f03e83c..e2dbdcbb939 100644 --- a/app/views/layouts/_broadcast.html.haml +++ b/app/views/layouts/_broadcast.html.haml @@ -1,2 +1,2 @@ -- BroadcastMessage.current.each do |message| +- BroadcastMessage.current&.each do |message| = broadcast_message(message) diff --git a/app/views/layouts/application.html.haml b/app/views/layouts/application.html.haml index 81f83b74826..38b95d11fd4 100644 --- a/app/views/layouts/application.html.haml +++ b/app/views/layouts/application.html.haml @@ -1,7 +1,7 @@ !!! 5 %html{ lang: I18n.locale, class: "#{page_class}" } = render "layouts/head" - %body{ class: @body_class, data: { page: body_data_page, project: "#{@project.path if @project}", group: "#{@group.path if @group}" } } + %body{ class: @body_class, data: { page: body_data_page, project: "#{@project.path if @project}", group: "#{@group.path if @group}", find_file: find_file_path } } = render "layouts/init_auto_complete" if @gfm_form - if show_new_nav? = render "layouts/header/new" diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index 25acdb700ef..bc3293fd100 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -91,8 +91,3 @@ = yield :header_content = render 'shared/outdated_browser' - -- if @project && !@project.empty_repo? - - if ref = @ref || @project.repository.root_ref - :javascript - var findFileURL = "#{project_find_file_path(@project, ref)}"; diff --git a/app/views/layouts/header/_new.html.haml b/app/views/layouts/header/_new.html.haml index 5bc15d68631..4697d91724b 100644 --- a/app/views/layouts/header/_new.html.haml +++ b/app/views/layouts/header/_new.html.haml @@ -84,8 +84,3 @@ = icon('times', class: 'js-navbar-toggle-left', style: 'display: none;') = render 'shared/outdated_browser' - -- if @project && !@project.empty_repo? - - if ref = @ref || @project.repository.root_ref - :javascript - var findFileURL = "#{project_find_file_path(@project, ref)}"; diff --git a/app/views/layouts/nav/_profile.html.haml b/app/views/layouts/nav/_profile.html.haml index ae1e1361f0f..424905ea890 100644 --- a/app/views/layouts/nav/_profile.html.haml +++ b/app/views/layouts/nav/_profile.html.haml @@ -29,7 +29,7 @@ = link_to profile_emails_path, title: 'Emails' do %span Emails - - unless current_user.ldap_user? + - if current_user.allow_password_authentication? = nav_link(controller: :passwords) do = link_to edit_profile_password_path, title: 'Password' do %span diff --git a/app/views/projects/jobs/_sidebar.html.haml b/app/views/projects/jobs/_sidebar.html.haml index bddb587ddc6..f2db71e8838 100644 --- a/app/views/projects/jobs/_sidebar.html.haml +++ b/app/views/projects/jobs/_sidebar.html.haml @@ -11,7 +11,7 @@ #js-details-block-vue - if can?(current_user, :read_build, @project) && (@build.artifacts? || @build.artifacts_expired?) - .block{ class: ("block-first" if !@build.coverage) } + .block .title Job artifacts - if @build.artifacts_expired? @@ -37,7 +37,7 @@ Browse - if @build.trigger_request - .build-widget + .build-widget.block %h4.title Trigger @@ -55,7 +55,7 @@ .js-build-variable.trigger-build-variable= key .js-build-value.trigger-build-value= value - .block + %div{ class: (@build.pipeline.stages_count > 1 ? "block" : "block-last") } %p Commit = link_to @build.pipeline.short_sha, project_commit_path(@project, @build.pipeline.sha), class: 'commit-sha link-commit' @@ -69,7 +69,7 @@ - if @build.pipeline.stages_count > 1 .dropdown.build-dropdown - .title + %div %span{ class: "ci-status-icon-#{@build.pipeline.status}" } = ci_icon_for_status(@build.pipeline.status) Pipeline diff --git a/app/views/projects/protected_branches/_branches_list.html.haml b/app/views/projects/protected_branches/_branches_list.html.haml index cf0db943865..5377d745371 100644 --- a/app/views/projects/protected_branches/_branches_list.html.haml +++ b/app/views/projects/protected_branches/_branches_list.html.haml @@ -1,28 +1,4 @@ -.panel.panel-default.protected-branches-list - - if @protected_branches.empty? - .panel-heading - %h3.panel-title - Protected branch (#{@protected_branches.size}) - %p.settings-message.text-center - There are currently no protected branches, protect a branch with the form above. - - else - - can_admin_project = can?(current_user, :admin_project, @project) +- can_admin_project = can?(current_user, :admin_project, @project) - %table.table.table-bordered - %colgroup - %col{ width: "25%" } - %col{ width: "30%" } - %col{ width: "25%" } - %col{ width: "20%" } - %thead - %tr - %th Protected branch (#{@protected_branches.size}) - %th Last commit - %th Allowed to merge - %th Allowed to push - - if can_admin_project - %th - %tbody - = render partial: 'projects/protected_branches/protected_branch', collection: @protected_branches, locals: { can_admin_project: can_admin_project} - - = paginate @protected_branches, theme: 'gitlab' += render layout: 'projects/protected_branches/shared/branches_list', locals: { can_admin_project: can_admin_project } do + = render partial: 'projects/protected_branches/protected_branch', collection: @protected_branches, locals: { can_admin_project: can_admin_project} diff --git a/app/views/projects/protected_branches/_create_protected_branch.html.haml b/app/views/projects/protected_branches/_create_protected_branch.html.haml index 99bc2516366..98d56a3e5c5 100644 --- a/app/views/projects/protected_branches/_create_protected_branch.html.haml +++ b/app/views/projects/protected_branches/_create_protected_branch.html.haml @@ -1,41 +1,14 @@ -= form_for [@project.namespace.becomes(Namespace), @project, @protected_branch] do |f| - .panel.panel-default - .panel-heading - %h3.panel-title - Protect a branch - .panel-body - .form-horizontal - = form_errors(@protected_branch) - .form-group - = f.label :name, class: 'col-md-2 text-right' do - Branch: - .col-md-10 - = render partial: "projects/protected_branches/dropdown", locals: { f: f } - .help-block - = link_to 'Wildcards', help_page_path('user/project/protected_branches', anchor: 'wildcard-protected-branches') - such as - %code *-stable - or - %code production/* - are supported - .form-group - %label.col-md-2.text-right{ for: 'merge_access_levels_attributes' } - Allowed to merge: - .col-md-10 - .merge_access_levels-container - = dropdown_tag('Select', - options: { toggle_class: 'js-allowed-to-merge wide', - dropdown_class: 'dropdown-menu-selectable capitalize-header', - data: { field_name: 'protected_branch[merge_access_levels_attributes][0][access_level]', input_id: 'merge_access_levels_attributes' }}) - .form-group - %label.col-md-2.text-right{ for: 'push_access_levels_attributes' } - Allowed to push: - .col-md-10 - .push_access_levels-container - = dropdown_tag('Select', - options: { toggle_class: 'js-allowed-to-push wide', - dropdown_class: 'dropdown-menu-selectable capitalize-header', - data: { field_name: 'protected_branch[push_access_levels_attributes][0][access_level]', input_id: 'push_access_levels_attributes' }}) +- content_for :merge_access_levels do + .merge_access_levels-container + = dropdown_tag('Select', + options: { toggle_class: 'js-allowed-to-merge wide', + dropdown_class: 'dropdown-menu-selectable capitalize-header', + data: { field_name: 'protected_branch[merge_access_levels_attributes][0][access_level]', input_id: 'merge_access_levels_attributes' }}) +- content_for :push_access_levels do + .push_access_levels-container + = dropdown_tag('Select', + options: { toggle_class: 'js-allowed-to-push wide', + dropdown_class: 'dropdown-menu-selectable capitalize-header', + data: { field_name: 'protected_branch[push_access_levels_attributes][0][access_level]', input_id: 'push_access_levels_attributes' }}) - .panel-footer - = f.submit 'Protect', class: 'btn-create btn', disabled: true += render 'projects/protected_branches/shared/create_protected_branch' diff --git a/app/views/projects/protected_branches/_index.html.haml b/app/views/projects/protected_branches/_index.html.haml index 5d2422bdf54..2f30fe33a97 100644 --- a/app/views/projects/protected_branches/_index.html.haml +++ b/app/views/projects/protected_branches/_index.html.haml @@ -1,26 +1,10 @@ -- expanded = Rails.env.test? - content_for :page_specific_javascripts do = page_specific_javascript_bundle_tag('protected_branches') -%section.settings - .settings-header - %h4 - Protected Branches - %button.btn.js-settings-toggle - = expanded ? 'Collapse' : 'Expand' - %p - Keep stable branches secure and force developers to use merge requests. - .settings-content.no-animate{ class: ('expanded' if expanded) } - %p - By default, protected branches are designed to: - %ul - %li prevent their creation, if not already created, from everybody except Masters - %li prevent pushes from everybody except Masters - %li prevent <strong>anyone</strong> from force pushing to the branch - %li prevent <strong>anyone</strong> from deleting the branch - %p Read more about #{link_to "protected branches", help_page_path("user/project/protected_branches"), class: "underlined-link"} and #{link_to "project permissions", help_page_path("user/permissions"), class: "underlined-link"}. +- content_for :create_protected_branch do + = render 'projects/protected_branches/create_protected_branch' - - if can? current_user, :admin_project, @project - = render 'projects/protected_branches/create_protected_branch' +- content_for :branches_list do + = render "projects/protected_branches/branches_list" - = render "projects/protected_branches/branches_list" += render 'projects/protected_branches/shared/index' diff --git a/app/views/projects/protected_branches/_protected_branch.html.haml b/app/views/projects/protected_branches/_protected_branch.html.haml index e4dadc42cc0..b12ae995ece 100644 --- a/app/views/projects/protected_branches/_protected_branch.html.haml +++ b/app/views/projects/protected_branches/_protected_branch.html.haml @@ -1,22 +1,2 @@ -%tr.js-protected-branch-edit-form{ data: { url: project_protected_branch_path(@project, protected_branch) } } - %td - %span.ref-name= protected_branch.name - - - if @project.root_ref?(protected_branch.name) - %span.label.label-info.prepend-left-5 default - %td - - if protected_branch.wildcard? - - matching_branches = protected_branch.matching(repository.branches) - = link_to pluralize(matching_branches.count, "matching branch"), project_protected_branch_path(@project, protected_branch) - - else - - if commit = protected_branch.commit - = link_to(commit.short_id, project_commit_path(@project, commit.id), class: 'commit-sha') - = time_ago_with_tooltip(commit.committed_date) - - else - (branch was removed from repository) - += render layout: 'projects/protected_branches/shared/protected_branch', locals: { protected_branch: protected_branch } do = render partial: 'projects/protected_branches/update_protected_branch', locals: { protected_branch: protected_branch } - - - if can_admin_project - %td - = link_to 'Unprotect', [@project.namespace.becomes(Namespace), @project, protected_branch], data: { confirm: 'Branch will be writable for developers. Are you sure?' }, method: :delete, class: 'btn btn-warning' diff --git a/app/views/projects/protected_branches/shared/_branches_list.html.haml b/app/views/projects/protected_branches/shared/_branches_list.html.haml new file mode 100644 index 00000000000..5c00bb6883c --- /dev/null +++ b/app/views/projects/protected_branches/shared/_branches_list.html.haml @@ -0,0 +1,28 @@ +.panel.panel-default.protected-branches-list + - if @protected_branches.empty? + .panel-heading + %h3.panel-title + Protected branch (#{@protected_branches.size}) + %p.settings-message.text-center + There are currently no protected branches, protect a branch with the form above. + - else + %table.table.table-bordered + %colgroup + %col{ width: "20%" } + %col{ width: "20%" } + %col{ width: "20%" } + %col{ width: "20%" } + - if can_admin_project + %col + %thead + %tr + %th Protected branch (#{@protected_branches.size}) + %th Last commit + %th Allowed to merge + %th Allowed to push + - if can_admin_project + %th + %tbody + = yield + + = paginate @protected_branches, theme: 'gitlab' diff --git a/app/views/projects/protected_branches/shared/_create_protected_branch.html.haml b/app/views/projects/protected_branches/shared/_create_protected_branch.html.haml new file mode 100644 index 00000000000..b619fa57e05 --- /dev/null +++ b/app/views/projects/protected_branches/shared/_create_protected_branch.html.haml @@ -0,0 +1,33 @@ += form_for [@project.namespace.becomes(Namespace), @project, @protected_branch] do |f| + .panel.panel-default + .panel-heading + %h3.panel-title + Protect a branch + .panel-body + .form-horizontal + = form_errors(@protected_branch) + .form-group + = f.label :name, class: 'col-md-2 text-right' do + Branch: + .col-md-10 + = render partial: "projects/protected_branches/shared/dropdown", locals: { f: f } + .help-block + = link_to 'Wildcards', help_page_path('user/project/protected_branches', anchor: 'wildcard-protected-branches') + such as + %code *-stable + or + %code production/* + are supported + .form-group + %label.col-md-2.text-right{ for: 'merge_access_levels_attributes' } + Allowed to merge: + .col-md-10 + = yield :merge_access_levels + .form-group + %label.col-md-2.text-right{ for: 'push_access_levels_attributes' } + Allowed to push: + .col-md-10 + = yield :push_access_levels + + .panel-footer + = f.submit 'Protect', class: 'btn-create btn', disabled: true diff --git a/app/views/projects/protected_branches/_dropdown.html.haml b/app/views/projects/protected_branches/shared/_dropdown.html.haml index 6e9c473494e..6e9c473494e 100644 --- a/app/views/projects/protected_branches/_dropdown.html.haml +++ b/app/views/projects/protected_branches/shared/_dropdown.html.haml diff --git a/app/views/projects/protected_branches/shared/_index.html.haml b/app/views/projects/protected_branches/shared/_index.html.haml new file mode 100644 index 00000000000..6a47cbdf724 --- /dev/null +++ b/app/views/projects/protected_branches/shared/_index.html.haml @@ -0,0 +1,24 @@ +- expanded = Rails.env.test? + +%section.settings + .settings-header + %h4 + Protected Branches + %button.btn.js-settings-toggle + = expanded ? 'Collapse' : 'Expand' + %p + Keep stable branches secure and force developers to use merge requests. + .settings-content.no-animate{ class: ('expanded' if expanded) } + %p + By default, protected branches are designed to: + %ul + %li prevent their creation, if not already created, from everybody except Masters + %li prevent pushes from everybody except Masters + %li prevent <strong>anyone</strong> from force pushing to the branch + %li prevent <strong>anyone</strong> from deleting the branch + %p Read more about #{link_to "protected branches", help_page_path("user/project/protected_branches"), class: "underlined-link"} and #{link_to "project permissions", help_page_path("user/permissions"), class: "underlined-link"}. + + - if can? current_user, :admin_project, @project + = content_for :create_protected_branch + + = content_for :branches_list diff --git a/app/views/projects/protected_branches/_matching_branch.html.haml b/app/views/projects/protected_branches/shared/_matching_branch.html.haml index 98793d632e6..98793d632e6 100644 --- a/app/views/projects/protected_branches/_matching_branch.html.haml +++ b/app/views/projects/protected_branches/shared/_matching_branch.html.haml diff --git a/app/views/projects/protected_branches/shared/_protected_branch.html.haml b/app/views/projects/protected_branches/shared/_protected_branch.html.haml new file mode 100644 index 00000000000..10b81e42572 --- /dev/null +++ b/app/views/projects/protected_branches/shared/_protected_branch.html.haml @@ -0,0 +1,24 @@ +- can_admin_project = can?(current_user, :admin_project, @project) + +%tr.js-protected-branch-edit-form{ data: { url: namespace_project_protected_branch_path(@project.namespace, @project, protected_branch) } } + %td + %span.ref-name= protected_branch.name + + - if @project.root_ref?(protected_branch.name) + %span.label.label-info.prepend-left-5 default + %td + - if protected_branch.wildcard? + - matching_branches = protected_branch.matching(repository.branches) + = link_to pluralize(matching_branches.count, "matching branch"), namespace_project_protected_branch_path(@project.namespace, @project, protected_branch) + - else + - if commit = protected_branch.commit + = link_to(commit.short_id, namespace_project_commit_path(@project.namespace, @project, commit.id), class: 'commit-sha') + = time_ago_with_tooltip(commit.committed_date) + - else + (branch was removed from repository) + + = yield + + - if can_admin_project + %td + = link_to 'Unprotect', [@project.namespace.becomes(Namespace), @project, protected_branch], data: { confirm: 'Branch will be writable for developers. Are you sure?' }, method: :delete, class: 'btn btn-warning' diff --git a/app/views/projects/protected_branches/show.html.haml b/app/views/projects/protected_branches/show.html.haml index a806a0756ec..1012ceefe93 100644 --- a/app/views/projects/protected_branches/show.html.haml +++ b/app/views/projects/protected_branches/show.html.haml @@ -19,7 +19,7 @@ %th Last commit %tbody - @matching_refs.each do |matching_branch| - = render partial: "matching_branch", object: matching_branch + = render partial: "projects/protected_branches/shared/matching_branch", object: matching_branch - else %p.settings-message.text-center Couldn't find any matching branches. diff --git a/app/views/projects/protected_tags/_create_protected_tag.html.haml b/app/views/projects/protected_tags/_create_protected_tag.html.haml index dd5b346d922..ea91e8af70e 100644 --- a/app/views/projects/protected_tags/_create_protected_tag.html.haml +++ b/app/views/projects/protected_tags/_create_protected_tag.html.haml @@ -1,32 +1,8 @@ -= form_for [@project.namespace.becomes(Namespace), @project, @protected_tag], html: { class: 'new-protected-tag js-new-protected-tag' } do |f| - .panel.panel-default - .panel-heading - %h3.panel-title - Protect a tag - .panel-body - .form-horizontal - = form_errors(@protected_tag) - .form-group - = f.label :name, class: 'col-md-2 text-right' do - Tag: - .col-md-10.protected-tags-dropdown - = render partial: "projects/protected_tags/dropdown", locals: { f: f } - .help-block - = link_to 'Wildcards', help_page_path('user/project/protected_tags', anchor: 'wildcard-protected-tags') - such as - %code v* - or - %code *-release - are supported - .form-group - %label.col-md-2.text-right{ for: 'create_access_levels_attributes' } - Allowed to create: - .col-md-10 - .create_access_levels-container - = dropdown_tag('Select', - options: { toggle_class: 'js-allowed-to-create wide', - dropdown_class: 'dropdown-menu-selectable', - data: { field_name: 'protected_tag[create_access_levels_attributes][0][access_level]', input_id: 'create_access_levels_attributes' }}) +- content_for :create_access_levels do + .create_access_levels-container + = dropdown_tag('Select', + options: { toggle_class: 'js-allowed-to-create wide', + dropdown_class: 'dropdown-menu-selectable', + data: { field_name: 'protected_tag[create_access_levels_attributes][0][access_level]', input_id: 'create_access_levels_attributes' }}) - .panel-footer - = f.submit 'Protect', class: 'btn-create btn', disabled: true += render 'projects/protected_tags/shared/create_protected_tag' diff --git a/app/views/projects/protected_tags/_index.html.haml b/app/views/projects/protected_tags/_index.html.haml index 8250f692a69..955220562a6 100644 --- a/app/views/projects/protected_tags/_index.html.haml +++ b/app/views/projects/protected_tags/_index.html.haml @@ -1,26 +1,10 @@ -- expanded = Rails.env.test? - content_for :page_specific_javascripts do = page_specific_javascript_bundle_tag('protected_tags') -%section.settings - .settings-header - %h4 - Protected Tags - %button.btn.js-settings-toggle - = expanded ? 'Collapse' : 'Expand' - %p - Limit access to creating and updating tags. - .settings-content.no-animate{ class: ('expanded' if expanded) } - %p - By default, protected tags are designed to: - %ul - %li Prevent tag creation by everybody except Masters - %li Prevent <strong>anyone</strong> from updating the tag - %li Prevent <strong>anyone</strong> from deleting the tag +- content_for :create_protected_tag do + = render 'projects/protected_tags/create_protected_tag' - %p Read more about #{link_to "protected tags", help_page_path("user/project/protected_tags"), class: "underlined-link"}. +- content_for :tag_list do + = render "projects/protected_tags/tags_list" - - if can? current_user, :admin_project, @project - = render 'projects/protected_tags/create_protected_tag' - - = render "projects/protected_tags/tags_list" += render 'projects/protected_tags/shared/index' diff --git a/app/views/projects/protected_tags/_protected_tag.html.haml b/app/views/projects/protected_tags/_protected_tag.html.haml index 5162da5e429..da1f97c8d6a 100644 --- a/app/views/projects/protected_tags/_protected_tag.html.haml +++ b/app/views/projects/protected_tags/_protected_tag.html.haml @@ -1,22 +1,2 @@ -%tr.js-protected-tag-edit-form{ data: { url: project_protected_tag_path(@project, protected_tag) } } - %td - %span.ref-name= protected_tag.name - - - if @project.root_ref?(protected_tag.name) - %span.label.label-info.prepend-left-5 default - %td - - if protected_tag.wildcard? - - matching_tags = protected_tag.matching(repository.tags) - = link_to pluralize(matching_tags.count, "matching tag"), project_protected_tag_path(@project, protected_tag) - - else - - if commit = protected_tag.commit - = link_to(commit.short_id, project_commit_path(@project, commit.id), class: 'commit-sha') - = time_ago_with_tooltip(commit.committed_date) - - else - (tag was removed from repository) - += render layout: 'projects/protected_tags/shared/protected_tag', locals: { protected_tag: protected_tag } do = render partial: 'projects/protected_tags/update_protected_tag', locals: { protected_tag: protected_tag } - - - if can_admin_project - %td - = link_to 'Unprotect', [@project.namespace.becomes(Namespace), @project, protected_tag], data: { confirm: 'Tag will be writable for developers. Are you sure?' }, method: :delete, class: 'btn btn-warning' diff --git a/app/views/projects/protected_tags/_tags_list.html.haml b/app/views/projects/protected_tags/_tags_list.html.haml index d432a5c9113..a6b18cc9f8f 100644 --- a/app/views/projects/protected_tags/_tags_list.html.haml +++ b/app/views/projects/protected_tags/_tags_list.html.haml @@ -1,30 +1,4 @@ -.panel.panel-default.protected-tags-list - - if @protected_tags.empty? - .panel-heading - %h3.panel-title - Protected tag (#{@protected_tags.size}) - %p.settings-message.text-center - There are currently no protected tags, protect a tag with the form above. - - else - - can_admin_project = can?(current_user, :admin_project, @project) +- can_admin_project = can?(current_user, :admin_project, @project) - %table.table.table-bordered - %colgroup - %col{ width: "25%" } - %col{ width: "25%" } - %col{ width: "50%" } - - if can_admin_project - %col - %thead - %tr - %th Protected tag (#{@protected_tags.size}) - %th Last commit - %th Allowed to create - - if can_admin_project - %th - %tbody - %tr - %td.flash-container{ colspan: 4 } - = render partial: 'projects/protected_tags/protected_tag', collection: @protected_tags, locals: { can_admin_project: can_admin_project} - - = paginate @protected_tags, theme: 'gitlab' += render layout: 'projects/protected_tags/shared/tags_list' do + = render partial: 'projects/protected_tags/protected_tag', collection: @protected_tags, locals: { can_admin_project: can_admin_project} diff --git a/app/views/projects/protected_tags/shared/_create_protected_tag.html.haml b/app/views/projects/protected_tags/shared/_create_protected_tag.html.haml new file mode 100644 index 00000000000..5a53c704fcb --- /dev/null +++ b/app/views/projects/protected_tags/shared/_create_protected_tag.html.haml @@ -0,0 +1,29 @@ += form_for [@project.namespace.becomes(Namespace), @project, @protected_tag], html: { class: 'new-protected-tag js-new-protected-tag' } do |f| + .panel.panel-default + .panel-heading + %h3.panel-title + Protect a tag + .panel-body + .form-horizontal + = form_errors(@protected_tag) + .form-group + = f.label :name, class: 'col-md-2 text-right' do + Tag: + .col-md-10.protected-tags-dropdown + = render partial: "projects/protected_tags/shared/dropdown", locals: { f: f } + .help-block + = link_to 'Wildcards', help_page_path('user/project/protected_tags', anchor: 'wildcard-protected-tags') + such as + %code v* + or + %code *-release + are supported + .form-group + %label.col-md-2.text-right{ for: 'create_access_levels_attributes' } + Allowed to create: + .col-md-10 + .create_access_levels-container + = yield :create_access_levels + + .panel-footer + = f.submit 'Protect', class: 'btn-create btn', disabled: true diff --git a/app/views/projects/protected_tags/_dropdown.html.haml b/app/views/projects/protected_tags/shared/_dropdown.html.haml index 9b6923210f7..9b6923210f7 100644 --- a/app/views/projects/protected_tags/_dropdown.html.haml +++ b/app/views/projects/protected_tags/shared/_dropdown.html.haml diff --git a/app/views/projects/protected_tags/shared/_index.html.haml b/app/views/projects/protected_tags/shared/_index.html.haml new file mode 100644 index 00000000000..c07bd454ff6 --- /dev/null +++ b/app/views/projects/protected_tags/shared/_index.html.haml @@ -0,0 +1,24 @@ +- expanded = Rails.env.test? + +%section.settings + .settings-header + %h4 + Protected Tags + %button.btn.js-settings-toggle + = expanded ? 'Collapse' : 'Expand' + %p + Limit access to creating and updating tags. + .settings-content.no-animate{ class: ('expanded' if expanded) } + %p + By default, protected tags are designed to: + %ul + %li Prevent tag creation by everybody except Masters + %li Prevent <strong>anyone</strong> from updating the tag + %li Prevent <strong>anyone</strong> from deleting the tag + + %p Read more about #{link_to "protected tags", help_page_path("user/project/protected_tags"), class: "underlined-link"}. + + - if can? current_user, :admin_project, @project + = yield :create_protected_tag + + = yield :tag_list diff --git a/app/views/projects/protected_tags/_matching_tag.html.haml b/app/views/projects/protected_tags/shared/_matching_tag.html.haml index 05f102d1ca3..05f102d1ca3 100644 --- a/app/views/projects/protected_tags/_matching_tag.html.haml +++ b/app/views/projects/protected_tags/shared/_matching_tag.html.haml diff --git a/app/views/projects/protected_tags/shared/_protected_tag.html.haml b/app/views/projects/protected_tags/shared/_protected_tag.html.haml new file mode 100644 index 00000000000..c778f7b9781 --- /dev/null +++ b/app/views/projects/protected_tags/shared/_protected_tag.html.haml @@ -0,0 +1,22 @@ +%tr.js-protected-tag-edit-form{ data: { url: project_protected_tag_path(@project, protected_tag) } } + %td + %span.ref-name= protected_tag.name + + - if @project.root_ref?(protected_tag.name) + %span.label.label-info.prepend-left-5 default + %td + - if protected_tag.wildcard? + - matching_tags = protected_tag.matching(repository.tags) + = link_to pluralize(matching_tags.count, "matching tag"), project_protected_tag_path(@project, protected_tag) + - else + - if commit = protected_tag.commit + = link_to(commit.short_id, project_commit_path(@project, commit.id), class: 'commit-sha') + = time_ago_with_tooltip(commit.committed_date) + - else + (tag was removed from repository) + + = yield + + - if can? current_user, :admin_project, @project + %td + = link_to 'Unprotect', [@project.namespace.becomes(Namespace), @project, protected_tag], data: { confirm: 'Tag will be writable for developers. Are you sure?' }, method: :delete, class: 'btn btn-warning' diff --git a/app/views/projects/protected_tags/shared/_tags_list.html.haml b/app/views/projects/protected_tags/shared/_tags_list.html.haml new file mode 100644 index 00000000000..6e3cd4ada71 --- /dev/null +++ b/app/views/projects/protected_tags/shared/_tags_list.html.haml @@ -0,0 +1,30 @@ +.panel.panel-default.protected-tags-list + - if @protected_tags.empty? + .panel-heading + %h3.panel-title + Protected tag (#{@protected_tags.size}) + %p.settings-message.text-center + There are currently no protected tags, protect a tag with the form above. + - else + - can_admin_project = can?(current_user, :admin_project, @project) + + %table.table.table-bordered + %colgroup + %col{ width: "25%" } + %col{ width: "25%" } + %col{ width: "50%" } + - if can_admin_project + %col + %thead + %tr + %th Protected tag (#{@protected_tags.size}) + %th Last commit + %th Allowed to create + - if can_admin_project + %th + %tbody + %tr + %td.flash-container{ colspan: 4 } + = yield + + = paginate @protected_tags, theme: 'gitlab' diff --git a/app/views/projects/protected_tags/show.html.haml b/app/views/projects/protected_tags/show.html.haml index 16fc02fe9f4..86629f1753b 100644 --- a/app/views/projects/protected_tags/show.html.haml +++ b/app/views/projects/protected_tags/show.html.haml @@ -19,7 +19,7 @@ %th Last commit %tbody - @matching_refs.each do |matching_tag| - = render partial: "matching_tag", object: matching_tag + = render partial: "projects/protected_tags/shared/matching_tag", object: matching_tag - else %p.settings-message.text-center Couldn't find any matching tags. diff --git a/app/views/projects/settings/repository/show.html.haml b/app/views/projects/settings/repository/show.html.haml index 40ea02abce9..0f20ecf8c69 100644 --- a/app/views/projects/settings/repository/show.html.haml +++ b/app/views/projects/settings/repository/show.html.haml @@ -6,6 +6,10 @@ = page_specific_javascript_bundle_tag('common_vue') = page_specific_javascript_bundle_tag('deploy_keys') +-# Protected branches & tags use a lot of nested partials. +-# The shared parts of the views can be found in the `shared` directory. +-# Those are used throughout the actual views. These `shared` views are then +-# reused in EE. = render "projects/protected_branches/index" = render "projects/protected_tags/index" = render @deploy_keys diff --git a/changelogs/unreleased/35035-sidebar-job-spaces.yml b/changelogs/unreleased/35035-sidebar-job-spaces.yml new file mode 100644 index 00000000000..a9a0211efd9 --- /dev/null +++ b/changelogs/unreleased/35035-sidebar-job-spaces.yml @@ -0,0 +1,4 @@ +--- +title: Fix vertical space in job details sidebar +merge_request: +author: diff --git a/changelogs/unreleased/fixes-for-internal-auth-disabled.yml b/changelogs/unreleased/fixes-for-internal-auth-disabled.yml new file mode 100644 index 00000000000..188d2770455 --- /dev/null +++ b/changelogs/unreleased/fixes-for-internal-auth-disabled.yml @@ -0,0 +1,4 @@ +--- +title: Fixes needed when GitLab sign-in is not enabled +merge_request: 12491 +author: Robin Bobbitt diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index eb4a1e390a9..ec7ce51b542 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -223,7 +223,7 @@ rescue ArgumentError # no user configured end Settings.gitlab['time_zone'] ||= nil Settings.gitlab['signup_enabled'] ||= true if Settings.gitlab['signup_enabled'].nil? -Settings.gitlab['signin_enabled'] ||= true if Settings.gitlab['signin_enabled'].nil? +Settings.gitlab['password_authentication_enabled'] ||= true if Settings.gitlab['password_authentication_enabled'].nil? Settings.gitlab['restricted_visibility_levels'] = Settings.__send__(:verify_constant_array, Gitlab::VisibilityLevel, Settings.gitlab['restricted_visibility_levels'], []) Settings.gitlab['username_changing_enabled'] = true if Settings.gitlab['username_changing_enabled'].nil? Settings.gitlab['issue_closing_pattern'] = '((?:[Cc]los(?:e[sd]?|ing)|[Ff]ix(?:e[sd]|ing)?|[Rr]esolv(?:e[sd]?|ing))(:?) +(?:(?:issues? +)?%{issue_ref}(?:(?:, *| +and +)?)|([A-Z][A-Z0-9_]+-\d+))+)' if Settings.gitlab['issue_closing_pattern'].nil? diff --git a/config/initializers/8_metrics.rb b/config/initializers/8_metrics.rb index 5e34aac6527..c80d28746d6 100644 --- a/config/initializers/8_metrics.rb +++ b/config/initializers/8_metrics.rb @@ -174,6 +174,10 @@ if Gitlab::Metrics.enabled? loc && loc[0].start_with?(models) && method.source =~ regex end end + + # Ability is in app/models, is not an ActiveRecord model, but should still + # be instrumented. + Gitlab::Metrics::Instrumentation.instrument_methods(Ability) end Gitlab::Metrics::Instrumentation.configure do |config| diff --git a/db/migrate/20170629171610_rename_application_settings_signin_enabled_to_password_authentication_enabled.rb b/db/migrate/20170629171610_rename_application_settings_signin_enabled_to_password_authentication_enabled.rb new file mode 100644 index 00000000000..858b3bebace --- /dev/null +++ b/db/migrate/20170629171610_rename_application_settings_signin_enabled_to_password_authentication_enabled.rb @@ -0,0 +1,15 @@ +class RenameApplicationSettingsSigninEnabledToPasswordAuthenticationEnabled < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + rename_column_concurrently :application_settings, :signin_enabled, :password_authentication_enabled + end + + def down + cleanup_concurrent_column_rename :application_settings, :password_authentication_enabled, :signin_enabled + end +end diff --git a/db/post_migrate/20170629180131_cleanup_application_settings_signin_enabled_rename.rb b/db/post_migrate/20170629180131_cleanup_application_settings_signin_enabled_rename.rb new file mode 100644 index 00000000000..52a773ddfee --- /dev/null +++ b/db/post_migrate/20170629180131_cleanup_application_settings_signin_enabled_rename.rb @@ -0,0 +1,15 @@ +class CleanupApplicationSettingsSigninEnabledRename < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + cleanup_concurrent_column_rename :application_settings, :signin_enabled, :password_authentication_enabled + end + + def down + rename_column_concurrently :application_settings, :password_authentication_enabled, :signin_enabled + end +end diff --git a/db/schema.rb b/db/schema.rb index 3dbe52c9c80..5264fc99557 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -41,7 +41,6 @@ ActiveRecord::Schema.define(version: 20170707184244) do create_table "application_settings", force: :cascade do |t| t.integer "default_projects_limit" t.boolean "signup_enabled" - t.boolean "signin_enabled" t.boolean "gravatar_enabled" t.text "sign_in_text" t.datetime "created_at" @@ -127,6 +126,7 @@ ActiveRecord::Schema.define(version: 20170707184244) do t.boolean "help_page_hide_commercial_content", default: false t.string "help_page_support_url" t.integer "performance_bar_allowed_group_id" + t.boolean "password_authentication_enabled" end create_table "audit_events", force: :cascade do |t| diff --git a/doc/api/settings.md b/doc/api/settings.md index eefbdda42ce..0b4cc98cea6 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -25,7 +25,7 @@ Example response: "id" : 1, "default_branch_protection" : 2, "restricted_visibility_levels" : [], - "signin_enabled" : true, + "password_authentication_enabled" : true, "after_sign_out_path" : null, "max_attachment_size" : 10, "user_oauth_applications" : true, @@ -63,7 +63,7 @@ PUT /application/settings | --------- | ---- | :------: | ----------- | | `default_projects_limit` | integer | no | Project limit per user. Default is `100000` | | `signup_enabled` | boolean | no | Enable registration. Default is `true`. | -| `signin_enabled` | boolean | no | Enable login via a GitLab account. Default is `true`. | +| `password_authentication_enabled` | boolean | no | Enable authentication via a GitLab account password. Default is `true`. | | `gravatar_enabled` | boolean | no | Enable Gravatar | | `sign_in_text` | string | no | Text on login page | | `home_page_url` | string | no | Redirect to this URL when not logged in | @@ -102,7 +102,7 @@ Example response: "id": 1, "default_projects_limit": 100000, "signup_enabled": true, - "signin_enabled": true, + "password_authentication_enabled": true, "gravatar_enabled": true, "sign_in_text": "", "created_at": "2015-06-12T15:51:55.432Z", diff --git a/doc/development/fe_guide/droplab/plugins/input_setter.md b/doc/development/fe_guide/droplab/plugins/input_setter.md index a549603c20d..94f3c8254a8 100644 --- a/doc/development/fe_guide/droplab/plugins/input_setter.md +++ b/doc/development/fe_guide/droplab/plugins/input_setter.md @@ -1,6 +1,6 @@ # InputSetter -`InputSetter` is a plugin that allows for udating DOM out of the scope of droplab when a list item is clicked. +`InputSetter` is a plugin that allows for updating DOM out of the scope of droplab when a list item is clicked. ## Usage diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 47742a164fc..09a88869063 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -621,7 +621,8 @@ module API expose :id expose :default_projects_limit expose :signup_enabled - expose :signin_enabled + expose :password_authentication_enabled + expose :password_authentication_enabled, as: :signin_enabled expose :gravatar_enabled expose :sign_in_text expose :after_sign_up_text diff --git a/lib/api/internal.rb b/lib/api/internal.rb index ef2c08e902c..465363da582 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -101,7 +101,7 @@ module API end get "/broadcast_message" do - if message = BroadcastMessage.current.last + if message = BroadcastMessage.current&.last present message, with: Entities::BroadcastMessage else {} diff --git a/lib/api/settings.rb b/lib/api/settings.rb index d598f9a62a2..b19095d1252 100644 --- a/lib/api/settings.rb +++ b/lib/api/settings.rb @@ -65,6 +65,7 @@ module API :shared_runners_enabled, :sidekiq_throttling_enabled, :sign_in_text, + :password_authentication_enabled, :signin_enabled, :signup_enabled, :terminal_max_session_time, @@ -95,7 +96,9 @@ module API requires :domain_blacklist, type: String, desc: 'Users with e-mail addresses that match these domain(s) will NOT be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com' end optional :after_sign_up_text, type: String, desc: 'Text shown after sign up' - optional :signin_enabled, type: Boolean, desc: 'Flag indicating if sign in is enabled' + optional :password_authentication_enabled, type: Boolean, desc: 'Flag indicating if password authentication is enabled' + optional :signin_enabled, type: Boolean, desc: 'Flag indicating if password authentication is enabled' + mutually_exclusive :password_authentication_enabled, :signin_enabled optional :require_two_factor_authentication, type: Boolean, desc: 'Require all users to setup Two-factor authentication' given require_two_factor_authentication: ->(val) { val } do requires :two_factor_grace_period, type: Integer, desc: 'Amount of time (in hours) that users are allowed to skip forced configuration of two-factor authentication' @@ -176,6 +179,10 @@ module API put "application/settings" do attrs = declared_params(include_missing: false) + if attrs.has_key?(:signin_enabled) + attrs[:password_authentication_enabled] = attrs.delete(:signin_enabled) + end + if current_settings.update_attributes(attrs) present current_settings, with: Entities::ApplicationSetting else diff --git a/lib/api/v3/entities.rb b/lib/api/v3/entities.rb index c848f52723b..3759250f7f6 100644 --- a/lib/api/v3/entities.rb +++ b/lib/api/v3/entities.rb @@ -161,7 +161,8 @@ module API expose :id expose :default_projects_limit expose :signup_enabled - expose :signin_enabled + expose :password_authentication_enabled + expose :password_authentication_enabled, as: :signin_enabled expose :gravatar_enabled expose :sign_in_text expose :after_sign_up_text diff --git a/lib/api/v3/settings.rb b/lib/api/v3/settings.rb index 748d6b97d4f..202011cfcbe 100644 --- a/lib/api/v3/settings.rb +++ b/lib/api/v3/settings.rb @@ -44,7 +44,9 @@ module API requires :domain_blacklist, type: String, desc: 'Users with e-mail addresses that match these domain(s) will NOT be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com' end optional :after_sign_up_text, type: String, desc: 'Text shown after sign up' - optional :signin_enabled, type: Boolean, desc: 'Flag indicating if sign in is enabled' + optional :password_authentication_enabled, type: Boolean, desc: 'Flag indicating if password authentication is enabled' + optional :signin_enabled, type: Boolean, desc: 'Flag indicating if password authentication is enabled' + mutually_exclusive :password_authentication_enabled, :signin_enabled optional :require_two_factor_authentication, type: Boolean, desc: 'Require all users to setup Two-factor authentication' given require_two_factor_authentication: ->(val) { val } do requires :two_factor_grace_period, type: Integer, desc: 'Amount of time (in hours) that users are allowed to skip forced configuration of two-factor authentication' @@ -116,7 +118,7 @@ module API :max_attachment_size, :session_expire_delay, :disabled_oauth_sign_in_sources, :user_oauth_applications, :user_default_external, :signup_enabled, :send_user_confirmation_email, :domain_whitelist, :domain_blacklist_enabled, - :after_sign_up_text, :signin_enabled, :require_two_factor_authentication, + :after_sign_up_text, :password_authentication_enabled, :signin_enabled, :require_two_factor_authentication, :home_page_url, :after_sign_out_path, :sign_in_text, :help_page_text, :shared_runners_enabled, :max_artifacts_size, :max_pages_size, :container_registry_token_expire_delay, :metrics_enabled, :sidekiq_throttling_enabled, :recaptcha_enabled, @@ -126,7 +128,13 @@ module API :housekeeping_enabled, :terminal_max_session_time end put "application/settings" do - if current_settings.update_attributes(declared_params(include_missing: false)) + attrs = declared_params(include_missing: false) + + if attrs.has_key?(:signin_enabled) + attrs[:password_authentication_enabled] = attrs.delete(:signin_enabled) + end + + if current_settings.update_attributes(attrs) present current_settings, with: Entities::ApplicationSetting else render_validation_error!(current_settings) diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index ccb5d886bab..9bed81e7327 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -37,7 +37,7 @@ module Gitlab rate_limit!(ip, success: result.success?, login: login) Gitlab::Auth::UniqueIpsLimiter.limit_user!(result.actor) - return result if result.success? || current_application_settings.signin_enabled? || Gitlab::LDAP::Config.enabled? + return result if result.success? || current_application_settings.password_authentication_enabled? || Gitlab::LDAP::Config.enabled? # If sign-in is disabled and LDAP is not configured, recommend a # personal access token on failed auth attempts @@ -48,6 +48,10 @@ module Gitlab # Avoid resource intensive login checks if password is not provided return unless password.present? + # Nothing to do here if internal auth is disabled and LDAP is + # not configured + return unless current_application_settings.password_authentication_enabled? || Gitlab::LDAP::Config.enabled? + Gitlab::Auth::UniqueIpsLimiter.limit_user! do user = User.by_login(login) diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index 8ab3b0498ff..d0f04d25db2 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -38,7 +38,7 @@ module Gitlab repo = options.delete(:repo) raise 'Gitlab::Git::Repository is required' unless repo.respond_to?(:log) - repo.log(options).map { |c| decorate(c) } + repo.log(options) end # Get single commit diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index bd9993f275d..639f5625d59 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -331,85 +331,10 @@ module Gitlab # ) # def log(options) - default_options = { - limit: 10, - offset: 0, - path: nil, - follow: false, - skip_merges: false, - disable_walk: false, - after: nil, - before: nil - } - - options = default_options.merge(options) - options[:limit] ||= 0 - options[:offset] ||= 0 - actual_ref = options[:ref] || root_ref - begin - sha = sha_from_ref(actual_ref) - rescue Rugged::OdbError, Rugged::InvalidError, Rugged::ReferenceError - # Return an empty array if the ref wasn't found - return [] - end - - if log_using_shell?(options) - log_by_shell(sha, options) - else - log_by_walk(sha, options) - end - end - - def log_using_shell?(options) - options[:path].present? || - options[:disable_walk] || - options[:skip_merges] || - options[:after] || - options[:before] - end - - def log_by_walk(sha, options) - walk_options = { - show: sha, - sort: Rugged::SORT_NONE, - limit: options[:limit], - offset: options[:offset] - } - Rugged::Walker.walk(rugged, walk_options).to_a - end - - def log_by_shell(sha, options) - limit = options[:limit].to_i - offset = options[:offset].to_i - use_follow_flag = options[:follow] && options[:path].present? - - # We will perform the offset in Ruby because --follow doesn't play well with --skip. - # See: https://gitlab.com/gitlab-org/gitlab-ce/issues/3574#note_3040520 - offset_in_ruby = use_follow_flag && options[:offset].present? - limit += offset if offset_in_ruby - - cmd = %W[#{Gitlab.config.git.bin_path} --git-dir=#{path} log] - cmd << "--max-count=#{limit}" - cmd << '--format=%H' - cmd << "--skip=#{offset}" unless offset_in_ruby - cmd << '--follow' if use_follow_flag - cmd << '--no-merges' if options[:skip_merges] - cmd << "--after=#{options[:after].iso8601}" if options[:after] - cmd << "--before=#{options[:before].iso8601}" if options[:before] - cmd << sha - - # :path can be a string or an array of strings - if options[:path].present? - cmd << '--' - cmd += Array(options[:path]) - end - - raw_output = IO.popen(cmd) { |io| io.read } - lines = offset_in_ruby ? raw_output.lines.drop(offset) : raw_output.lines - - lines.map! { |c| Rugged::Commit.new(rugged, c.strip) } + raw_log(options).map { |c| Commit.decorate(c) } end + # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/382 def count_commits(options) cmd = %W[#{Gitlab.config.git.bin_path} --git-dir=#{path} rev-list] cmd << "--after=#{options[:after].iso8601}" if options[:after] @@ -454,7 +379,7 @@ module Gitlab # Counts the amount of commits between `from` and `to`. def count_commits_between(from, to) - commits_between(from, to).size + Commit.between(self, from, to).size end # Returns the SHA of the most recent common ancestor of +from+ and +to+ @@ -912,6 +837,89 @@ module Gitlab private + def raw_log(options) + default_options = { + limit: 10, + offset: 0, + path: nil, + follow: false, + skip_merges: false, + disable_walk: false, + after: nil, + before: nil + } + + options = default_options.merge(options) + options[:limit] ||= 0 + options[:offset] ||= 0 + actual_ref = options[:ref] || root_ref + begin + sha = sha_from_ref(actual_ref) + rescue Rugged::OdbError, Rugged::InvalidError, Rugged::ReferenceError + # Return an empty array if the ref wasn't found + return [] + end + + if log_using_shell?(options) + log_by_shell(sha, options) + else + log_by_walk(sha, options) + end + end + + def log_using_shell?(options) + options[:path].present? || + options[:disable_walk] || + options[:skip_merges] || + options[:after] || + options[:before] + end + + def log_by_walk(sha, options) + walk_options = { + show: sha, + sort: Rugged::SORT_NONE, + limit: options[:limit], + offset: options[:offset] + } + Rugged::Walker.walk(rugged, walk_options).to_a + end + + # Gitaly note: JV: although #log_by_shell shells out to Git I think the + # complexity is such that we should migrate it as Ruby before trying to + # do it in Go. + def log_by_shell(sha, options) + limit = options[:limit].to_i + offset = options[:offset].to_i + use_follow_flag = options[:follow] && options[:path].present? + + # We will perform the offset in Ruby because --follow doesn't play well with --skip. + # See: https://gitlab.com/gitlab-org/gitlab-ce/issues/3574#note_3040520 + offset_in_ruby = use_follow_flag && options[:offset].present? + limit += offset if offset_in_ruby + + cmd = %W[#{Gitlab.config.git.bin_path} --git-dir=#{path} log] + cmd << "--max-count=#{limit}" + cmd << '--format=%H' + cmd << "--skip=#{offset}" unless offset_in_ruby + cmd << '--follow' if use_follow_flag + cmd << '--no-merges' if options[:skip_merges] + cmd << "--after=#{options[:after].iso8601}" if options[:after] + cmd << "--before=#{options[:before].iso8601}" if options[:before] + cmd << sha + + # :path can be a string or an array of strings + if options[:path].present? + cmd << '--' + cmd += Array(options[:path]) + end + + raw_output = IO.popen(cmd) { |io| io.read } + lines = offset_in_ruby ? raw_output.lines.drop(offset) : raw_output.lines + + lines.map! { |c| Rugged::Commit.new(rugged, c.strip) } + end + # We are trying to deprecate this method because it does a lot of work # but it seems to be used only to look up submodule URL's. # https://gitlab.com/gitlab-org/gitaly/issues/329 diff --git a/lib/gitlab/git_ref_validator.rb b/lib/gitlab/git_ref_validator.rb index 0e87ee30c98..a3c6b21a6a1 100644 --- a/lib/gitlab/git_ref_validator.rb +++ b/lib/gitlab/git_ref_validator.rb @@ -1,3 +1,5 @@ +# Gitaly note: JV: does not need to be migrated, works without a repo. + module Gitlab module GitRefValidator extend self diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb index 0baea092e6a..4366ff336ef 100644 --- a/lib/gitlab/shell.rb +++ b/lib/gitlab/shell.rb @@ -1,3 +1,6 @@ +# Gitaly note: JV: two sets of straightforward RPC's. 1 Hard RPC: fork_repository. +# SSH key operations are not part of Gitaly so will never be migrated. + require 'securerandom' module Gitlab @@ -68,6 +71,7 @@ module Gitlab # Ex. # add_repository("/path/to/storage", "gitlab/gitlab-ci") # + # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/387 def add_repository(storage, name) gitlab_shell_fast_execute([gitlab_shell_projects_path, 'add-project', storage, "#{name}.git"]) @@ -81,6 +85,7 @@ module Gitlab # Ex. # import_repository("/path/to/storage", "gitlab/gitlab-ci", "https://github.com/randx/six.git") # + # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/387 def import_repository(storage, name, url) # Timeout should be less than 900 ideally, to prevent the memory killer # to silently kill the process without knowing we are timing out here. @@ -99,6 +104,7 @@ module Gitlab # Ex. # fetch_remote("gitlab/gitlab-ci", "upstream") # + # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/387 def fetch_remote(storage, name, remote, forced: false, no_tags: false) args = [gitlab_shell_projects_path, 'fetch-remote', storage, "#{name}.git", remote, "#{Gitlab.config.gitlab_shell.git_timeout}"] args << '--force' if forced @@ -115,6 +121,7 @@ module Gitlab # Ex. # mv_repository("/path/to/storage", "gitlab/gitlab-ci", "randx/gitlab-ci-new") # + # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/387 def mv_repository(storage, path, new_path) gitlab_shell_fast_execute([gitlab_shell_projects_path, 'mv-project', storage, "#{path}.git", "#{new_path}.git"]) @@ -129,6 +136,7 @@ module Gitlab # Ex. # fork_repository("/path/to/forked_from/storage", "gitlab/gitlab-ci", "/path/to/forked_to/storage", "randx") # + # Gitaly note: JV: not easy to migrate because this involves two Gitaly servers, not one. def fork_repository(forked_from_storage, path, forked_to_storage, fork_namespace) gitlab_shell_fast_execute([gitlab_shell_projects_path, 'fork-project', forked_from_storage, "#{path}.git", forked_to_storage, @@ -143,6 +151,7 @@ module Gitlab # Ex. # remove_repository("/path/to/storage", "gitlab/gitlab-ci") # + # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/387 def remove_repository(storage, name) gitlab_shell_fast_execute([gitlab_shell_projects_path, 'rm-project', storage, "#{name}.git"]) @@ -194,6 +203,7 @@ module Gitlab # Ex. # add_namespace("/path/to/storage", "gitlab") # + # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/385 def add_namespace(storage, name) path = full_path(storage, name) FileUtils.mkdir_p(path, mode: 0770) unless exists?(storage, name) @@ -207,6 +217,7 @@ module Gitlab # Ex. # rm_namespace("/path/to/storage", "gitlab") # + # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/385 def rm_namespace(storage, name) FileUtils.rm_r(full_path(storage, name), force: true) end @@ -216,6 +227,7 @@ module Gitlab # Ex. # mv_namespace("/path/to/storage", "gitlab", "gitlabhq") # + # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/385 def mv_namespace(storage, old_name, new_name) return false if exists?(storage, new_name) || !exists?(storage, old_name) @@ -241,6 +253,7 @@ module Gitlab # exists?(storage, 'gitlab') # exists?(storage, 'gitlab/cookies.git') # + # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/385 def exists?(storage, dir_name) File.exist?(full_path(storage, dir_name)) end diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index a2720c9b81e..1641bddea11 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -30,6 +30,15 @@ describe ApplicationController do expect(controller).not_to receive(:redirect_to) controller.send(:check_password_expiration) end + + it 'does not redirect if the user is over their password expiry but sign-in is disabled' do + stub_application_setting(password_authentication_enabled: false) + user.password_expires_at = Time.new(2002) + allow(controller).to receive(:current_user).and_return(user) + expect(controller).not_to receive(:redirect_to) + + controller.send(:check_password_expiration) + end end describe "#authenticate_user_from_token!" do diff --git a/spec/controllers/passwords_controller_spec.rb b/spec/controllers/passwords_controller_spec.rb new file mode 100644 index 00000000000..2955d01fad0 --- /dev/null +++ b/spec/controllers/passwords_controller_spec.rb @@ -0,0 +1,29 @@ +require 'spec_helper' + +describe PasswordsController do + describe '#check_password_authentication_available' do + before do + @request.env["devise.mapping"] = Devise.mappings[:user] + end + + context 'when password authentication is disabled' do + it 'prevents a password reset' do + stub_application_setting(password_authentication_enabled: false) + + post :create + + expect(flash[:alert]).to eq 'Password authentication is unavailable.' + end + end + + context 'when reset email belongs to an ldap user' do + let(:user) { create(:omniauth_user, provider: 'ldapmain', email: 'ldapuser@gitlab.com') } + + it 'prevents a password reset' do + post :create, user: { email: user.email } + + expect(flash[:alert]).to eq 'Password authentication is unavailable.' + end + end + end +end diff --git a/spec/features/profiles/password_spec.rb b/spec/features/profiles/password_spec.rb index 67975a68ee2..26d6d6658aa 100644 --- a/spec/features/profiles/password_spec.rb +++ b/spec/features/profiles/password_spec.rb @@ -1,44 +1,74 @@ require 'spec_helper' describe 'Profile > Password', feature: true do - let(:user) { create(:user, password_automatically_set: true) } + context 'Password authentication enabled' do + let(:user) { create(:user, password_automatically_set: true) } - before do - sign_in(user) - visit edit_profile_password_path - end + before do + sign_in(user) + visit edit_profile_password_path + end - def fill_passwords(password, confirmation) - fill_in 'New password', with: password - fill_in 'Password confirmation', with: confirmation + def fill_passwords(password, confirmation) + fill_in 'New password', with: password + fill_in 'Password confirmation', with: confirmation - click_button 'Save password' - end + click_button 'Save password' + end + + context 'User with password automatically set' do + describe 'User puts different passwords in the field and in the confirmation' do + it 'shows an error message' do + fill_passwords('mypassword', 'mypassword2') - context 'User with password automatically set' do - describe 'User puts different passwords in the field and in the confirmation' do - it 'shows an error message' do - fill_passwords('mypassword', 'mypassword2') + page.within('.alert-danger') do + expect(page).to have_content("Password confirmation doesn't match Password") + end + end + + it 'does not contain the current password field after an error' do + fill_passwords('mypassword', 'mypassword2') - page.within('.alert-danger') do - expect(page).to have_content("Password confirmation doesn't match Password") + expect(page).to have_no_field('user[current_password]') end end - it 'does not contain the current password field after an error' do - fill_passwords('mypassword', 'mypassword2') + describe 'User puts the same passwords in the field and in the confirmation' do + it 'shows a success message' do + fill_passwords('mypassword', 'mypassword') - expect(page).to have_no_field('user[current_password]') + page.within('.flash-notice') do + expect(page).to have_content('Password was successfully updated. Please login with it') + end + end end end + end - describe 'User puts the same passwords in the field and in the confirmation' do - it 'shows a success message' do - fill_passwords('mypassword', 'mypassword') + context 'Password authentication unavailable' do + before do + gitlab_sign_in(user) + end - page.within('.flash-notice') do - expect(page).to have_content('Password was successfully updated. Please login with it') - end + context 'Regular user' do + let(:user) { create(:user) } + + it 'renders 404 when sign-in is disabled' do + stub_application_setting(password_authentication_enabled: false) + + visit edit_profile_password_path + + expect(page).to have_http_status(404) + end + end + + context 'LDAP user' do + let(:user) { create(:omniauth_user, provider: 'ldapmain') } + + it 'renders 404' do + visit edit_profile_password_path + + expect(page).to have_http_status(404) end end end diff --git a/spec/features/projects/no_password_spec.rb b/spec/features/projects/no_password_spec.rb index 53ac18fa7cc..d22a6daac08 100644 --- a/spec/features/projects/no_password_spec.rb +++ b/spec/features/projects/no_password_spec.rb @@ -30,7 +30,7 @@ feature 'No Password Alert' do let(:user) { create(:omniauth_user, extern_uid: 'my-uid', provider: 'saml') } before do - stub_application_setting(signin_enabled?: false) + stub_application_setting(password_authentication_enabled?: false) stub_omniauth_saml_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], providers: [mock_saml_config]) end diff --git a/spec/helpers/button_helper_spec.rb b/spec/helpers/button_helper_spec.rb index 661327d4432..7ecb75da8ce 100644 --- a/spec/helpers/button_helper_spec.rb +++ b/spec/helpers/button_helper_spec.rb @@ -35,7 +35,7 @@ describe ButtonHelper do context 'with internal auth disabled' do before do - stub_application_setting(signin_enabled?: false) + stub_application_setting(password_authentication_enabled?: false) end context 'when user has no personal access tokens' do diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index c462d9006ea..45066a60f50 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -160,7 +160,7 @@ describe ProjectsHelper do context 'user requires a personal access token' do it 'returns true' do - stub_application_setting(signin_enabled?: false) + stub_application_setting(password_authentication_enabled?: false) expect(helper.show_no_password_message?).to be_truthy end @@ -184,7 +184,7 @@ describe ProjectsHelper do let(:user) { create(:user) } it 'returns link to create a personal access token' do - stub_application_setting(signin_enabled?: false) + stub_application_setting(password_authentication_enabled?: false) expect(helper.link_to_set_password).to match %r{<a href="#{profile_personal_access_tokens_path}">create a personal access token</a>} end diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index d09da951869..55780518230 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -206,7 +206,7 @@ describe Gitlab::Auth, lib: true do end it 'throws an error suggesting user create a PAT when internal auth is disabled' do - allow_any_instance_of(ApplicationSetting).to receive(:signin_enabled?) { false } + allow_any_instance_of(ApplicationSetting).to receive(:password_authentication_enabled?) { false } expect { gl_auth.find_for_git_client('foo', 'bar', project: nil, ip: 'ip') }.to raise_error(Gitlab::Auth::MissingPersonalTokenError) end @@ -279,6 +279,16 @@ describe Gitlab::Auth, lib: true do gl_auth.find_with_user_password('ldap_user', 'password') end end + + context "with sign-in disabled" do + before do + stub_application_setting(password_authentication_enabled: false) + end + + it "does not find user by valid login/password" do + expect(gl_auth.find_with_user_password(username, password)).to be_nil + end + end end private diff --git a/spec/lib/gitlab/fake_application_settings_spec.rb b/spec/lib/gitlab/fake_application_settings_spec.rb index b793176d84a..34322c2a693 100644 --- a/spec/lib/gitlab/fake_application_settings_spec.rb +++ b/spec/lib/gitlab/fake_application_settings_spec.rb @@ -1,25 +1,25 @@ require 'spec_helper' describe Gitlab::FakeApplicationSettings do - let(:defaults) { { signin_enabled: false, foobar: 'asdf', signup_enabled: true, 'test?' => 123 } } + let(:defaults) { { password_authentication_enabled: false, foobar: 'asdf', signup_enabled: true, 'test?' => 123 } } subject { described_class.new(defaults) } it 'wraps OpenStruct variables properly' do - expect(subject.signin_enabled).to be_falsey + expect(subject.password_authentication_enabled).to be_falsey expect(subject.signup_enabled).to be_truthy expect(subject.foobar).to eq('asdf') end it 'defines predicate methods' do - expect(subject.signin_enabled?).to be_falsey + expect(subject.password_authentication_enabled?).to be_falsey expect(subject.signup_enabled?).to be_truthy end it 'predicate method changes when value is updated' do - subject.signin_enabled = true + subject.password_authentication_enabled = true - expect(subject.signin_enabled?).to be_truthy + expect(subject.password_authentication_enabled?).to be_truthy end it 'does not define a predicate method' do diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index acffd335e43..10fa5f4044b 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -705,9 +705,9 @@ describe Gitlab::Git::Repository, seed_helper: true do # Add new commits so that there's a renamed file in the commit history repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH).rugged - commit_with_old_name = new_commit_edit_old_file(repo) - rename_commit = new_commit_move_file(repo) - commit_with_new_name = new_commit_edit_new_file(repo) + commit_with_old_name = Gitlab::Git::Commit.decorate(new_commit_edit_old_file(repo)) + rename_commit = Gitlab::Git::Commit.decorate(new_commit_move_file(repo)) + commit_with_new_name = Gitlab::Git::Commit.decorate(new_commit_edit_new_file(repo)) end after(:context) do @@ -880,8 +880,8 @@ describe Gitlab::Git::Repository, seed_helper: true do context "compare results between log_by_walk and log_by_shell" do let(:options) { { ref: "master" } } - let(:commits_by_walk) { repository.log(options).map(&:oid) } - let(:commits_by_shell) { repository.log(options.merge({ disable_walk: true })).map(&:oid) } + let(:commits_by_walk) { repository.log(options).map(&:id) } + let(:commits_by_shell) { repository.log(options.merge({ disable_walk: true })).map(&:id) } it { expect(commits_by_walk).to eq(commits_by_shell) } @@ -924,7 +924,7 @@ describe Gitlab::Git::Repository, seed_helper: true do expect(commits.size).to be > 0 expect(commits).to satisfy do |commits| - commits.all? { |commit| commit.time >= options[:after] } + commits.all? { |commit| commit.committed_date >= options[:after] } end end end @@ -937,7 +937,7 @@ describe Gitlab::Git::Repository, seed_helper: true do expect(commits.size).to be > 0 expect(commits).to satisfy do |commits| - commits.all? { |commit| commit.time <= options[:before] } + commits.all? { |commit| commit.committed_date <= options[:before] } end end end @@ -946,7 +946,7 @@ describe Gitlab::Git::Repository, seed_helper: true do let(:options) { { ref: 'master', path: ['PROCESS.md', 'README.md'] } } def commit_files(commit) - commit.diff(commit.parent_ids.first).deltas.flat_map do |delta| + commit.diff_from_parent.deltas.flat_map do |delta| [delta.old_file[:path], delta.new_file[:path]].uniq.compact end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index c70f916a8bd..c614df8e98a 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1932,4 +1932,26 @@ describe User, models: true do user.invalidate_merge_request_cache_counts end end + + describe '#allow_password_authentication?' do + context 'regular user' do + let(:user) { build(:user) } + + it 'returns true when sign-in is enabled' do + expect(user.allow_password_authentication?).to be_truthy + end + + it 'returns false when sign-in is disabled' do + stub_application_setting(password_authentication_enabled: false) + + expect(user.allow_password_authentication?).to be_falsey + end + end + + it 'returns false for ldap user' do + user = create(:omniauth_user, provider: 'ldapmain') + + expect(user.allow_password_authentication?).to be_falsey + end + end end diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb index 453eb4683a0..beaaf346283 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -35,6 +35,17 @@ describe API::Internal do expect(json_response).to be_empty end end + + context 'nil broadcast message' do + it 'returns nothing' do + allow(BroadcastMessage).to receive(:current).and_return(nil) + + get api('/internal/broadcast_message'), secret_token: secret_token + + expect(response).to have_http_status(200) + expect(json_response).to be_empty + end + end end describe 'GET /internal/broadcast_messages' do diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index ede48b1c888..b71ac6c30b5 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -10,7 +10,7 @@ describe API::Settings, 'Settings' do expect(response).to have_http_status(200) expect(json_response).to be_an Hash expect(json_response['default_projects_limit']).to eq(42) - expect(json_response['signin_enabled']).to be_truthy + expect(json_response['password_authentication_enabled']).to be_truthy expect(json_response['repository_storage']).to eq('default') expect(json_response['koding_enabled']).to be_falsey expect(json_response['koding_url']).to be_nil @@ -32,7 +32,7 @@ describe API::Settings, 'Settings' do it "updates application settings" do put api("/application/settings", admin), default_projects_limit: 3, - signin_enabled: false, + password_authentication_enabled: false, repository_storage: 'custom', koding_enabled: true, koding_url: 'http://koding.example.com', @@ -46,7 +46,7 @@ describe API::Settings, 'Settings' do help_page_support_url: 'http://example.com/help' expect(response).to have_http_status(200) expect(json_response['default_projects_limit']).to eq(3) - expect(json_response['signin_enabled']).to be_falsey + expect(json_response['password_authentication_enabled']).to be_falsey expect(json_response['repository_storage']).to eq('custom') expect(json_response['repository_storages']).to eq(['custom']) expect(json_response['koding_enabled']).to be_truthy diff --git a/spec/requests/api/v3/settings_spec.rb b/spec/requests/api/v3/settings_spec.rb index 41d039b7da0..291f6dcc2aa 100644 --- a/spec/requests/api/v3/settings_spec.rb +++ b/spec/requests/api/v3/settings_spec.rb @@ -10,7 +10,7 @@ describe API::V3::Settings, 'Settings' do expect(response).to have_http_status(200) expect(json_response).to be_an Hash expect(json_response['default_projects_limit']).to eq(42) - expect(json_response['signin_enabled']).to be_truthy + expect(json_response['password_authentication_enabled']).to be_truthy expect(json_response['repository_storage']).to eq('default') expect(json_response['koding_enabled']).to be_falsey expect(json_response['koding_url']).to be_nil @@ -28,11 +28,11 @@ describe API::V3::Settings, 'Settings' do it "updates application settings" do put v3_api("/application/settings", admin), - default_projects_limit: 3, signin_enabled: false, repository_storage: 'custom', koding_enabled: true, koding_url: 'http://koding.example.com', + default_projects_limit: 3, password_authentication_enabled: false, repository_storage: 'custom', koding_enabled: true, koding_url: 'http://koding.example.com', plantuml_enabled: true, plantuml_url: 'http://plantuml.example.com' expect(response).to have_http_status(200) expect(json_response['default_projects_limit']).to eq(3) - expect(json_response['signin_enabled']).to be_falsey + expect(json_response['password_authentication_enabled']).to be_falsey expect(json_response['repository_storage']).to eq('custom') expect(json_response['repository_storages']).to eq(['custom']) expect(json_response['koding_enabled']).to be_truthy diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index d0443a450a2..d043ab2a974 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -463,7 +463,7 @@ describe 'Git HTTP requests', lib: true do context 'when internal auth is disabled' do before do - allow_any_instance_of(ApplicationSetting).to receive(:signin_enabled?) { false } + allow_any_instance_of(ApplicationSetting).to receive(:password_authentication_enabled?) { false } end it 'rejects pulls with personal access token error message' do diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb index 5e4cf05748e..8d79ea3dd40 100644 --- a/spec/requests/jwt_controller_spec.rb +++ b/spec/requests/jwt_controller_spec.rb @@ -101,7 +101,7 @@ describe JwtController do context 'when internal auth is disabled' do it 'rejects the authorization attempt with personal access token message' do - allow_any_instance_of(ApplicationSetting).to receive(:signin_enabled?) { false } + allow_any_instance_of(ApplicationSetting).to receive(:password_authentication_enabled?) { false } get '/jwt/auth', parameters, headers expect(response).to have_http_status(401) |