Skip to content
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

Invalid domain used when ldap record doesn't have email field #21169

Closed
pezhovski opened this issue Sep 14, 2022 · 3 comments · Fixed by #25425
Closed

Invalid domain used when ldap record doesn't have email field #21169

pezhovski opened this issue Sep 14, 2022 · 3 comments · Fixed by #25425
Labels

Comments

@pezhovski
Copy link
Contributor

Description

When syncing users from ldap, if user doesn't have email field, username@localhost is used https://github.com/go-gitea/gitea/blob/v1.17.2/services/auth/source/ldap/source_sync.go#L88

Later when trying to edit this user from admin page, I'm getting an error that email is invalid

Gitea Version

1.17.0

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

No response

How are you running Gitea?

Downloaded binary from release page, run with systemd

Database

No response

@lunny
Copy link
Member

lunny commented Jun 21, 2023

Maybe we should ignore this user with no email input?

@pezhovski
Copy link
Contributor Author

@lunny I think it would be kinda unfair to ignore such users. In case some organizations may not have email set up, or it is a guest or automation user with no email within organization.

I remember investigating this a little bit more, and the root cause of this behavior was the go library that validates email addresses. When I last checked it required fqdn and, probably, just localhost is not a valid fqdn for this library, sorry for not mentioning this in the issue.

So, I believe faster approach would be to change localhost to localhost.local or localhost.localhost, but I am not sure if it could potentially lead to security or compatibility issues.

@Zettat123
Copy link
Contributor

The Email field has an Email tag, so when we try to edit a user with the default email (username@localhost), the validation will fail.

Email string `binding:"Required;Email;MaxSize(254)"`

So, I believe faster approach would be to change localhost to localhost.local or localhost.localhost, but I am not sure if it could potentially lead to security or compatibility issues.

Since we shouldn't ignore the users with no email, I think changing the email suffix would be a solution.

6543 pushed a commit that referenced this issue Jun 22, 2023
Fixes #21169

Change `localhost` to `localhost.local`
GiteaBot pushed a commit to GiteaBot/gitea that referenced this issue Jun 22, 2023
6543 pushed a commit that referenced this issue Jun 22, 2023
Backport #25425 by @Zettat123

Fixes #21169

Change `localhost` to `localhost.local`

Co-authored-by: Zettat123 <[email protected]>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 7, 2023
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