diff --git a/gerrit_util.py b/gerrit_util.py index 46bf79477..79fb9df0f 100755 --- a/gerrit_util.py +++ b/gerrit_util.py @@ -98,6 +98,15 @@ class CookiesAuthenticator(Authenticator): self.netrc = self._get_netrc() self.gitcookies = self._get_gitcookies() + @classmethod + def get_new_password_url(cls, host): + assert not host.startswith('http') + # Assume *.googlesource.com pattern. + parts = host.split('.') + if not parts[0].endswith('-review'): + parts[0] += '-review' + return 'https://%s/new-password' % ('.'.join(parts)) + @classmethod def get_new_password_message(cls, host): assert not host.startswith('http') diff --git a/git_cl.py b/git_cl.py index a2de0c448..3d3589a78 100755 --- a/git_cl.py +++ b/git_cl.py @@ -3845,6 +3845,12 @@ class _GitCookiesChecker(object): prefix = git_host.split('.', 1)[0] return prefix + '-review.' + self._GOOGLESOURCE + def _get_counterpart_host(self, host): + assert host.endswith(self._GOOGLESOURCE) + git = self._canonical_git_googlesource_host(host) + gerrit = self._canonical_gerrit_googlesource_host(git) + return git if gerrit == host else gerrit + def has_generic_host(self): """Returns whether generic .googlesource.com has been configured. @@ -3870,14 +3876,14 @@ class _GitCookiesChecker(object): def get_partially_configured_hosts(self): return set( - host for host, identities_pair in - self._get_git_gerrit_identity_pairs().iteritems() - if None in identities_pair and host != '.' + self._GOOGLESOURCE) + (host if i1 else self._canonical_gerrit_googlesource_host(host)) + for host, (i1, i2) in self._get_git_gerrit_identity_pairs().iteritems() + if None in (i1, i2) and host != '.' + self._GOOGLESOURCE) def get_conflicting_hosts(self): return set( - host for host, (i1, i2) in - self._get_git_gerrit_identity_pairs().iteritems() + host + for host, (i1, i2) in self._get_git_gerrit_identity_pairs().iteritems() if None not in (i1, i2) and i1 != i2) def get_duplicated_hosts(self): @@ -3904,61 +3910,83 @@ class _GitCookiesChecker(object): return hosts @staticmethod - def print_hosts(hosts, extra_column_func=None): + def _format_hosts(hosts, extra_column_func=None): hosts = sorted(hosts) assert hosts if extra_column_func is None: extras = [''] * len(hosts) else: extras = [extra_column_func(host) for host in hosts] - tmpl = ' %%-%ds %%-%ds' % (max(map(len, hosts)), max(map(len, extras))) + tmpl = '%%-%ds %%-%ds' % (max(map(len, hosts)), max(map(len, extras))) + lines = [] for he in zip(hosts, extras): - print(tmpl % he) - print() - - def find_and_report_problems(self): - """Returns True if there was at least one problem, else False.""" - problems = [False] - def add_problem(): - if not problems[0]: - print('\n\n.gitcookies problem report:\n') - problems[0] = True + lines.append(tmpl % he) + return lines + def _find_problems(self): if self.has_generic_host(): - add_problem() - print(' .googlesource.com record detected\n' - ' Chrome Infrastructure team recommends to list full host names ' - 'explicitly.\n') + yield ('.googlesource.com wildcard record detected', + ['Chrome Infrastructure team recommends to list full host names ' + 'explicitly.'], + None) dups = self.get_duplicated_hosts() if dups: - add_problem() - print(' The following hosts were defined twice:\n') - self.print_hosts(dups) + yield ('The following hosts were defined twice', + self._format_hosts(dups), + None) partial = self.get_partially_configured_hosts() if partial: - add_problem() - print(' Credentials should come in pairs for Git and Gerrit hosts. ' - 'These hosts are missing:') - self.print_hosts(partial) + yield ('Credentials should come in pairs for Git and Gerrit hosts. ' + 'These hosts are missing', + self._format_hosts(partial, lambda host: 'but %s defined' % + self._get_counterpart_host(host)), + partial) conflicting = self.get_conflicting_hosts() if conflicting: - add_problem() - print(' The following Git hosts have differing credentials from their ' - 'Gerrit counterparts:\n') - self.print_hosts(conflicting, lambda host: '%s vs %s' % - tuple(self._get_git_gerrit_identity_pairs()[host])) + yield ('The following Git hosts have differing credentials from their ' + 'Gerrit counterparts', + self._format_hosts(conflicting, lambda host: '%s vs %s' % + tuple(self._get_git_gerrit_identity_pairs()[host])), + conflicting) wrong = self.get_hosts_with_wrong_identities() if wrong: - add_problem() - print(' These hosts likely use wrong identity:\n') - self.print_hosts(wrong, lambda host: '%s but %s recommended' % - (self._get_git_gerrit_identity_pairs()[host][0], - self._EXPECTED_HOST_IDENTITY_DOMAINS[host])) - return problems[0] + yield ('These hosts likely use wrong identity', + self._format_hosts(wrong, lambda host: '%s but %s recommended' % + (self._get_git_gerrit_identity_pairs()[host][0], + self._EXPECTED_HOST_IDENTITY_DOMAINS[host])), + wrong) + + def find_and_report_problems(self): + """Returns True if there was at least one problem, else False.""" + found = False + bad_hosts = set() + for title, sublines, hosts in self._find_problems(): + if not found: + found = True + print('\n\n.gitcookies problem report:\n') + bad_hosts.update(hosts or []) + print(' %s%s' % (title , (':' if sublines else ''))) + if sublines: + print() + print(' %s' % '\n '.join(sublines)) + print() + + if bad_hosts: + assert found + print(' You can manually remove corresponding lines in your %s file and ' + 'visit the following URLs with correct account to generate ' + 'correct credential lines:\n' % + gerrit_util.CookiesAuthenticator.get_gitcookies_path()) + print(' %s' % '\n '.join(sorted(set( + gerrit_util.CookiesAuthenticator().get_new_password_url( + self._canonical_git_googlesource_host(host)) + for host in bad_hosts + )))) + return found def CMDcreds_check(parser, args): diff --git a/tests/git_cl_creds_check_report.txt b/tests/git_cl_creds_check_report.txt index 87ce55b68..7c94c3fa7 100644 --- a/tests/git_cl_creds_check_report.txt +++ b/tests/git_cl_creds_check_report.txt @@ -1,6 +1,7 @@ .gitcookies problem report: - .googlesource.com record detected + .googlesource.com wildcard record detected: + Chrome Infrastructure team recommends to list full host names explicitly. The following hosts were defined twice: @@ -8,13 +9,23 @@ dup.googlesource.com Credentials should come in pairs for Git and Gerrit hosts. These hosts are missing: - partial.googlesource.com + + gpartial-review.googlesource.com but gpartial.googlesource.com defined + partial.googlesource.com but partial-review.googlesource.com defined The following Git hosts have differing credentials from their Gerrit counterparts: - conflict.googlesource.com git-example.google.com vs git-example.chromium.org + conflict.googlesource.com git-example.google.com vs git-example.chromium.org These hosts likely use wrong identity: - chrome-internal.googlesource.com git-example.chromium.org but google.com recommended - chromium.googlesource.com git-example.google.com but chromium.org recommended + chrome-internal.googlesource.com git-example.chromium.org but google.com recommended + chromium.googlesource.com git-example.google.com but chromium.org recommended + + You can manually remove corresponding lines in your ~/.gitcookies file and visit the following URLs with correct account to generate correct credential lines: + + https://chrome-internal-review.googlesource.com/new-password + https://chromium-review.googlesource.com/new-password + https://conflict-review.googlesource.com/new-password + https://gpartial-review.googlesource.com/new-password + https://partial-review.googlesource.com/new-password diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 91dffcab2..a3b383002 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -550,13 +550,15 @@ class GitCookiesCheckerTest(TestCase): ('dup', 'git-example.google.com'), ('dup-review', 'git-example.google.com'), ('partial', 'git-example.google.com'), + ('gpartial-review', 'git-example.google.com'), ]) self.assertTrue(self.c.has_generic_host()) self.assertEqual(set(['conflict.googlesource.com']), self.c.get_conflicting_hosts()) self.assertEqual(set(['dup.googlesource.com']), self.c.get_duplicated_hosts()) - self.assertEqual(set(['partial.googlesource.com']), + self.assertEqual(set(['partial.googlesource.com', + 'gpartial-review.googlesource.com']), self.c.get_partially_configured_hosts()) self.assertEqual(set(['chromium.googlesource.com', 'chrome-internal.googlesource.com']), @@ -571,12 +573,15 @@ class GitCookiesCheckerTest(TestCase): def test_report(self): self.test_analysis() self.mock(sys, 'stdout', StringIO.StringIO()) + self.mock(git_cl.gerrit_util.CookiesAuthenticator, 'get_gitcookies_path', + classmethod(lambda _: '~/.gitcookies')) self.assertTrue(self.c.find_and_report_problems()) with open(os.path.join(os.path.dirname(__file__), 'git_cl_creds_check_report.txt')) as f: expected = f.read() def by_line(text): return [l.rstrip() for l in text.rstrip().splitlines()] + self.maxDiff = 10000 self.assertEqual(by_line(sys.stdout.getvalue().strip()), by_line(expected)) class TestGitCl(TestCase):