- Revision
- 294491
- Author
- ab...@igalia.com
- Date
- 2022-05-19 10:13:30 -0700 (Thu, 19 May 2022)
Log Message
git-webkit setup: Fix various pitfalls with credentials input
https://bugs.webkit.org/show_bug.cgi?id=240574
Reviewed by Jonathan Bedard.
Yesterday I tried to run `git webkit setup`.
To put it mildly, it wasn't a smooth ride. I ended up having to debug the
tooling for hours just to be able to get it running.
This patch fixes several issues I found during the process, so that the next
unlucky person doesn't have to go through this again.
1. Whenever a request failed, the response from the server was not shown in
anyway, instead printing an unhelpful generic message. This patch adds code to
write to the screen the responses obtained from the GitHub API, so that the
next person having problems with it doesn't need to add debugging code to know
what is wrong.
2. When copying and pasting tokens from the browser it's very easy to
accidentally grab some leading or trailing whitespace. This is especially easy
to miss for the blind terminal key prompt. This patch adds code to trim these
fields. This is generally good UX practice since leading and trailing spaces
are virtually always accidental. [1]
3. The validation code for GitHub username and token was not run under `git
webkit setup`. Looking at the code it's clear the intention was for that
validation to be done to check (1) a plain GitHub username is used instead of
an email address associated to that account and (2) that a test log-in
succeeds. But because the credentials function is called in many places, the
first instance that actually gets called happens to not set validate=True in
the arguments.
Validating passwords that have just been input for the first time should not
be an optional feature that is disabled by default. Otherwise any mistake in
the credentials input can cause cryptic errors and the user is left on their
own to figure out what is going on, and eventually, how to manually interact
with the keychain to remove or edit the bogus username and/or token. This patch
makes changes so that validation is made whenever the user is prompt for
username and token, no matter in what codepath this becomes necessary.
[1] https://tonyshowoff.com/articles/should-you-trim-passwords/
* Tools/Scripts/libraries/webkitbugspy/webkitbugspy/bugzilla.py:
(Tracker.credentials):
* Tools/Scripts/libraries/webkitbugspy/webkitbugspy/github.py:
* Tools/Scripts/libraries/webkitcorepy/webkitcorepy/credentials.py:
(credentials):
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/setup.py:
(Setup.github):
Canonical link: https://commits.webkit.org/250750@main
Modified Paths
Diff
Modified: trunk/Tools/Scripts/libraries/webkitbugspy/webkitbugspy/bugzilla.py (294490 => 294491)
--- trunk/Tools/Scripts/libraries/webkitbugspy/webkitbugspy/bugzilla.py 2022-05-19 17:10:02 UTC (rev 294490)
+++ trunk/Tools/Scripts/libraries/webkitbugspy/webkitbugspy/bugzilla.py 2022-05-19 17:13:30 UTC (rev 294491)
@@ -125,7 +125,8 @@
url=""
required=required,
prompt=self.url.split('//')[-1],
- validater=validater if validate else None,
+ validater=validater,
+ validate_existing_credentials=validate
)
def _login_arguments(self, required=False, query=None):
Modified: trunk/Tools/Scripts/libraries/webkitbugspy/webkitbugspy/github.py (294490 => 294491)
--- trunk/Tools/Scripts/libraries/webkitbugspy/webkitbugspy/github.py 2022-05-19 17:10:02 UTC (rev 294490)
+++ trunk/Tools/Scripts/libraries/webkitbugspy/webkitbugspy/github.py 2022-05-19 17:13:30 UTC (rev 294491)
@@ -125,7 +125,8 @@
name=self.url.split('/')[2].replace('.', '_').upper(),
prompt=prompt,
key_name='token',
- validater=validater if validate else None,
+ validater=validater,
+ validate_existing_credentials=validate,
save_in_keyring=save_in_keyring,
)
Modified: trunk/Tools/Scripts/libraries/webkitcorepy/webkitcorepy/credentials.py (294490 => 294491)
--- trunk/Tools/Scripts/libraries/webkitcorepy/webkitcorepy/credentials.py 2022-05-19 17:10:02 UTC (rev 294490)
+++ trunk/Tools/Scripts/libraries/webkitcorepy/webkitcorepy/credentials.py 2022-05-19 17:13:30 UTC (rev 294491)
@@ -29,7 +29,7 @@
_cache = dict()
-def credentials(url, required=True, name=None, prompt=None, key_name='password', validater=None, retry=3, save_in_keyring=None):
+def credentials(url, required=True, name=None, prompt=None, key_name='password', validater=None, validate_existing_credentials=False, retry=3, save_in_keyring=None):
global _cache
name = name or url.split('/')[2].replace('.', '_')
@@ -69,7 +69,7 @@
prompt = prompt()
sys.stderr.write("Authentication required to use {}\n".format(prompt or name))
sys.stderr.write('Username: ')
- username = Terminal.input()
+ username = Terminal.input().strip()
username_prompted = True
if not key and username:
@@ -82,10 +82,11 @@
if not key and required:
if not sys.stderr.isatty() or not sys.stdin.isatty():
raise OSError('No tty to prompt user for username')
- key = getpass.getpass('{}: '.format(key_name.capitalize()))
+ key = getpass.getpass('{}: '.format(key_name.capitalize())).strip()
key_prompted = True
- if username and key and (not validater or validater(username, key)):
+ should_validate = validater and (username_prompted or key_prompted or validate_existing_credentials)
+ if username and key and (not should_validate or validater(username, key)):
_cache[name] = (username, key)
break
Modified: trunk/Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/setup.py (294490 => 294491)
--- trunk/Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/setup.py 2022-05-19 17:10:02 UTC (rev 294490)
+++ trunk/Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/setup.py 2022-05-19 17:13:30 UTC (rev 294491)
@@ -80,6 +80,7 @@
), auth=auth, headers=dict(Accept='application/vnd.github.v3+json'))
if response.status_code not in (200, 202):
sys.stderr.write("Failed to create a fork of '{}' belonging to '{}'\n".format(forked_name, username))
+ sys.stderr.write("URL: {}\nServer replied with status code {}:\n{}\n".format(response.url, response.status_code, response.text))
return 1
set_name = response.json().get('name', '')
@@ -93,6 +94,7 @@
), auth=auth, headers=dict(Accept='application/vnd.github.v3+json'))
if response.status_code not in (200, 202):
sys.stderr.write("Fork created with name '{}' belonging to '{}'\n Failed to change name to {}\n".format(set_name, username, forked_name))
+ sys.stderr.write("URL: {}\nServer replied with status code {}:\n{}\n".format(response.url, response.status_code, response.text))
return 1
response = None
