From 3749b2486630d8fa7f1ccbe863fcf3304ecc4519 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miquel=20Sabat=C3=A9=20Sol=C3=A0?= Date: Thu, 3 Sep 2015 13:45:27 +0200 Subject: [PATCH] Introducing LDAP support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is an introduction to LDAP support. There are some questions that need to be addressed, but at least the basic structure is in place now. Fixes #150 Signed-off-by: Miquel Sabaté Solà --- CHANGELOG.md | 1 + Gemfile | 1 + Gemfile.lock | 2 + app/controllers/application_controller.rb | 17 +- app/models/user.rb | 16 +- app/views/devise/registrations/edit.html.slim | 79 ++++---- app/views/shared/_header.html.slim | 2 +- config/config.yml | 13 +- config/initializers/config.rb | 21 +- config/initializers/devise.rb | 5 + config/initializers/ldap_authenticatable.rb | 2 + .../20150831131727_add_ldap_name_to_users.rb | 6 + db/schema.rb | 4 +- lib/assets/.keep | 0 lib/portus/ldap.rb | 109 ++++++++++ spec/api/v2/token_spec.rb | 18 +- spec/features/application_spec.rb | 27 +++ spec/features/gravatar_spec.rb | 4 +- spec/lib/portus/ldap_spec.rb | 188 ++++++++++++++++++ spec/models/user_spec.rb | 17 +- spec/spec_helper.rb | 3 + 21 files changed, 479 insertions(+), 56 deletions(-) create mode 100644 config/initializers/ldap_authenticatable.rb create mode 100644 db/migrate/20150831131727_add_ldap_name_to_users.rb delete mode 100644 lib/assets/.keep create mode 100644 lib/portus/ldap.rb create mode 100644 spec/features/application_spec.rb create mode 100644 spec/lib/portus/ldap_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index f8e43858d..93aee3ae1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## Upcoming Version +- Introduced LDAP support. See PR [#301](https://github.com/SUSE/Portus/pull/301). - Users will not be able to create namespaces without a Registry currently existing. - PhantomJS is now being used in the testing infrastructure. See the following diff --git a/Gemfile b/Gemfile index a80f668ef..28b79ceaa 100644 --- a/Gemfile +++ b/Gemfile @@ -18,6 +18,7 @@ gem "mysql2" gem "search_cop" gem "kaminari" gem "crono" +gem "net-ldap" # Assets group. # diff --git a/Gemfile.lock b/Gemfile.lock index 1cd952452..5f6267e83 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -146,6 +146,7 @@ GEM multi_json (1.11.1) multipart-post (2.0.0) mysql2 (0.3.18) + net-ldap (0.11) nokogiri (1.6.6.2) mini_portile (~> 0.6.0) octokit (2.0.0) @@ -339,6 +340,7 @@ DEPENDENCIES jwt kaminari mysql2 + net-ldap poltergeist pry-rails public_activity diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 8ee2e934e..70de6ba7c 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,6 +1,7 @@ class ApplicationController < ActionController::Base before_action :check_requirements before_action :authenticate_user! + before_action :force_update_profile! protect_from_forgery with: :exception include Pundit @@ -8,8 +9,12 @@ class ApplicationController < ActionController::Base respond_to :html + # Two things can happen when signing in. + # 1. The current user has no email: this happens on LDAP registration. In + # this case, the user will be asked to submit an email. + # 2. Everything is fine, go to the root url. def after_sign_in_path_for(_resource) - root_url + current_user.email.empty? ? edit_user_registration_url : root_url end def after_sign_out_path_for(_resource) @@ -39,6 +44,16 @@ def fixes [fix_secrets, fix_ssl] end + # Redirect users to their profile page if they haven't set up their email + # account (this happens when signing up through LDAP suppor). + def force_update_profile! + return unless current_user && current_user.email.empty? + + controller = params[:controller] + return if controller == "auth/registrations" || controller == "auth/sessions" + redirect_to edit_user_registration_url + end + def deny_access render text: "Access Denied", status: :unauthorized end diff --git a/app/models/user.rb b/app/models/user.rb index 5f7acee5e..b5d15ce4b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -2,10 +2,14 @@ class User < ActiveRecord::Base devise :database_authenticatable, :registerable, :recoverable, :rememberable, :trackable, :validatable, authentication_keys: [:username] - validates :username, presence: true, uniqueness: true, - format: { with: /\A[a-z0-9]{4,30}\Z/, - message: 'Accepted format: "\A[a-z0-9]{4,30}\Z"' } + USERNAME_CHARS = "a-z0-9" + USERNAME_FORMAT = /\A[#{USERNAME_CHARS}]{4,30}\Z/ + validates :username, presence: true, uniqueness: true, + format: { + with: USERNAME_FORMAT, + message: "Only alphanumeric characters are allowed. Minimum 4 characters, maximum 30." + } validate :private_namespace_available, on: :create has_many :team_users @@ -16,6 +20,12 @@ class User < ActiveRecord::Base scope :enabled, -> { not_portus.where enabled: true } scope :admins, -> { not_portus.where enabled: true, admin: true } + # Special method used by Devise to require an email on signup. This is always + # true except for LDAP. + def email_required? + !(Portus::LDAP.enabled? && !ldap_name.nil?) + end + def private_namespace_available return unless Namespace.exists?(name: username) errors.add(:username, "cannot be used as name for private namespace") diff --git a/app/views/devise/registrations/edit.html.slim b/app/views/devise/registrations/edit.html.slim index 7dea21f7f..1e1c2f776 100644 --- a/app/views/devise/registrations/edit.html.slim +++ b/app/views/devise/registrations/edit.html.slim @@ -7,48 +7,55 @@ ' Public Profile .panel-body = form_for(resource, as: resource_name, url: registration_path(resource_name), html: { method: :put, class: 'profile' }) do |f| - .form-group + - if current_user.email.empty? + p + | Your profile is not complete. Please, submit an email to be used in this Portus instance. + .form-group class=(current_user.email.empty? ? "has-error" : "") .field - = f.label :email - = f.text_field(:email, class: 'form-control', required: true) + - if current_user.email.empty? + = f.label :email, "Email", class: "control-label", title: "This profile is not complete. You need to provide an email first" + - else + = f.label :email + = f.text_field(:email, class: 'form-control', required: true, autofocus: true) .form-group .actions = f.submit('Update', class: 'btn btn-primary', disabled: true) - .panel.panel-default - .panel-heading - h5 - ' Change Password - .panel-body - = form_for(resource, as: resource_name, url: registration_path(resource_name), html: { method: :put, class: 'password' }) do |f| - - if devise_mapping.confirmable? && resource.pending_reconfirmation? - div - Currently waiting confirmation for: #{resource.unconfirmed_email} - .form-group - .field - = f.label :current_password, class: 'control-label' - = f.password_field :current_password, autocomplete: 'off', class: 'form-control' - br - .field - = f.label :password, class: 'control-label' - = f.password_field :password, autocomplete: 'off', class: 'form-control' - br - .field - = f.label :password_confirmation, class: 'control-label' - = f.password_field :password_confirmation, autocomplete: 'off', class: 'form-control' - .form-group - .actions - = f.submit('Change', class: 'btn btn-primary', disabled: true) - - - unless current_user.admin? && @admin_count == 1 + - unless current_user.email.empty? .panel.panel-default .panel-heading h5 - ' Disable account + ' Change Password .panel-body - = form_tag(toggle_enabled_path(current_user), method: :put, remote: true, id: 'disable-form') do - .form-group - p - | By disabling the account, you won't be able to access Portus with it, and - any affiliations with any team will be lost. - = submit_tag('Disable', class: 'btn btn-primary btn-danger') + = form_for(resource, as: resource_name, url: registration_path(resource_name), html: { method: :put, class: 'password' }) do |f| + - if devise_mapping.confirmable? && resource.pending_reconfirmation? + div + Currently waiting confirmation for: #{resource.unconfirmed_email} + .form-group + .field + = f.label :current_password, class: 'control-label' + = f.password_field :current_password, autocomplete: 'off', class: 'form-control' + br + .field + = f.label :password, class: 'control-label' + = f.password_field :password, autocomplete: 'off', class: 'form-control' + br + .field + = f.label :password_confirmation, class: 'control-label' + = f.password_field :password_confirmation, autocomplete: 'off', class: 'form-control' + .form-group + .actions + = f.submit('Change', class: 'btn btn-primary', disabled: true) + + - unless current_user.admin? && @admin_count == 1 + .panel.panel-default + .panel-heading + h5 + ' Disable account + .panel-body + = form_tag(toggle_enabled_path(current_user), method: :put, remote: true, id: 'disable-form') do + .form-group + p + | By disabling the account, you won't be able to access Portus with it, and + any affiliations with any team will be lost. + = submit_tag('Disable', class: 'btn btn-primary btn-danger') diff --git a/app/views/shared/_header.html.slim b/app/views/shared/_header.html.slim index b66ed4f11..86db854eb 100644 --- a/app/views/shared/_header.html.slim +++ b/app/views/shared/_header.html.slim @@ -13,7 +13,7 @@ i.fa.fa-search .username-logout .hidden-xs - - if APP_CONFIG['gravatar'] + - if APP_CONFIG.enabled?("gravatar") = gravatar_image_tag(current_user.email) - else i.fa.fa-user.fa-1x diff --git a/config/config.yml b/config/config.yml index 46af0ae32..66c673f99 100644 --- a/config/config.yml +++ b/config/config.yml @@ -2,5 +2,14 @@ # application. In order to change them, write your own config-local.yml file # (it will be ignored by git). -settings: - gravatar: true +gravatar: + enabled: true + +ldap: + enabled: false + + hostname: "ldap_hostname" + port: 389 + + # The base where users are located (e.g. "ou=users,dc=example,dc=com"). + base: "" diff --git a/config/initializers/config.rb b/config/initializers/config.rb index 8788d8efd..e2bba613b 100644 --- a/config/initializers/config.rb +++ b/config/initializers/config.rb @@ -1,12 +1,17 @@ +# TODO: (mssola) move this into its own file in the `lib` directory. +# TODO: (mssola) take advantage of YAML syntax for inheriting values. This way +# we could define different values for different environments (useful for +# testing). + config = File.join(Rails.root, "config", "config.yml") local = File.join(Rails.root, "config", "config-local.yml") -app_config = YAML.load_file(config)["settings"] || {} +app_config = YAML.load_file(config) || {} if File.exist?(local) # Check for bad user input in the local config.yml file. - local_config = YAML.load_file(local)["settings"] + local_config = YAML.load_file(local) if local_config.nil? || !local_config.is_a?(Hash) raise StandardError, "Wrong format for the config-local file!" end @@ -14,4 +19,16 @@ app_config = app_config.merge(local_config) end +class << app_config + # The `enabled?` method is a convenient method that checks whether a specific + # feature is enabled or not. This method takes advantage of the convention + # that each feature has the "enabled" key inside of it. If this key exists in + # the checked feature, and it's set to true, then this method will return + # true. It returns false otherwise. + def enabled?(feature) + return false if !self[feature] || self[feature].empty? + self[feature]["enabled"].eql?(true) + end +end + APP_CONFIG = app_config diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index b1b29a0cf..771a19181 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -243,6 +243,11 @@ # manager.intercept_401 = false # manager.default_strategies(scope: :user).unshift :some_external_strategy # end + if Portus::LDAP.enabled? && !Rails.env.test? + config.warden do |manager| + manager.default_strategies(scope: :user).unshift :ldap_authenticatable + end + end # ==> Mountable engine configurations # When using Devise inside an engine, let's call it `MyEngine`, and this engine diff --git a/config/initializers/ldap_authenticatable.rb b/config/initializers/ldap_authenticatable.rb new file mode 100644 index 000000000..c7f29c780 --- /dev/null +++ b/config/initializers/ldap_authenticatable.rb @@ -0,0 +1,2 @@ +require "portus/ldap" +Warden::Strategies.add(:ldap_authenticatable, Portus::LDAP) diff --git a/db/migrate/20150831131727_add_ldap_name_to_users.rb b/db/migrate/20150831131727_add_ldap_name_to_users.rb new file mode 100644 index 000000000..79d3682c5 --- /dev/null +++ b/db/migrate/20150831131727_add_ldap_name_to_users.rb @@ -0,0 +1,6 @@ +class AddLdapNameToUsers < ActiveRecord::Migration + def change + add_column :users, :ldap_name, :string, default: nil + add_index :users, :ldap_name, unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index c89757787..61af8e299 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20150805130722) do +ActiveRecord::Schema.define(version: 20150831131727) do create_table "activities", force: :cascade do |t| t.integer "trackable_id", limit: 4 @@ -136,9 +136,11 @@ t.datetime "updated_at" t.boolean "admin", limit: 1, default: false t.boolean "enabled", limit: 1, default: true + t.string "ldap_name", limit: 255 end add_index "users", ["email"], name: "index_users_on_email", unique: true, using: :btree + add_index "users", ["ldap_name"], name: "index_users_on_ldap_name", unique: true, using: :btree add_index "users", ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true, using: :btree add_index "users", ["username"], name: "index_users_on_username", unique: true, using: :btree diff --git a/lib/assets/.keep b/lib/assets/.keep deleted file mode 100644 index e69de29bb..000000000 diff --git a/lib/portus/ldap.rb b/lib/portus/ldap.rb new file mode 100644 index 000000000..ff1c89fe7 --- /dev/null +++ b/lib/portus/ldap.rb @@ -0,0 +1,109 @@ +require "net/ldap" +require "devise/strategies/authenticatable" + +module Portus + # Portus::LDAP implements Devise's authenticatable for LDAP servers. + class LDAP < Devise::Strategies::Authenticatable + # Re-implemented from Devise::Strategies::Authenticatable to authenticate + # the user. + def authenticate! + ldap = load_configuration + + if ldap && ldap.bind_as(bind_options) + user = find_or_create_user! + user.valid? ? success!(user) : raise(:invalid_login) + else + raise(:invalid_login) + end + end + + # Returns true if LDAP has been enabled in the application, false + # otherwise. + def self.enabled? + APP_CONFIG.enabled?("ldap") + end + + protected + + # Loads the configuration and authenticates the current user. + def load_configuration + return nil if !::Portus::LDAP.enabled? || params[:user].nil? + + cfg = APP_CONFIG["ldap"] + adapter(host: cfg["hostname"], port: cfg["port"]) + end + + # Returns the class to be used for LDAP support. Mainly declared this way + # so tests can mock this away. This can also be useful if we decide to jump + # on another gem for LDAP support. + def adapter(opts) + Net::LDAP.new(opts) + end + + # Returns the option hash to be used for the `bind_as` call. It's a mix + # from information that we can gather from the parameters and from the + # configuration. + def bind_options + {}.tap do |opts| + opts[:filter] = "(uid=#{username})" + opts[:password] = password + opts[:base] = APP_CONFIG["ldap"]["base"] unless APP_CONFIG["ldap"]["base"].empty? + end + end + + # Retrieve the given user as an LDAP user. If it doesn't exist, create it + # with the parameters given in the form. + def find_or_create_user! + user = User.find_by(ldap_name: username) + + # The user does not exist in Portus yet, let's create it. If it does + # not match a valid username, it will be transformed into a proper one. + unless user + ldap_name = username.dup + if User::USERNAME_FORMAT.match(ldap_name) + name = ldap_name + else + name = ldap_name.gsub(/[^#{User::USERNAME_CHARS}]/, "") + end + + # This is to check that no clashes occur. This is quite improbable to + # happen, since it would mean that the name contains characters like + # "$", "!", etc. + name = generate_random_name(name) if User.exists?(username: name) + + user = User.create( + username: name, + + # TODO: (mssola) the password for LDAP users is ignored anyways, + # don't store it. This way we'd forget about issues like the password + # in LDAP being to short for Portus and such. + password: password, + ldap_name: ldap_name + ) + end + user + end + + # It generates a new name that doesn't clash with any of the existing ones. + def generate_random_name(name) + 10.times do + nn = "#{name}#{Random.rand(101)}" + return nn unless User.exists?(username: nn) + end + + # We have not been able to generate a new name, let's raise an exception. + raise(:invalid_login) + end + + ## + # Parameters. + + def username + params[:user][:username] + end + + def password + params[:user][:password] + end + end +end diff --git a/spec/api/v2/token_spec.rb b/spec/api/v2/token_spec.rb index 12d4db39a..d47f206ce 100644 --- a/spec/api/v2/token_spec.rb +++ b/spec/api/v2/token_spec.rb @@ -65,14 +65,22 @@ it "does not allow to pull a private namespace from another team" do # It works for the regular user get v2_token_url, - { service: registry.hostname, account: user.username, scope: "repository:#{user.username}/busybox:push,pull" }, + { + service: registry.hostname, + account: user.username, + scope: "repository:#{user.username}/busybox:push,pull" + }, "HTTP_AUTHORIZATION" => auth_mech.encode_credentials(user.username, password) expect(response.status).to eq 200 # But not for another get v2_token_url, - { service: registry.hostname, account: another.username, scope: "repository:#{user.username}/busybox:push,pull" }, + { + service: registry.hostname, + account: another.username, + scope: "repository:#{user.username}/busybox:push,pull" + }, "HTTP_AUTHORIZATION" => auth_mech.encode_credentials(another.username, password) expect(response.status).to eq 401 @@ -162,7 +170,11 @@ allow_any_instance_of(NamespacePolicy).to receive(:pull?).and_return(true) get v2_token_url, - { service: registry.hostname, account: user.username, scope: "repository:#{user.username}/busybox:push,pull" }, + { + service: registry.hostname, + account: user.username, + scope: "repository:#{user.username}/busybox:push,pull" + }, valid_auth_header expect(response.status).to eq 200 diff --git a/spec/features/application_spec.rb b/spec/features/application_spec.rb new file mode 100644 index 000000000..a5ddc04d7 --- /dev/null +++ b/spec/features/application_spec.rb @@ -0,0 +1,27 @@ +require "rails_helper" + +feature "Global application" do + let!(:registry) { create(:registry) } + let!(:user) { create(:admin) } + + describe "#force_update_profile!" do + it "does nothing for accounts with a proper email", js: true do + login_as user, scope: :user + visit root_path + expect(current_path).to eq root_path + end + + it "redirects properly for accounts without email", js: true do + APP_CONFIG["ldap"] = { "enabled" => true } + incomplete = create(:user, email: "", ldap_name: "user") + login_as incomplete, scope: :user + + visit root_path + expect(current_path).to eq edit_user_registration_path + + expect(page).to have_content("Your profile is not complete.") + find("#logout").click + expect(current_path).to eq new_user_session_path + end + end +end diff --git a/spec/features/gravatar_spec.rb b/spec/features/gravatar_spec.rb index 6e129f027..ab9d9d52f 100644 --- a/spec/features/gravatar_spec.rb +++ b/spec/features/gravatar_spec.rb @@ -10,13 +10,13 @@ end scenario "If gravatar support is on, there should be an image" do - APP_CONFIG["gravatar"] = true + APP_CONFIG["gravatar"] = { "enabled" => true } visit root_url expect(page).to have_selector(".user-header img") end scenario "If gravatar suppor is disabled, there should be an icon" do - APP_CONFIG["gravatar"] = false + APP_CONFIG["gravatar"] = { "enabled" => false } visit root_url expect(page).to have_selector(".user-header .fa-user") end diff --git a/spec/lib/portus/ldap_spec.rb b/spec/lib/portus/ldap_spec.rb new file mode 100644 index 000000000..c18606e33 --- /dev/null +++ b/spec/lib/portus/ldap_spec.rb @@ -0,0 +1,188 @@ +require "rails_helper" + +class LdapMockAdapter + attr_accessor :opts + + def initialize(o) + @opts = o + end + + def bind_as(_) + true + end +end + +class LdapMock < Portus::LDAP + attr_reader :params, :user, :ad + + def initialize(params) + @params = { user: params } + end + + def load_configuration_test + load_configuration + end + + def bind_options_test + bind_options + end + + def find_or_create_user_test! + find_or_create_user! + end + + def generate_random_name_test(name) + generate_random_name(name) + end + + def success!(user) + @user = user + end + + protected + + def adapter(opts) + LdapMockAdapter.new(opts) + end +end + +describe Portus::LDAP do + let(:ldap_config) do + { + "enabled" => true, + "hostname" => "hostname", + "port" => 389, + "base" => "ou=users,dc=example,dc=com" + } + end + + it "sets self.enabled? accordingly" do + expect(Portus::LDAP.enabled?).to be_falsey + + APP_CONFIG["ldap"] = {} + expect(Portus::LDAP.enabled?).to be_falsey + + APP_CONFIG["ldap"] = { "enabled" => "lala" } + expect(Portus::LDAP.enabled?).to be_falsey + + APP_CONFIG["ldap"] = { "enabled" => false } + expect(Portus::LDAP.enabled?).to be_falsey + + APP_CONFIG["ldap"] = { "enabled" => true } + expect(Portus::LDAP.enabled?).to be true + end + + it "loads the configuration properly" do + lm = LdapMock.new(nil) + expect(lm.load_configuration_test).to be nil + + APP_CONFIG["ldap"] = { "enabled" => true } + expect(lm.load_configuration_test).to be nil + + APP_CONFIG["ldap"] = ldap_config + lm = LdapMock.new(username: "name", password: "1234") + cfg = lm.load_configuration_test + + expect(cfg).to_not be nil + expect(cfg.opts[:host]).to eq "hostname" + expect(cfg.opts[:port]).to eq 389 + end + + it "fetches the right bind options" do + APP_CONFIG["ldap"] = { "enabled" => true, "base" => "" } + lm = LdapMock.new(username: "name", password: "1234") + opts = lm.bind_options_test + expect(opts.size).to eq 2 + expect(opts[:filter]).to eq "(uid=name)" + expect(opts[:password]).to eq "1234" + + APP_CONFIG["ldap"] = ldap_config + opts = lm.bind_options_test + expect(opts.size).to eq 3 + expect(opts[:filter]).to eq "(uid=name)" + expect(opts[:password]).to eq "1234" + expect(opts[:base]).to eq "ou=users,dc=example,dc=com" + end + + describe "#find_or_create_user!" do + before :each do + APP_CONFIG["ldap"] = { "enabled" => true } + end + + it "the ldap user could be located" do + user = create(:user, ldap_name: "name") + lm = LdapMock.new(username: "name", password: "1234") + ret = lm.find_or_create_user_test! + expect(ret.id).to eq user.id + end + + it "creates a new ldap user" do + lm = LdapMock.new(username: "name", password: "12341234") + lm.find_or_create_user_test! + + expect(User.count).to eq 1 + expect(User.find_by(ldap_name: "name")).to_not be nil + end + + it "creates a new ldap user even if it has weird characters" do + lm = LdapMock.new(username: "name!o", password: "12341234") + lm.find_or_create_user_test! + + expect(User.count).to eq 1 + user = User.find_by(ldap_name: "name!o") + expect(user.username).to eq "nameo" + expect(user.ldap_name).to eq "name!o" + end + + it "creates a new ldap user with a new username if it clashes" do + create(:user, username: "name") + lm = LdapMock.new(username: "name", password: "12341234") + lm.find_or_create_user_test! + + expect(User.count).to eq 2 + created = User.find_by(ldap_name: "name") + expect(created.username).to_not eq "name" + expect(created.username.start_with?("name")).to be_truthy + end + end + + describe "#generate_random_name" do + it "creates a random name" do + lm = LdapMock.new(nil) + name = lm.generate_random_name_test("name") + expect(name).to_not eq "name" + expect(name).to be_truthy + end + + it "raises an exception if it fails" do + # Let's make sure that there will be a clash. + create(:user, username: "name") + 101.times do |i| + create(:user, username: "name#{i}") + end + + lm = LdapMock.new(nil) + expect { lm.generate_random_name_test("name") }.to raise_error(StandardError) + end + end + + describe "#authenticate!" do + it "raises an exception if ldap is not supported" do + lm = LdapMock.new(username: "name", password: "1234") + expect { lm.authenticate! }.to raise_error(StandardError) + end + + it "raises an exception if the user could not created" do + APP_CONFIG["ldap"] = { "enabled" => true } + lm = LdapMock.new(username: "", password: "1234") + expect { lm.authenticate! }.to raise_error(StandardError) + end + + it "returns a success if it was successful" do + APP_CONFIG["ldap"] = { "enabled" => true, "base" => "" } + lm = LdapMock.new(username: "name", password: "12341234") + expect { lm.authenticate! }.to_not raise_error + expect(lm.user.username).to eq "name" + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index cc88ff3cf..42ba61997 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1,7 +1,6 @@ require "rails_helper" describe User do - subject { create(:user) } it { should validate_uniqueness_of(:email) } @@ -19,8 +18,17 @@ .to match_array([:username, "cannot be used as name for private namespace"]) end - describe "#create_personal_namespace!" do + it "#email_required?" do + expect(subject.email_required?).to be true + + APP_CONFIG["ldap"] = { "enabled" => true } + incomplete = create(:user, email: "", ldap_name: "user") + + expect(subject.email_required?).to be true + expect(incomplete.email_required?).to be false + end + describe "#create_personal_namespace!" do context "no registry defined yet" do before :each do expect(Registry.count).to be(0) @@ -50,7 +58,6 @@ expect(team).to be_hidden end end - end describe "admins" do @@ -91,8 +98,8 @@ describe "disabling" do let!(:admin) { create(:admin) } - let!(:user) { create(:user) } - let!(:team) { create(:team, owners: [admin], viewers: [user]) } + let!(:user) { create(:user) } + let!(:team) { create(:team, owners: [admin], viewers: [user]) } it "interacts with Devise as expected" do expect(user.active_for_authentication?).to be true diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index ef432ce3f..dc66cf523 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -34,6 +34,9 @@ # after returning from it. config.before :each do Timecop.return + + # Clear the global config before each test. + APP_CONFIG.clear end config.order = :random