Skip to content

Commit

Permalink
Merge pull request ManageIQ#13068 from abellotti/api_sys_auth_authori…
Browse files Browse the repository at this point in the history
…ze_user

Enhance API to authorize users with system token authenticated requests.
  • Loading branch information
gtanzillo committed Dec 12, 2016
2 parents 83afa4c + 68160e7 commit fadf640
Show file tree
Hide file tree
Showing 9 changed files with 157 additions and 15 deletions.
4 changes: 3 additions & 1 deletion app/controllers/api/base_controller/authentication.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def user_settings
end

def userid_to_userobj(userid)
User.find_by_userid(userid)
User.lookup_by_identity(userid)
end

def authorize_user_group(user_obj)
Expand Down Expand Up @@ -146,6 +146,8 @@ def authenticate_with_system_token(x_miq_token)
validate_system_token_server(@miq_token_hash[:server_guid])
validate_system_token_timestamp(@miq_token_hash[:timestamp])

User.authorize_user(@miq_token_hash[:userid])

@auth_user = @miq_token_hash[:userid]
@auth_user_obj = userid_to_userobj(@auth_user)

Expand Down
19 changes: 15 additions & 4 deletions app/models/authenticator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,19 @@ def uses_stored_password?
false
end

def user_authorizable_without_authentication?
false
end

def authorize_user(userid)
return unless user_authorizable_without_authentication?
authenticate(userid, "", {}, {:require_user => true, :authorize_only => true})
end

def authenticate(username, password, request = nil, options = {})
options = options.dup
options[:require_user] ||= false
options[:authorize_only] ||= false
fail_message = _("Authentication failed")

user_or_taskid = nil
Expand All @@ -46,11 +56,12 @@ def authenticate(username, password, request = nil, options = {})

username = normalize_username(username)

if _authenticate(username, password, request)
authenticated = options[:authorize_only] || _authenticate(username, password, request)
if authenticated
AuditEvent.success(audit.merge(:message => "User #{username} successfully validated by #{self.class.proper_name}"))

if authorize?
user_or_taskid = authorize_queue(username, request)
user_or_taskid = authorize_queue(username, request, options)
else
# If role_mode == database we will only use the external system for authentication. Also, the user must exist in our database
# otherwise we will fail authentication
Expand Down Expand Up @@ -205,7 +216,7 @@ def encrypt_ldap_password(config)
config[:bind_pwd] = MiqPassword.try_encrypt(config[:bind_pwd])
end

def authorize_queue(username, _request, *args)
def authorize_queue(username, _request, _options, *args)
task = MiqTask.create(:name => "#{self.class.proper_name} User Authorization of '#{username}'", :userid => username)
if authorize_queue?
encrypt_ldap_password(config) if MiqLdap.using_ldap?
Expand Down Expand Up @@ -254,7 +265,7 @@ def match_groups(external_group_names)
return [] if external_group_names.empty?
external_group_names = external_group_names.collect(&:downcase)

internal_groups = MiqGroup.order(:sequence).to_a
internal_groups = MiqGroup.in_my_region.order(:sequence).to_a

external_group_names.each { |g| _log.debug("External Group: #{g}") }
internal_groups.each { |g| _log.debug("Internal Group: #{g.description.downcase}") }
Expand Down
2 changes: 1 addition & 1 deletion app/models/authenticator/amazon.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def _authenticate(username, password, _request)
end
end

def find_external_identity(username)
def find_external_identity(username, *_args)
# Amazon IAM will be used for authentication and role assignment
_log.info("AWS key: [#{config[:amazon_key]}]")
_log.info(" User: [#{username}]")
Expand Down
62 changes: 54 additions & 8 deletions app/models/authenticator/httpd.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@ def self.proper_name
'External httpd'
end

def authorize_queue(username, request)
user_attrs = {:username => username,
:fullname => request.headers['X-REMOTE-USER-FULLNAME'],
:firstname => request.headers['X-REMOTE-USER-FIRSTNAME'],
:lastname => request.headers['X-REMOTE-USER-LASTNAME'],
:email => request.headers['X-REMOTE-USER-EMAIL']}
membership_list = (request.headers['X-REMOTE-USER-GROUPS'] || '').split(/[;:]/)
def authorize_queue(username, request, options, *_args)
user_attrs, membership_list =
if options[:authorize_only]
user_details_from_external_directory(username)
else
user_details_from_headers(username, request)
end

super(username, request, user_attrs, membership_list)
super(username, request, {}, user_attrs, membership_list)
end

# We don't talk to an external system in #find_external_identity /
Expand All @@ -21,6 +21,10 @@ def authorize_queue?
false
end

def user_authorizable_without_authentication?
true
end

def _authenticate(_username, _password, request)
request.present? &&
request.headers['X-REMOTE-USER'].present?
Expand Down Expand Up @@ -48,5 +52,47 @@ def update_user_attributes(user, _username, identity)
user.last_name = user_attrs[:lastname]
user.email = user_attrs[:email] unless user_attrs[:email].blank?
end

private

def user_details_from_external_directory(username)
ext_user_attrs = user_attrs_from_external_directory(username)
user_attrs = {:username => username,
:fullname => ext_user_attrs["displayname"],
:firstname => ext_user_attrs["givenname"],
:lastname => ext_user_attrs["sn"],
:email => ext_user_attrs["mail"]}
[user_attrs, MiqGroup.get_httpd_groups_by_user(username)]
end

def user_details_from_headers(username, request)
user_attrs = {:username => username,
:fullname => request.headers['X-REMOTE-USER-FULLNAME'],
:firstname => request.headers['X-REMOTE-USER-FIRSTNAME'],
:lastname => request.headers['X-REMOTE-USER-LASTNAME'],
:email => request.headers['X-REMOTE-USER-EMAIL']}
[user_attrs, (request.headers['X-REMOTE-USER-GROUPS'] || '').split(/[;:]/)]
end

def user_attrs_from_external_directory(username)
return unless username
require "dbus"

attrs_needed = %w(mail givenname sn displayname)

sysbus = DBus.system_bus
ifp_service = sysbus["org.freedesktop.sssd.infopipe"]
ifp_object = ifp_service.object "/org/freedesktop/sssd/infopipe"
ifp_object.introspect
ifp_interface = ifp_object["org.freedesktop.sssd.infopipe"]
begin
user_attrs = ifp_interface.GetUserAttr(username, attrs_needed).first
rescue => err
raise _("Unable to get attributes for external user %{user_name} - %{error}") %
{:user_name => username, :error => err}
end

attrs_needed.each_with_object({}) { |attr, hash| hash[attr] = Array(user_attrs[attr]).first }
end
end
end
6 changes: 5 additions & 1 deletion app/models/authenticator/ldap.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ def lookup_by_identity(username)
find_or_create_by_ldap(username)
end

def user_authorizable_without_authentication?
true
end

private

def ldap
Expand Down Expand Up @@ -76,7 +80,7 @@ def _authenticate(username, password, _request)
ldap_bind(username, password)
end

def find_external_identity(username)
def find_external_identity(username, *_args)
# Ldap will be used for authentication and role assignment
_log.info("Bind DN: [#{config[:bind_dn]}]")
_log.info(" User FQDN: [#{username}]")
Expand Down
9 changes: 9 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,11 @@ def self.lookup_by_identity(username)
authenticator(username).lookup_by_identity(username)
end

def self.authorize_user(userid)
return if userid.blank? || admin?(userid)
authenticator(userid).authorize_user(userid)
end

def logoff
self.lastlogoff = Time.now.utc
save
Expand Down Expand Up @@ -194,6 +199,10 @@ def miq_groups=(groups)
end

def admin?
self.class.admin?(userid)
end

def self.admin?(userid)
userid == "admin"
end

Expand Down
44 changes: 44 additions & 0 deletions spec/models/authenticator/httpd_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@
end
end

describe '.user_authorizable_without_authentication?' do
it "is true" do
expect(subject.user_authorizable_without_authentication?).to be_truthy
end
end

describe '#lookup_by_identity' do
it "finds existing users" do
expect(subject.lookup_by_identity('alice')).to eq(alice)
Expand Down Expand Up @@ -290,6 +296,44 @@ def authenticate
end
end
end

describe ".user_attrs_from_external_directory" do
before do
require "dbus"
sysbus = double('sysbus')
ifp_service = double('ifp_service')
ifp_object = double('ifp_object')
@ifp_interface = double('ifp_interface')

allow(DBus).to receive(:system_bus).and_return(sysbus)
allow(sysbus).to receive(:[]).with("org.freedesktop.sssd.infopipe").and_return(ifp_service)
allow(ifp_service).to receive(:object).with("/org/freedesktop/sssd/infopipe").and_return(ifp_object)
allow(ifp_object).to receive(:introspect)
allow(ifp_object).to receive(:[]).with("org.freedesktop.sssd.infopipe").and_return(@ifp_interface)
end

it "should return nil for unspecified user" do
expect(subject.send(:user_attrs_from_external_directory, nil)).to be_nil
end

it "should return user attributes hash for valid user" do
requested_attrs = %w(mail givenname sn displayname)

jdoe_attrs = [{"mail" => ["jdoe@example.com"],
"givenname" => ["John"],
"sn" => ["Doe"],
"displayname" => ["John Doe"]}]

expected_jdoe_attrs = {"mail" => "jdoe@example.com",
"givenname" => "John",
"sn" => "Doe",
"displayname" => "John Doe"}

allow(@ifp_interface).to receive(:GetUserAttr).with('jdoe', requested_attrs).and_return(jdoe_attrs)

expect(subject.send(:user_attrs_from_external_directory, 'jdoe')).to eq(expected_jdoe_attrs)
end
end
end
end
end
6 changes: 6 additions & 0 deletions spec/models/authenticator/ldap_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,12 @@ def normalize(dn)
end
end

describe ".user_authorizable_without_authentication?" do
it "is true" do
expect(subject.user_authorizable_without_authentication?).to be_truthy
end
end

describe '#lookup_by_identity' do
it "finds existing users" do
expect(subject.lookup_by_identity('alice')).to eq(alice)
Expand Down
20 changes: 20 additions & 0 deletions spec/models/user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -527,4 +527,24 @@
expect(User.super_admin).to be_super_admin_user
end
end

context ".admin?" do
it "admin? succeeds with admin account" do
expect(User.admin?("admin")).to be_truthy
end

it "admin? fails with non-admin account" do
expect(User.admin?("regular_user")).to be_falsey
end
end

context ".authorize_user" do
it "returns nil with blank userid" do
expect(User.authorize_user("")).to be_nil
end

it "returns nil with admin userid" do
expect(User.authorize_user("admin")).to be_nil
end
end
end

0 comments on commit fadf640

Please sign in to comment.