Match method order for implementation and class

This will prevent a mess, and keeps everything clean.
In addition i made the non interface methods private
parent 1a59ce4e
...@@ -114,6 +114,20 @@ module Gitlab ...@@ -114,6 +114,20 @@ module Gitlab
end end
end end
# Loop throug all ldap conneted groups, and update the users link with it
def update_ldap_group_links(user)
gitlab_groups_with_ldap_link.each do |group|
active_group_links = group.ldap_group_links.where(cn: cns_with_access(get_ldap_user(user)))
if active_group_links.any?
group.add_users([user.id], fetch_group_access(group, user, active_group_links))
else
group.users.delete(user)
end
end
end
private
def ldap_groups def ldap_groups
@ldap_groups ||= ::LdapGroupLink.distinct(:cn).pluck(:cn).map do |cn| @ldap_groups ||= ::LdapGroupLink.distinct(:cn).pluck(:cn).map do |cn|
Gitlab::LDAP::Group.find_by_cn(cn, adapter) Gitlab::LDAP::Group.find_by_cn(cn, adapter)
...@@ -132,19 +146,6 @@ module Gitlab ...@@ -132,19 +146,6 @@ module Gitlab
where.not(ldap_group_links: { id: nil }) where.not(ldap_group_links: { id: nil })
end end
# Loop throug all ldap conneted groups, and update the users link with it
def update_ldap_group_links(user)
gitlab_groups_with_ldap_link.each do |group|
active_group_links = group.ldap_group_links.where(cn: cns_with_access(get_ldap_user(user)))
if active_group_links.any?
group.add_users([user.id], fetch_group_access(group, user, active_group_links))
else
group.users.delete(user)
end
end
end
# Get the group_access for a give user. # Get the group_access for a give user.
# Always respect the current level, never downgrade it. # Always respect the current level, never downgrade it.
def fetch_group_access(group, user, active_group_links) def fetch_group_access(group, user, active_group_links)
......
...@@ -4,41 +4,37 @@ describe Gitlab::LDAP::Access do ...@@ -4,41 +4,37 @@ describe Gitlab::LDAP::Access do
let(:access) { Gitlab::LDAP::Access.new } let(:access) { Gitlab::LDAP::Access.new }
let(:user) { create(:user) } let(:user) { create(:user) }
describe :update_user_email do
let(:user_ldap) { create(:user, provider: 'ldap', extern_uid: "66048")}
it "should not update email if email attribute is not set" do describe :allowed? do
entry = Net::LDAP::Entry.new subject { access.allowed?(user) }
Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry) }
updated = access.update_email(user_ldap)
updated.should == false
end
it "should not update the email if the user has the same email in GitLab and in LDAP" do context 'when the user cannot be found' do
entry = Net::LDAP::Entry.new before { Gitlab::LDAP::Person.stub(find_by_dn: nil) }
entry['mail'] = [user_ldap.email]
Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry) }
updated = access.update_email(user_ldap)
updated.should == false
end
it "should not update the email if the user has the same email GitLab and in LDAP, but with upper case in LDAP" do it { should be_false }
entry = Net::LDAP::Entry.new
entry['mail'] = [user_ldap.email.upcase]
Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry) }
updated = access.update_email(user_ldap)
updated.should == false
end end
it "should update the email if the user email is different" do context 'when the user is found' do
entry = Net::LDAP::Entry.new before { Gitlab::LDAP::Person.stub(find_by_dn: :ldap_user) }
entry['mail'] = ["new_email@example.com"]
Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry) } context 'and the user is diabled via active directory' do
updated = access.update_email(user_ldap) before { Gitlab::LDAP::Person.stub(disabled_via_active_directory?: true) }
updated.should == true
it { should be_false }
end
context 'and has no disabled flag in active diretory' do
before { Gitlab::LDAP::Person.stub(disabled_via_active_directory?: false) }
it { should be_true }
end
end end
end end
describe :update_permissions do
end
describe :update_ssh_keys do describe :update_ssh_keys do
let(:user_ldap) { create(:user, provider: 'ldap', extern_uid: "66049")} let(:user_ldap) { create(:user, provider: 'ldap', extern_uid: "66049")}
let(:ssh_key) { 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCrSQHff6a1rMqBdHFt+FwIbytMZ+hJKN3KLkTtOWtSvNIriGhnTdn4rs+tjD/w+z+revytyWnMDM9dS7J8vQi006B16+hc9Xf82crqRoPRDnBytgAFFQY1G/55ql2zdfsC5yvpDOFzuwIJq5dNGsojS82t6HNmmKPq130fzsenFnj5v1pl3OJvk513oduUyKiZBGTroWTn7H/eOPtu7s9MD7pAdEjqYKFLeaKmyidiLmLqQlCRj3Tl2U9oyFg4PYNc0bL5FZJ/Z6t0Ds3i/a2RanQiKxrvgu3GSnUKMx7WIX373baL4jeM7cprRGiOY/1NcS+1cAjfJ8oaxQF/1dYj' } let(:ssh_key) { 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCrSQHff6a1rMqBdHFt+FwIbytMZ+hJKN3KLkTtOWtSvNIriGhnTdn4rs+tjD/w+z+revytyWnMDM9dS7J8vQi006B16+hc9Xf82crqRoPRDnBytgAFFQY1G/55ql2zdfsC5yvpDOFzuwIJq5dNGsojS82t6HNmmKPq130fzsenFnj5v1pl3OJvk513oduUyKiZBGTroWTn7H/eOPtu7s9MD7pAdEjqYKFLeaKmyidiLmLqQlCRj3Tl2U9oyFg4PYNc0bL5FZJ/Z6t0Ds3i/a2RanQiKxrvgu3GSnUKMx7WIX373baL4jeM7cprRGiOY/1NcS+1cAjfJ8oaxQF/1dYj' }
...@@ -105,35 +101,44 @@ describe Gitlab::LDAP::Access do ...@@ -105,35 +101,44 @@ describe Gitlab::LDAP::Access do
expect(user_ldap.keys.size).to be(0) expect(user_ldap.keys.size).to be(0)
end end
end end
end end
describe :allowed? do describe :update_user_email do
subject { access.allowed?(user) } let(:user_ldap) { create(:user, provider: 'ldap', extern_uid: "66048")}
context 'when the user cannot be found' do
before { Gitlab::LDAP::Person.stub(find_by_dn: nil) }
it { should be_false } it "should not update email if email attribute is not set" do
entry = Net::LDAP::Entry.new
Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry) }
updated = access.update_email(user_ldap)
updated.should == false
end end
context 'when the user is found' do it "should not update the email if the user has the same email in GitLab and in LDAP" do
before { Gitlab::LDAP::Person.stub(find_by_dn: :ldap_user) } entry = Net::LDAP::Entry.new
entry['mail'] = [user_ldap.email]
context 'and the user is diabled via active directory' do Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry) }
before { Gitlab::LDAP::Person.stub(disabled_via_active_directory?: true) } updated = access.update_email(user_ldap)
updated.should == false
it { should be_false } end
end
context 'and has no disabled flag in active diretory' do it "should not update the email if the user has the same email GitLab and in LDAP, but with upper case in LDAP" do
before { Gitlab::LDAP::Person.stub(disabled_via_active_directory?: false) } entry = Net::LDAP::Entry.new
entry['mail'] = [user_ldap.email.upcase]
Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry) }
updated = access.update_email(user_ldap)
updated.should == false
end
it { should be_true } it "should update the email if the user email is different" do
end entry = Net::LDAP::Entry.new
entry['mail'] = ["new_email@example.com"]
Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry) }
updated = access.update_email(user_ldap)
updated.should == true
end end
end end
describe :update_admin_status do describe :update_admin_status do
let(:gitlab_user) { create(:user, provider: 'ldap', extern_uid: "admin2")} let(:gitlab_user) { create(:user, provider: 'ldap', extern_uid: "admin2")}
let(:gitlab_admin) { create(:admin, provider: 'ldap', extern_uid: "admin2")} let(:gitlab_admin) { create(:admin, provider: 'ldap', extern_uid: "admin2")}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment