Skip to content

Commit

Permalink
Fixes #21626 - "Domain Users" are not a valid AD group
Browse files Browse the repository at this point in the history
'Domain Users' is a special group in AD. This group users' cannot
be queried through regular LDAP, it can only be seen on the Windows
AD UI.

This confuses people who think that could add this group but they
find that no users are found after adding this group.

This PR adds a warning when you try to do that, so that hopefully we
don't get more bug reports about this.
  • Loading branch information
dLobatog authored and tbrisker committed Nov 13, 2017
1 parent c18a547 commit 5dcb9bc
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 0 deletions.
13 changes: 13 additions & 0 deletions app/models/external_usergroup.rb
Expand Up @@ -10,6 +10,7 @@ class ExternalUsergroup < ApplicationRecord
validates :name, :auth_source, :usergroup, :presence => true
validate :hidden_authsource_restricted
validate :in_auth_source?, :if => Proc.new { |eu| eu.auth_source.respond_to?(:valid_group?) }
validate :domain_users_forbidden

def refresh
return false unless auth_source.respond_to?(:users_in_group)
Expand Down Expand Up @@ -47,4 +48,16 @@ def hidden_authsource_restricted
errors.add :auth_source, _("is not permitted")
end
end

def domain_users_forbidden
if auth_source.server_type == 'active_directory' &&
name.downcase == 'domain users'
errors.add(
:name,
_("Domain Users is a special group in AD. Unfortunately, we cannot "\
"obtain membership information from a LDAP search and therefore "\
"sync it.")
)
end
end
end
9 changes: 9 additions & 0 deletions test/models/external_usergroup_test.rb
Expand Up @@ -5,4 +5,13 @@ class ExternalUsergroupTest < ActiveSupport::TestCase
eug = FactoryBot.build(:external_usergroup, :auth_source => AuthSourceHidden.first)
refute_valid eug, :auth_source, /permitted/
end

test 'should not allow "Domain Users" as name for AD sources' do
auth_source = FactoryBot.build(:auth_source_ldap, :active_directory)
eug = FactoryBot.build(:external_usergroup,
:name => 'Domain Users',
:auth_source => auth_source)
eug.valid?
assert_match(/special/, eug.errors[:name].first)
end
end

0 comments on commit 5dcb9bc

Please sign in to comment.