Removing user from organization decrements member counter twice if MySQL is used as DB

[SagePtr]

• Gitea version (or commit ref): both 1.5.0-rc2 and f847884
• Git version: 2.7.4
• Operating system: linux ubuntu xenial (amd64)
• Database (use [x]):
  • PostgreSQL
  • MySQL
  • MSSQL
  • SQLite
• Can you reproduce the bug at https://try.gitea.io:
  • Yes (provide example URL)
  • No
  • Not relevant
• Log gist:

Description

Removing user from organization removes user twice if MySQL is used as database.

How to reproduce:

1. Install gitea, use MySQL as database.
2. Create two users (e.g. user1 and user2), login as one of them.
3. Create new organization (e.g. org1). User1 will be owner, num_members = 1, num_teams = 1 (as expected)
4. Create new team (e.g. team1). num_members = 1, num_teams = 2 (as expected)
5. Add user2 to team1. num_members = 2, num_teams = 2 (as expected)
6. Remove user2 from organization (not from team). num_members = 0, num_teams = 2 (bug)

User user2 is first removed from organization, then removed from team, but removeTeamMember function automatically checks if user is removed from last team he belongs to and calls removeOrgUser again.

removeOrgUser checks if user is team member, but when SQLite is used, this check passes first time and fails second time, but when MySQL is used, this check is passed both times and user deletion is triggered twice, member counter is decrementing twice.

Consequent adding and removing user from organization decreases counter and it becomes negative.

Digging into xorm.log showed me that DELETE FROM `org_user` WHERE `id`=? AND `uid`=? AND `org_id`=? AND `id`=? is executed in place (before second check), but maybe it's executed out of transaction and row deletion is not visible to consequent SELECT `id`, `uid`, `org_id`, `is_public` FROM `org_user` WHERE (uid=?) AND (org_id=?) LIMIT 1 inside transaction.

I digged further and converted org_user table from InnoDB to MyISAM storage (MyISAM doesn't support transactions) and bug disappeared. Converting back to InnoDB returned bug. So it's very likely this issue is related to bad transaction handling.

Here is part of xorm.log (between BEGIN TRANSACTION and COMMIT):
MySQL (buggy):

2018/07/26 18:16:47 [I] [SQL] BEGIN TRANSACTION
2018/07/26 18:16:47 [I] [SQL] SELECT `id`, `uid`, `org_id`, `is_public` FROM `org_user` WHERE (uid=?) AND (org_id=?) LIMIT 1 []interface {}{2, 3}
2018/07/26 18:16:47 [I] [SQL] SELECT `id`, `lower_name`, `name`, `full_name`, `email`, `keep_email_private`, `passwd`, `login_type`, `login_source`, `login_name`, `type`, `location`, `website`, `rands`, `salt`, `language`, `created_unix`, `updated_unix`, `last_login_unix`, `last_repo_visibility`, `max_repo_creation`, `is_active`, `is_admin`, `allow_git_hook`, `allow_import_local`, `allow_create_organization`, `prohibit_login`, `avatar`, `avatar_email`, `use_custom_avatar`, `num_followers`, `num_following`, `num_stars`, `num_repos`, `description`, `num_teams`, `num_members`, `diff_view_style` FROM `user` WHERE `id`=? LIMIT 1 []interface {}{3}
2018/07/26 18:16:47 [I] [SQL] SELECT `id`, `org_id`, `lower_name`, `name`, `description`, `authorize`, `num_repos`, `num_members` FROM `team` WHERE `org_id`=? AND `name`=? LIMIT 1 []interface {}{3, "Owners"}
2018/07/26 18:16:47 [I] [SQL] SELECT * FROM team_user WHERE (org_id=?) AND (team_id=?) AND (uid=?) LIMIT 1 []interface {}{3, 1, 2}
2018/07/26 18:16:47 [I] [SQL] DELETE FROM `org_user` WHERE `id`=? AND `uid`=? AND `org_id`=? AND `id`=? []interface {}{8, 2, 3, 8}
2018/07/26 18:16:47 [I] [SQL] UPDATE `user` SET num_members=num_members-1 WHERE id=? []interface {}{3}
2018/07/26 18:16:47 [I] [SQL] SELECT `team`.`id` FROM `team` INNER JOIN team_user ON `team_user`.team_id = team.id WHERE (`team_user`.org_id = ?) AND (`team_user`.uid = ?) []interface {}{3, 2}
2018/07/26 18:16:47 [I] [SQL] SELECT `repository`.`id` FROM `repository` INNER JOIN team_repo ON `team_repo`.repo_id=`repository`.id WHERE ((`repository`.is_private=? AND `repository`.owner_id=?) OR team_repo.team_id IN (?)) GROUP BY `repository`.id,`repository`.updated_unix ORDER BY updated_unix DESC []interface {}{false, 3, 2}
2018/07/26 18:16:47 [I] [SQL] SELECT * FROM `team` INNER JOIN team_user ON team_user.team_id = team.id WHERE (team.org_id = ?) AND (team_user.uid=?) []interface {}{3, 2}
2018/07/26 18:16:47 [I] [SQL] SELECT * FROM team_user WHERE (org_id=?) AND (team_id=?) AND (uid=?) LIMIT 1 []interface {}{3, 2, 2}
2018/07/26 18:16:47 [I] [SQL] SELECT * FROM `repository` INNER JOIN team_repo ON repository.id = team_repo.repo_id WHERE (team_repo.team_id=?) []interface {}{2}
2018/07/26 18:16:47 [I] [SQL] DELETE FROM `team_user` WHERE `org_id`=? AND `team_id`=? AND `uid`=? []interface {}{3, 2, 2}
2018/07/26 18:16:47 [I] [SQL] UPDATE `team` SET `num_members` = ? WHERE `id`=? []interface {}{0, 2}
2018/07/26 18:16:47 [I] [SQL] SELECT count(*) FROM `team_user` WHERE `org_id`=? AND `uid`=? []interface {}{3, 2}
2018/07/26 18:16:47 [I] [SQL] SELECT `id`, `uid`, `org_id`, `is_public` FROM `org_user` WHERE (uid=?) AND (org_id=?) LIMIT 1 []interface {}{2, 3}
2018/07/26 18:16:47 [I] [SQL] SELECT `id`, `lower_name`, `name`, `full_name`, `email`, `keep_email_private`, `passwd`, `login_type`, `login_source`, `login_name`, `type`, `location`, `website`, `rands`, `salt`, `language`, `created_unix`, `updated_unix`, `last_login_unix`, `last_repo_visibility`, `max_repo_creation`, `is_active`, `is_admin`, `allow_git_hook`, `allow_import_local`, `allow_create_organization`, `prohibit_login`, `avatar`, `avatar_email`, `use_custom_avatar`, `num_followers`, `num_following`, `num_stars`, `num_repos`, `description`, `num_teams`, `num_members`, `diff_view_style` FROM `user` WHERE `id`=? LIMIT 1 []interface {}{3}
2018/07/26 18:16:47 [I] [SQL] SELECT `id`, `org_id`, `lower_name`, `name`, `description`, `authorize`, `num_repos`, `num_members` FROM `team` WHERE `org_id`=? AND `name`=? LIMIT 1 []interface {}{3, "Owners"}
2018/07/26 18:16:47 [I] [SQL] SELECT * FROM team_user WHERE (org_id=?) AND (team_id=?) AND (uid=?) LIMIT 1 []interface {}{3, 1, 2}
2018/07/26 18:16:47 [I] [SQL] DELETE FROM `org_user` WHERE `id`=? AND `uid`=? AND `org_id`=? AND `id`=? []interface {}{8, 2, 3, 8}
2018/07/26 18:16:47 [I] [SQL] UPDATE `user` SET num_members=num_members-1 WHERE id=? []interface {}{3}
2018/07/26 18:16:47 [I] [SQL] SELECT `team`.`id` FROM `team` INNER JOIN team_user ON `team_user`.team_id = team.id WHERE (`team_user`.org_id = ?) AND (`team_user`.uid = ?) []interface {}{3, 2}
2018/07/26 18:16:47 [I] [SQL] SELECT `repository`.`id` FROM `repository` INNER JOIN team_repo ON `team_repo`.repo_id=`repository`.id WHERE ((`repository`.is_private=? AND `repository`.owner_id=?) OR team_repo.team_id IN (?)) GROUP BY `repository`.id,`repository`.updated_unix ORDER BY updated_unix DESC []interface {}{false, 3, 2}
2018/07/26 18:16:47 [I] [SQL] SELECT * FROM `team` INNER JOIN team_user ON team_user.team_id = team.id WHERE (team.org_id = ?) AND (team_user.uid=?) []interface {}{3, 2}
2018/07/26 18:16:47 [I] [SQL] COMMIT

SQLITE (correct):

2018/07/26 18:18:05 [I] [SQL] BEGIN TRANSACTION
2018/07/26 18:18:05 [I] [SQL] SELECT `id`, `uid`, `org_id`, `is_public` FROM `org_user` WHERE (uid=?) AND (org_id=?) LIMIT 1 []interface {}{2, 3}
2018/07/26 18:18:05 [I] [SQL] SELECT `id`, `lower_name`, `name`, `full_name`, `email`, `keep_email_private`, `passwd`, `login_type`, `login_source`, `login_name`, `type`, `location`, `website`, `rands`, `salt`, `language`, `created_unix`, `updated_unix`, `last_login_unix`, `last_repo_visibility`, `max_repo_creation`, `is_active`, `is_admin`, `allow_git_hook`, `allow_import_local`, `allow_create_organization`, `prohibit_login`, `avatar`, `avatar_email`, `use_custom_avatar`, `num_followers`, `num_following`, `num_stars`, `num_repos`, `description`, `num_teams`, `num_members`, `diff_view_style` FROM `user` WHERE `id`=? LIMIT 1 []interface {}{3}
2018/07/26 18:18:05 [I] [SQL] SELECT `id`, `org_id`, `lower_name`, `name`, `description`, `authorize`, `num_repos`, `num_members` FROM `team` WHERE `org_id`=? AND `name`=? LIMIT 1 []interface {}{3, "Owners"}
2018/07/26 18:18:05 [I] [SQL] SELECT * FROM team_user WHERE (org_id=?) AND (team_id=?) AND (uid=?) LIMIT 1 []interface {}{3, 1, 2}
2018/07/26 18:18:05 [I] [SQL] DELETE FROM `org_user` WHERE `id`=? AND `uid`=? AND `org_id`=? AND `id`=? []interface {}{4, 2, 3, 4}
2018/07/26 18:18:05 [I] [SQL] UPDATE `user` SET num_members=num_members-1 WHERE id=? []interface {}{3}
2018/07/26 18:18:05 [I] [SQL] SELECT `team`.`id` FROM `team` INNER JOIN team_user ON `team_user`.team_id = team.id WHERE (`team_user`.org_id = ?) AND (`team_user`.uid = ?) []interface {}{3, 2}
2018/07/26 18:18:05 [I] [SQL] SELECT `repository`.`id` FROM `repository` INNER JOIN team_repo ON `team_repo`.repo_id=`repository`.id WHERE ((`repository`.is_private=? AND `repository`.owner_id=?) OR team_repo.team_id IN (?)) GROUP BY `repository`.id,`repository`.updated_unix ORDER BY updated_unix DESC []interface {}{false, 3, 2}
2018/07/26 18:18:05 [I] [SQL] SELECT * FROM `team` INNER JOIN team_user ON team_user.team_id = team.id WHERE (team.org_id = ?) AND (team_user.uid=?) []interface {}{3, 2}
2018/07/26 18:18:05 [I] [SQL] SELECT * FROM team_user WHERE (org_id=?) AND (team_id=?) AND (uid=?) LIMIT 1 []interface {}{3, 2, 2}
2018/07/26 18:18:05 [I] [SQL] SELECT * FROM `repository` INNER JOIN team_repo ON repository.id = team_repo.repo_id WHERE (team_repo.team_id=?) []interface {}{2}
2018/07/26 18:18:05 [I] [SQL] DELETE FROM `team_user` WHERE `org_id`=? AND `team_id`=? AND `uid`=? []interface {}{3, 2, 2}
2018/07/26 18:18:05 [I] [SQL] UPDATE `team` SET `num_members` = ? WHERE `id`=? []interface {}{0, 2}
2018/07/26 18:18:05 [I] [SQL] SELECT count(*) FROM `team_user` WHERE `org_id`=? AND `uid`=? []interface {}{3, 2}
2018/07/26 18:18:05 [I] [SQL] SELECT `id`, `uid`, `org_id`, `is_public` FROM `org_user` WHERE (uid=?) AND (org_id=?) LIMIT 1 []interface {}{2, 3}
2018/07/26 18:18:05 [I] [SQL] COMMIT

Screenshots

After 8 consequent additions and removal of user: