Improve the REMOTE_USER (for example mod_auth_kerb) support #967

adelton · 2013-10-21T07:29:53Z

This series of patches addresses http://projects.theforeman.org/issues/3312, making it possible to set up a protected "logon" location in Apache via

<Location /users/extlogin>
...
</Location>

and configure mod_auth_* authentication modules for that location. It was tested with mod_auth_kerb.

The possibilities and issues with the existing REMOTE_USER support are listed at http://projects.theforeman.org/projects/foreman/wiki/Foreman_and_mod_auth_kerb, together with explanations introduced by this pull request.

This is a refactoring of #958, addressing all comments in that pull request.

dLobatog · 2013-10-21T12:23:24Z

[test]

ares · 2013-10-21T14:58:11Z

tests added in adelton#2, worked on my machine, @adelton could you please merge/rebase?

adelton · 2013-10-22T02:04:27Z

Rebased on develop, including Marek H.'s tests: 34defdc55765700eff0723e5852c2bc5ac1d2774.

lzap · 2013-10-22T07:09:18Z

Thanks [test]

sodabrew · 2013-10-23T15:14:23Z

Oh this is great, would it work with mod_auth_openid? https://github.com/bmuller/mod_auth_openid

adelton · 2013-10-24T01:42:02Z

I did not verify it specifically with mod_auth_openid but the plan is to support anything that populates REMOTE_USER which mod_auth_openid does.

ares · 2013-10-24T06:05:54Z

app/assets/.directory

@@ -0,0 +1,4 @@
+[Dolphin]


oops, could you please remove .directory files? I accidentaly added them to my commit

On Wed, Oct 23, 2013 at 11:05:57PM -0700, Marek Hulán wrote:

@@ -0,0 +1,4 @@
+[Dolphin]

oops, could you please remove .directory files? I accidentaly added them to my commit

Done, and rebased on latest develop: 1ecfb77.

Jan Pazdziora
Principal Software Engineer, Identity Management Engineering, Red Hat

adelton · 2013-10-24T06:22:17Z

On Wed, Oct 23, 2013 at 11:05:57PM -0700, Marek Hulán wrote:

@@ -0,0 +1,4 @@
+[Dolphin]

oops, could you please remove .directory files? I accidentaly added them to my commit

Done, and rebased on latest develop: 1ecfb77.

ares · 2013-10-24T06:46:54Z

I went through this once again and when adelton/pull/3 gets in, you have my 👍 The only unresolved comment from #958 is about authorize_login_delegation_auth_source_user_autocreate type - @domcleal?

adelton · 2013-10-24T08:14:11Z

The adelton#3 is in e8426b3.

ohadlevy · 2013-10-24T13:43:06Z

[test]

adelton · 2013-10-28T02:35:48Z

What is preventing this pull request from being merged / rebased on develop?

ohadlevy · 2013-10-28T07:21:52Z

app/models/user.rb

+        if auth_source.nil?
+          auth_source = AuthSourceExternal.new(:name => auth_source_name)
+          auth_source.save!
+        end


can be simplified:

options = { :name => auth_source_name } auth_source = AuthSourceExternal.where(options).first || AuthSourceExternal.create!(options) User.create!(:login => login, :auth_source => auth_source)

this first ensures we are always talking about External Authsources vs any authsource.

secondly, I'm wondering what happens if AutoSource is created, but user does not, I did not try, but we could try using the build method instead and save them in one go... maybe something like

options = { :name => auth_source_name } auth_source = AuthSourceExternal.where(options).first || AuthSourceExternal.build(options) User.create!(:login => login, :auth_source => auth_source)

AFAIK (require a bit testing) would create both user and auth source in the same transaction. (would nice to see a test like that).

The goal is to be able to name any auth_source, not necessarily the external one.

If the build approach works, by all means go for it.

Attempt to use the build approach leads to

x undefined method `build' for #<Class:0x000000070bcb90>

right.. i guess it should be auth_source.build(:type =>
'AuthSourceExternal'...)
but that would also need to include Foreman::STI to the AuthSource class
for it to work.

Sorry, no idea what Foreman::STI is and how to include it in this patch. I'll go with the

options = { :name => auth_source_name } auth_source = AuthSourceExternal.where(options).first || AuthSourceExternal.create!(options) User.create!(:login => login, :auth_source => auth_source)

approach.

sure, but as you said, you might need to use AuthSource.where (vs.
AuthSourceExternal.where)

Right. Pushed out as 12d820e. Thanks.

… on REMOTE_USER.

adelton · 2013-10-28T08:20:37Z

Rebased on latest develop and added commit 8a1f17c9fa03d51f9a28fd94362a37d372975f4a to give the new user the Anonymous role upon login, like form-based logons do.

…ser_autocreate is set, creates the unknown authenticated user.

…ogin so that it can be used for external logins as well.

adelton · 2013-10-28T08:42:20Z

New rebase to address Ohad's comments: 59f9484.

domcleal · 2013-11-04T13:09:41Z

I'm having problems testing this after seeing up Foreman as per the wiki page. My browser begins looping around the /users/extlogin page with a series of 302 and 401 HTTP responses.

http://people.redhat.com/~dcleal/extlogin_redirect.png (browser logs)
http://pastie.org/private/zaponz70lg0pdszlcmfoa (httpd logs)
http://pastie.org/private/ngudp6v82kjyljrrraxnng (production.log)

Any thoughts?

ares · 2013-11-04T14:11:41Z

Production log contains only 302 so 401 is rendered by apache before it gets to foreman? How did you set the Setting['login_delegation_logout_url']? Did you set kerberos login only on /users/extlogin sub-uri?

domcleal · 2013-11-04T14:23:03Z

I hadn't set the logout_url setting, but have now have - it makes no difference:

:login_delegation_logout_url: https://rhel6c.ipa.fm.example.net/users/extlogout

Yes, I'm only using mod_auth_kerb on /users/extlogin using /etc/httpd/conf.d/auth_kerb.conf:

 <Location /users/extlogin>
 AuthType Kerberos
 AuthName "Kerberos Login" 
 KrbMethodNegotiate On
 KrbMethodK5Passwd Off
 KrbAuthRealms IPA.FM.EXAMPLE.NET
 Krb5KeyTab /etc/http.keytab
 KrbLocalUserMapping On
 require valid-user
 ErrorDocument 401 'Kerberos authentication did not pass.'
 ErrorDocument 500 'Kerberos authentication did not pass.' 
 </Location>

adelton · 2013-11-05T02:00:11Z

My output after successful Kergeros login is

Started GET "/users/extlogin" for 10.36.112.30 at 2013-11-04 20:54:46 -0500
Processing by UsersController#extlogin as HTML
Authorized user admin(Admin User)
Redirected to https://foreman.example.com/hosts
Completed 302 Found in 21ms (ActiveRecord: 11.7ms)

Since I don't see that Authorized user ... message in http://pastie.org/private/ngudp6v82kjyljrrraxnng I assume something has gone wrong in your setup. In any case, I am not able to reproduce that loop of yours (I just did another fresh installation from http://yum.theforeman.org/releases/1.3/el6/$basearch).

adelton · 2013-11-05T02:14:00Z

I assume the culprit could be in

  def login_user(user)
    session[:user]         = user.id
    uri                    = session[:original_uri]
    session[:original_uri] = nil
    redirect_to (uri || hosts_path)
  end

-- if you have /users/extlogin in session[:original_uri], that might cause the loop. Can you tweak the code to check if that is the case?

But I have no idea how that /users/extlogin would get there in the first place.

adelton · 2013-11-07T08:20:04Z

Did you have chance to test this? If the endless loop comes from this code, we might want to not do the redirect_to if the uri is already the same as request.fullpath.

domcleal · 2013-11-12T11:30:13Z

PEBKAC, sorry for the noise. I'd done a nightly install and attempted to apply the patch on top, but somehow it hadn't fully applied. An RPM build of the branch worked much better.

domcleal · 2013-11-12T12:00:47Z

Rebased and merged as b7589c3 for Foreman 1.4.0. Thanks very much @adelton, and my apologies for the delay.

sodabrew · 2013-11-12T14:45:48Z

Awesome work! I'm very excited about this merge, thanks @adelton!

adelton · 2013-11-12T23:36:54Z

Thanks for the merge. I'll now rebase #986 and work on it.

This was referenced Oct 21, 2013

[Obsolete by https://github.com/theforeman/foreman/pull/967] Improve the REMOTE_USER (for example mod_auth_kerb) support #958

Closed

Reuse existing SSO method logic adelton/foreman#1

Closed

adelton mentioned this pull request Oct 22, 2013

Add tests adelton/foreman#2

Merged

ares reviewed Oct 24, 2013
View reviewed changes

ohadlevy reviewed Oct 28, 2013
View reviewed changes

fixes theforeman#3312 - support /users/extlogin as a login page based…

f649d6d

… on REMOTE_USER.

adelton and others added 5 commits October 28, 2013 16:22

fixes theforeman#3312 - support logout from REMOTE_USER-based logons.

a22f033

fixes theforeman#3312 - when authorize_login_delegation_auth_source_u…

12d820e

…ser_autocreate is set, creates the unknown authenticated user.

fixes theforeman#3312 - tweaking nil tests; private.

294be0d

fixes theforeman#3312 - add tests.

6e98dc7

fixes theforeman#3312 - factor out the post-login logic from try_to_l…

59f9484

…ogin so that it can be used for external logins as well.

adelton mentioned this pull request Oct 28, 2013

Populate first name, last name, and email, if available. #986

Closed

domcleal closed this Nov 12, 2013

adelton deleted the mod_auth_kerb-2 branch November 12, 2013 23:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve the REMOTE_USER (for example mod_auth_kerb) support #967

Improve the REMOTE_USER (for example mod_auth_kerb) support #967

adelton commented Oct 21, 2013

dLobatog commented Oct 21, 2013

ares commented Oct 21, 2013

adelton commented Oct 22, 2013

lzap commented Oct 22, 2013

sodabrew commented Oct 23, 2013

adelton commented Oct 24, 2013

ares Oct 24, 2013

adelton Oct 24, 2013

adelton commented Oct 24, 2013

ares commented Oct 24, 2013

adelton commented Oct 24, 2013

ohadlevy commented Oct 24, 2013

adelton commented Oct 28, 2013

ohadlevy Oct 28, 2013

adelton Oct 28, 2013

adelton Oct 28, 2013

ohadlevy Oct 28, 2013

adelton Oct 28, 2013

ohadlevy Oct 28, 2013

adelton Oct 28, 2013

adelton commented Oct 28, 2013

adelton commented Oct 28, 2013

domcleal commented Nov 4, 2013

ares commented Nov 4, 2013

domcleal commented Nov 4, 2013

adelton commented Nov 5, 2013

adelton commented Nov 5, 2013

adelton commented Nov 7, 2013

domcleal commented Nov 12, 2013

domcleal commented Nov 12, 2013

sodabrew commented Nov 12, 2013

adelton commented Nov 12, 2013

Improve the REMOTE_USER (for example mod_auth_kerb) support #967

Improve the REMOTE_USER (for example mod_auth_kerb) support #967

Conversation

adelton commented Oct 21, 2013

dLobatog commented Oct 21, 2013

ares commented Oct 21, 2013

adelton commented Oct 22, 2013

lzap commented Oct 22, 2013

sodabrew commented Oct 23, 2013

adelton commented Oct 24, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adelton commented Oct 24, 2013

ares commented Oct 24, 2013

adelton commented Oct 24, 2013

ohadlevy commented Oct 24, 2013

adelton commented Oct 28, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adelton commented Oct 28, 2013

adelton commented Oct 28, 2013

domcleal commented Nov 4, 2013

ares commented Nov 4, 2013

domcleal commented Nov 4, 2013

adelton commented Nov 5, 2013

adelton commented Nov 5, 2013

adelton commented Nov 7, 2013

domcleal commented Nov 12, 2013

domcleal commented Nov 12, 2013

sodabrew commented Nov 12, 2013

adelton commented Nov 12, 2013