Commit 672a68d3 authored by Robin Bobbitt's avatar Robin Bobbitt

Fixes needed when GitLab sign-in is not enabled

When sign-in is disabled:
 - skip password expiration checks
 - prevent password reset requests
 - don’t show Password tab in User Settings
 - don’t allow login with username/password for Git over HTTP requests
 - render 404 on requests to Profiles::PasswordsController
parent 31ada792
...@@ -113,6 +113,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController ...@@ -113,6 +113,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
:html_emails_enabled, :html_emails_enabled,
:koding_enabled, :koding_enabled,
:koding_url, :koding_url,
:password_authentication_enabled,
:plantuml_enabled, :plantuml_enabled,
:plantuml_url, :plantuml_url,
:max_artifacts_size, :max_artifacts_size,
...@@ -135,7 +136,6 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController ...@@ -135,7 +136,6 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
:require_two_factor_authentication, :require_two_factor_authentication,
:session_expire_delay, :session_expire_delay,
:sign_in_text, :sign_in_text,
:signin_enabled,
:signup_enabled, :signup_enabled,
:sentry_dsn, :sentry_dsn,
:sentry_enabled, :sentry_enabled,
......
...@@ -170,7 +170,7 @@ class ApplicationController < ActionController::Base ...@@ -170,7 +170,7 @@ class ApplicationController < ActionController::Base
end end
def check_password_expiration 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 return redirect_to new_profile_password_path
end end
end end
......
class PasswordsController < Devise::PasswordsController class PasswordsController < Devise::PasswordsController
include Gitlab::CurrentSettings
before_action :resource_from_email, only: [:create] 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] before_action :throttle_reset, only: [:create]
def edit def edit
...@@ -25,7 +27,7 @@ class PasswordsController < Devise::PasswordsController ...@@ -25,7 +27,7 @@ class PasswordsController < Devise::PasswordsController
def update def update
super do |resource| super do |resource|
if resource.valid? && resource.require_password? if resource.valid? && resource.require_password_creation?
resource.update_attribute(:password_automatically_set, false) resource.update_attribute(:password_automatically_set, false)
end end
end end
...@@ -38,11 +40,11 @@ class PasswordsController < Devise::PasswordsController ...@@ -38,11 +40,11 @@ class PasswordsController < Devise::PasswordsController
self.resource = resource_class.find_by_email(email) self.resource = resource_class.find_by_email(email)
end end
def prevent_ldap_reset def check_password_authentication_available
return unless resource && resource.ldap_user? 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), redirect_to after_sending_reset_password_instructions_path_for(resource_name),
alert: "Cannot reset password for LDAP user." alert: "Password authentication is unavailable."
end end
def throttle_reset def throttle_reset
......
...@@ -77,7 +77,7 @@ class Profiles::PasswordsController < Profiles::ApplicationController ...@@ -77,7 +77,7 @@ class Profiles::PasswordsController < Profiles::ApplicationController
end end
def authorize_change_password! def authorize_change_password!
return render_404 if @user.ldap_user? render_404 unless @user.allow_password_authentication?
end end
def user_params def user_params
......
...@@ -58,7 +58,7 @@ class SessionsController < Devise::SessionsController ...@@ -58,7 +58,7 @@ class SessionsController < Devise::SessionsController
user = User.admins.last user = User.admins.last
return unless user && user.require_password? return unless user && user.require_password_creation?
Users::UpdateService.new(user).execute do |user| Users::UpdateService.new(user).execute do |user|
@token = user.generate_reset_token @token = user.generate_reset_token
......
module ApplicationSettingsHelper module ApplicationSettingsHelper
delegate :gravatar_enabled?, delegate :gravatar_enabled?,
:signup_enabled?, :signup_enabled?,
:signin_enabled?, :password_authentication_enabled?,
:akismet_enabled?, :akismet_enabled?,
:koding_enabled?, :koding_enabled?,
to: :current_application_settings to: :current_application_settings
......
...@@ -50,12 +50,12 @@ module ButtonHelper ...@@ -50,12 +50,12 @@ module ButtonHelper
def http_clone_button(project, placement = 'right', append_link: true) def http_clone_button(project, placement = 'right', append_link: true)
klass = 'http-selector' 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 protocol = gitlab_config.protocol.upcase
tooltip_title = 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 } _("Set a password on your account to pull or push via %{protocol}.") % { protocol: protocol }
else else
_("Create a personal access token on your account to pull or push via %{protocol}.") % { protocol: protocol } _("Create a personal access token on your account to pull or push via %{protocol}.") % { protocol: protocol }
......
...@@ -214,11 +214,11 @@ module ProjectsHelper ...@@ -214,11 +214,11 @@ module ProjectsHelper
def show_no_password_message? def show_no_password_message?
cookies[:hide_no_password_message].blank? && !current_user.hide_no_password && 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 end
def link_to_set_password 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 link_to s_('SetPasswordToCloneLink|set a password'), edit_profile_password_path
else else
link_to s_('CreateTokenToCloneLink|create a personal access token'), profile_personal_access_tokens_path link_to s_('CreateTokenToCloneLink|create a personal access token'), profile_personal_access_tokens_path
......
...@@ -237,6 +237,7 @@ class ApplicationSetting < ActiveRecord::Base ...@@ -237,6 +237,7 @@ class ApplicationSetting < ActiveRecord::Base
koding_url: nil, koding_url: nil,
max_artifacts_size: Settings.artifacts['max_size'], max_artifacts_size: Settings.artifacts['max_size'],
max_attachment_size: Settings.gitlab['max_attachment_size'], max_attachment_size: Settings.gitlab['max_attachment_size'],
password_authentication_enabled: Settings.gitlab['password_authentication_enabled'],
performance_bar_allowed_group_id: nil, performance_bar_allowed_group_id: nil,
plantuml_enabled: false, plantuml_enabled: false,
plantuml_url: nil, plantuml_url: nil,
...@@ -251,7 +252,6 @@ class ApplicationSetting < ActiveRecord::Base ...@@ -251,7 +252,6 @@ class ApplicationSetting < ActiveRecord::Base
shared_runners_text: nil, shared_runners_text: nil,
sidekiq_throttling_enabled: false, sidekiq_throttling_enabled: false,
sign_in_text: nil, sign_in_text: nil,
signin_enabled: Settings.gitlab['signin_enabled'],
signup_enabled: Settings.gitlab['signup_enabled'], signup_enabled: Settings.gitlab['signup_enabled'],
terminal_max_session_time: 0, terminal_max_session_time: 0,
two_factor_grace_period: 48, two_factor_grace_period: 48,
......
...@@ -580,16 +580,20 @@ class User < ActiveRecord::Base ...@@ -580,16 +580,20 @@ class User < ActiveRecord::Base
keys.count == 0 && Gitlab::ProtocolAccess.allowed?('ssh') keys.count == 0 && Gitlab::ProtocolAccess.allowed?('ssh')
end end
def require_password? def require_password_creation?
password_automatically_set? && !ldap_user? && current_application_settings.signin_enabled? password_automatically_set? && allow_password_authentication?
end end
def require_personal_access_token? def require_personal_access_token_creation_for_git_auth?
return false if current_application_settings.signin_enabled? || ldap_user? return false if allow_password_authentication? || ldap_user?
PersonalAccessTokensFinder.new(user: self, impersonation: false, state: 'active').execute.none? PersonalAccessTokensFinder.new(user: self, impersonation: false, state: 'active').execute.none?
end end
def allow_password_authentication?
!ldap_user? && current_application_settings.password_authentication_enabled?
end
def can_change_username? def can_change_username?
gitlab_config.username_changing_enabled gitlab_config.username_changing_enabled
end end
......
...@@ -145,9 +145,9 @@ ...@@ -145,9 +145,9 @@
.form-group .form-group
.col-sm-offset-2.col-sm-10 .col-sm-offset-2.col-sm-10
.checkbox .checkbox
= f.label :signin_enabled do = f.label :password_authentication_enabled do
= f.check_box :signin_enabled = f.check_box :password_authentication_enabled
Sign-in enabled Password authentication enabled
- if omniauth_enabled? && button_based_providers.any? - if omniauth_enabled? && button_based_providers.any?
.form-group .form-group
= f.label :enabled_oauth_sign_in_sources, 'Enabled OAuth sign-in sources', class: 'control-label col-sm-2' = f.label :enabled_oauth_sign_in_sources, 'Enabled OAuth sign-in sources', class: 'control-label col-sm-2'
......
...@@ -6,15 +6,15 @@ ...@@ -6,15 +6,15 @@
- else - else
= render 'devise/shared/tabs_normal' = render 'devise/shared/tabs_normal'
.tab-content .tab-content
- if signin_enabled? || ldap_enabled? || crowd_enabled? - if password_authentication_enabled? || ldap_enabled? || crowd_enabled?
= render 'devise/shared/signin_box' = render 'devise/shared/signin_box'
-# Signup only makes sense if you can also sign-in -# 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' = render 'devise/shared/signup_box'
-# Show a message if none of the mechanisms above are enabled -# 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 %div
No authentication methods configured. No authentication methods configured.
......
...@@ -7,12 +7,12 @@ ...@@ -7,12 +7,12 @@
.login-box.tab-pane{ id: "#{server['provider_name']}", role: 'tabpanel', class: active_when(i.zero? && !crowd_enabled?) } .login-box.tab-pane{ id: "#{server['provider_name']}", role: 'tabpanel', class: active_when(i.zero? && !crowd_enabled?) }
.login-body .login-body
= render 'devise/sessions/new_ldap', server: server = render 'devise/sessions/new_ldap', server: server
- if signin_enabled? - if password_authentication_enabled?
.login-box.tab-pane{ id: 'ldap-standard', role: 'tabpanel' } .login-box.tab-pane{ id: 'ldap-standard', role: 'tabpanel' }
.login-body .login-body
= render 'devise/sessions/new_base' = render 'devise/sessions/new_base'
- elsif signin_enabled? - elsif password_authentication_enabled?
.login-box.tab-pane.active{ id: 'login-pane', role: 'tabpanel' } .login-box.tab-pane.active{ id: 'login-pane', role: 'tabpanel' }
.login-body .login-body
= render 'devise/sessions/new_base' = render 'devise/sessions/new_base'
...@@ -5,9 +5,9 @@ ...@@ -5,9 +5,9 @@
- @ldap_servers.each_with_index do |server, i| - @ldap_servers.each_with_index do |server, i|
%li{ class: active_when(i.zero? && !crowd_enabled?) } %li{ class: active_when(i.zero? && !crowd_enabled?) }
= link_to server['label'], "##{server['provider_name']}", 'data-toggle' => 'tab' = link_to server['label'], "##{server['provider_name']}", 'data-toggle' => 'tab'
- if signin_enabled? - if password_authentication_enabled?
%li %li
= link_to 'Standard', '#ldap-standard', 'data-toggle' => 'tab' = link_to 'Standard', '#ldap-standard', 'data-toggle' => 'tab'
- if signin_enabled? && signup_enabled? - if password_authentication_enabled? && signup_enabled?
%li %li
= link_to 'Register', '#register-pane', 'data-toggle' => 'tab' = link_to 'Register', '#register-pane', 'data-toggle' => 'tab'
%ul.nav-links.new-session-tabs.nav-tabs{ role: 'tablist' } %ul.nav-links.new-session-tabs.nav-tabs{ role: 'tablist' }
%li.active{ role: 'presentation' } %li.active{ role: 'presentation' }
%a{ href: '#login-pane', data: { toggle: 'tab' }, role: 'tab' } Sign in %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' } %li{ role: 'presentation' }
%a{ href: '#register-pane', data: { toggle: 'tab' }, role: 'tab' } Register %a{ href: '#register-pane', data: { toggle: 'tab' }, role: 'tab' } Register
...@@ -29,7 +29,7 @@ ...@@ -29,7 +29,7 @@
= link_to profile_emails_path, title: 'Emails' do = link_to profile_emails_path, title: 'Emails' do
%span %span
Emails Emails
- unless current_user.ldap_user? - if current_user.allow_password_authentication?
= nav_link(controller: :passwords) do = nav_link(controller: :passwords) do
= link_to edit_profile_password_path, title: 'Password' do = link_to edit_profile_password_path, title: 'Password' do
%span %span
......
---
title: Fixes needed when GitLab sign-in is not enabled
merge_request: 12491
author: Robin Bobbitt
...@@ -223,7 +223,7 @@ rescue ArgumentError # no user configured ...@@ -223,7 +223,7 @@ rescue ArgumentError # no user configured
end end
Settings.gitlab['time_zone'] ||= nil Settings.gitlab['time_zone'] ||= nil
Settings.gitlab['signup_enabled'] ||= true if Settings.gitlab['signup_enabled'].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['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['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? 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?
......
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
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
...@@ -41,7 +41,6 @@ ActiveRecord::Schema.define(version: 20170707184244) do ...@@ -41,7 +41,6 @@ ActiveRecord::Schema.define(version: 20170707184244) do
create_table "application_settings", force: :cascade do |t| create_table "application_settings", force: :cascade do |t|
t.integer "default_projects_limit" t.integer "default_projects_limit"
t.boolean "signup_enabled" t.boolean "signup_enabled"
t.boolean "signin_enabled"
t.boolean "gravatar_enabled" t.boolean "gravatar_enabled"
t.text "sign_in_text" t.text "sign_in_text"
t.datetime "created_at" t.datetime "created_at"
...@@ -127,6 +126,7 @@ ActiveRecord::Schema.define(version: 20170707184244) do ...@@ -127,6 +126,7 @@ ActiveRecord::Schema.define(version: 20170707184244) do
t.boolean "help_page_hide_commercial_content", default: false t.boolean "help_page_hide_commercial_content", default: false
t.string "help_page_support_url" t.string "help_page_support_url"
t.integer "performance_bar_allowed_group_id" t.integer "performance_bar_allowed_group_id"
t.boolean "password_authentication_enabled"
end end
create_table "audit_events", force: :cascade do |t| create_table "audit_events", force: :cascade do |t|
......
...@@ -25,7 +25,7 @@ Example response: ...@@ -25,7 +25,7 @@ Example response:
"id" : 1, "id" : 1,
"default_branch_protection" : 2, "default_branch_protection" : 2,
"restricted_visibility_levels" : [], "restricted_visibility_levels" : [],
"signin_enabled" : true, "password_authentication_enabled" : true,
"after_sign_out_path" : null, "after_sign_out_path" : null,
"max_attachment_size" : 10, "max_attachment_size" : 10,
"user_oauth_applications" : true, "user_oauth_applications" : true,
...@@ -63,7 +63,7 @@ PUT /application/settings ...@@ -63,7 +63,7 @@ PUT /application/settings
| --------- | ---- | :------: | ----------- | | --------- | ---- | :------: | ----------- |
| `default_projects_limit` | integer | no | Project limit per user. Default is `100000` | | `default_projects_limit` | integer | no | Project limit per user. Default is `100000` |
| `signup_enabled` | boolean | no | Enable registration. Default is `true`. | | `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 | | `gravatar_enabled` | boolean | no | Enable Gravatar |
| `sign_in_text` | string | no | Text on login page | | `sign_in_text` | string | no | Text on login page |
| `home_page_url` | string | no | Redirect to this URL when not logged in | | `home_page_url` | string | no | Redirect to this URL when not logged in |
...@@ -102,7 +102,7 @@ Example response: ...@@ -102,7 +102,7 @@ Example response:
"id": 1, "id": 1,
"default_projects_limit": 100000, "default_projects_limit": 100000,
"signup_enabled": true, "signup_enabled": true,
"signin_enabled": true, "password_authentication_enabled": true,
"gravatar_enabled": true, "gravatar_enabled": true,
"sign_in_text": "", "sign_in_text": "",
"created_at": "2015-06-12T15:51:55.432Z", "created_at": "2015-06-12T15:51:55.432Z",
......
...@@ -614,7 +614,8 @@ module API ...@@ -614,7 +614,8 @@ module API
expose :id expose :id
expose :default_projects_limit expose :default_projects_limit
expose :signup_enabled expose :signup_enabled
expose :signin_enabled expose :password_authentication_enabled
expose :password_authentication_enabled, as: :signin_enabled
expose :gravatar_enabled expose :gravatar_enabled
expose :sign_in_text expose :sign_in_text
expose :after_sign_up_text expose :after_sign_up_text
......
...@@ -65,6 +65,7 @@ module API ...@@ -65,6 +65,7 @@ module API
:shared_runners_enabled, :shared_runners_enabled,
:sidekiq_throttling_enabled, :sidekiq_throttling_enabled,
:sign_in_text, :sign_in_text,
:password_authentication_enabled,
:signin_enabled, :signin_enabled,
:signup_enabled, :signup_enabled,
:terminal_max_session_time, :terminal_max_session_time,
...@@ -95,7 +96,9 @@ module API ...@@ -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' 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 end
optional :after_sign_up_text, type: String, desc: 'Text shown after sign up' 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' optional :require_two_factor_authentication, type: Boolean, desc: 'Require all users to setup Two-factor authentication'
given require_two_factor_authentication: ->(val) { val } do 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' 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 ...@@ -176,6 +179,10 @@ module API
put "application/settings" do put "application/settings" do
attrs = 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) if current_settings.update_attributes(attrs)
present current_settings, with: Entities::ApplicationSetting present current_settings, with: Entities::ApplicationSetting
else else
......
...@@ -161,7 +161,8 @@ module API ...@@ -161,7 +161,8 @@ module API
expose :id expose :id
expose :default_projects_limit expose :default_projects_limit
expose :signup_enabled expose :signup_enabled
expose :signin_enabled expose :password_authentication_enabled
expose :password_authentication_enabled, as: :signin_enabled
expose :gravatar_enabled expose :gravatar_enabled
expose :sign_in_text expose :sign_in_text
expose :after_sign_up_text expose :after_sign_up_text
......
...@@ -44,7 +44,9 @@ module API ...@@ -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' 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 end
optional :after_sign_up_text, type: String, desc: 'Text shown after sign up' 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' optional :require_two_factor_authentication, type: Boolean, desc: 'Require all users to setup Two-factor authentication'
given require_two_factor_authentication: ->(val) { val } do 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' 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 ...@@ -116,7 +118,7 @@ module API
:max_attachment_size, :session_expire_delay, :disabled_oauth_sign_in_sources, :max_attachment_size, :session_expire_delay, :disabled_oauth_sign_in_sources,
:user_oauth_applications, :user_default_external, :signup_enabled, :user_oauth_applications, :user_default_external, :signup_enabled,
:send_user_confirmation_email, :domain_whitelist, :domain_blacklist_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, :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, :shared_runners_enabled, :max_artifacts_size, :max_pages_size, :container_registry_token_expire_delay,
:metrics_enabled, :sidekiq_throttling_enabled, :recaptcha_enabled, :metrics_enabled, :sidekiq_throttling_enabled, :recaptcha_enabled,
...@@ -126,7 +128,13 @@ module API ...@@ -126,7 +128,13 @@ module API
:housekeeping_enabled, :terminal_max_session_time :housekeeping_enabled, :terminal_max_session_time
end end
put "application/settings" do 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 present current_settings, with: Entities::ApplicationSetting
else else
render_validation_error!(current_settings) render_validation_error!(current_settings)
......
...@@ -37,7 +37,7 @@ module Gitlab ...@@ -37,7 +37,7 @@ module Gitlab
rate_limit!(ip, success: result.success?, login: login) rate_limit!(ip, success: result.success?, login: login)
Gitlab::Auth::UniqueIpsLimiter.limit_user!(result.actor) 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 # If sign-in is disabled and LDAP is not configured, recommend a
# personal access token on failed auth attempts # personal access token on failed auth attempts
...@@ -48,6 +48,10 @@ module Gitlab ...@@ -48,6 +48,10 @@ module Gitlab
# Avoid resource intensive login checks if password is not provided # Avoid resource intensive login checks if password is not provided
return unless password.present? 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 Gitlab::Auth::UniqueIpsLimiter.limit_user! do
user = User.by_login(login) user = User.by_login(login)
......
...@@ -30,6 +30,15 @@ describe ApplicationController do ...@@ -30,6 +30,15 @@ describe ApplicationController do
expect(controller).not_to receive(:redirect_to) expect(controller).not_to receive(:redirect_to)
controller.send(:check_password_expiration) controller.send(:check_password_expiration)
end 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 end
describe "#authenticate_user_from_token!" do describe "#authenticate_user_from_token!" do
......
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
require 'spec_helper' require 'spec_helper'
describe 'Profile > Password', feature: true do 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 before do
sign_in(user) sign_in(user)
visit edit_profile_password_path visit edit_profile_password_path
end end
def fill_passwords(password, confirmation) def fill_passwords(password, confirmation)
fill_in 'New password', with: password fill_in 'New password', with: password
fill_in 'Password confirmation', with: confirmation fill_in 'Password confirmation', with: confirmation
click_button 'Save password' click_button 'Save password'
end 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 page.within('.alert-danger') do
describe 'User puts different passwords in the field and in the confirmation' do expect(page).to have_content("Password confirmation doesn't match Password")
it 'shows an error message' do end
fill_passwords('mypassword', 'mypassword2') 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_no_field('user[current_password]')
expect(page).to have_content("Password confirmation doesn't match Password")
end end
end end
it 'does not contain the current password field after an error' do describe 'User puts the same passwords in the field and in the confirmation' do
fill_passwords('mypassword', 'mypassword2') 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 end
end
describe 'User puts the same passwords in the field and in the confirmation' do context 'Password authentication unavailable' do
it 'shows a success message' do before do
fill_passwords('mypassword', 'mypassword') gitlab_sign_in(user)
end
page.within('.flash-notice') do context 'Regular user' do
expect(page).to have_content('Password was successfully updated. Please login with it') let(:user) { create(:user) }
end
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 end
end end
......
...@@ -30,7 +30,7 @@ feature 'No Password Alert' do ...@@ -30,7 +30,7 @@ feature 'No Password Alert' do
let(:user) { create(:omniauth_user, extern_uid: 'my-uid', provider: 'saml') } let(:user) { create(:omniauth_user, extern_uid: 'my-uid', provider: 'saml') }
before do 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]) stub_omniauth_saml_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], providers: [mock_saml_config])
end end
......
...@@ -35,7 +35,7 @@ describe ButtonHelper do ...@@ -35,7 +35,7 @@ describe ButtonHelper do
context 'with internal auth disabled' do context 'with internal auth disabled' do
before do before do
stub_application_setting(signin_enabled?: false) stub_application_setting(password_authentication_enabled?: false)
end end
context 'when user has no personal access tokens' do context 'when user has no personal access tokens' do
......
...@@ -160,7 +160,7 @@ describe ProjectsHelper do ...@@ -160,7 +160,7 @@ describe ProjectsHelper do
context 'user requires a personal access token' do context 'user requires a personal access token' do
it 'returns true' 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 expect(helper.show_no_password_message?).to be_truthy
end end
...@@ -184,7 +184,7 @@ describe ProjectsHelper do ...@@ -184,7 +184,7 @@ describe ProjectsHelper do
let(:user) { create(:user) } let(:user) { create(:user) }
it 'returns link to create a personal access token' do 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>} expect(helper.link_to_set_password).to match %r{<a href="#{profile_personal_access_tokens_path}">create a personal access token</a>}
end end
......
...@@ -206,7 +206,7 @@ describe Gitlab::Auth, lib: true do ...@@ -206,7 +206,7 @@ describe Gitlab::Auth, lib: true do
end end
it 'throws an error suggesting user create a PAT when internal auth is disabled' do 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) expect { gl_auth.find_for_git_client('foo', 'bar', project: nil, ip: 'ip') }.to raise_error(Gitlab::Auth::MissingPersonalTokenError)
end end
...@@ -279,6 +279,16 @@ describe Gitlab::Auth, lib: true do ...@@ -279,6 +279,16 @@ describe Gitlab::Auth, lib: true do
gl_auth.find_with_user_password('ldap_user', 'password') gl_auth.find_with_user_password('ldap_user', 'password')
end end
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 end
private private
......
require 'spec_helper' require 'spec_helper'
describe Gitlab::FakeApplicationSettings do 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) } subject { described_class.new(defaults) }
it 'wraps OpenStruct variables properly' do 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.signup_enabled).to be_truthy
expect(subject.foobar).to eq('asdf') expect(subject.foobar).to eq('asdf')
end end
it 'defines predicate methods' do 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 expect(subject.signup_enabled?).to be_truthy
end end
it 'predicate method changes when value is updated' do 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 end
it 'does not define a predicate method' do it 'does not define a predicate method' do
......
...@@ -1920,4 +1920,26 @@ describe User, models: true do ...@@ -1920,4 +1920,26 @@ describe User, models: true do
user.invalidate_merge_request_cache_counts user.invalidate_merge_request_cache_counts
end end
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 end
...@@ -10,7 +10,7 @@ describe API::Settings, 'Settings' do ...@@ -10,7 +10,7 @@ describe API::Settings, 'Settings' do
expect(response).to have_http_status(200) expect(response).to have_http_status(200)
expect(json_response).to be_an Hash expect(json_response).to be_an Hash
expect(json_response['default_projects_limit']).to eq(42) 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['repository_storage']).to eq('default')
expect(json_response['koding_enabled']).to be_falsey expect(json_response['koding_enabled']).to be_falsey
expect(json_response['koding_url']).to be_nil expect(json_response['koding_url']).to be_nil
...@@ -32,7 +32,7 @@ describe API::Settings, 'Settings' do ...@@ -32,7 +32,7 @@ describe API::Settings, 'Settings' do
it "updates application settings" do it "updates application settings" do
put api("/application/settings", admin), put api("/application/settings", admin),
default_projects_limit: 3, default_projects_limit: 3,
signin_enabled: false, password_authentication_enabled: false,
repository_storage: 'custom', repository_storage: 'custom',
koding_enabled: true, koding_enabled: true,
koding_url: 'http://koding.example.com', koding_url: 'http://koding.example.com',
...@@ -46,7 +46,7 @@ describe API::Settings, 'Settings' do ...@@ -46,7 +46,7 @@ describe API::Settings, 'Settings' do
help_page_support_url: 'http://example.com/help' help_page_support_url: 'http://example.com/help'
expect(response).to have_http_status(200) expect(response).to have_http_status(200)
expect(json_response['default_projects_limit']).to eq(3) 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_storage']).to eq('custom')
expect(json_response['repository_storages']).to eq(['custom']) expect(json_response['repository_storages']).to eq(['custom'])
expect(json_response['koding_enabled']).to be_truthy expect(json_response['koding_enabled']).to be_truthy
......
...@@ -10,7 +10,7 @@ describe API::V3::Settings, 'Settings' do ...@@ -10,7 +10,7 @@ describe API::V3::Settings, 'Settings' do
expect(response).to have_http_status(200) expect(response).to have_http_status(200)
expect(json_response).to be_an Hash expect(json_response).to be_an Hash
expect(json_response['default_projects_limit']).to eq(42) 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['repository_storage']).to eq('default')
expect(json_response['koding_enabled']).to be_falsey expect(json_response['koding_enabled']).to be_falsey
expect(json_response['koding_url']).to be_nil expect(json_response['koding_url']).to be_nil
...@@ -28,11 +28,11 @@ describe API::V3::Settings, 'Settings' do ...@@ -28,11 +28,11 @@ describe API::V3::Settings, 'Settings' do
it "updates application settings" do it "updates application settings" do
put v3_api("/application/settings", admin), 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' plantuml_enabled: true, plantuml_url: 'http://plantuml.example.com'
expect(response).to have_http_status(200) expect(response).to have_http_status(200)
expect(json_response['default_projects_limit']).to eq(3) 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_storage']).to eq('custom')
expect(json_response['repository_storages']).to eq(['custom']) expect(json_response['repository_storages']).to eq(['custom'])
expect(json_response['koding_enabled']).to be_truthy expect(json_response['koding_enabled']).to be_truthy
......
...@@ -463,7 +463,7 @@ describe 'Git HTTP requests', lib: true do ...@@ -463,7 +463,7 @@ describe 'Git HTTP requests', lib: true do
context 'when internal auth is disabled' do context 'when internal auth is disabled' do
before 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 end
it 'rejects pulls with personal access token error message' do it 'rejects pulls with personal access token error message' do
......
...@@ -101,7 +101,7 @@ describe JwtController do ...@@ -101,7 +101,7 @@ describe JwtController do
context 'when internal auth is disabled' do context 'when internal auth is disabled' do
it 'rejects the authorization attempt with personal access token message' 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 get '/jwt/auth', parameters, headers
expect(response).to have_http_status(401) expect(response).to have_http_status(401)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment