Users mentions don't filter by viewing permissions

[guillep2k]

• Gitea version (or commit ref): c6fb7fe

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.

[guillep2k]

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").

[lunny]

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

[guillep2k]

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]

@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
}