github-starred/gitea
2
0
Fork 0
mirror of https://github.com/go-gitea/gitea.git synced 2026-03-14 03:46:23 -05:00
Code Issues 2.6k Packages Projects Releases 100 Wiki Activity

LDAP BindDN Multiple Log In Options Results in user already exists error #4690

New Issue
Closed
opened 2025-11-02 05:59:37 -06:00 by GiteaMirror · 7 comments
GiteaMirror commented 2025-11-02 05:59:37 -06:00
Owner
Copy Link

Originally created by @iamdoubz on GitHub (Jan 20, 2020).

  • Gitea version (or commit ref): 1.11.0+rc1-41-gce756ee89 and 1.11.0+rc1-42-gf96c1a2c7
  • Git version: 2.25.0.windows.1
  • Operating system: Windows Server 2012 R2
  • Database (use [x]):
    • PostgreSQL
    • MySQL (MariaDB Server version: 10.1.34-MariaDB)
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)
    • No (Can't really test out my AD environment in a public instance)
    • Not relevant
  • Log gist (trace):
2020/01/20 10:00:11 ...es/auth/ldap/ldap.go:216:SearchEntry() [T] LDAP will use BindDN.
2020/01/20 10:00:11 ...es/auth/ldap/ldap.go:226:SearchEntry() [T] Bound as BindDN CN=Gitea,OU=Users,DC=my,DC=domain
2020/01/20 10:00:11 ...es/auth/ldap/ldap.go:85:findUserDN() [T] Search for LDAP user: 334455
2020/01/20 10:00:11 ...es/auth/ldap/ldap.go:93:findUserDN() [T] Searching for DN using filter (&(objectClass=Person)(|(sAMAccountName=334455)(mail=334455)(employeeNumber=334455))) and base OU=Users,DC=my,DC=domain
2020/01/20 10:00:11 ...es/auth/ldap/ldap.go:144:bindUser() [T] Binding with userDN: CN=First M. Lastname,OU=Users,DC=my,DC=domain
2020/01/20 10:00:11 ...es/auth/ldap/ldap.go:150:bindUser() [T] Bound successfully with userDN: CN=First M. Lastname,OU=Users,DC=my,DC=domain
2020/01/20 10:00:11 ...es/auth/ldap/ldap.go:257:SearchEntry() [T] Fetching attributes 'sAMAccountName', 'givenName', 'sn', 'mail', '' with filter (&(objectClass=Person)(|(sAMAccountName=334455)(mail=334455)(employeeNumber=334455))) and base CN=First M. Lastname,OU=Users,DC=my,DC=domain
2020/01/20 10:00:11 ...es/auth/ldap/ldap.go:156:checkAdmin() [T] Checking admin with filter (memberOf=CN=GiteaAdmins,OU=Users,DC=my,DC=domain) and base CN=First M. Lastname,OU=Users,DC=my,DC=domain
2020/01/20 10:00:11 .../xorm/session_raw.go:78:queryRows() [I] [SQL] SELECT `id`, `lower_name`, `name`, `full_name`, `email`, `keep_email_private`, `email_notifications_preference`, `passwd`, `passwd_hash_algo`, `must_change_password`, `login_type`, `login_source`, `login_name`, `type`, `location`, `website`, `rands`, `salt`, `language`, `description`, `created_unix`, `updated_unix`, `last_login_unix`, `last_repo_visibility`, `max_repo_creation`, `is_active`, `is_admin`, `allow_git_hook`, `allow_import_local`, `allow_create_organization`, `prohibit_login`, `avatar`, `avatar_email`, `use_custom_avatar`, `num_followers`, `num_following`, `num_stars`, `num_repos`, `num_teams`, `num_members`, `visibility`, `repo_admin_change_team_access`, `diff_view_style`, `theme` FROM `user` WHERE (id!=?) AND `lower_name`=? LIMIT 1 []interface {}{0, "fmlastname"} - took: 1.1474ms
2020/01/20 10:00:11 .../xorm/session_raw.go:78:queryRows() [I] [SQL] SELECT `id`, `lower_name`, `name`, `full_name`, `email`, `keep_email_private`, `email_notifications_preference`, `passwd`, `passwd_hash_algo`, `must_change_password`, `login_type`, `login_source`, `login_name`, `type`, `location`, `website`, `rands`, `salt`, `language`, `description`, `created_unix`, `updated_unix`, `last_login_unix`, `last_repo_visibility`, `max_repo_creation`, `is_active`, `is_admin`, `allow_git_hook`, `allow_import_local`, `allow_create_organization`, `prohibit_login`, `avatar`, `avatar_email`, `use_custom_avatar`, `num_followers`, `num_following`, `num_stars`, `num_repos`, `num_teams`, `num_members`, `visibility`, `repo_admin_change_team_access`, `diff_view_style`, `theme` FROM `user` WHERE `lower_name`=? LIMIT 1 []interface {}{"fmlastname"} - took: 999.2µs
2020/01/20 10:00:11 .../xorm/session_raw.go:85:queryRows() [I] [SQL] SELECT `id`, `lower_name`, `name`, `full_name`, `email`, `keep_email_private`, `email_notifications_preference`, `passwd`, `passwd_hash_algo`, `must_change_password`, `login_type`, `login_source`, `login_name`, `type`, `location`, `website`, `rands`, `salt`, `language`, `description`, `created_unix`, `updated_unix`, `last_login_unix`, `last_repo_visibility`, `max_repo_creation`, `is_active`, `is_admin`, `allow_git_hook`, `allow_import_local`, `allow_create_organization`, `prohibit_login`, `avatar`, `avatar_email`, `use_custom_avatar`, `num_followers`, `num_following`, `num_stars`, `num_repos`, `num_teams`, `num_members`, `visibility`, `repo_admin_change_team_access`, `diff_view_style`, `theme` FROM `user` WHERE (id!=?) AND `lower_name`=? LIMIT 1 []interface {}{0, "fmlastname"} - took: 1.0001ms
2020/01/20 10:00:11 ...dels/login_source.go:805:UserSignIn() [W] Failed to login '334455' via 'LDAP-Bind': user already exists [name: fmlastname]
2020/01/20 10:00:11 ...s/context/context.go:139:HTML() [D] Template: user/auth/signin
2020/01/20 10:00:11 routers/user/auth.go:171:SignInPost() [I] Failed authentication attempt for 334455 from 192.168.10.17:3418
2020/01/20 10:00:12 .../xorm/session_raw.go:78:queryRows() [I] [SQL] SELECT `id`, `type`, `name`, `is_actived`, `is_sync_enabled`, `cfg`, `created_unix`, `updated_unix` FROM `login_source` WHERE (is_actived = ? and type = ?) []interface {}{true, 7} - took: 15.0985ms
2020/01/20 10:00:12 ...s/context/context.go:330:func1() [D] Session ID: bf99b215ba3a0c69
2020/01/20 10:00:12 ...s/context/context.go:331:func1() [D] CSRF Token: ZXQVKwqrWZaiGXr1oKI3LlfQ6Rk6MTU3OTUzNTU1MTU0MTQ4MTcwMA
2020/01/20 10:00:12 ...s/context/context.go:139:HTML() [D] Template: pwa/manifest_json
2020/01/20 10:00:13 .../xorm/session_raw.go:78:queryRows() [I] [SQL] SELECT `id`, `type`, `name`, `is_actived`, `is_sync_enabled`, `cfg`, `created_unix`, `updated_unix` FROM `login_source` WHERE (is_actived = ? and type = ?) []interface {}{true, 7} - took: 1.1432ms
2020/01/20 10:00:13 ...s/context/context.go:330:func1() [D] Session ID: bf99b215ba3a0c69
2020/01/20 10:00:13 ...s/context/context.go:331:func1() [D] CSRF Token: ZXQVKwqrWZaiGXr1oKI3LlfQ6Rk6MTU3OTUzNTU1MTU0MTQ4MTcwMA
2020/01/20 10:00:13 ...s/context/context.go:139:HTML() [D] Template: pwa/serviceworker_js

Description

When logging in with LDAP with multiple log in options that are not the typical "username" or "email" uid's, you must check if the user exists and then authenticate using what is defined in "Username Attribute".

What should occur

When logging in with a non-standard option like employeeNumber, it should authenticate to LDAP, if passes then check if user already exists, if not, create user, if already exists and password is good, log the user in.

What is happening

When logging in with a non-standard option like employeeNumber, it will try to create a new user without checking to make sure that user does not already exist which results in an invalid credential on log in page.

Relevant lines of code:

models/login_source.go Lines 527 - 533.

	err := CreateUser(user)

	if err == nil && isAttributeSSHPublicKeySet && addLdapSSHPublicKeys(user, source, sr.SSHPublicKey) {
		err = RewriteAllPublicKeys()
	}

	return user,

Screenshots

gitea_employeeNumber_login

My environment

  1. Compiled from source and built using git clone https://github.com/go-gitea/gitea.git&&git checkout release/v1.11&&TAGS="bindata sqlite sqlite_unlock_notify" make build
  2. Go v1.13.6
  3. Make v4.2
  4. Gitea v1.11.0+rc1-41-gce756ee89
  5. IIS reverse proxy to localhost on custom port

Other

Semi-relevant ticket that fixed the 500 error associated with same scenario.

Originally created by @iamdoubz on GitHub (Jan 20, 2020). - Gitea version (or commit ref): **1.11.0+rc1-41-gce756ee89** and **1.11.0+rc1-42-gf96c1a2c7** - Git version: **2.25.0.windows.1** - Operating system: **Windows Server 2012 R2** - Database (use `[x]`): - [ ] PostgreSQL - [x] MySQL (MariaDB Server version: 10.1.34-MariaDB) - [ ] MSSQL - [ ] SQLite - Can you reproduce the bug at https://try.gitea.io: - [ ] Yes (provide example URL) - [x] No (Can't really test out my AD environment in a public instance) - [ ] Not relevant - Log gist (trace): ``` 2020/01/20 10:00:11 ...es/auth/ldap/ldap.go:216:SearchEntry() [T] LDAP will use BindDN. 2020/01/20 10:00:11 ...es/auth/ldap/ldap.go:226:SearchEntry() [T] Bound as BindDN CN=Gitea,OU=Users,DC=my,DC=domain 2020/01/20 10:00:11 ...es/auth/ldap/ldap.go:85:findUserDN() [T] Search for LDAP user: 334455 2020/01/20 10:00:11 ...es/auth/ldap/ldap.go:93:findUserDN() [T] Searching for DN using filter (&(objectClass=Person)(|(sAMAccountName=334455)(mail=334455)(employeeNumber=334455))) and base OU=Users,DC=my,DC=domain 2020/01/20 10:00:11 ...es/auth/ldap/ldap.go:144:bindUser() [T] Binding with userDN: CN=First M. Lastname,OU=Users,DC=my,DC=domain 2020/01/20 10:00:11 ...es/auth/ldap/ldap.go:150:bindUser() [T] Bound successfully with userDN: CN=First M. Lastname,OU=Users,DC=my,DC=domain 2020/01/20 10:00:11 ...es/auth/ldap/ldap.go:257:SearchEntry() [T] Fetching attributes 'sAMAccountName', 'givenName', 'sn', 'mail', '' with filter (&(objectClass=Person)(|(sAMAccountName=334455)(mail=334455)(employeeNumber=334455))) and base CN=First M. Lastname,OU=Users,DC=my,DC=domain 2020/01/20 10:00:11 ...es/auth/ldap/ldap.go:156:checkAdmin() [T] Checking admin with filter (memberOf=CN=GiteaAdmins,OU=Users,DC=my,DC=domain) and base CN=First M. Lastname,OU=Users,DC=my,DC=domain 2020/01/20 10:00:11 .../xorm/session_raw.go:78:queryRows() [I] [SQL] SELECT `id`, `lower_name`, `name`, `full_name`, `email`, `keep_email_private`, `email_notifications_preference`, `passwd`, `passwd_hash_algo`, `must_change_password`, `login_type`, `login_source`, `login_name`, `type`, `location`, `website`, `rands`, `salt`, `language`, `description`, `created_unix`, `updated_unix`, `last_login_unix`, `last_repo_visibility`, `max_repo_creation`, `is_active`, `is_admin`, `allow_git_hook`, `allow_import_local`, `allow_create_organization`, `prohibit_login`, `avatar`, `avatar_email`, `use_custom_avatar`, `num_followers`, `num_following`, `num_stars`, `num_repos`, `num_teams`, `num_members`, `visibility`, `repo_admin_change_team_access`, `diff_view_style`, `theme` FROM `user` WHERE (id!=?) AND `lower_name`=? LIMIT 1 []interface {}{0, "fmlastname"} - took: 1.1474ms 2020/01/20 10:00:11 .../xorm/session_raw.go:78:queryRows() [I] [SQL] SELECT `id`, `lower_name`, `name`, `full_name`, `email`, `keep_email_private`, `email_notifications_preference`, `passwd`, `passwd_hash_algo`, `must_change_password`, `login_type`, `login_source`, `login_name`, `type`, `location`, `website`, `rands`, `salt`, `language`, `description`, `created_unix`, `updated_unix`, `last_login_unix`, `last_repo_visibility`, `max_repo_creation`, `is_active`, `is_admin`, `allow_git_hook`, `allow_import_local`, `allow_create_organization`, `prohibit_login`, `avatar`, `avatar_email`, `use_custom_avatar`, `num_followers`, `num_following`, `num_stars`, `num_repos`, `num_teams`, `num_members`, `visibility`, `repo_admin_change_team_access`, `diff_view_style`, `theme` FROM `user` WHERE `lower_name`=? LIMIT 1 []interface {}{"fmlastname"} - took: 999.2µs 2020/01/20 10:00:11 .../xorm/session_raw.go:85:queryRows() [I] [SQL] SELECT `id`, `lower_name`, `name`, `full_name`, `email`, `keep_email_private`, `email_notifications_preference`, `passwd`, `passwd_hash_algo`, `must_change_password`, `login_type`, `login_source`, `login_name`, `type`, `location`, `website`, `rands`, `salt`, `language`, `description`, `created_unix`, `updated_unix`, `last_login_unix`, `last_repo_visibility`, `max_repo_creation`, `is_active`, `is_admin`, `allow_git_hook`, `allow_import_local`, `allow_create_organization`, `prohibit_login`, `avatar`, `avatar_email`, `use_custom_avatar`, `num_followers`, `num_following`, `num_stars`, `num_repos`, `num_teams`, `num_members`, `visibility`, `repo_admin_change_team_access`, `diff_view_style`, `theme` FROM `user` WHERE (id!=?) AND `lower_name`=? LIMIT 1 []interface {}{0, "fmlastname"} - took: 1.0001ms 2020/01/20 10:00:11 ...dels/login_source.go:805:UserSignIn() [W] Failed to login '334455' via 'LDAP-Bind': user already exists [name: fmlastname] 2020/01/20 10:00:11 ...s/context/context.go:139:HTML() [D] Template: user/auth/signin 2020/01/20 10:00:11 routers/user/auth.go:171:SignInPost() [I] Failed authentication attempt for 334455 from 192.168.10.17:3418 2020/01/20 10:00:12 .../xorm/session_raw.go:78:queryRows() [I] [SQL] SELECT `id`, `type`, `name`, `is_actived`, `is_sync_enabled`, `cfg`, `created_unix`, `updated_unix` FROM `login_source` WHERE (is_actived = ? and type = ?) []interface {}{true, 7} - took: 15.0985ms 2020/01/20 10:00:12 ...s/context/context.go:330:func1() [D] Session ID: bf99b215ba3a0c69 2020/01/20 10:00:12 ...s/context/context.go:331:func1() [D] CSRF Token: ZXQVKwqrWZaiGXr1oKI3LlfQ6Rk6MTU3OTUzNTU1MTU0MTQ4MTcwMA 2020/01/20 10:00:12 ...s/context/context.go:139:HTML() [D] Template: pwa/manifest_json 2020/01/20 10:00:13 .../xorm/session_raw.go:78:queryRows() [I] [SQL] SELECT `id`, `type`, `name`, `is_actived`, `is_sync_enabled`, `cfg`, `created_unix`, `updated_unix` FROM `login_source` WHERE (is_actived = ? and type = ?) []interface {}{true, 7} - took: 1.1432ms 2020/01/20 10:00:13 ...s/context/context.go:330:func1() [D] Session ID: bf99b215ba3a0c69 2020/01/20 10:00:13 ...s/context/context.go:331:func1() [D] CSRF Token: ZXQVKwqrWZaiGXr1oKI3LlfQ6Rk6MTU3OTUzNTU1MTU0MTQ4MTcwMA 2020/01/20 10:00:13 ...s/context/context.go:139:HTML() [D] Template: pwa/serviceworker_js ``` ## Description When logging in with LDAP with multiple log in options that are not the typical "username" or "email" uid's, you must check if the user exists and then authenticate using what is defined in "Username Attribute". ## What should occur When logging in with a non-standard option like `employeeNumber`, it should authenticate to LDAP, if passes then check if user already exists, if not, create user, if already exists and password is good, log the user in. ## What is happening When logging in with a non-standard option like `employeeNumber`, it will try to create a new user without checking to make sure that user does not already exist which results in an invalid credential on log in page. **Relevant lines of code:** `models/login_source.go` Lines [527](https://github.com/go-gitea/gitea/blob/f96c1a2c79a86a157f5eb697a5f1ac89ee33267f/models/login_source.go#L527) - 533. ``` err := CreateUser(user) if err == nil && isAttributeSSHPublicKeySet && addLdapSSHPublicKeys(user, source, sr.SSHPublicKey) { err = RewriteAllPublicKeys() } return user, ``` ## Screenshots ![gitea_employeeNumber_login](https://user-images.githubusercontent.com/4871781/72746541-fcfff000-3b77-11ea-94fd-00d60099aa90.png) ## My environment 1. Compiled from source and built using `git clone https://github.com/go-gitea/gitea.git&&git checkout release/v1.11&&TAGS="bindata sqlite sqlite_unlock_notify" make build` 2. Go v1.13.6 3. Make v4.2 4. Gitea v1.11.0+rc1-41-gce756ee89 5. IIS reverse proxy to localhost on custom port ## Other Semi-relevant ticket that fixed the [500 error](https://github.com/go-gitea/gitea/issues/9809) associated with same scenario.
GiteaMirror added the type/bug label 2025-11-02 05:59:37 -06:00
GiteaMirror commented 2025-11-02 05:59:37 -06:00
Author
Owner
Copy Link

@zeripath commented on GitHub (Jan 20, 2020):

I don't understand this definitely checks if the user is in existence

@zeripath commented on GitHub (Jan 20, 2020): I don't understand this definitely checks if the user is in existence
GiteaMirror commented 2025-11-02 05:59:38 -06:00
Author
Owner
Copy Link

@zeripath commented on GitHub (Jan 20, 2020):

ah I do.

@zeripath commented on GitHub (Jan 20, 2020): ah I do.
GiteaMirror commented 2025-11-02 05:59:38 -06:00
Author
Owner
Copy Link

@zeripath commented on GitHub (Jan 20, 2020):

Sorry I missed fixing this... I presumed that the ErrUserExist would be handled correctly.

That's what I get for presuming.

@zeripath commented on GitHub (Jan 20, 2020): Sorry I missed fixing this... I presumed that the ErrUserExist would be handled correctly. That's what I get for presuming.
GiteaMirror commented 2025-11-02 05:59:38 -06:00
Author
Owner
Copy Link

@iamdoubz commented on GitHub (Jan 20, 2020):

Sorry I missed fixing this... I presumed that the ErrUserExist would be handled correctly.

That's what I get for presuming.

Don't fret. You did fix the initial ticket (500 error no longer happens) so it was technically correct and you did fix the problem... Now referencing the age old adage of, "Assume makes an a** out of u and me" 🤦‍♂ I did know that this would happen as that is the error I got in the logs with Gogs.

@iamdoubz commented on GitHub (Jan 20, 2020): > Sorry I missed fixing this... I presumed that the ErrUserExist would be handled correctly. > > That's what I get for presuming. Don't fret. You did fix the initial ticket (500 error no longer happens) so it was technically correct and you did fix the problem... Now referencing the age old adage of, "Assume makes an a** out of u and me" 🤦‍♂ I did know that this would happen as that is the error I got in the logs with [Gogs](https://github.com/gogs/gogs/issues/5897).
GiteaMirror commented 2025-11-02 05:59:38 -06:00
Author
Owner
Copy Link

@zeripath commented on GitHub (Jan 20, 2020):

So the issue is that the signature for ExternalUserLogin is wrong.

It is:

27c6b8fc07/models/login_source.go (L694)

It's only ever called in two ways:

27c6b8fc07/models/login_source.go (L786)

and

27c6b8fc07/models/login_source.go (L800)

In the first case user cannot be nil so detecting whether we need to do autoRegister doesn't need to be a separate argument.

The same broken signature is passed to LoginViaLDAP too.

@zeripath commented on GitHub (Jan 20, 2020): So the issue is that the signature for ExternalUserLogin is wrong. It is: https://github.com/go-gitea/gitea/blob/27c6b8fc07eab2dd579b94cd92136738ace61706/models/login_source.go#L694 It's only ever called in two ways: https://github.com/go-gitea/gitea/blob/27c6b8fc07eab2dd579b94cd92136738ace61706/models/login_source.go#L786 and https://github.com/go-gitea/gitea/blob/27c6b8fc07eab2dd579b94cd92136738ace61706/models/login_source.go#L800 In the first case user cannot be nil so detecting whether we need to do autoRegister doesn't need to be a separate argument. The same broken signature is passed to LoginViaLDAP too.
GiteaMirror commented 2025-11-02 05:59:39 -06:00
Author
Owner
Copy Link

@iamdoubz commented on GitHub (Jan 21, 2020):

How do I git pull this patch directly to test it?

@iamdoubz commented on GitHub (Jan 21, 2020): How do I `git pull` [this patch](https://github.com/go-gitea/gitea/pull/9900) directly to test it?
GiteaMirror commented 2025-11-02 05:59:39 -06:00
Author
Owner
Copy Link

@iamdoubz commented on GitHub (Jan 21, 2020):

Nevermind:

curl -L https://github.com/go-gitea/gitea/pull/9900.patch > 9900.patch
git apply --check 9900.patch
git apply 9900.patch

PS IT WORKS!!!

@iamdoubz commented on GitHub (Jan 21, 2020): Nevermind: ``` curl -L https://github.com/go-gitea/gitea/pull/9900.patch > 9900.patch git apply --check 9900.patch git apply 9900.patch ``` PS IT WORKS!!!
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
$20
$250
$50
$500
backport/done
💎 Bounty
docs-update-needed
good first issue
hacktoberfest
issue/bounty
issue/confirmed
issue/critical
issue/duplicate
issue/needs-feedback
issue/not-a-bug
issue/regression
issue/stale
issue/workaround
lgtm/need 2
modifies/api
modifies/translation
outdated/backport/v1.18
outdated/theme/markdown
outdated/theme/timetracker
performance/bigrepo
performance/cpu
performance/memory
performance/speed
pr/breaking
proposal/accepted
proposal/rejected
pr/wip
pull-request
Mirrored from GitHub Pull Request
 reviewed/wontfix
💰 Rewarded
skip-changelog
status/blocked
topic/accessibility
topic/api
topic/authentication
topic/build
topic/code-linting
topic/commit-signing
topic/content-rendering
topic/deployment
topic/distribution
topic/federation
topic/gitea-actions
topic/issues
topic/lfs
topic/mobile
topic/moderation
topic/packages
topic/pr
topic/projects
topic/repo
topic/repo-migration
topic/security
topic/theme
topic/ui
topic/ui-interaction
topic/ux
topic/webhooks
topic/wiki
type/bug
type/deprecation
type/docs
type/enhancement
type/feature
type/miscellaneous
type/proposal
type/question
type/refactoring
type/summary
type/testing
type/upstream
No Label type/bug
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/gitea#4690
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?