Users mentions don't filter by viewing permissions #4026

Closed
opened 2025-11-02 05:34:43 -06:00 by GiteaMirror · 4 comments
Owner

Originally created by @guillep2k on GitHub (Sep 27, 2019).

Description

When a comment in an issue or PR mentions a user using @username, the mentioned user receives a mail notification even if they don't have permission to see the originating repository.

Originally created by @guillep2k on GitHub (Sep 27, 2019). - Gitea version (or commit ref): c6fb7fe27c16c4e43d4d8dbe4d2ff4b3c4c52a29 ## Description When a comment in an issue or PR mentions a user using `@username`, the mentioned user receives a mail notification even if they don't have permission to see the originating repository.
GiteaMirror added the type/bug label 2025-11-02 05:34:43 -06:00
Author
Owner

@guillep2k commented on GitHub (Oct 5, 2019):

I'm working on this issue but I'm stuck with how mentions are processed regarding organizations.

If the commenter adds @user to a comment, this is parsed in the following way:

  • An entry is created in issue_user for @user
  • A mail is sent to @user.

If the commenter adds @org-name to a comment, this is parsed in the following way:

  • An entry is created in issue_user for each and every organization member.
  • No mails are sent.

My question is: it this by design? To avoid mass mailing? Since I'm rewriting these functions I need to know how to proceed with this. My options are:

  1. Reproduce this behavior to the letter (I consider this a bit inconsistent).
  2. For every issue_user created, a mail should be sent.
  3. Never process organization mentions.
  4. A combination of the above, limiting this process to comments created by org owners and site admins.

Note: issue_user is the table used to list user mentions in the GUI (e.g. "issues mentioning you").

@guillep2k commented on GitHub (Oct 5, 2019): I'm working on this issue but I'm stuck with how mentions are processed regarding organizations. If the commenter adds `@user` to a comment, this is parsed in the following way: - An entry is created in `issue_user` for `@user` - A mail is sent to `@user`. If the commenter adds `@org-name` to a comment, this is parsed in the following way: - An entry is created in `issue_user` for *each and every organization member*. - **No mails are sent**. My question is: it this by design? To avoid mass mailing? Since I'm rewriting these functions I need to know how to proceed with this. My options are: 1. Reproduce this behavior to the letter (I consider this a bit inconsistent). 2. For every `issue_user` created, a mail should be sent. 3. Never process organization mentions. 4. A combination of the above, limiting this process to comments created by org owners and site admins. Note: `issue_user` is the table used to list user mentions in the GUI (e.g. *"issues mentioning you"*).
Author
Owner

@lunny commented on GitHub (Oct 5, 2019):

I would like to not do anything when added @org-name on comments. But @maintainers should be handled.

@lunny commented on GitHub (Oct 5, 2019): I would like to not do anything when added `@org-name` on comments. But `@maintainers` should be handled.
Author
Owner

@guillep2k commented on GitHub (Oct 5, 2019):

I would like to not do anything when added @org-name on comments. But @maintainers should be handled.

Sorry, @lunny , you mean @team-name ? If I understand correctly, @mention will:

  • If @mention is for a normal user (@user-name), add an issue_user count and mail the user.
  • If @mention is for a normal team (@team-name), for every user in the team:
    • Add an issue_user count and mail the user.
  • If @mention is for an organization (@org-name), just ignore it.

I'll be working with this in mind. Nonetheless, I'll check individual permissions for each user and ignore those with no access.

@guillep2k commented on GitHub (Oct 5, 2019): > > > I would like to not do anything when added `@org-name` on comments. But `@maintainers` should be handled. Sorry, @lunny , you mean `@team-name` ? If I understand correctly, `@mention` will: - If `@mention` is for a normal user (`@user-name`), add an `issue_user` count and mail the user. - If `@mention` is for a normal team (`@team-name`), for every user in the team: - Add an `issue_user` count and mail the user. - If `@mention` is for an organization (`@org-name`), just ignore it. I'll be working with this in mind. Nonetheless, I'll check individual permissions for each user and ignore those with no access.
Author
Owner

@guillep2k commented on GitHub (Oct 5, 2019):

@lunny and others, this is what I'm working on. Please let me know if this is what you have in mind:

// ResolveMentionsByVisibility returns the users mentioned in an issue, removing those that
// don't have access to reading it. Teams are expanded into their users, but organizations are ignored.
func (issue *Issue) ResolveMentionsByVisibility(ctx DBContext, doer *User, mentions []string) (users[]*User, err error) {
	if len(mentions) == 0 {
		return
	}
	if err = issue.loadRepo(ctx.e); err != nil {
		return
	}
	resolved := make(map[string]bool,len(mentions))
	for _, name := range mentions {
		name := strings.ToLower(name)
		if _, ok := resolved[name]; ok {
			continue
		}
		resolved[name] = false
	}

	if err := issue.Repo.getOwner(ctx.e); err != nil {
		return nil, err
	}

	names := make([]string,0,len(resolved))
	for name, _ := range resolved {
		names = append(names, name)
	}

	if issue.Repo.Owner.IsOrganization() {

		// Since there can be users with names that match the name of a team,
		// if the team exists and can read the issue, the team takes precedence.
		teams := make([]*Team,0,len(names))
		if err := ctx.e.
			Join("INNER", "team_repo", "team_repo.team_id = team.id").
			Where("team_repo.repo_id=?", issue.Repo.ID).
			In("team.lower_name", names).
			Find(&teams);
			err != nil {
			return nil, fmt.Errorf("find mentioned teams: %v", err)
		}
		if len(teams) != 0 {
			checked := make([]*Team,0,len(teams))
			unittype := UnitTypeIssues
			if issue.IsPull {
				unittype = UnitTypePullRequests
			}
			for _, team := range teams {
				if team.Authorize >= AccessModeOwner {
					checked = append(checked, team)
					resolved[team.LowerName] = true
					continue
				}
				has, err := ctx.e.Get(&TeamUnit{OrgID: issue.Repo.Owner.ID, TeamID: team.ID, Type: unittype})
				if err != nil {
					return nil, fmt.Errorf("get team units (%d): %v", team.ID, err)
				}
				if has {
					checked = append(checked, team)
					resolved[team.LowerName] = true
				}
			}
			if len(checked) != 0 {
				ids := make([]int64,len(checked))
				for i, _ := range checked {
					ids[i] = checked[i].ID
				}
				if err := ctx.e.
					Join("INNER", "team_user", "team_user.team_id = `user`.id").
					Where("`user`.prohibit_login", false).
					And("`user`.is_active", true).
					In("`team_user`.team_id", ids).
					Distinct().
					Find(users); err != nil {
					return nil, fmt.Errorf("get teams users: %v", err)
				}
				for _, user := range users {
					resolved[user.LowerName] = true
				}
			}
		}

		// Remove names already in the list
		names = make([]string,0,len(resolved))
		for name, already := range resolved {
			if !already {
				names = append(names, name)
			}
		}
	}

	unchecked := make([]*User,0,len(names))
	if err := ctx.e.
			Where("`user`.prohibit_login", false).
			And("`user`.is_active", true).
			In("`user`.lower_name", names).
			Find(&unchecked); err != nil {
		return nil, fmt.Errorf("find mentioned users: %v", err)
	}

	for _, user := range unchecked {
		if _, already := resolved[user.LowerName]; already || user.IsOrganization() {
			continue
		}
		// Normal users must have read access to the referencing issue
		perm, err := getUserRepoPermission(ctx.e, issue.Repo, user)
		if err != nil {
			return nil, fmt.Errorf("getUserRepoPermission [%d]: %v", user.ID, err)
		}
		if !perm.CanReadIssuesOrPulls(issue.IsPull) {
			continue
		}
		users = append(users, user)
		resolved[user.LowerName] = true
	}

	return
}
@guillep2k commented on GitHub (Oct 5, 2019): @lunny and others, this is what I'm working on. Please let me know if this is what you have in mind: ```go // ResolveMentionsByVisibility returns the users mentioned in an issue, removing those that // don't have access to reading it. Teams are expanded into their users, but organizations are ignored. func (issue *Issue) ResolveMentionsByVisibility(ctx DBContext, doer *User, mentions []string) (users[]*User, err error) { if len(mentions) == 0 { return } if err = issue.loadRepo(ctx.e); err != nil { return } resolved := make(map[string]bool,len(mentions)) for _, name := range mentions { name := strings.ToLower(name) if _, ok := resolved[name]; ok { continue } resolved[name] = false } if err := issue.Repo.getOwner(ctx.e); err != nil { return nil, err } names := make([]string,0,len(resolved)) for name, _ := range resolved { names = append(names, name) } if issue.Repo.Owner.IsOrganization() { // Since there can be users with names that match the name of a team, // if the team exists and can read the issue, the team takes precedence. teams := make([]*Team,0,len(names)) if err := ctx.e. Join("INNER", "team_repo", "team_repo.team_id = team.id"). Where("team_repo.repo_id=?", issue.Repo.ID). In("team.lower_name", names). Find(&teams); err != nil { return nil, fmt.Errorf("find mentioned teams: %v", err) } if len(teams) != 0 { checked := make([]*Team,0,len(teams)) unittype := UnitTypeIssues if issue.IsPull { unittype = UnitTypePullRequests } for _, team := range teams { if team.Authorize >= AccessModeOwner { checked = append(checked, team) resolved[team.LowerName] = true continue } has, err := ctx.e.Get(&TeamUnit{OrgID: issue.Repo.Owner.ID, TeamID: team.ID, Type: unittype}) if err != nil { return nil, fmt.Errorf("get team units (%d): %v", team.ID, err) } if has { checked = append(checked, team) resolved[team.LowerName] = true } } if len(checked) != 0 { ids := make([]int64,len(checked)) for i, _ := range checked { ids[i] = checked[i].ID } if err := ctx.e. Join("INNER", "team_user", "team_user.team_id = `user`.id"). Where("`user`.prohibit_login", false). And("`user`.is_active", true). In("`team_user`.team_id", ids). Distinct(). Find(users); err != nil { return nil, fmt.Errorf("get teams users: %v", err) } for _, user := range users { resolved[user.LowerName] = true } } } // Remove names already in the list names = make([]string,0,len(resolved)) for name, already := range resolved { if !already { names = append(names, name) } } } unchecked := make([]*User,0,len(names)) if err := ctx.e. Where("`user`.prohibit_login", false). And("`user`.is_active", true). In("`user`.lower_name", names). Find(&unchecked); err != nil { return nil, fmt.Errorf("find mentioned users: %v", err) } for _, user := range unchecked { if _, already := resolved[user.LowerName]; already || user.IsOrganization() { continue } // Normal users must have read access to the referencing issue perm, err := getUserRepoPermission(ctx.e, issue.Repo, user) if err != nil { return nil, fmt.Errorf("getUserRepoPermission [%d]: %v", user.ID, err) } if !perm.CanReadIssuesOrPulls(issue.IsPull) { continue } users = append(users, user) resolved[user.LowerName] = true } return } ```
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/gitea#4026