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 c2bf796d7..d66f7e2e7 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -2,6 +2,7 @@ class ApplicationController < ActionController::Base before_action :check_requirements helper_method :fixes before_action :authenticate_user! + before_action :force_update_profile! protect_from_forgery with: :exception include Pundit @@ -9,8 +10,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 check_requirements redirect_to "/errors/500" 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/controllers/auth/registrations_controller.rb b/app/controllers/auth/registrations_controller.rb index e105cdbba..051822f18 100644 --- a/app/controllers/auth/registrations_controller.rb +++ b/app/controllers/auth/registrations_controller.rb @@ -1,6 +1,7 @@ class Auth::RegistrationsController < Devise::RegistrationsController layout "authentication", except: :edit + before_action :check_ldap, only: [:new, :create] before_action :check_admin, only: [:new, :create] before_action :configure_sign_up_params, only: [:create] before_action :authenticate_user!, only: [:disable] @@ -86,6 +87,11 @@ def configure_sign_up_params protected + # Redirect to the login page if LDAP is enabled. + def check_ldap + redirect_to new_user_session_path if Portus::LDAP.enabled? + end + # Returns true if the contents of the `params` hash contains the needed keys # to update the password of the user. def password_update? diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb index 8a1dfd761..e1d304c16 100644 --- a/app/controllers/auth/sessions_controller.rb +++ b/app/controllers/auth/sessions_controller.rb @@ -1,10 +1,11 @@ class Auth::SessionsController < Devise::SessionsController layout "authentication" - # Re-implementing. The logic is: if there is already a user that can log in, - # work as usual. Otherwise, redirect always to the signup page. + # Re-implementing. The logic is: if there is already a user that can log in + # or LDAP support is enabled, work as usual. Otherwise, redirect always to + # the signup page. def new - if User.not_portus.any? + if User.not_portus.any? || Portus::LDAP.enabled? super else # For some reason if we get here from the root path, we'll get a flashy 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..c0bdf761d 100644 --- a/app/views/devise/registrations/edit.html.slim +++ b/app/views/devise/registrations/edit.html.slim @@ -7,48 +7,56 @@ ' 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 - .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 + - unless current_user.email.empty? + - if current_user.ldap_name.nil? + .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 - 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') + .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/devise/sessions/new.html.slim b/app/views/devise/sessions/new.html.slim index c52767558..de2dbbeb5 100644 --- a/app/views/devise/sessions/new.html.slim +++ b/app/views/devise/sessions/new.html.slim @@ -8,6 +8,14 @@ section.row-0 = f.text_field :username, class: 'input form-control input-lg first', placeholder: 'Username', autofocus: true, required: true = f.password_field :password, class: 'input form-control input-lg last', placeholder: 'Password', autocomplete: false, required: true = f.button class: 'classbutton btn btn-primary btn-block btn-lg' do - i.fa.fa-check Login + - if Portus::LDAP.enabled? + i.fa.fa-check LDAP Login + - else + i.fa.fa-check Login - .text-center = link_to 'or, Create a new account', new_user_registration_url + - if Portus::LDAP.enabled? + - unless User.not_portus.any? + p + | NOTE: The first user to be created will have admin permissions ! + - else + .text-center = link_to 'or, Create a new account', new_user_registration_url 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..2b3e028d5 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -243,6 +243,12 @@ # 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| + # Let's put LDAP in front of every other strategy. + 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/config/locales/devise.en.yml b/config/locales/devise.en.yml index 26a10f292..15f7b7e76 100644 --- a/config/locales/devise.en.yml +++ b/config/locales/devise.en.yml @@ -16,6 +16,8 @@ en: timeout: "Your session expired. Please sign in again to continue." unauthenticated: "You need to sign in or sign up before continuing." unconfirmed: "You have to confirm your email address before continuing." + user: + invalid_login: "Invalid %{authentication_keys} or password." mailer: confirmation_instructions: subject: "Confirmation instructions" 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..c0d64e1c9 --- /dev/null +++ b/lib/portus/ldap.rb @@ -0,0 +1,141 @@ +require "net/ldap" +require "devise/strategies/authenticatable" + +module Portus + # Portus::LDAP implements Devise's authenticatable for LDAP servers. This + # class will fallback to other strategies if LDAP support is not enabled. + # + # If we can bind to the server with the given credentials, we assume that + # the authentication was successful. In this case, if this is the first time + # that this user enters Portus, it will be saved inside of Portus' DB. There + # are some issues while doing this: + # + # 1. The 'email' is not provided in a standard way: some LDAP servers may + # provide it, some others won't. Therefore, once an LDAP user logs in + # Portus for the first time, the email will be left blank. This should + # be handled by the controller layer. + # 2. The 'password' is stored in the DB but it's not really used. This is + # because the DB requires the password to not be blank, but in order to + # authenticate we always want to check with the LDAP server. + # + # This class is only useful if LDAP is enabled in the `config/config.yml` + # file. Take a look at this file in order to read more on the different + # configurable values. + class LDAP < Devise::Strategies::Authenticatable + # Re-implemented from Devise::Strategies::Authenticatable to authenticate + # the user. + def authenticate! + ldap = load_configuration + + # If LDAP is enabled try to authenticate through the LDAP server. + # Otherwise we fall back to the next strategy. + if ldap + # Try to bind to the LDAP server. If there's any failure, the + # authentication process will fail without going to the any other + # strategy. + if ldap.bind_as(bind_options) + user = find_or_create_user! + user.valid? ? success!(user) : fail!(:invalid_login) + else + fail!(:invalid_login) + end + else + # rubocop:disable Style/SignalException + fail(:invalid_login) + # rubocop:enable Style/SignalException + 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.new(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 + Net::LDAP + end + + # Returns the option hash to be used in order to authenticate the user in + # the LDAP server. + 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. We also check that the name is longer than 4 + # (requirement from Docker). + if name.length < 4 || User.exists?(username: name) + name = generate_random_name(name) + end + + user = User.create( + username: name, + password: password, + admin: !User.not_portus.any?, + 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) + # Even if the name has just one character, adding a number of at least + # three digits would make the name valid. + offset = name.length < 4 ? 100 : 0 + + 10.times do + nn = "#{name}#{Random.rand(offset + 101)}" + return nn unless User.exists?(username: nn) + end + + # We have not been able to generate a new name, let's raise an exception. + fail!(: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 db400b994..f070af8a5 100644 --- a/spec/api/v2/token_spec.rb +++ b/spec/api/v2/token_spec.rb @@ -65,16 +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 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/auth/login_feature_spec.rb b/spec/features/auth/login_feature_spec.rb index 80815c2d5..b9c1b07c6 100644 --- a/spec/features/auth/login_feature_spec.rb +++ b/spec/features/auth/login_feature_spec.rb @@ -14,6 +14,20 @@ expect(page).to_not have_content("You need to sign in or sign up before continuing.") end + scenario "It does show a warning for the admin creation in LDAP support", js: true do + User.delete_all + APP_CONFIG["ldap"] = { "enabled" => true } + visit new_user_session_path + + expect(page).to have_content("The first user to be created will have admin permissions !") + expect(page).to_not have_content("Create a new account") + + create(:admin) + + visit new_user_session_path + expect(page).to_not have_content("The first user to be created will have admin permissions !") + end + scenario "Existing user is able using his login and password to login into Portus", js: true do expect(page).to_not have_content("Invalid username or password") diff --git a/spec/features/auth/signup_feature_spec.rb b/spec/features/auth/signup_feature_spec.rb index 24e576364..4394308da 100644 --- a/spec/features/auth/signup_feature_spec.rb +++ b/spec/features/auth/signup_feature_spec.rb @@ -1,7 +1,6 @@ require "rails_helper" feature "Signup feature" do - before do create(:admin) visit new_user_registration_url @@ -59,6 +58,17 @@ expect(page).to have_css("section.first-user") end + scenario "It always redirects to the signin page when there are no users but LDAP is enabled" do + User.delete_all + APP_CONFIG["ldap"] = { "enabled" => true } + + visit new_user_session_url + expect(current_url).to eq new_user_session_url + + visit new_user_registration_url + expect(current_url).to eq new_user_session_url + end + scenario "I am readirected to the signup page if only the portus user exists" do User.delete_all create(:admin, username: "portus") 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..501b5a8c4 --- /dev/null +++ b/spec/lib/portus/ldap_spec.rb @@ -0,0 +1,236 @@ +require "rails_helper" + +class LdapMockAdapter + attr_accessor :opts + + def initialize(o) + @opts = o + end + + def bind_as(_) + true + end +end + +class LdapFailedBindAdapter < LdapMockAdapter + def bind_as(_) + false + end +end + +class LdapOriginal < Portus::LDAP + def adapter + super + end +end + +class LdapMock < Portus::LDAP + attr_reader :params, :user, :last_symbol + attr_accessor :bind_result + + def initialize(params) + @params = { user: params } + @bind_result = true + @last_symbol = :ok + 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 + + def fail!(symbol) + @last_symbol = symbol + end + + alias_method :fail, :fail! + + protected + + def adapter + @bind_result ? LdapMockAdapter : LdapFailedBindAdapter + 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 + + # Let's make code coverage happy + it "calls the right adapter" do + ldap = LdapOriginal.new(nil) + expect(ldap.adapter.to_s).to eq "Net::LDAP" + 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" + expect(user.admin?).to be_truthy + end + + it "creates a new ldap user with a new username if it clashes" do + create(:admin, 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 + expect(created.admin?).to be_falsey + 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.start_with?("name")).to be_truthy + end + + it "generates a name that is large enough" do + lm = LdapMock.new(nil) + name = lm.generate_random_name_test("n") + expect(name).to_not eq "n" + expect(name.start_with?("n")).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) + lm.generate_random_name_test("name") + expect(lm.last_symbol).to be :invalid_login + end + end + + describe "#authenticate!" do + it "raises an exception if ldap is not supported" do + lm = LdapMock.new(username: "name", password: "1234") + lm.authenticate! + expect(lm.last_symbol).to be :invalid_login + end + + it "fails if the user couldn't bind" do + APP_CONFIG["ldap"] = { "enabled" => true, "base" => "" } + lm = LdapMock.new(username: "name", password: "12341234") + lm.bind_result = false + lm.authenticate! + expect(lm.last_symbol).to be :invalid_login + end + + it "raises an exception if the user could not created" do + APP_CONFIG["ldap"] = { "enabled" => true, "base" => "" } + lm = LdapMock.new(username: "", password: "1234") + lm.authenticate! + expect(lm.last_symbol).to be :invalid_login + end + + it "returns a success if it was successful" do + APP_CONFIG["ldap"] = { "enabled" => true, "base" => "" } + lm = LdapMock.new(username: "name", password: "12341234") + lm.authenticate! + expect(lm.last_symbol).to be :ok + 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