Commit be3bddf9 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'tc-saml-fix-false-empty' into 'master'

SAML fix when auth hash attributes do not define groups

Closes #38923

See merge request gitlab-org/gitlab-ce!14913
parents 0fa2f148 b40ff634
---
title: Fix SAML error 500 when no groups are defined for user
merge_request: 14913
author:
type: fixed
...@@ -2,7 +2,7 @@ module Gitlab ...@@ -2,7 +2,7 @@ module Gitlab
module Saml module Saml
class AuthHash < Gitlab::OAuth::AuthHash class AuthHash < Gitlab::OAuth::AuthHash
def groups def groups
get_raw(Gitlab::Saml::Config.groups) Array.wrap(get_raw(Gitlab::Saml::Config.groups))
end end
private private
......
require 'spec_helper'
describe Gitlab::Saml::AuthHash do
include LoginHelpers
let(:raw_info_attr) { { 'groups' => %w(Developers Freelancers) } }
subject(:saml_auth_hash) { described_class.new(omniauth_auth_hash) }
let(:info_hash) do
{
name: 'John',
email: 'john@mail.com'
}
end
let(:omniauth_auth_hash) do
OmniAuth::AuthHash.new(uid: 'my-uid',
provider: 'saml',
info: info_hash,
extra: { raw_info: OneLogin::RubySaml::Attributes.new(raw_info_attr) } )
end
before do
stub_saml_group_config(%w(Developers Freelancers Designers))
end
describe '#groups' do
it 'returns array of groups' do
expect(saml_auth_hash.groups).to eq(%w(Developers Freelancers))
end
context 'raw info hash attributes empty' do
let(:raw_info_attr) { {} }
it 'returns an empty array' do
expect(saml_auth_hash.groups).to be_a(Array)
end
end
end
end
...@@ -2,13 +2,15 @@ require 'spec_helper' ...@@ -2,13 +2,15 @@ require 'spec_helper'
describe Gitlab::Saml::User do describe Gitlab::Saml::User do
include LdapHelpers include LdapHelpers
include LoginHelpers
let(:saml_user) { described_class.new(auth_hash) } let(:saml_user) { described_class.new(auth_hash) }
let(:gl_user) { saml_user.gl_user } let(:gl_user) { saml_user.gl_user }
let(:uid) { 'my-uid' } let(:uid) { 'my-uid' }
let(:dn) { 'uid=user1,ou=People,dc=example' } let(:dn) { 'uid=user1,ou=People,dc=example' }
let(:provider) { 'saml' } let(:provider) { 'saml' }
let(:auth_hash) { OmniAuth::AuthHash.new(uid: uid, provider: provider, info: info_hash, extra: { raw_info: OneLogin::RubySaml::Attributes.new({ 'groups' => %w(Developers Freelancers Designers) }) }) } let(:raw_info_attr) { { 'groups' => %w(Developers Freelancers Designers) } }
let(:auth_hash) { OmniAuth::AuthHash.new(uid: uid, provider: provider, info: info_hash, extra: { raw_info: OneLogin::RubySaml::Attributes.new(raw_info_attr) }) }
let(:info_hash) do let(:info_hash) do
{ {
name: 'John', name: 'John',
...@@ -18,22 +20,6 @@ describe Gitlab::Saml::User do ...@@ -18,22 +20,6 @@ describe Gitlab::Saml::User do
let(:ldap_user) { Gitlab::LDAP::Person.new(Net::LDAP::Entry.new, 'ldapmain') } let(:ldap_user) { Gitlab::LDAP::Person.new(Net::LDAP::Entry.new, 'ldapmain') }
describe '#save' do describe '#save' do
def stub_omniauth_config(messages)
allow(Gitlab.config.omniauth).to receive_messages(messages)
end
def stub_ldap_config(messages)
allow(Gitlab::LDAP::Config).to receive_messages(messages)
end
def stub_basic_saml_config
allow(Gitlab::Saml::Config).to receive_messages({ options: { name: 'saml', args: {} } })
end
def stub_saml_group_config(groups)
allow(Gitlab::Saml::Config).to receive_messages({ options: { name: 'saml', groups_attribute: 'groups', external_groups: groups, args: {} } })
end
before do before do
stub_basic_saml_config stub_basic_saml_config
end end
...@@ -402,4 +388,16 @@ describe Gitlab::Saml::User do ...@@ -402,4 +388,16 @@ describe Gitlab::Saml::User do
end end
end end
end end
describe '#find_user' do
context 'raw info hash attributes empty' do
let(:raw_info_attr) { {} }
it 'does not mark user as external' do
stub_saml_group_config(%w(Freelancers))
expect(saml_user.find_user.external).to be_falsy
end
end
end
end end
...@@ -15,10 +15,7 @@ module LdapHelpers ...@@ -15,10 +15,7 @@ module LdapHelpers
# admin_group: 'my-admin-group' # admin_group: 'my-admin-group'
# ) # )
def stub_ldap_config(messages) def stub_ldap_config(messages)
messages.each do |config, value| allow_any_instance_of(::Gitlab::LDAP::Config).to receive_messages(messages)
allow_any_instance_of(::Gitlab::LDAP::Config)
.to receive(config.to_sym).and_return(value)
end
end end
# Stub an LDAP person search and provide the return entry. Specify `nil` for # Stub an LDAP person search and provide the return entry. Specify `nil` for
......
...@@ -120,4 +120,16 @@ module LoginHelpers ...@@ -120,4 +120,16 @@ module LoginHelpers
allow_any_instance_of(Object).to receive(:user_saml_omniauth_authorize_path).and_return('/users/auth/saml') allow_any_instance_of(Object).to receive(:user_saml_omniauth_authorize_path).and_return('/users/auth/saml')
allow_any_instance_of(Object).to receive(:omniauth_authorize_path).with(:user, "saml").and_return('/users/auth/saml') allow_any_instance_of(Object).to receive(:omniauth_authorize_path).with(:user, "saml").and_return('/users/auth/saml')
end end
def stub_omniauth_config(messages)
allow(Gitlab.config.omniauth).to receive_messages(messages)
end
def stub_basic_saml_config
allow(Gitlab::Saml::Config).to receive_messages({ options: { name: 'saml', args: {} } })
end
def stub_saml_group_config(groups)
allow(Gitlab::Saml::Config).to receive_messages({ options: { name: 'saml', groups_attribute: 'groups', external_groups: groups, args: {} } })
end
end end
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