diff --git a/gerrit_util.py b/gerrit_util.py index 1a38cf05bc..9294d46cb0 100644 --- a/gerrit_util.py +++ b/gerrit_util.py @@ -30,7 +30,7 @@ from dataclasses import dataclass from io import StringIO from multiprocessing.pool import ThreadPool from typing import Any, Container, Dict, List, Optional -from typing import Tuple, Type, TypedDict, cast +from typing import Tuple, TypedDict, cast import httplib2 import httplib2.socks @@ -1640,6 +1640,32 @@ def GetAccountDetails(host, account_id='self'): return ReadHttpJsonResponse(conn, accept_statuses=[200, 404]) +class EmailRecord(TypedDict): + email: str + preferred: bool # This should be NotRequired[bool] in 3.11+ + + +def GetAccountEmails(host, account_id='self') -> Optional[List[EmailRecord]]: + """Returns all emails for this account, and an indication of which of these + is preferred. + + If account_id is not given, uses magic value 'self' which corresponds to + whichever account user is authenticating as. + + Requires Modify Account permission to view emails other than 'self'. + + Documentation: + https://gerrit-review.googlesource.com/Documentation/rest-api-accounts.html#list-account-emails + + Returns None if account is not found (i.e. Gerrit returned 404). + """ + conn = CreateHttpConn(host, '/accounts/%s/emails' % account_id) + resp = ReadHttpJsonResponse(conn, accept_statuses=[200, 404]) + if resp is None: + return None + return cast(List[EmailRecord], resp) + + def ValidAccounts(host, accounts, max_threads=10): """Returns a mapping from valid account to its details. diff --git a/git_cl.py b/git_cl.py index 883c82c6d3..497316da15 100755 --- a/git_cl.py +++ b/git_cl.py @@ -2320,7 +2320,8 @@ class Changelist(object): DieWithError(msg) def EnsureCanUploadPatchset(self, force): - if not self.GetIssue(): + issue = self.GetIssue() + if not issue: return status = self._GetChangeDetail()['status'] @@ -2338,30 +2339,41 @@ class Changelist(object): self.SetIssue() return - # TODO(vadimsh): For some reason the chunk of code below was skipped if - # 'is_gce' is True. I'm just refactoring it to be 'skip if not cookies'. - # Apparently this check is not very important? Otherwise get_auth_email - # could have been added to other implementations of Authenticator. - cookies_auth = gerrit_util.Authenticator.get() - if not isinstance(cookies_auth, gerrit_util.CookiesAuthenticator): + # Check to see if the currently authenticated account is the issue + # owner. + + # first, grab the issue owner email + owner = self.GetIssueOwner() + + # do a quick check to see if this matches the local git config's + # configured email. + git_config_email = scm.GIT.GetConfig(settings.GetRoot(), 'user.email') + if git_config_email == owner: + # Good enough - Gerrit will reject this if the user is doing funny things + # with user.email. return - cookies_user = cookies_auth.get_auth_email(self.GetGerritHost()) - if self.GetIssueOwner() == cookies_user: + # However, the user may have linked accounts in Gerrit, so pull up the + # list of all known emails for the currently authenticated account. + emails = gerrit_util.GetAccountEmails(self.GetGerritHost(), 'self') + if not emails: + print('WARNING: Gerrit does not have a record for your account.') + print('Please browse to https://{self.GetGerritHost()} and log in.') return - logging.debug('change %s owner is %s, cookies user is %s', - self.GetIssue(), self.GetIssueOwner(), cookies_user) - # Maybe user has linked accounts or something like that, - # so ask what Gerrit thinks of this user. - details = gerrit_util.GetAccountDetails(self.GetGerritHost(), 'self') - if details['email'] == self.GetIssueOwner(): + + # If the issue owner is one of the emails for the currently + # authenticated account, Gerrit will accept the upload. + if any(owner == e['email'] for e in emails): return + if not force: print( - 'WARNING: Change %s is owned by %s, but you authenticate to Gerrit ' - 'as %s.\n' - 'Uploading may fail due to lack of permissions.' % - (self.GetIssue(), self.GetIssueOwner(), details['email'])) + f'WARNING: Change {issue} is owned by {owner}, but Gerrit knows you as:' + ) + for email in emails: + tag = ' (preferred)' if email.get('preferred') else '' + print(f' * {email["email"]}{tag}') + print('Uploading may fail due to lack of permissions.') confirm_or_exit(action='upload') def GetStatus(self):