Modify

Opened 13 years ago

Closed 11 years ago

Last modified 8 years ago

#8545 closed defect (fixed)

Authentication always fails

Reported by: Xeross Owned by: Steffen Hoffmann
Priority: normal Component: AccountManagerPlugin
Severity: blocker Keywords: auth failure
Cc: Trac Release: 0.12

Description

I've been experiencing problems since I've updated trac (Well I think they started around then), the built-in authentication still works however both my custom auth for AccountManager and the built in HtPasswd store fail to login.

I added debug output to my own custom "bridge" and the user is found, and the passwords match, yet I get an incorrect username and/or password error.

This happens with multiple trac installs that also use the HtPasswd store, so I'm kind of confused as what this can be.

Trac is running in WSGI mode behind an NGINX HTTPd.

The problem might be specific to me because I can't find anything about it, but I can't really think of anything that it might be that I haven't checked.

So is there any way to get better details on what's going on, because even debug logging doesn't give much. And are there known cases of this problem?

Attachments (0)

Change History (22)

comment:1 Changed 13 years ago by Steffen Hoffmann

Keywords: failure added; fails removed

I didn't check, if debug logging is where it would be most helpful, sorry. So certainly the reason why you don't come up with more details here is partially because of absence of such logging at critical places. It's a matter of fact that we're far from perfect now. Recent development introduced some new bugs that we're about to fix.

Still mentioning your OS, Python version, AcctMgr revision and some more details, i.e. all used authentication stores, would give us a better start. Thanks in advance.

comment:2 Changed 13 years ago by Xeross

Trac 0.12.3dev-r10609 TracAccountManager 0.3dev-r9785

Current authentication stores: HtPasswdStore

comment:3 Changed 13 years ago by Xeross

I guess I could try downgrading to Trac 0.12.2 but if I recall correctly that had the same issue, guess I could try again though :/

This is kind of annoying as I have like 4 trac installs that are all currently malfunctioning :/

comment:4 Changed 13 years ago by Steffen Hoffmann

We've just found a major glitch with password file handling on non-*nix systems (see #8487). I still don't know your OS, but try trunk at [9923], at least if it's any Windows flavor, please.

comment:5 Changed 13 years ago by anonymous

Well I am using Linux (Debian 5.0 to be exact), so yeah. And it doesn't work with either a password file or my login-bridge that connects with MySQL.

I guess I could try setting up an isolated environment on a test machine, see if that works.

comment:6 in reply to:  5 Changed 13 years ago by Steffen Hoffmann

Replying to anonymous:

Well I am using Linux (Debian 5.0 to be exact), so yeah. And it doesn't work with either a password file or my login-bridge that connects with MySQL.

Ok, so my previous guess was irrelevant.

I guess I could try setting up an isolated environment on a test machine, see if that works.

I'll need to add more debug code then, at least for you.

comment:7 Changed 13 years ago by Xeross

All I can think of is that my current environment is messed up causing them all to malfunction, but I haven't done anything that could cause that.

comment:8 Changed 13 years ago by Steffen Hoffmann

Keywords: needinfo added

There has been many changes including various fixes for improper password file handling. I recommend to recheck with latest revision.

If this still fails, I'll prepare an 'excessive debug' patch for you to help with localizing the weak spot, ok?

comment:9 Changed 12 years ago by anatoly techtonik

The same problem Trac 0.12.2, AccMgr 3.2, FastCGI through Nginx. Debian.

Hasienda, it would be better if you could just add logging with the 'trace' level to critical parts and leave them there. It could also help if methods to check pass/user are exposed and run from Python console.

comment:10 Changed 12 years ago by anonymous

Keywords: needinfo removed
Severity: normalblocker

comment:11 Changed 12 years ago by anatoly techtonik

Ok. For me the problem was introduced in [9556] which was an attempt to fix #5964. It seems that in my environment which uses http://trac.edgewall.org/wiki/TracFastCgi script for NGINX the REMOTE_USER variable is already present in environment.

comment:12 in reply to:  11 ; Changed 12 years ago by Steffen Hoffmann

Replying to techtonik:

Ok. For me the problem was introduced in [9556] which was an attempt to fix #5964. It seems that in my environment which uses http://trac.edgewall.org/wiki/TracFastCgi script for NGINX the REMOTE_USER variable is already present in environment.

Seems like this is related to AuthenticationMiddleware (see T:browser:trunk/trac/web/standalone.py?rev=10594#L55), right? In AccountManagerPlugin's LoginModule we'll need another way to decide, if authentication has already happened, but without setting 'REMOTE_USER' in the request object on our own (see accountmanagerplugin/trunk/acct_mgr/web_ui.py#L542).

Last edited 8 years ago by Ryan J Ollos (previous) (diff)

comment:13 in reply to:  12 ; Changed 12 years ago by anatoly techtonik

Replying to hasienda:

Replying to techtonik:

Ok. For me the problem was introduced in [9556] which was an attempt to fix #5964. It seems that in my environment which uses http://trac.edgewall.org/wiki/TracFastCgi script for NGINX the REMOTE_USER variable is already present in environment.

Seems like this is related to AuthenticationMiddleware (see T:browser:trunk/trac/web/standalone.py?rev=10594#L55), right?

Not really. I use 2nd script from trac:TracFastCgi#SimpleNginxConfiguration that imports trac.web._fcgi directly and as a result bypasses AuthenticationMiddleware defined in standalone.py

In AccountManagerPlugin's LoginModule we'll need another way to decide, if authentication has already happened, but without setting 'REMOTE_USER' in the request object on our own (see accountmanagerplugin/trunk/acct_mgr/web_ui.py#L542).

If we think about it more. REMOTE_USER should be set by web server if and only if the resource is protected and the user is authenticated. So, if the variable is already set when AccountManager has full right to report an error with advice to re-check web-server config files to see who sets REMOTE_USER variable.

In the Nginx config file above, I've disabled authentication by Nginx, but didn't disable environment variables passed to FastCGI script (well, because I wasn't aware of the issue). To protect other users from the same mistake, I'll add a notice to example, but it will also help to implement a workaround that check for empty string REMOTE_USER, complains and still sets it. If REMOTE_USER is already set is not empty - just complain.

How is that solution?

Last edited 8 years ago by Ryan J Ollos (previous) (diff)

comment:14 in reply to:  13 Changed 12 years ago by Steffen Hoffmann

Status: newassigned

Replying to techtonik:

How is that solution?

Thank you very much for your thoughts and investigation. Sadly current AcctMgr logic uses (abuses?) that value to prevent multiple calls to LoginModule as hinted in [9556]. We'll have to find another way for that loop-prevention then.

comment:15 Changed 12 years ago by anonymous

is there any plan to fix that?

it make this wonder full plugin useless for me ...

comment:16 Changed 12 years ago by Steffen Hoffmann

I've #5964 as well as this one open yet for thinking about it, but no good idea yet.

Especially it's hard to see, how [9556] could have caused issues, because before the REMOTE_USER has been always set unconditionally, and no one complained. Still I want it fixed, even for acct_mgr-0.4, if possible.

comment:17 in reply to:  15 Changed 11 years ago by Steffen Hoffmann

Replying to anonymous:

is there any plan to fix that?

it make this wonder full plugin useless for me ...

Huh, I would be very interested to hear details about the Why.

As pointed out before the AccountManagerPlugin while using its login form does authentication itself, so should be entitled to set REMOTE_USER itself. With acct_mgr.web_ui.LoginModule disabled there neither was nor is an issue of this plugin interfering with other authenticators.

If you still feel, that there are applications, where setting REMOTE_USER should still left to another component (the web-server?), I didn't get that by now, and you should better tell me. Making this conditional for good reason would be easy while I'm on it. So speak-up now, please.

comment:18 in reply to:  16 Changed 11 years ago by Steffen Hoffmann

Replying to hasienda:

I've #5964 as well as this one open yet for thinking about it, but no good idea yet.

Glad to tell, that I got over it and there's a patch, that could get pushed soonish.

Still I want it fixed, even for acct_mgr-0.4, if possible.

Likely acct_mgr-0.4.1 now.

comment:19 Changed 11 years ago by Steffen Hoffmann

(In [12444]) AccountManagerPlugin: Revert [9556] and implement a better solution, refs #5964 and #8545.

Undesired recursion on LoginModule.authenticate is prevented by always setting 'user_locked' on the request now. New option 'environ_auth_overwrite' allows to control previously hard-coded (re-)set of environment variable REMOTE_USER.

Finally some log output should ease future authentication issue debugging.

comment:20 Changed 11 years ago by Steffen Hoffmann

Resolution: fixed
Status: assignedclosed

(In [12482]) AccountManagerPlugin: Publish maintenance release 0.4.1, closes #5964, #8545, #10134, #10625, #10700 and #10701.

This is an update for current stable acct_mgr-0.4 with a number of fixes for issues resolved within the last weeks, i.e.:

  • a final fix for Single-Sign-On functionality (refs #9676),
  • a long-standing HttpAuth login issue and
  • one for acct_mgr.LoginModule, that is relevant if used with web-servers, that evaluate the REMOTE_USER environment variable.

Changeset [12468] is included, that may require a Trac db fix-up. Run python ./contrib/fix-session_attribute-failed_logins.py <env> once on any Trac environment, that had account locking enabled with time constraints before.

comment:21 Changed 11 years ago by Steffen Hoffmann

Glad we fixed this, as the DEBUG logging introduced here proved very helpful in debugging #10826.

comment:22 Changed 11 years ago by anatoly techtonik

Because this ticket is closed, it will be better to discuss this in #10827.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Steffen Hoffmann.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.