Modify

Opened 3 years ago

Closed 19 months ago

Last modified 18 months ago

#8545 closed defect (fixed)

Authentication always fails

Reported by: xeross Owned by: hasienda
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 3 years ago by hasienda

  • 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 3 years ago by xeross

Trac 0.12.3dev-r10609
TracAccountManager 0.3dev-r9785

Current authentication stores:
HtPasswdStore

comment:3 Changed 3 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 3 years ago by hasienda

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 follow-up: Changed 3 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 3 years ago by hasienda

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 3 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 3 years ago by hasienda

  • 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 3 years ago by 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 3 years ago by anonymous

  • Keywords needinfo removed
  • Severity changed from normal to blocker

comment:11 follow-up: Changed 3 years ago by 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 ; follow-up: Changed 3 years ago by 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? 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.pyL542).

comment:13 in reply to: ↑ 12 ; follow-up: Changed 3 years ago by 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 http://trac.edgewall.org/wiki/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?

comment:14 in reply to: ↑ 13 Changed 3 years ago by hasienda

  • Status changed from new to assigned

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 follow-up: Changed 23 months ago by anonymous

is there any plan to fix that?

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

comment:16 follow-up: Changed 23 months ago by hasienda

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 20 months ago by hasienda

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 20 months ago by hasienda

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 20 months ago by hasienda

(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 19 months ago by hasienda

  • Resolution set to fixed
  • Status changed from assigned to closed

(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 18 months ago by hasienda

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

comment:22 Changed 18 months ago by techtonik

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

Add Comment

Modify Ticket

Action
as closed .
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from hasienda. Next status will be 'closed'.
The resolution will be deleted. Next status will be 'reopened'.
Author


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

 
Note: See TracTickets for help on using tickets.