diff options
| author | Douwe Maan <douwe@gitlab.com> | 2017-11-23 13:16:16 +0000 |
|---|---|---|
| committer | Douwe Maan <douwe@gitlab.com> | 2017-11-23 13:16:16 +0000 |
| commit | b8db9c8c8c406d4abc03ba7246bbfecbecfbc784 (patch) | |
| tree | afaaddcdc16ac407d72b7b4c0e96d951a141c268 | |
| parent | a6cafbcbe8d6802a81055c3469312f889cd73c9a (diff) | |
| parent | 257fd5713485a05460a9170190100643199a7e48 (diff) | |
| download | gitlab-ce-b8db9c8c8c406d4abc03ba7246bbfecbecfbc784.tar.gz | |
Merge branch 'feature/disable-password-authentication' into 'master'
Allow password authentication to be disabled entirely
Closes #37320
See merge request gitlab-org/gitlab-ce!15223
52 files changed, 299 insertions, 110 deletions
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index b2ec491146f..ee21d81f23e 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -196,7 +196,7 @@ class ApplicationController < ActionController::Base end def check_password_expiration - return if session[:impersonator_id] || current_user&.ldap_user? + return if session[:impersonator_id] || !current_user&.allow_password_authentication? password_expires_at = current_user&.password_expires_at diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb index 0982a61902b..04b29aa2384 100644 --- a/app/controllers/invites_controller.rb +++ b/app/controllers/invites_controller.rb @@ -51,7 +51,7 @@ class InvitesController < ApplicationController return if current_user notice = "To accept this invitation, sign in" - notice << " or create an account" if current_application_settings.signup_enabled? + notice << " or create an account" if current_application_settings.allow_signup? notice << "." store_location_for :user, request.fullpath diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 56baa19f864..e3c18cba1dd 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -140,7 +140,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController label = Gitlab::OAuth::Provider.label_for(oauth['provider']) message = "Signing in using your #{label} account without a pre-existing GitLab account is not allowed." - if current_application_settings.signup_enabled? + if current_application_settings.allow_signup? message << " Create a GitLab account first, and then connect it to your #{label} account." end diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb index fda944adecd..68a52f40342 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_creation? + if resource.valid? && resource.password_automatically_set? resource.update_attribute(:password_automatically_set, false) end end @@ -38,11 +40,15 @@ class PasswordsController < Devise::PasswordsController self.resource = resource_class.find_by_email(email) end - def prevent_ldap_reset - return unless resource&.ldap_user? + def check_password_authentication_available + if resource + return if resource.allow_password_authentication? + else + return if current_application_settings.password_authentication_enabled? + end 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 dcfcb855ab5..fa72f67c77e 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! - render_404 if @user.ldap_user? + render_404 unless @user.allow_password_authentication? end def user_params diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index c01be42c3ee..d79108c88fb 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -63,7 +63,7 @@ class SessionsController < Devise::SessionsController user = User.admins.last - return unless user && user.require_password_creation? + return unless user && user.require_password_creation_for_web? Users::UpdateService.new(current_user, user: 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 e5d2693b01e..6fc4248b245 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -3,9 +3,9 @@ module ApplicationSettingsHelper include Gitlab::CurrentSettings - delegate :gravatar_enabled?, - :signup_enabled?, - :password_authentication_enabled?, + delegate :allow_signup?, + :gravatar_enabled?, + :password_authentication_enabled_for_web?, :akismet_enabled?, :koding_enabled?, to: :current_application_settings @@ -203,7 +203,7 @@ module ApplicationSettingsHelper :metrics_port, :metrics_sample_interval, :metrics_timeout, - :password_authentication_enabled, + :password_authentication_enabled_for_web, :performance_bar_allowed_group_id, :performance_bar_enabled, :plantuml_enabled, diff --git a/app/helpers/button_helper.rb b/app/helpers/button_helper.rb index 48cf30a48ab..8e8feeea1d8 100644 --- a/app/helpers/button_helper.rb +++ b/app/helpers/button_helper.rb @@ -58,12 +58,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_creation?) || current_user.try(:require_personal_access_token_creation_for_git_auth?) + klass << ' has-tooltip' if current_user.try(:require_extra_setup_for_git_auth?) protocol = gitlab_config.protocol.upcase tooltip_title = - if current_user.try(:require_password_creation?) + if current_user.try(:require_password_creation_for_git?) _("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 f48d47953e4..4a6b22b5ff6 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -234,11 +234,11 @@ module ProjectsHelper def show_no_password_message? cookies[:hide_no_password_message].blank? && !current_user.hide_no_password && - ( current_user.require_password_creation? || current_user.require_personal_access_token_creation_for_git_auth? ) + current_user.require_extra_setup_for_git_auth? end def link_to_set_password - if current_user.require_password_creation? + if current_user.require_password_creation_for_git? 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 diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index a7e0219b03a..01455a52d2a 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -276,7 +276,8 @@ 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'], + password_authentication_enabled_for_web: Settings.gitlab['signin_enabled'], + password_authentication_enabled_for_git: true, performance_bar_allowed_group_id: nil, rsa_key_restriction: 0, plantuml_enabled: false, @@ -474,6 +475,14 @@ class ApplicationSetting < ActiveRecord::Base has_attribute?(attr_name) ? public_send(attr_name) : FORBIDDEN_KEY_VALUE # rubocop:disable GitlabSecurity/PublicSend end + def allow_signup? + signup_enabled? && password_authentication_enabled_for_web? + end + + def password_authentication_enabled? + password_authentication_enabled_for_web? || password_authentication_enabled_for_git? + end + private def ensure_uuid! diff --git a/app/models/user.rb b/app/models/user.rb index f98165754ca..6c773b3ce7d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -633,18 +633,34 @@ class User < ActiveRecord::Base count.zero? && Gitlab::ProtocolAccess.allowed?('ssh') end - def require_password_creation? - password_automatically_set? && allow_password_authentication? + def require_password_creation_for_web? + allow_password_authentication_for_web? && password_automatically_set? + end + + def require_password_creation_for_git? + allow_password_authentication_for_git? && password_automatically_set? end def require_personal_access_token_creation_for_git_auth? - return false if current_application_settings.password_authentication_enabled? || ldap_user? + return false if allow_password_authentication_for_git? || ldap_user? PersonalAccessTokensFinder.new(user: self, impersonation: false, state: 'active').execute.none? end + def require_extra_setup_for_git_auth? + require_password_creation_for_git? || require_personal_access_token_creation_for_git_auth? + end + def allow_password_authentication? - !ldap_user? && current_application_settings.password_authentication_enabled? + allow_password_authentication_for_web? || allow_password_authentication_for_git? + end + + def allow_password_authentication_for_web? + current_application_settings.password_authentication_enabled_for_web? && !ldap_user? + end + + def allow_password_authentication_for_git? + current_application_settings.password_authentication_enabled_for_git? && !ldap_user? end def can_change_username? diff --git a/app/services/users/build_service.rb b/app/services/users/build_service.rb index 6f05500adea..61f1568f366 100644 --- a/app/services/users/build_service.rb +++ b/app/services/users/build_service.rb @@ -34,7 +34,7 @@ module Users private def can_create_user? - (current_user.nil? && current_application_settings.signup_enabled?) || current_user&.admin? + (current_user.nil? && current_application_settings.allow_signup?) || current_user&.admin? end # Allowed params for creating a user (admins only) diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index 12658dddc06..64249c91dd0 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -160,9 +160,22 @@ .form-group .col-sm-offset-2.col-sm-10 .checkbox - = f.label :password_authentication_enabled do - = f.check_box :password_authentication_enabled - Sign-in enabled + = f.label :password_authentication_enabled_for_web do + = f.check_box :password_authentication_enabled_for_web + Password authentication enabled for web interface + .help-block + When disabled, an external authentication provider must be used. + .form-group + .col-sm-offset-2.col-sm-10 + .checkbox + = f.label :password_authentication_enabled_for_git do + = f.check_box :password_authentication_enabled_for_git + Password authentication enabled for Git over HTTP(S) + .help-block + When disabled, a Personal Access Token + - if Gitlab::LDAP::Config.enabled? + or LDAP password + must be used to authenticate. - 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/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml index 2f0143c7eff..a24516355bf 100644 --- a/app/views/admin/dashboard/index.html.haml +++ b/app/views/admin/dashboard/index.html.haml @@ -45,10 +45,10 @@ .well-segment.admin-well.admin-well-features %h4 Features - sign_up = "Sign up" - %p{ "aria-label" => "#{sign_up}: status " + (signup_enabled? ? "on" : "off") } + %p{ "aria-label" => "#{sign_up}: status " + (allow_signup? ? "on" : "off") } = sign_up %span.light.pull-right - = boolean_to_icon signup_enabled? + = boolean_to_icon allow_signup? - ldap = "LDAP" %p{ "aria-label" => "#{ldap}: status " + (Gitlab.config.ldap.enabled ? "on" : "off") } = ldap diff --git a/app/views/devise/sessions/new.html.haml b/app/views/devise/sessions/new.html.haml index dd61dcf2a7b..34d4293bd45 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 password_authentication_enabled? || ldap_enabled? || crowd_enabled? + - if password_authentication_enabled_for_web? || ldap_enabled? || crowd_enabled? = render 'devise/shared/signin_box' -# Signup only makes sense if you can also sign-in - - if password_authentication_enabled? && signup_enabled? + - if allow_signup? = render 'devise/shared/signup_box' -# Show a message if none of the mechanisms above are enabled - - if !password_authentication_enabled? && !ldap_enabled? && !(omniauth_enabled? && devise_mapping.omniauthable?) + - if !password_authentication_enabled_for_web? && !ldap_enabled? && !(omniauth_enabled? && devise_mapping.omniauthable?) %div No authentication methods configured. diff --git a/app/views/devise/shared/_links.erb b/app/views/devise/shared/_links.erb index 49e99e25c1d..6e1cc244f26 100644 --- a/app/views/devise/shared/_links.erb +++ b/app/views/devise/shared/_links.erb @@ -2,7 +2,7 @@ <%= link_to "Sign in", new_session_path(resource_name), class: "btn" %><br /> <% end -%> -<%- if devise_mapping.registerable? && controller_name != 'registrations' && gitlab_config.signup_enabled %> +<%- if devise_mapping.registerable? && controller_name != 'registrations' && allow_signup? %> <%= link_to "Sign up", new_registration_path(resource_name) %><br /> <% end -%> diff --git a/app/views/devise/shared/_signin_box.html.haml b/app/views/devise/shared/_signin_box.html.haml index 3b06008febe..6087f4a0b37 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 password_authentication_enabled? + - if password_authentication_enabled_for_web? .login-box.tab-pane{ id: 'ldap-standard', role: 'tabpanel' } .login-body = render 'devise/sessions/new_base' -- elsif password_authentication_enabled? +- elsif password_authentication_enabled_for_web? .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 6d0243a325d..94f19ccd44c 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 password_authentication_enabled? + - if password_authentication_enabled_for_web? %li = link_to 'Standard', '#ldap-standard', 'data-toggle' => 'tab' - - if password_authentication_enabled? && signup_enabled? + - if allow_signup? %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 212856c0676..1ba6d390875 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 password_authentication_enabled? && signup_enabled? + - if allow_signup? %li{ role: 'presentation' } %a{ href: '#register-pane', data: { toggle: 'tab' }, role: 'tab' } Register diff --git a/app/views/layouts/nav/sidebar/_profile.html.haml b/app/views/layouts/nav/sidebar/_profile.html.haml index 458b5010d36..7e23f9c1f05 100644 --- a/app/views/layouts/nav/sidebar/_profile.html.haml +++ b/app/views/layouts/nav/sidebar/_profile.html.haml @@ -73,7 +73,7 @@ = link_to profile_emails_path do %strong.fly-out-top-item-name #{ _('Emails') } - - unless current_user.ldap_user? + - if current_user.allow_password_authentication? = nav_link(controller: :passwords) do = link_to edit_profile_password_path do .nav-icon-container diff --git a/app/views/notify/new_user_email.html.haml b/app/views/notify/new_user_email.html.haml index 6b9b42dcf37..00e1b5faae3 100644 --- a/app/views/notify/new_user_email.html.haml +++ b/app/views/notify/new_user_email.html.haml @@ -1,7 +1,7 @@ %p Hi #{@user['name']}! %p - - if Gitlab.config.gitlab.signup_enabled + - if current_application_settings.allow_signup? Your account has been created successfully. - else The Administrator created an account for you. Now you are a member of the company GitLab application. diff --git a/changelogs/unreleased/feature-disable-password-authentication.yml b/changelogs/unreleased/feature-disable-password-authentication.yml new file mode 100644 index 00000000000..999203f12ce --- /dev/null +++ b/changelogs/unreleased/feature-disable-password-authentication.yml @@ -0,0 +1,5 @@ +--- +title: Allow password authentication to be disabled entirely +merge_request: 15223 +author: Markus Koller +type: changed diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index f7c83f7b0f7..f10f0cdf42c 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -256,7 +256,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['password_authentication_enabled'] ||= true if Settings.gitlab['password_authentication_enabled'].nil? +Settings.gitlab['signin_enabled'] ||= true if Settings.gitlab['signin_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)|[Ii]mplement(?:s|ed|ing)?)(:?) +(?:(?:issues? +)?%{issue_ref}(?:(?:, *| +and +)?)|([A-Z][A-Z0-9_]+-\d+))+)' if Settings.gitlab['issue_closing_pattern'].nil? diff --git a/db/migrate/20171106133143_rename_application_settings_password_authentication_enabled_to_password_authentication_enabled_for_web.rb b/db/migrate/20171106133143_rename_application_settings_password_authentication_enabled_to_password_authentication_enabled_for_web.rb new file mode 100644 index 00000000000..6d369e93361 --- /dev/null +++ b/db/migrate/20171106133143_rename_application_settings_password_authentication_enabled_to_password_authentication_enabled_for_web.rb @@ -0,0 +1,15 @@ +class RenameApplicationSettingsPasswordAuthenticationEnabledToPasswordAuthenticationEnabledForWeb < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + rename_column_concurrently :application_settings, :password_authentication_enabled, :password_authentication_enabled_for_web + end + + def down + cleanup_concurrent_column_rename :application_settings, :password_authentication_enabled_for_web, :password_authentication_enabled + end +end diff --git a/db/migrate/20171106133911_add_password_authentication_enabled_for_git_to_application_settings.rb b/db/migrate/20171106133911_add_password_authentication_enabled_for_git_to_application_settings.rb new file mode 100644 index 00000000000..b8aa600864e --- /dev/null +++ b/db/migrate/20171106133911_add_password_authentication_enabled_for_git_to_application_settings.rb @@ -0,0 +1,9 @@ +class AddPasswordAuthenticationEnabledForGitToApplicationSettings < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + add_column :application_settings, :password_authentication_enabled_for_git, :boolean, default: true, null: false + end +end diff --git a/db/post_migrate/20171106133144_cleanup_application_settings_password_authentication_enabled_rename.rb b/db/post_migrate/20171106133144_cleanup_application_settings_password_authentication_enabled_rename.rb new file mode 100644 index 00000000000..d54ff3d5f5e --- /dev/null +++ b/db/post_migrate/20171106133144_cleanup_application_settings_password_authentication_enabled_rename.rb @@ -0,0 +1,15 @@ +class CleanupApplicationSettingsPasswordAuthenticationEnabledRename < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + cleanup_concurrent_column_rename :application_settings, :password_authentication_enabled, :password_authentication_enabled_for_web + end + + def down + rename_column_concurrently :application_settings, :password_authentication_enabled_for_web, :password_authentication_enabled + end +end diff --git a/db/schema.rb b/db/schema.rb index bca8b96b2c1..d10561099b7 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -129,7 +129,6 @@ ActiveRecord::Schema.define(version: 20171121144800) do t.boolean "prometheus_metrics_enabled", default: false, null: false t.boolean "help_page_hide_commercial_content", default: false t.string "help_page_support_url" - t.boolean "password_authentication_enabled" t.integer "performance_bar_allowed_group_id" t.boolean "hashed_storage_enabled", default: false, null: false t.boolean "project_export_enabled", default: true, null: false @@ -149,6 +148,8 @@ ActiveRecord::Schema.define(version: 20171121144800) do t.boolean "throttle_authenticated_web_enabled", default: false, null: false t.integer "throttle_authenticated_web_requests_per_period", default: 7200, null: false t.integer "throttle_authenticated_web_period_in_seconds", default: 3600, null: false + t.boolean "password_authentication_enabled_for_web" + t.boolean "password_authentication_enabled_for_git", default: true end create_table "audit_events", force: :cascade do |t| diff --git a/doc/administration/auth/ldap.md b/doc/administration/auth/ldap.md index ad903aef896..6b5a0f139c5 100644 --- a/doc/administration/auth/ldap.md +++ b/doc/administration/auth/ldap.md @@ -30,6 +30,12 @@ immediately block all access. >**Note**: GitLab EE supports a configurable sync time, with a default of one hour. +## Git password authentication + +LDAP-enabled users can always authenticate with Git using their GitLab username +or email and LDAP password, even if password authentication for Git is disabled +in the application settings. + ## Configuration To enable LDAP integration you need to add your LDAP server settings in diff --git a/doc/api/settings.md b/doc/api/settings.md index b27220f57f4..22fb2baa8ec 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" : [], - "password_authentication_enabled" : true, + "password_authentication_enabled_for_web" : true, "after_sign_out_path" : null, "max_attachment_size" : 10, "user_oauth_applications" : true, @@ -117,7 +117,8 @@ PUT /application/settings | `metrics_port` | integer | no | The UDP port to use for connecting to InfluxDB | | `metrics_sample_interval` | integer | yes (if `metrics_enabled` is `true`) | The sampling interval in seconds. | | `metrics_timeout` | integer | yes (if `metrics_enabled` is `true`) | The amount of seconds after which InfluxDB will time out. | -| `password_authentication_enabled` | boolean | no | Enable authentication via a GitLab account password. Default is `true`. | +| `password_authentication_enabled_for_web` | boolean | no | Enable authentication for the web interface via a GitLab account password. Default is `true`. | +| `password_authentication_enabled_for_git` | boolean | no | Enable authentication for Git over HTTP(S) via a GitLab account password. Default is `true`. | | `performance_bar_allowed_group_id` | string | no | The group that is allowed to enable the performance bar | | `performance_bar_enabled` | boolean | no | Allow enabling the performance bar | | `plantuml_enabled` | boolean | no | Enable PlantUML integration. Default is `false`. | @@ -165,7 +166,7 @@ Example response: "id": 1, "default_projects_limit": 100000, "signup_enabled": true, - "password_authentication_enabled": true, + "password_authentication_enabled_for_web": true, "gravatar_enabled": true, "sign_in_text": "", "created_at": "2015-06-12T15:51:55.432Z", diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 16ae99b5c6c..e45c87e4f4f 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -763,7 +763,10 @@ module API expose(:default_project_visibility) { |setting, _options| Gitlab::VisibilityLevel.string_level(setting.default_project_visibility) } expose(:default_snippet_visibility) { |setting, _options| Gitlab::VisibilityLevel.string_level(setting.default_snippet_visibility) } expose(:default_group_visibility) { |setting, _options| Gitlab::VisibilityLevel.string_level(setting.default_group_visibility) } - expose :password_authentication_enabled, as: :signin_enabled + + # support legacy names, can be removed in v5 + expose :password_authentication_enabled_for_web, as: :password_authentication_enabled + expose :password_authentication_enabled_for_web, as: :signin_enabled end class Release < Grape::Entity diff --git a/lib/api/settings.rb b/lib/api/settings.rb index 851b226e9e5..06373fe5069 100644 --- a/lib/api/settings.rb +++ b/lib/api/settings.rb @@ -44,9 +44,11 @@ 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 :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 :password_authentication_enabled_for_web, type: Boolean, desc: 'Flag indicating if password authentication is enabled for the web interface' + optional :password_authentication_enabled, type: Boolean, desc: 'Flag indicating if password authentication is enabled for the web interface' # support legacy names, can be removed in v5 + optional :signin_enabled, type: Boolean, desc: 'Flag indicating if password authentication is enabled for the web interface' # support legacy names, can be removed in v5 + mutually_exclusive :password_authentication_enabled_for_web, :password_authentication_enabled, :signin_enabled + optional :password_authentication_enabled_for_git, type: Boolean, desc: 'Flag indicating if password authentication is enabled for Git over HTTP(S)' 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' @@ -135,8 +137,11 @@ module API put "application/settings" do attrs = declared_params(include_missing: false) + # support legacy names, can be removed in v5 if attrs.has_key?(:signin_enabled) - attrs[:password_authentication_enabled] = attrs.delete(:signin_enabled) + attrs[:password_authentication_enabled_for_web] = attrs.delete(:signin_enabled) + elsif attrs.has_key?(:password_authentication_enabled) + attrs[:password_authentication_enabled_for_web] = attrs.delete(:password_authentication_enabled) end if current_settings.update_attributes(attrs) diff --git a/lib/api/v3/entities.rb b/lib/api/v3/entities.rb index afdd7b83998..c17b6f45ed8 100644 --- a/lib/api/v3/entities.rb +++ b/lib/api/v3/entities.rb @@ -172,8 +172,8 @@ module API expose :id expose :default_projects_limit expose :signup_enabled - expose :password_authentication_enabled - expose :password_authentication_enabled, as: :signin_enabled + expose :password_authentication_enabled_for_web, as: :password_authentication_enabled + expose :password_authentication_enabled_for_web, 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 202011cfcbe..9b4ab7630fb 100644 --- a/lib/api/v3/settings.rb +++ b/lib/api/v3/settings.rb @@ -44,8 +44,8 @@ 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 :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' + optional :password_authentication_enabled, type: Boolean, desc: 'Flag indicating if password authentication is enabled for the web interface' + optional :signin_enabled, type: Boolean, desc: 'Flag indicating if password authentication is enabled for the web interface' 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 @@ -131,7 +131,9 @@ module API attrs = declared_params(include_missing: false) if attrs.has_key?(:signin_enabled) - attrs[:password_authentication_enabled] = attrs.delete(:signin_enabled) + attrs[:password_authentication_enabled_for_web] = attrs.delete(:signin_enabled) + elsif attrs.has_key?(:password_authentication_enabled) + attrs[:password_authentication_enabled_for_web] = attrs.delete(:password_authentication_enabled) end if current_settings.update_attributes(attrs) diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index cbbc51db99e..9670207a105 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -34,7 +34,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.password_authentication_enabled? || Gitlab::LDAP::Config.enabled? + return result if result.success? || authenticate_using_internal_or_ldap_password? # If sign-in is disabled and LDAP is not configured, recommend a # personal access token on failed auth attempts @@ -45,6 +45,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 authenticate_using_internal_or_ldap_password? + Gitlab::Auth::UniqueIpsLimiter.limit_user! do user = User.by_login(login) @@ -52,10 +56,8 @@ module Gitlab # LDAP users are only authenticated via LDAP if user.nil? || user.ldap_user? # Second chance - try LDAP authentication - return unless Gitlab::LDAP::Config.enabled? - Gitlab::LDAP::Authentication.login(login, password) - else + elsif current_application_settings.password_authentication_enabled_for_git? user if user.active? && user.valid_password?(password) end end @@ -84,6 +86,10 @@ module Gitlab private + def authenticate_using_internal_or_ldap_password? + current_application_settings.password_authentication_enabled_for_git? || Gitlab::LDAP::Config.enabled? + end + def service_request_check(login, password, project) matched_login = /(?<service>^[a-zA-Z]*-ci)-token$/.match(login) diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index 112d4939582..2adcc9809b3 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -79,7 +79,7 @@ module Gitlab def features_usage_data_ce { - signup: current_application_settings.signup_enabled?, + signup: current_application_settings.allow_signup?, ldap: Gitlab.config.ldap.enabled, gravatar: current_application_settings.gravatar_enabled?, omniauth: Gitlab.config.omniauth.enabled, diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 768c7e99c96..fe95d1ef9cd 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -41,14 +41,13 @@ describe ApplicationController do controller.send(:check_password_expiration) end - it 'redirects if the user is over their password expiry and sign-in is disabled' do - stub_application_setting(password_authentication_enabled: false) + it 'does not redirect if the user is over their password expiry but password authentication is disabled for the web interface' do + stub_application_setting(password_authentication_enabled_for_web: false) + stub_application_setting(password_authentication_enabled_for_git: false) user.password_expires_at = Time.new(2002) - expect(user.ldap_user?).to be_falsey allow(controller).to receive(:current_user).and_return(user) - expect(controller).to receive(:redirect_to) - expect(controller).to receive(:new_profile_password_path) + expect(controller).not_to receive(:redirect_to) controller.send(:check_password_expiration) end diff --git a/spec/controllers/passwords_controller_spec.rb b/spec/controllers/passwords_controller_spec.rb index 8778bff1190..4d31cfedbd2 100644 --- a/spec/controllers/passwords_controller_spec.rb +++ b/spec/controllers/passwords_controller_spec.rb @@ -1,18 +1,20 @@ require 'spec_helper' describe PasswordsController do - describe '#prevent_ldap_reset' 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 'allows password reset' do - stub_application_setting(password_authentication_enabled: false) + context 'when password authentication is disabled for the web interface and Git' do + it 'prevents a password reset' do + stub_application_setting(password_authentication_enabled_for_web: false) + stub_application_setting(password_authentication_enabled_for_git: false) post :create expect(response).to have_gitlab_http_status(302) + expect(flash[:alert]).to eq 'Password authentication is unavailable.' end end @@ -22,7 +24,7 @@ describe PasswordsController do it 'prevents a password reset' do post :create, user: { email: user.email } - expect(flash[:alert]).to eq('Cannot reset password for LDAP user.') + expect(flash[:alert]).to eq 'Password authentication is unavailable.' end end end diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index 1d3ddfbd220..346944fd5b0 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -118,7 +118,8 @@ describe RegistrationsController do context 'user does not require password confirmation' do before do - stub_application_setting(password_authentication_enabled: false) + stub_application_setting(password_authentication_enabled_for_web: false) + stub_application_setting(password_authentication_enabled_for_git: false) end it 'fails if username confirmation is not provided' do diff --git a/spec/features/profiles/password_spec.rb b/spec/features/profiles/password_spec.rb index fb4355074df..4665626f114 100644 --- a/spec/features/profiles/password_spec.rb +++ b/spec/features/profiles/password_spec.rb @@ -53,12 +53,13 @@ describe 'Profile > Password' do context 'Regular user' do let(:user) { create(:user) } - it 'renders 200 when sign-in is disabled' do - stub_application_setting(password_authentication_enabled: false) + it 'renders 404 when password authentication is disabled for the web interface and Git' do + stub_application_setting(password_authentication_enabled_for_web: false) + stub_application_setting(password_authentication_enabled_for_git: false) visit edit_profile_password_path - expect(page).to have_gitlab_http_status(200) + expect(page).to have_gitlab_http_status(404) end end diff --git a/spec/features/projects/no_password_spec.rb b/spec/features/projects/no_password_spec.rb index 6aff079dd39..b3b3212556c 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(password_authentication_enabled?: false) + stub_application_setting(password_authentication_enabled_for_git?: 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 4423560ecaa..e5158761333 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(password_authentication_enabled?: false) + stub_application_setting(password_authentication_enabled_for_git?: 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 5777b5c4025..ede9d232efd 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -150,17 +150,26 @@ describe ProjectsHelper do end end - context 'user requires a password' do - let(:user) { create(:user, password_automatically_set: true) } + context 'user has hidden the message' do + it 'returns false' do + allow(helper).to receive(:cookies).and_return(hide_no_password_message: true) + + expect(helper.show_no_password_message?).to be_falsey + end + end + context 'user requires a password for Git' do it 'returns true' do + allow(user).to receive(:require_password_creation_for_git?).and_return(true) + expect(helper.show_no_password_message?).to be_truthy end end - context 'user requires a personal access token' do + context 'user requires a personal access token for Git' do it 'returns true' do - stub_application_setting(password_authentication_enabled?: false) + allow(user).to receive(:require_password_creation_for_git?).and_return(false) + allow(user).to receive(:require_personal_access_token_creation_for_git_auth?).and_return(true) expect(helper.show_no_password_message?).to be_truthy end @@ -168,23 +177,23 @@ describe ProjectsHelper do end describe '#link_to_set_password' do + let(:user) { create(:user, password_automatically_set: true) } + before do allow(helper).to receive(:current_user).and_return(user) end - context 'user requires a password' do - let(:user) { create(:user, password_automatically_set: true) } - + context 'password authentication is enabled for Git' do it 'returns link to set a password' do + stub_application_setting(password_authentication_enabled_for_git?: true) + expect(helper.link_to_set_password).to match %r{<a href="#{edit_profile_password_path}">set a password</a>} end end - context 'user requires a personal access token' do - let(:user) { create(:user) } - + context 'password authentication is disabled for Git' do it 'returns link to create a personal access token' do - stub_application_setting(password_authentication_enabled?: false) + stub_application_setting(password_authentication_enabled_for_git?: 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 3164d2ebf04..5e822a0026a 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -251,7 +251,7 @@ describe Gitlab::Auth do end it 'throws an error suggesting user create a PAT when internal auth is disabled' do - allow_any_instance_of(ApplicationSetting).to receive(:password_authentication_enabled?) { false } + allow_any_instance_of(ApplicationSetting).to receive(:password_authentication_enabled_for_git?) { false } expect { gl_auth.find_for_git_client('foo', 'bar', project: nil, ip: 'ip') }.to raise_error(Gitlab::Auth::MissingPersonalAccessTokenError) end @@ -324,6 +324,26 @@ describe Gitlab::Auth do gl_auth.find_with_user_password('ldap_user', 'password') end end + + context "with password authentication disabled for Git" do + before do + stub_application_setting(password_authentication_enabled_for_git: 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 + + context "with ldap enabled" do + before do + allow(Gitlab::LDAP::Config).to receive(:enabled?).and_return(true) + end + + it "does not find non-ldap user by valid login/password" do + expect(gl_auth.find_with_user_password(username, password)).to be_nil + end + 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 34322c2a693..af12e13d36d 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) { { password_authentication_enabled: false, foobar: 'asdf', signup_enabled: true, 'test?' => 123 } } + let(:defaults) { { password_authentication_enabled_for_web: false, foobar: 'asdf', signup_enabled: true, 'test?' => 123 } } subject { described_class.new(defaults) } it 'wraps OpenStruct variables properly' do - expect(subject.password_authentication_enabled).to be_falsey + expect(subject.password_authentication_enabled_for_web).to be_falsey expect(subject.signup_enabled).to be_truthy expect(subject.foobar).to eq('asdf') end it 'defines predicate methods' do - expect(subject.password_authentication_enabled?).to be_falsey + expect(subject.password_authentication_enabled_for_web?).to be_falsey expect(subject.signup_enabled?).to be_truthy end it 'predicate method changes when value is updated' do - subject.password_authentication_enabled = true + subject.password_authentication_enabled_for_web = true - expect(subject.password_authentication_enabled?).to be_truthy + expect(subject.password_authentication_enabled_for_web?).to be_truthy end it 'does not define a predicate method' do diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index a4c1113ae37..b5f2a15ada3 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -103,7 +103,7 @@ describe Gitlab::UsageData do subject { described_class.features_usage_data_ce } it 'gathers feature usage data' do - expect(subject[:signup]).to eq(current_application_settings.signup_enabled?) + expect(subject[:signup]).to eq(current_application_settings.allow_signup?) expect(subject[:ldap]).to eq(Gitlab.config.ldap.enabled) expect(subject[:gravatar]).to eq(current_application_settings.gravatar_enabled?) expect(subject[:omniauth]).to eq(Gitlab.config.omniauth.enabled) diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 47b7150d36f..51bf4e65e5d 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -564,4 +564,22 @@ describe ApplicationSetting do expect(setting.key_restriction_for(:foo)).to eq(described_class::FORBIDDEN_KEY_VALUE) end end + + describe '#allow_signup?' do + it 'returns true' do + expect(setting.allow_signup?).to be_truthy + end + + it 'returns false if signup is disabled' do + allow(setting).to receive(:signup_enabled?).and_return(false) + + expect(setting.allow_signup?).to be_falsey + end + + it 'returns false if password authentication is disabled for the web interface' do + allow(setting).to receive(:password_authentication_enabled_for_web?).and_return(false) + + expect(setting.allow_signup?).to be_falsey + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 86647ddf6ce..b27c1b2cd1a 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2143,25 +2143,47 @@ describe User do end end - describe '#allow_password_authentication?' do + describe '#allow_password_authentication_for_web?' 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 + it 'returns true when password authentication is enabled for the web interface' do + expect(user.allow_password_authentication_for_web?).to be_truthy end - it 'returns false when sign-in is disabled' do - stub_application_setting(password_authentication_enabled: false) + it 'returns false when password authentication is disabled for the web interface' do + stub_application_setting(password_authentication_enabled_for_web: false) - expect(user.allow_password_authentication?).to be_falsey + expect(user.allow_password_authentication_for_web?).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 + expect(user.allow_password_authentication_for_web?).to be_falsey + end + end + + describe '#allow_password_authentication_for_git?' do + context 'regular user' do + let(:user) { build(:user) } + + it 'returns true when password authentication is enabled for Git' do + expect(user.allow_password_authentication_for_git?).to be_truthy + end + + it 'returns false when password authentication is disabled Git' do + stub_application_setting(password_authentication_enabled_for_git: false) + + expect(user.allow_password_authentication_for_git?).to be_falsey + end + end + + it 'returns false for ldap user' do + user = create(:omniauth_user, provider: 'ldapmain') + + expect(user.allow_password_authentication_for_git?).to be_falsey end end @@ -2381,7 +2403,8 @@ describe User do let(:expected) { !(password_automatically_set || ldap_user || password_authentication_disabled) } before do - stub_application_setting(password_authentication_enabled: !password_authentication_disabled) + stub_application_setting(password_authentication_enabled_for_web: !password_authentication_disabled) + stub_application_setting(password_authentication_enabled_for_git: !password_authentication_disabled) end it 'returns false unless all inputs are true' do diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index 5d3e78dd7c8..63175c40a18 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_gitlab_http_status(200) expect(json_response).to be_an Hash expect(json_response['default_projects_limit']).to eq(42) - expect(json_response['password_authentication_enabled']).to be_truthy + expect(json_response['password_authentication_enabled_for_web']).to be_truthy expect(json_response['repository_storages']).to eq(['default']) expect(json_response['koding_enabled']).to be_falsey expect(json_response['koding_url']).to be_nil @@ -37,7 +37,7 @@ describe API::Settings, 'Settings' do it "updates application settings" do put api("/application/settings", admin), default_projects_limit: 3, - password_authentication_enabled: false, + password_authentication_enabled_for_web: false, repository_storages: ['custom'], koding_enabled: true, koding_url: 'http://koding.example.com', @@ -58,7 +58,7 @@ describe API::Settings, 'Settings' do expect(response).to have_gitlab_http_status(200) expect(json_response['default_projects_limit']).to eq(3) - expect(json_response['password_authentication_enabled']).to be_falsey + expect(json_response['password_authentication_enabled_for_web']).to be_falsey expect(json_response['repository_storages']).to eq(['custom']) expect(json_response['koding_enabled']).to be_truthy expect(json_response['koding_url']).to eq('http://koding.example.com') diff --git a/spec/requests/api/v3/settings_spec.rb b/spec/requests/api/v3/settings_spec.rb index 25fa0a8aabd..985bfbfa09c 100644 --- a/spec/requests/api/v3/settings_spec.rb +++ b/spec/requests/api/v3/settings_spec.rb @@ -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, password_authentication_enabled: false, repository_storage: 'custom', koding_enabled: true, koding_url: 'http://koding.example.com', + default_projects_limit: 3, password_authentication_enabled_for_web: 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_gitlab_http_status(200) expect(json_response['default_projects_limit']).to eq(3) - expect(json_response['password_authentication_enabled']).to be_falsey + expect(json_response['password_authentication_enabled_for_web']).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 cd52194033a..a16f98bec36 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -463,7 +463,7 @@ describe 'Git HTTP requests' do context 'when internal auth is disabled' do before do - allow_any_instance_of(ApplicationSetting).to receive(:password_authentication_enabled?) { false } + allow_any_instance_of(ApplicationSetting).to receive(:password_authentication_enabled_for_git?) { 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 94e04ce5608..6f40a02aaa9 100644 --- a/spec/requests/jwt_controller_spec.rb +++ b/spec/requests/jwt_controller_spec.rb @@ -105,7 +105,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(:password_authentication_enabled?) { false } + allow_any_instance_of(ApplicationSetting).to receive(:password_authentication_enabled_for_git?) { false } get '/jwt/auth', parameters, headers expect(response).to have_gitlab_http_status(401) diff --git a/spec/support/matchers/have_gitlab_http_status.rb b/spec/support/matchers/have_gitlab_http_status.rb index 3198f1b9edd..e7e418cdde4 100644 --- a/spec/support/matchers/have_gitlab_http_status.rb +++ b/spec/support/matchers/have_gitlab_http_status.rb @@ -8,7 +8,11 @@ RSpec::Matchers.define :have_gitlab_http_status do |expected| end failure_message do |actual| + # actual can be either an ActionDispatch::TestResponse (which uses #response_code) + # or a Capybara::Session (which uses #status_code) + response_code = actual.try(:response_code) || actual.try(:status_code) + "expected the response to have status code #{expected.inspect}" \ - " but it was #{actual.response_code}. The response was: #{actual.body}" + " but it was #{response_code}. The response was: #{actual.body}" end end |