summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSean McGivern <sean@mcgivern.me.uk>2017-07-14 08:49:36 +0000
committerSean McGivern <sean@mcgivern.me.uk>2017-07-14 08:49:36 +0000
commit1afdbe320e60aa227dfd75ae1f1f128106660c8a (patch)
tree38e91b764c45b2f0074829560325d0f4b2b4ae39
parentfebd3884a293859a63d2a7ae83b969fc33448f26 (diff)
parent672a68d3724bcae676d18244c85566e7d664a169 (diff)
downloadgitlab-ce-1afdbe320e60aa227dfd75ae1f1f128106660c8a.tar.gz
Merge branch 'fixes-for-internal-auth-disabled' into 'master'
Fixes needed when GitLab sign-in is not enabled See merge request !12491
-rw-r--r--app/controllers/admin/application_settings_controller.rb2
-rw-r--r--app/controllers/application_controller.rb2
-rw-r--r--app/controllers/passwords_controller.rb12
-rw-r--r--app/controllers/profiles/passwords_controller.rb2
-rw-r--r--app/controllers/sessions_controller.rb2
-rw-r--r--app/helpers/application_settings_helper.rb2
-rw-r--r--app/helpers/button_helper.rb4
-rw-r--r--app/helpers/projects_helper.rb4
-rw-r--r--app/models/application_setting.rb2
-rw-r--r--app/models/user.rb12
-rw-r--r--app/views/admin/application_settings/_form.html.haml6
-rw-r--r--app/views/devise/sessions/new.html.haml6
-rw-r--r--app/views/devise/shared/_signin_box.html.haml4
-rw-r--r--app/views/devise/shared/_tabs_ldap.html.haml4
-rw-r--r--app/views/devise/shared/_tabs_normal.html.haml2
-rw-r--r--app/views/layouts/nav/_profile.html.haml2
-rw-r--r--changelogs/unreleased/fixes-for-internal-auth-disabled.yml4
-rw-r--r--config/initializers/1_settings.rb2
-rw-r--r--db/migrate/20170629171610_rename_application_settings_signin_enabled_to_password_authentication_enabled.rb15
-rw-r--r--db/post_migrate/20170629180131_cleanup_application_settings_signin_enabled_rename.rb15
-rw-r--r--db/schema.rb2
-rw-r--r--doc/api/settings.md6
-rw-r--r--lib/api/entities.rb3
-rw-r--r--lib/api/settings.rb9
-rw-r--r--lib/api/v3/entities.rb3
-rw-r--r--lib/api/v3/settings.rb14
-rw-r--r--lib/gitlab/auth.rb6
-rw-r--r--spec/controllers/application_controller_spec.rb9
-rw-r--r--spec/controllers/passwords_controller_spec.rb29
-rw-r--r--spec/features/profiles/password_spec.rb80
-rw-r--r--spec/features/projects/no_password_spec.rb2
-rw-r--r--spec/helpers/button_helper_spec.rb2
-rw-r--r--spec/helpers/projects_helper_spec.rb4
-rw-r--r--spec/lib/gitlab/auth_spec.rb12
-rw-r--r--spec/lib/gitlab/fake_application_settings_spec.rb10
-rw-r--r--spec/models/user_spec.rb22
-rw-r--r--spec/requests/api/settings_spec.rb6
-rw-r--r--spec/requests/api/v3/settings_spec.rb6
-rw-r--r--spec/requests/git_http_spec.rb2
-rw-r--r--spec/requests/jwt_controller_spec.rb2
40 files changed, 247 insertions, 86 deletions
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/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..6a62c7ab0be 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
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/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/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/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/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/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/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/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/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/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/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)