Skip to content

LDAP BindDN Multiple Log In Options Results in 500 #9809

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
2 of 7 tasks
iamdoubz opened this issue Jan 16, 2020 · 6 comments · Fixed by #9881
Closed
2 of 7 tasks

LDAP BindDN Multiple Log In Options Results in 500 #9809

iamdoubz opened this issue Jan 16, 2020 · 6 comments · Fixed by #9881
Labels

Comments

@iamdoubz
Copy link

iamdoubz commented Jan 16, 2020

  • Gitea version (or commit ref): 1.11.0+rc1-24-g6d1f7e90c
  • 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): gitea.log.txt

The log file contains when I tried to log in with my employeeNumber (error), then immediately trying to log in with my email (works).

Description

Adding multiple options for end users to log in results in a 500 error ...les/context/panic.go:35:1() [E] PANIC:: runtime error: invalid memory address or nil pointer dereference.

Using an Authentication Source of LDAP (via BindDN), with a User Filter of (&(objectClass=Person)(|(sAMAccountName=%[1]s)(mail=%[1]s)(employeeNumber=%[1]s))) and Username Attribute of sAMAccountName. Oddly enough, end users can log in using sAMAccountName or their email and it works great. However, when they go to log in with their employeeNumber, it will result in a 500 error.

If it helps, all the fields I am using in Microsoft AD are of Syntax DirectoryString.

Gogs error is here.

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-24-g6d1f7e90c
  5. IIS reverse proxy to localhost on custom port

Screenshots

N/A

@lafriks
Copy link
Member

lafriks commented Jan 16, 2020

Could it be that there are multiple records with same employeeNumber?

@iamdoubz
Copy link
Author

Could it be that there are multiple records with same employeeNumber?

First off, this is a great question. I would not have thought of this if you had not pointed out. However, the short answer is no.

Right now, there are only two users it Gitea: an admin account (local), and my account (AD). As for the Domain itself, no. using the Powershell command Get-ADUser -Filter 'employeeNumber -like "334455"' -SearchBase "OU=Users,DC=my,DC=domain" returns one result (me).

And each employee at the company gets a unique employeeNumber. Once issued, it is never used again by another user.

@zeripath
Copy link
Contributor

So looking at your stack trace here is the causative line:

!user.ProhibitLogin && len(source.LDAP().AdminFilter) > 0 && user.IsAdmin != sr.IsAdmin {

I suspect that user is nil at this point

@zeripath
Copy link
Contributor

yup. It would be. Because the user is trying to login with their EID not their username - so gitea hasn't matched that username.

@zeripath
Copy link
Contributor

zeripath commented Jan 19, 2020

authUser, err := ExternalUserLogin(nil, username, password, source, true)

And lo and behold it is nil

@iamdoubz
Copy link
Author

@zeripath Thank you for your excellent tracking down skills and for the patch. It is greatly appreciated and you did great work!

@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants