diff --git a/app/assets/javascripts/api.js.coffee b/app/assets/javascripts/api.js.coffee index 0e7494cb2c..0be89c1130 100644 --- a/app/assets/javascripts/api.js.coffee +++ b/app/assets/javascripts/api.js.coffee @@ -2,7 +2,7 @@ users_path: "/api/:version/users.json" user_path: "/api/:version/users/:id.json" notes_path: "/api/:version/projects/:id/notes.json" - ldap_groups_path: "/api/:version/ldap/groups.json" + ldap_groups_path: "/api/:version/ldap/:provider/groups.json" namespaces_path: "/api/:version/namespaces.json" project_users_path: "/api/:version/projects/:id/users.json" @@ -89,8 +89,9 @@ return url.replace(':version', gon.api_version) # Return LDAP groups list. Filtered by query - ldap_groups: (query, callback) -> + ldap_groups: (query, provider, callback) -> url = Api.buildUrl(Api.ldap_groups_path) + url = url.replace(':provider', provider); $.ajax( url: url diff --git a/app/assets/javascripts/ldap_groups_select.js.coffee b/app/assets/javascripts/ldap_groups_select.js.coffee index 83758da1da..5f0180d2db 100644 --- a/app/assets/javascripts/ldap_groups_select.js.coffee +++ b/app/assets/javascripts/ldap_groups_select.js.coffee @@ -12,7 +12,8 @@ $ -> placeholder: "Search for a LDAP group" minimumInputLength: 1 query: (query) -> - Api.ldap_groups query.term, (groups) -> + provider = $('#ldap_group_link_provider').val(); + Api.ldap_groups query.term, provider, (groups) -> data = { results: groups } query.callback(data) @@ -26,3 +27,5 @@ $ -> dropdownCssClass: "ajax-groups-dropdown" formatNoMatches: (nomatch) -> "Match not found; try refining your search query." + $('#ldap_group_link_provider').on 'change', -> + $('.ajax-ldap-groups-select').select2('data', null) \ No newline at end of file diff --git a/app/controllers/groups/ldap_group_links_controller.rb b/app/controllers/groups/ldap_group_links_controller.rb index ea59bb596d..bc4f8cc04d 100644 --- a/app/controllers/groups/ldap_group_links_controller.rb +++ b/app/controllers/groups/ldap_group_links_controller.rb @@ -36,6 +36,6 @@ class Groups::LdapGroupLinksController < ApplicationController end def ldap_group_link_params - params.require(:ldap_group_link).permit(:cn, :group_access) + params.require(:ldap_group_link).permit(:cn, :group_access, :provider) end end diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb index 0dc53dedeb..82a2fdb2bf 100644 --- a/app/helpers/groups_helper.rb +++ b/app/helpers/groups_helper.rb @@ -6,7 +6,7 @@ module GroupsHelper def leave_group_message(group) "Are you sure you want to leave \"#{group}\" group?" end - + def should_user_see_group_roles?(user, group) if user user.is_admin? || group.members.exists?(user_id: user.id) diff --git a/app/helpers/selects_helper.rb b/app/helpers/selects_helper.rb index c04e214e40..25b01f621e 100644 --- a/app/helpers/selects_helper.rb +++ b/app/helpers/selects_helper.rb @@ -18,4 +18,12 @@ module SelectsHelper project_id = opts[:project_id] || @project.id hidden_field_tag(id, value, class: css_class, 'data-placeholder' => placeholder, 'data-project-id' => project_id) end + + def ldap_server_select_options + options_from_collection_for_select( + Gitlab::LDAP::Config.servers, + 'provider_name', + 'label' + ) + end end diff --git a/app/models/ldap_group_link.rb b/app/models/ldap_group_link.rb index c331bab510..94ace766d7 100644 --- a/app/models/ldap_group_link.rb +++ b/app/models/ldap_group_link.rb @@ -3,10 +3,25 @@ class LdapGroupLink < ActiveRecord::Base belongs_to :group validates :cn, :group_access, :group_id, presence: true - validates :cn, uniqueness: { scope: :group_id } + validates :cn, uniqueness: { scope: [:group_id, :provider] } validates :group_access, inclusion: { in: Gitlab::Access.all_values } + scope :with_provider, ->(provider) { where(provider: provider) } + def access_field group_access end + + def config + Gitlab::LDAP::Config.new(provider) + end + + # default to the first LDAP server + def provider + read_attribute(:provider) || Gitlab::LDAP::Config.providers.first + end + + def provider_label + config.label + end end diff --git a/app/views/ldap_group_links/_form.html.haml b/app/views/ldap_group_links/_form.html.haml index 8d58d8b6a4..59a5c0312c 100644 --- a/app/views/ldap_group_links/_form.html.haml +++ b/app/views/ldap_group_links/_form.html.haml @@ -3,6 +3,11 @@ %fieldset %legend %div.form-holder + .form-group.clearfix + = f.label :cn, class: 'control-label' do + LDAP Server + .col-sm-10 + = f.select :provider, ldap_server_select_options .form-group.clearfix = f.label :cn, class: 'control-label' do LDAP Group cn diff --git a/app/views/ldap_group_links/_ldap_group_link.html.haml b/app/views/ldap_group_links/_ldap_group_link.html.haml index 329e0cad0f..27441b5128 100644 --- a/app/views/ldap_group_links/_ldap_group_link.html.haml +++ b/app/views/ldap_group_links/_ldap_group_link.html.haml @@ -1,6 +1,6 @@ %li = ldap_group_link.cn - %small.light== as #{ldap_group_link.human_access} + %small.light== as #{ldap_group_link.human_access} on #{ldap_group_link.provider_label} .pull-right = link_to group_ldap_group_link_path(group, ldap_group_link), method: :delete, class: 'btn btn-danger btn-small' do = fa_icon('unlink', text: 'unlink') diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 7e30155a78..c091d03a03 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -61,6 +61,7 @@ Settings.ldap['sync_time'] = 3600 if Settings.ldap['sync_time'].nil? if Settings.ldap['enabled'] || Rails.env.test? if Settings.ldap['host'].present? server = Settings.ldap.except('sync_time') + server = Settingslogic.new(server) server['label'] = 'LDAP' server['provider_name'] = 'ldap' Settings.ldap['servers'] = { @@ -68,12 +69,14 @@ if Settings.ldap['enabled'] || Rails.env.test? } end - Settings.ldap['servers'].each do |key, server| + Settings.ldap['servers'].map do |key, server| + server = Settingslogic.new(server) server['allow_username_or_email_login'] = false if server['allow_username_or_email_login'].nil? server['active_directory'] = true if server['active_directory'].nil? server['provider_name'] ||= "ldap#{key}".downcase server['sync_time'] = 3600 if server['sync_time'].nil? server['provider_class'] = OmniAuth::Utils.camelize(server['provider_name']) + Settings.ldap['servers'][key] = server end end diff --git a/config/initializers/7_omniauth.rb b/config/initializers/7_omniauth.rb index 3c1e8329d3..4cc14fba24 100644 --- a/config/initializers/7_omniauth.rb +++ b/config/initializers/7_omniauth.rb @@ -1,13 +1,15 @@ -module OmniAuth::Strategies - Gitlab::LDAP::Config.servers.each do |server| - # do not redeclare LDAP - next if server['provider_name'] == 'ldap' - const_set(server['provider_class'], Class.new(LDAP)) +if Gitlab::LDAP::Config.enabled? + module OmniAuth::Strategies + Gitlab::LDAP::Config.servers.each do |server| + # do not redeclare LDAP + next if server['provider_name'] == 'ldap' + const_set(server['provider_class'], Class.new(LDAP)) + end end -end -OmniauthCallbacksController.class_eval do - Gitlab::LDAP::Config.servers.each do |server| - alias_method server['provider_name'], :ldap + OmniauthCallbacksController.class_eval do + Gitlab::LDAP::Config.servers.each do |server| + alias_method server['provider_name'], :ldap + end end -end +end \ No newline at end of file diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 232e465318..b8f1e32798 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -204,7 +204,7 @@ Devise.setup do |config| # manager.default_strategies(scope: :user).unshift :some_external_strategy # end - if Gitlab.config.ldap.enabled + if Gitlab::LDAP::Config.enabled? Gitlab::LDAP::Config.servers.each do |server| if server['allow_username_or_email_login'] email_stripping_proc = ->(name) {name.gsub(/@.*$/,'')} diff --git a/db/migrate/20141010132608_add_provider_to_ldap_group_links.rb b/db/migrate/20141010132608_add_provider_to_ldap_group_links.rb new file mode 100644 index 0000000000..2858d97cb6 --- /dev/null +++ b/db/migrate/20141010132608_add_provider_to_ldap_group_links.rb @@ -0,0 +1,5 @@ +class AddProviderToLdapGroupLinks < ActiveRecord::Migration + def change + add_column :ldap_group_links, :provider, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 6b39439308..1d728f5c5b 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: 20141007100818) do +ActiveRecord::Schema.define(version: 20141010132608) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -155,6 +155,7 @@ ActiveRecord::Schema.define(version: 20141007100818) do t.integer "group_id", null: false t.datetime "created_at" t.datetime "updated_at" + t.string "provider" end create_table "members", force: true do |t| diff --git a/lib/api/ldap.rb b/lib/api/ldap.rb index 735fbba42a..2a5b6a1aec 100644 --- a/lib/api/ldap.rb +++ b/lib/api/ldap.rb @@ -4,18 +4,32 @@ module API before { authenticate! } resource :ldap do + helpers do + def get_group_list(provider, search) + Gitlab::LDAP::Adapter.new(provider).groups("#{search}*", 20) + end + end + # Get a LDAP groups list. Limit size to 20 of them. # Filter results by name using search param # # Example Request: # GET /ldap/groups get 'groups' do - # NOTE: this should be deprecated in favour of /ldap/PROVIDER_NAME/groups - # for now we just select the first LDAP server provider = Gitlab::LDAP::Config.servers.first['provider_name'] @groups = Gitlab::LDAP::Adapter.new(provider).groups("#{params[:search]}*", 20) present @groups, with: Entities::LdapGroup end + + # Get a LDAP groups list by the requested provider. Lited size to 20 of them. + # Filter results by name using search param + # + # Example Request: + # GET /ldap/ldapmain/groups + get ':provider/groups' do + @groups = get_group_list(params[:provider], params[:search]) + present @groups, with: Entities::LdapGroup + end end end end diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb index ac0e7ec8cf..593d8719f4 100644 --- a/lib/gitlab/ldap/access.rb +++ b/lib/gitlab/ldap/access.rb @@ -137,7 +137,7 @@ module Gitlab end def ldap_groups - @ldap_groups ||= ::LdapGroupLink.distinct(:cn).pluck(:cn).map do |cn| + @ldap_groups ||= ::LdapGroupLink.with_provider(provider).distinct(:cn).pluck(:cn).map do |cn| Gitlab::LDAP::Group.find_by_cn(cn, adapter) end.compact end @@ -164,7 +164,8 @@ module Gitlab private def gitlab_groups_with_ldap_link ::Group.includes(:ldap_group_links).references(:ldap_group_links). - where.not(ldap_group_links: { id: nil }) + where.not(ldap_group_links: { id: nil }). + where(ldap_group_links: { provider: provider }) end # Get the group_access for a give user. diff --git a/lib/gitlab/ldap/config.rb b/lib/gitlab/ldap/config.rb index d41bfba9b0..3f31e9f4bf 100644 --- a/lib/gitlab/ldap/config.rb +++ b/lib/gitlab/ldap/config.rb @@ -44,6 +44,10 @@ module Gitlab options['uid'] end + def label + options['label'] + end + def sync_ssh_keys? sync_ssh_keys.present? end diff --git a/lib/gitlab/ldap/group.rb b/lib/gitlab/ldap/group.rb index 8adf3c7aae..2fd3fad952 100644 --- a/lib/gitlab/ldap/group.rb +++ b/lib/gitlab/ldap/group.rb @@ -1,8 +1,7 @@ module Gitlab module LDAP class Group - def self.find_by_cn(cn, adapter=nil) - adapter ||= Gitlab::LDAP::Adapter.new + def self.find_by_cn(cn, adapter) adapter.group(cn) end @@ -66,10 +65,6 @@ module Gitlab def entry @entry end - - def adapter - @adapter ||= Gitlab::LDAP::Adapter.new - end end end end diff --git a/lib/tasks/migrate/ldap.rake b/lib/tasks/migrate/ldap.rake new file mode 100644 index 0000000000..cf22aa8a86 --- /dev/null +++ b/lib/tasks/migrate/ldap.rake @@ -0,0 +1,19 @@ +desc "GITLAB | migrate provider names to multiple ldap setup" +namespace :gitlab do + task migrate_ldap_providers: :environment do + config = Gitlab::LDAP::Config + raise 'No LDAP server hash defined. See config/gitlab.yml.example for an example' unless config.servers.any? + + provider = config.servers.first['provider_name'] + valid_providers = config.providers + unmigrated_group_links = LdapGroupLink.where('provider IS NULL OR provider NOT IN (?)', config.providers) + puts "found #{unmigrated_group_links.count} unmigrated LDAP links" + puts "setting provider to #{provider}" + unmigrated_group_links.update_all provider: provider + + unmigrated_ldap_users = User.where(provider: 'ldap') + puts "found #{unmigrated_ldap_users.count} unmigrated LDAP users" + puts "setting provider to #{provider}" + unmigrated_ldap_users.update_all provider: provider + end +end diff --git a/spec/factories.rb b/spec/factories.rb index 15899d8c3c..2d54849b79 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -182,4 +182,11 @@ FactoryGirl.define do deploy_key project end + + factory :ldap_group_link do + cn 'group1' + group_access Gitlab::Access::GUEST + provider 'ldapmain' + group + end end diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb index 4656442a6f..5be2ffeed1 100644 --- a/spec/lib/gitlab/ldap/access_spec.rb +++ b/spec/lib/gitlab/ldap/access_spec.rb @@ -208,7 +208,7 @@ objectclass: posixGroup context "non existing access for group-1, allowed via ldap-group1 as MASTER" do before do gitlab_group_1.ldap_group_links.create({ - cn: 'ldap-group1', group_access: Gitlab::Access::MASTER }) + cn: 'ldap-group1', group_access: Gitlab::Access::MASTER, provider: 'ldapmain' }) end it "gives the user master access for group 1" do @@ -220,7 +220,8 @@ objectclass: posixGroup context "existing access as guest for group-1, allowed via ldap-group1 as DEVELOPER" do before do gitlab_group_1.group_members.guests.create(user_id: user.id) - gitlab_group_1.ldap_group_links.create cn: 'ldap-group1', group_access: Gitlab::Access::MASTER + gitlab_group_1.ldap_group_links.create({ + cn: 'ldap-group1', group_access: Gitlab::Access::MASTER, provider: 'ldapmain' }) end it "upgrades the users access to master for group 1" do @@ -232,7 +233,8 @@ objectclass: posixGroup context "existing access as MASTER for group-1, allowed via ldap-group1 as DEVELOPER" do before do gitlab_group_1.group_members.masters.create(user_id: user.id) - gitlab_group_1.ldap_group_links.create cn: 'ldap-group1', group_access: Gitlab::Access::DEVELOPER + gitlab_group_1.ldap_group_links.create({ + cn: 'ldap-group1', group_access: Gitlab::Access::DEVELOPER, provider: 'ldapmain' }) end it "keeps the users master access for group 1" do @@ -244,7 +246,8 @@ objectclass: posixGroup context "existing access as master for group-1, not allowed" do before do gitlab_group_1.group_members.masters.create(user_id: user.id) - gitlab_group_1.ldap_group_links.create cn: 'ldap-group1', group_access: Gitlab::Access::MASTER + gitlab_group_1.ldap_group_links.create({ + cn: 'ldap-group1', group_access: Gitlab::Access::MASTER, provider: 'ldapmain'}) access.stub(cns_with_access: ['ldap-group2']) end @@ -270,7 +273,8 @@ objectclass: posixGroup end it "returns an interator of LDAP Groups" do - ::LdapGroupLink.create cn: 'example', group_access: Gitlab::Access::DEVELOPER, group_id: 42 + ::LdapGroupLink.create({ + cn: 'example', group_access: Gitlab::Access::DEVELOPER, group_id: 42, provider: 'ldapmain' }) Gitlab::LDAP::Adapter.any_instance.stub(:group) { Gitlab::LDAP::Group.new(ldap_group_1) } expect(access.ldap_groups.first).to be_a Gitlab::LDAP::Group diff --git a/spec/models/ldap_group_link_spec.rb b/spec/models/ldap_group_link_spec.rb new file mode 100644 index 0000000000..b70e98d64a --- /dev/null +++ b/spec/models/ldap_group_link_spec.rb @@ -0,0 +1,36 @@ +require 'spec_helper' + +describe LdapGroupLink do + let(:klass) { LdapGroupLink } + let(:ldap_group_link) { build :ldap_group_link } + + describe "validation" do + describe "cn" do + it "validates uniquiness based on group_id and provider" do + create(:ldap_group_link, cn: 'group1', group_id: 1, provider: 'ldapmain') + + group_link = build(:ldap_group_link, + cn: 'group1', group_id: 1, provider: 'ldapmain') + expect(group_link).to_not be_valid + + group_link.group_id = 2 + expect(group_link).to be_valid + + group_link.group_id = 1 + group_link.provider = 'ldapalt' + expect(group_link).to be_valid + end + end + + describe :provider do + it "shows the set value" do + ldap_group_link.provider = '1235' + expect( ldap_group_link.provider ).to eql '1235' + end + + it "defaults to the first ldap server if empty" do + expect( klass.new.provider ).to eql Gitlab::LDAP::Config.providers.first + end + end + end +end \ No newline at end of file diff --git a/spec/requests/api/ldap_spec.rb b/spec/requests/api/ldap_spec.rb index 4d24fef143..7913e0c480 100644 --- a/spec/requests/api/ldap_spec.rb +++ b/spec/requests/api/ldap_spec.rb @@ -33,4 +33,23 @@ describe API::API do end end end + + describe "GET /ldap/ldapmain/groups" do + context "when unauthenticated" do + it "should return authentication error" do + get api("/ldap/ldapmain/groups") + response.status.should == 401 + end + end + + context "when authenticated as user" do + it "should return an array of ldap groups" do + get api("/ldap/ldapmain/groups", user) + response.status.should == 200 + json_response.should be_an Array + json_response.length.should == 2 + json_response.first['cn'].should == 'developers' + end + end + end end