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

GetUserByEmailContext erroneous handling of email case #18830

Closed
smunaut opened this issue Feb 19, 2022 · 3 comments · Fixed by #18833
Closed

GetUserByEmailContext erroneous handling of email case #18830

smunaut opened this issue Feb 19, 2022 · 3 comments · Fixed by #18833
Labels
Milestone

Comments

@smunaut
Copy link

smunaut commented Feb 19, 2022

Gitea Version

1.16.1

Git Version

No response

Operating System

No response

How are you running Gitea?

Irrelevant, I can see the bug in the code ...

Database

No response

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Description

The GetUserByEmailContext function lowers the case of the email before searching for it ( email = strings.ToLower(email) ), but the email stored in the DB is with whatever case the user entered it and is not necessarily lower case.

Also as a side note, technically only the host part of the email is case insensitive, there is nothing in the spec saying the part before the @ has to be insensitive AFAICT. It'd be dumb and confusing, but technically spec-compliant.

Screenshots

No response

@lunny
Copy link
Member

lunny commented Feb 20, 2022

Please paste the code here.

@smunaut
Copy link
Author

smunaut commented Feb 20, 2022

func GetUserByEmailContext(ctx context.Context, email string) (*User, error) {
	if len(email) == 0 {
		return nil, ErrUserNotExist{0, email, 0}
	}

	email = strings.ToLower(email)
	// First try to find the user by primary email
	user := &User{Email: email}
	has, err := db.GetEngine(ctx).Get(user)
	if err != nil {
		return nil, err
	}
	if has {
		return user, nil
	}

	// Otherwise, check in alternative list for activated email addresses
	emailAddress := &EmailAddress{Email: email, IsActivated: true}
	has, err = db.GetEngine(ctx).Get(emailAddress)
	if err != nil {
		return nil, err
	}
	if has {
		return GetUserByIDCtx(ctx, emailAddress.UID)
	}

        ///......
}

@smunaut
Copy link
Author

smunaut commented Feb 20, 2022

AFAICT this has been introduced in b9d611e when instead of lowering the email manually a new LowerEmail was added, but here it still matches against Email.

@lunny lunny added the type/bug label Feb 20, 2022
@lunny lunny added this to the 1.16.2 milestone Feb 20, 2022
zeripath pushed a commit that referenced this issue Feb 21, 2022
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
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.

2 participants