Introducing LDAP support

mssola · mssola · commit 7c534db2da12 · 2015-09-02T17:20:08.000+02:00
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à <msabate@suse.com>
diff --git 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
@@ -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
@@ -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
@@ -1,15 +1,20 @@
 class ApplicationController < ActionController::Base
   before_action :check_requirements
   before_action :authenticate_user!
+  before_action :force_update_profile!
   protect_from_forgery with: :exception
 
   include Pundit
   rescue_from Pundit::NotAuthorizedError, with: :deny_access
 
   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
@@ -2,9 +2,12 @@ class User < ActiveRecord::Base
   devise :database_authenticatable, :registerable,
          :recoverable, :rememberable, :trackable, :validatable, authentication_keys: [:username]
 
+  USERNAME_CHARS  = "a-z0-9"
+  USERNAME_FORMAT = /\A[#{USERNAME_CHARS}]{4,30}\Z/
+
   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"' }
+                       format: { with:    USERNAME_FORMAT,
+                                 message: "Only alphanumeric characters are allowed" }
 
   validate :private_namespace_available, on: :create
 
@@ -16,6 +19,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.empty?)
+  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
@@ -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
@@ -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
@@ -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
@@ -1,17 +1,34 @@
 
+# 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
 
   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
@@ -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
@@ -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
@@ -0,0 +1,5 @@
+class AddLdapNameToUsers < ActiveRecord::Migration
+  def change
+    add_column :users, :ldap_name, :string, default: ""
+  end
+end
diff --git 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,6 +136,7 @@
     t.datetime "updated_at"
     t.boolean  "admin",                  limit: 1,   default: false
     t.boolean  "enabled",                limit: 1,   default: true
+    t.string   "ldap_name",              limit: 255, default: ""
   end
 
   add_index "users", ["email"], name: "index_users_on_email", unique: true, using: :btree
diff --git a/lib/assets/.keep b/lib/assets/.keep
diff --git a/lib/portus/ldap.rb b/lib/portus/ldap.rb
@@ -0,0 +1,89 @@
+require "net/ldap"
+require "devise/strategies/authenticatable"
+
+module Portus
+  class LDAP < Devise::Strategies::Authenticatable
+    # Re-implemented from Devise::Strategies::Authenticatable to authenticate
+    # the user.
+    # TODO: test
+    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 !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
+
+        user = User.create(
+          username:  name,
+          password:  password,
+          ldap_name: ldap_name
+        )
+      end
+      user
+    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
diff --git a/spec/features/application_spec.rb b/spec/features/application_spec.rb
diff --git a/spec/features/gravatar_spec.rb b/spec/features/gravatar_spec.rb
diff --git a/spec/lib/portus/ldap_spec.rb b/spec/lib/portus/ldap_spec.rb
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb

Original file line number	Diff line number	Diff line change
`@@ -18,6 +18,7 @@ gem "mysql2"`
`18`	`18`	`gem "search_cop"`
`19`	`19`	`gem "kaminari"`
`20`	`20`	`gem "crono"`
	`21`	`+gem "net-ldap"`
`21`	`22`
`22`	`23`	`# Assets group.`
`23`	`24`	`#`