go-gitea / gitea

Git with a cup of tea! Painless self-hosted all-in-one software development service, including Git hosting, code review, team collaboration, package registry and CI/CD
https://gitea.com
MIT License
45.09k stars 5.49k forks source link

Gitea deactivates LDAP users #7949

Closed aravindhsampath closed 4 years ago

aravindhsampath commented 5 years ago

Description

While using Gitea with FreeIPA as authentication source, Gitea repeatedly deactivates valid user account every time auth source is synchronised. This is rather annoying to the extent that it makes Gitea unusable in a FreeIPA environment. There are already several issues opened in the past for this very same issue, but they either went stale or have been closed pointing to a hack that does not quite address the root cause of the issue. This is why I am creating this meta-issue referring to previous closed issues. If this issue also gets closed without a solution in sight, I will have to abandon using Gitea and move our stuff over to something else. :-( Making a desperate attempt to keep using Gitea because it is what I want.

Past issues:

  1. https://github.com/go-gitea/gitea/issues/4067 State = Closed ; Reason = a hack to NOT sync LDAP users was supplied.
  2. https://github.com/go-gitea/gitea/issues/4433 State = Stale & Closed
  3. https://github.com/go-gitea/gitea/issues/4404 State = Stale & Closed
  4. https://github.com/go-gitea/gitea/issues/4402 State = Stale & Closed
  5. https://github.com/go-gitea/gitea/issues/3815 State = Open

Issue # 3815 refers to LDAP call failing leading to users being de-activated. I cannot verify whether this is happening in my case - the logs don't quite show any LDAP activity in the first place.. I tried running in dev instead of prod as well without luck.

Even if LDAP call fails, de-activating users should not be the default activity! Even in good conditions LDAP server may go offline temporarily, which will lead to disruption in Gitea unnecessarily. This feature needs to be modified such that activation/de-activation of users happen only on a successful LDAP call.

Alternatively, a new config option may be added for e.g local_activation = true which skips the activation deactivation field of the SQL query if the config var is set, and allow the Gitea administrator to activate or de-activate all users(local+LDAP) in Gitea admin panel.

Why am I not okay with the hack? The hack is to supply UPDATE_EXISTING = false for LDAP sync. I have users who changed their passwords in FreeIPA, and Gitea will not take the new password. As an admin, I do not have the ability to sync this owner's LDAP data alone. I end up re-enabling LDAP sync and resort to manually activate all users after that 👎 only for the same to happen the next time a user changes their password.

Happy to provide any information from my installation. Desperately looking for help. ...

Screenshots

jorgytim commented 5 years ago

I see the exact same issue, which is making this unusable for my development team. We have our Gitea instance using Active Directory as our LDAP server, but the exact same thing occurs whenever a directory sync is performed, all existing users lose their activated status. The only fix is to go in and manually edit every account to set them as activated.

We absolutely need to leave existing accounts activated whenever an LDAP sync is performed, it's almost like the sync is overwriting the local user account info entirely, rather than just merging changes.

lafriks commented 5 years ago

Problem is that no one from developers are able to reproduce issue, if there is no clear ways to reproduce that it is hard to fix it also. In company I work for we use microsoft AD and had no such problem ever. I created that functionality so most probably that is my bug but I can't really fix what I have not seen :/

jorgytim commented 5 years ago

I completely understand that a developer may not be able to reproduce this, but with multiple issues being reported, doesn't it at least warrant a code review to see what the ldap synchronization is doing when it creates local user accounts. Of course we all understand we have access to the source code to review this ourselves, but we are just asking for someone to at least give it a review with this concern in mind, as it would be much faster for an existing developer to figure out than the spin up time it would be for us in the community to figure out what is going on.

aravindhsampath commented 5 years ago

It is reproducible with FreeIPA as an auth source(which Gitea has guidance about in its documentation) in the sense that I tried different versions of Gitea in different servers with same FreeIPA. I can understand that it is harder for a dev to fix something they don't even know where is broken. But, I and several others who created the issues above refer to the same problem that LDAP sync is doing something that it should not be doing in the first place - deactivating accounts without formal confirmation that those accounts were de-activated by an admin.

So, IMHO this should be categorised as a logical bug, and solution can be to either 1. ignore the activation/de-activation field altogether from LDAP sync and tell the Gitea admin that it is their responsibility to use the Gitea admin panel to do so, or 2. have a config parameter "local_activation" as I mentioned in the orig. post that prevents this behaviour. or 3. have the Gitea LDAP sync code de-activate accounts if and only if it can see that the account was de-activated in LDAP source and not just because an LDAP query failed or because the user supplied a "Username attribute" that is not aligned with the LDAP data.

davidsvantesson commented 5 years ago

@aravindhsampath I continue discussion in this issue

If receiving an empty list of users from LDAP, they will still be deactivated.

I dont understand this logic. If Gitea receives an empty list from LDAP how can it assume that ALL users are to be de-activated? Shouldn't Gitea de-activate users ONLY if it is certain that LDAP says that they are infact de-activated? In my mid, an empty list from LDAP should lead to Gitea doing nothing

First: If you manage to make a successful LDAP query, why is the list empty? That sound as a fault in the LDAP service/server, and should rather be an issue towards that than Gitea!? (It could of course also be a "valid" error that Gitea doesn't detect, but an empty list can't be a fault in itself.) I'm newbie to LDAP, but what I understand there is no standardized way to know if an account was deactivated. But if a user is not longer in the list from the LDAP query, that should normally mean the user shall not have access any longer.

Anyhow, maybe this could be possible features to add to Gitea to solve this:

I looked into this because I had some problems with LDAP. But in my case the problem was we had two LDAP sources, and Gitea doesn't automatically move users from one to the other. So the user has to be moved manually to the other LDAP source. I know this is not your issue, but just wanted to mention in case someone else having that issue.

aravindhsampath commented 5 years ago

@davidsvantesson Thanks for taking the time to explain the reasoning behind your work and #7960 .

On a successful query, why is the list empty? I'm not an LDAP expert either, but I have a case in hand where LDAP connection seems successful (since users totally new to Gitea are able to login because their creds are supplied by LDAP source), but everytime LDAP sync runs, ALL users are de-activated. So, Gitea is possibly receiving an empty list or a malformed list from LDAP source and decides to annoy the Gitea admin - me :-| So, it is evidently possible to have a working LDAP source but receive a bad response leading to this behavior.

Another corner case is LDAP server going down temporarily(which is more common than not). Gitea cannot assume that all users must be locked out because of this.

Atleast in FreeIPA (my LDAP source), there is a field for activation/de-activation status of an account. Perhaps Gitea could use such a field to justifiably disable an account - or else under a case of lacking evidence of de-activation, relying on Gitea admin would be wise.

I dont know how I can make Gitea write out the results of this LDAP query in the log so that we all can get some data to see. I tried putting Gitea in "dev" mode, which did not do anything to the logs. Is there are log level I can set to get such info? do you know?

I agree that, if not changing the default behaviour, there must be a config option to turn off this "LDAP based de-activation" feature so that users like me can continue to use Gitea for all its awesomeness!

lafriks commented 5 years ago

Can you provide your ldap auth source settings?

aravindhsampath commented 5 years ago
Screenshot 2019-08-25 at 22 44 32 Screenshot 2019-08-25 at 22 44 52
aravindhsampath commented 5 years ago

I tried various options for "Username Attribute" field, all resulting in the same behaviour of de-activation. I tried uid(which I believe is correct according to FreeIPA), and several other options listed in the listed issues.

Going to sleep now as it is late where I am. I will update more info as possible after 10 hours.

guillep2k commented 5 years ago

@aravindhsampath I suggest you try Gitea again when #7960 is released (I'm guessing 1.10.0 or even 1.9.3). From what you describe, it seems that the list was empty because Gitea was not detecting the error that came with it (e.g. a temporary connection problem). I have the feeling that with this fix your problem will be gone.

lafriks commented 5 years ago

Username attribute should be uid in your case

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 weeks. Thank you for your contributions.

6543 commented 5 years ago

@aravindhsampath do you have this problem with v1.9.5 too?

jscolaire commented 5 years ago

I have 1.11.0 version and it has this issue too

6543 commented 5 years ago

@jscolaire thanks for your report :)

PipeItToDevNull commented 5 years ago

I am having this issue for my LDAP users when synchronizing from AD on v1.9.5

davidsvantesson commented 5 years ago

@jscolaire @PipeItToDevNull Occasionally or at every sync?

PipeItToDevNull commented 5 years ago

@davidsvantesson I believe it only occurs when my AD is in err and the sync does not succeed (the AD is offline or there is a network issue)

jscolaire commented 5 years ago

@jscolaire I disabled sync IPA on authentication source and it looks well

davidsvantesson commented 5 years ago

@PipeItToDevNull Have you enabled trace logging and what can you see in logs regarding ldap when this occurs?

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 weeks. Thank you for your contributions.

zeripath commented 4 years ago

Hmm - ok so I would guess we're being too aggressive with cleaning up when there is an error.

zeripath commented 4 years ago

https://github.com/go-gitea/gitea/blob/d171cd41b178874ce22791c07d35283b126ebdf3/models/user.go#L1765

Ok so presumably there are ldaps out there that will error but not return an error, but instead return an empty set.

I would guess a solution would be to log an error on empty return from an LDAP and quit there unless there is a setting set which will allow an LDAP to delete all of its users?

guillep2k commented 4 years ago

I would guess a solution would be to log an error on empty return from an LDAP and quit there unless there is a setting set which will allow an LDAP to delete all of its users?

I don't have much experience with LDAP, but perhaps there are different kinds of empty responses? (like empty set != nil). If that's not the case, I would say the setting is the lesser evil.

PipeItToDevNull commented 4 years ago

I am on 1.10.2 talking to Windows AD and this is still an issue.

I can see the main issue being Windows not sending a proper response time to time, it would be nice if this fix was in place so the admin needs to disable/delete accounts not the sync service.

davidsvantesson commented 4 years ago

We never see this on Windows AD. I don't know if it is related to version of Windows AD or something with the server environment, or why only some people see this.

zeripath commented 4 years ago

@PipeItToDevNull are you using PagedSearch? @davidsvantesson how about you?

Maybe there's something about AD doing some limiting around unpaged searches etc?

lafriks commented 4 years ago

There are definitely limits for not paged searches but imho if you are over limit it should return error

vtmb commented 4 years ago

Hello, my ldap-users are deactivated after some time. How to prevent this?

PipeItToDevNull commented 4 years ago

I use this at home, so I do not have new users being made. I disabled the LDAP sync to my DC and it stopped disabling users

vtmb commented 4 years ago

I this is bad news to me. I have a lot of users that are stored via an MS AD. If I disable the sync, no new users can be created by "just logging in", I suppose. I could tell everyone "log in today and after your profile is created I will deactivate syncing" but this is not that handy, isn't it?

PipeItToDevNull commented 4 years ago

I completely understand that, my work-around is hardly a solution

vtmb commented 4 years ago

Too bad. But thank you for your suggestion. It could have been a solution if it wasn't that many users i rely on. Have a nice day.

zeripath commented 4 years ago

@vtmb what version of Gitea are you using? "Allow Deactivate All" should be set to false by default from 1.10+

If you're running something older than 1.12 you should upgrade.

6543 commented 4 years ago

this is an old issue for 1.9.1 / is it realy the same bug - or can we open a new issue?

vtmb commented 4 years ago

Actually I am using gitea 1.12.5

lafriks commented 4 years ago

It has to be something with filters you have set as I have never seen such behavior and I have been using ldap for years already

zeripath commented 4 years ago

Have you set AllowDeactivateAll to false?

lafriks commented 4 years ago

@zeripath no I have not

6543 commented 4 years ago

since nobody has created a new issue, I'll reopen this one! (aktive conversation on closed issues are a bad habit)

vtmb commented 4 years ago

I think I have solved this, it had to do with the filters. The filter for users to deactivate was * by default and I haven't seen it the whole time. I am not very good at seeing, I am glad I saw that single Asteriks while checking!

So sorry for the trouble - it was my fault or my eyes fault!

zeripath commented 4 years ago

Thanks for the update

lcnittl commented 4 years ago

So: As far as I understand, when having Allow an empty search result to deactivate all users unchecked, users should never be deactivated?

All my users (user emails) got deactivated on LDAP sync, as I was having a faulty filter in place. I assume that this did not return an empty list, but usually gives an error when using ldapsearch in CLI, but maybe this is treated as empty list?

$ ldapsearch -x -h ldap.lxc0 -b "ou=users,dc=our,dc=domain" "(obviouslyFaultyFilter)"
# extended LDIF
#
# LDAPv3
# base <ou=users,dc=our,dc=domain> with scope subtree
# filter: (obviouslyFaultyFilter)
# requesting: ALL
#

ldap_search_ext: Bad search filter (-7)

Maybe a new feature could be a "test sync" or "test filters" button like it is available for the SMTP Mailer Configuration (Send Testing Email)?

I can open a new issue, if desired.

PS: Gitea version is 1.12.4 ; OpenLDAP version is 2.4.47+dfsg-3+deb10u3

lafriks commented 4 years ago

Yes please open other issue