<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[186927] trunk/Tools</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/186927">186927</a></dd>
<dt>Author</dt> <dd>commit-queue@webkit.org</dd>
<dt>Date</dt> <dd>2015-07-16 20:59:31 -0700 (Thu, 16 Jul 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>webkit-patch uses incorrect credentials from keychain to login until account lockout
https://bugs.webkit.org/show_bug.cgi?id=146923
Patch by Dean Johnson <dean_johnson@apple.com> on 2015-07-16
Reviewed by Daniel Bates.
When `webkit-patch upload` asks for login credentials for bugs.webkit.org, if you
provide incorrect credentials then ask for them to be saved to the keychain
webkit-patch will then use those incorrect credentials until 5 total attempts
have been made to log in. This will lock the user out of Bugzilla for a
half hour.
* Scripts/webkitpy/common/net/bugzilla/bugzilla.py:
(Bugzilla.authenticate): Added argument to read_credentials() that
passes in whether or not to use stored credentials.
* Scripts/webkitpy/common/net/credentials.py:
(Credentials.read_credentials): Added optional argument 'use_stored_credentials'
that specifies whether or not to use credentials stored on the system. If it
does, we do not auto-fill credentials since either the credentials caused
the failure, or they weren't used in the first place so there is no reason
to try and use them again if a failure happened.
* Scripts/webkitpy/common/net/credentials_unittest.py: Added unit test for newly added
use_stored_credentials argument.
(test_do_not_use_stored_credentials): Added unit test to make sure functions that
retrieve username/password information from the local system (environment, Git, keychain,
keyring) are never called. It also tests that we get back our mocked username and password.
(test_do_not_use_stored_credentials.MockKeyring): Mock keyring
(test_do_not_use_stored_credentials.MockKeyring.get_password): Assert this is never called.
(test_do_not_use_stored_credentials.FakeCredentials): Mock credentials
(test_do_not_use_stored_credentials.FakeCredentials._credentials_from_keychain): Assert this
is never called.
(test_do_not_use_stored_credentials.FakeCredentials._credentials_from_environment): Ditto
(test_do_not_use_stored_credentials.FakeCredentials._offer_to_store_credentials_in_keyring): Ditto
(test_do_not_use_stored_credentials.FakeUser): Mock the User class
(test_do_not_use_stored_credentials.FakeUser.prompt): Returns a username to assert against.
(test_do_not_use_stored_credentials.FakeUser.prompt_password): Returns a password to assert
against.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkToolsChangeLog">trunk/Tools/ChangeLog</a></li>
<li><a href="#trunkToolsScriptswebkitpycommonnetbugzillabugzillapy">trunk/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py</a></li>
<li><a href="#trunkToolsScriptswebkitpycommonnetcredentialspy">trunk/Tools/Scripts/webkitpy/common/net/credentials.py</a></li>
<li><a href="#trunkToolsScriptswebkitpycommonnetcredentials_unittestpy">trunk/Tools/Scripts/webkitpy/common/net/credentials_unittest.py</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkToolsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Tools/ChangeLog (186926 => 186927)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/ChangeLog 2015-07-17 03:59:22 UTC (rev 186926)
+++ trunk/Tools/ChangeLog 2015-07-17 03:59:31 UTC (rev 186927)
</span><span class="lines">@@ -1,3 +1,42 @@
</span><ins>+2015-07-16 Dean Johnson <dean_johnson@apple.com>
+
+ webkit-patch uses incorrect credentials from keychain to login until account lockout
+ https://bugs.webkit.org/show_bug.cgi?id=146923
+
+ Reviewed by Daniel Bates.
+
+ When `webkit-patch upload` asks for login credentials for bugs.webkit.org, if you
+ provide incorrect credentials then ask for them to be saved to the keychain
+ webkit-patch will then use those incorrect credentials until 5 total attempts
+ have been made to log in. This will lock the user out of Bugzilla for a
+ half hour.
+
+ * Scripts/webkitpy/common/net/bugzilla/bugzilla.py:
+ (Bugzilla.authenticate): Added argument to read_credentials() that
+ passes in whether or not to use stored credentials.
+ * Scripts/webkitpy/common/net/credentials.py:
+ (Credentials.read_credentials): Added optional argument 'use_stored_credentials'
+ that specifies whether or not to use credentials stored on the system. If it
+ does, we do not auto-fill credentials since either the credentials caused
+ the failure, or they weren't used in the first place so there is no reason
+ to try and use them again if a failure happened.
+ * Scripts/webkitpy/common/net/credentials_unittest.py: Added unit test for newly added
+ use_stored_credentials argument.
+ (test_do_not_use_stored_credentials): Added unit test to make sure functions that
+ retrieve username/password information from the local system (environment, Git, keychain,
+ keyring) are never called. It also tests that we get back our mocked username and password.
+ (test_do_not_use_stored_credentials.MockKeyring): Mock keyring
+ (test_do_not_use_stored_credentials.MockKeyring.get_password): Assert this is never called.
+ (test_do_not_use_stored_credentials.FakeCredentials): Mock credentials
+ (test_do_not_use_stored_credentials.FakeCredentials._credentials_from_keychain): Assert this
+ is never called.
+ (test_do_not_use_stored_credentials.FakeCredentials._credentials_from_environment): Ditto
+ (test_do_not_use_stored_credentials.FakeCredentials._offer_to_store_credentials_in_keyring): Ditto
+ (test_do_not_use_stored_credentials.FakeUser): Mock the User class
+ (test_do_not_use_stored_credentials.FakeUser.prompt): Returns a username to assert against.
+ (test_do_not_use_stored_credentials.FakeUser.prompt_password): Returns a password to assert
+ against.
+
</ins><span class="cx"> 2015-07-16 Gyuyoung Kim <gyuyoung.kim@webkit.org>
</span><span class="cx">
</span><span class="cx"> Unreviewed, remove my redundant email information.
</span></span></pre></div>
<a id="trunkToolsScriptswebkitpycommonnetbugzillabugzillapy"></a>
<div class="modfile"><h4>Modified: trunk/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py (186926 => 186927)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py 2015-07-17 03:59:22 UTC (rev 186926)
+++ trunk/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py 2015-07-17 03:59:31 UTC (rev 186927)
</span><span class="lines">@@ -527,7 +527,7 @@
</span><span class="cx"> attempts = 0
</span><span class="cx"> while not self.authenticated:
</span><span class="cx"> attempts += 1
</span><del>- username, password = credentials.read_credentials()
</del><ins>+ username, password = credentials.read_credentials(use_stored_credentials=attempts == 1)
</ins><span class="cx">
</span><span class="cx"> _log.info("Logging in as %s..." % username)
</span><span class="cx"> self.browser.open(config_urls.bug_server_url +
</span></span></pre></div>
<a id="trunkToolsScriptswebkitpycommonnetcredentialspy"></a>
<div class="modfile"><h4>Modified: trunk/Tools/Scripts/webkitpy/common/net/credentials.py (186926 => 186927)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/Scripts/webkitpy/common/net/credentials.py 2015-07-17 03:59:22 UTC (rev 186926)
+++ trunk/Tools/Scripts/webkitpy/common/net/credentials.py 2015-07-17 03:59:31 UTC (rev 186927)
</span><span class="lines">@@ -136,19 +136,21 @@
</span><span class="cx"> except:
</span><span class="cx"> pass
</span><span class="cx">
</span><del>- def read_credentials(self, user=User):
- username, password = self._credentials_from_environment()
- # FIXME: We don't currently support pulling the username from one
- # source and the password from a separate source.
- if not username or not password:
- username, password = self._credentials_from_git()
- if not username or not password:
- username, password = self._credentials_from_keychain(username)
</del><ins>+ def read_credentials(self, user=User, use_stored_credentials=True):
+ username, password = None, None
+ if use_stored_credentials:
+ username, password = self._credentials_from_environment()
+ # FIXME: We don't currently support pulling the username from one
+ # source and the password from a separate source.
+ if not username or not password:
+ username, password = self._credentials_from_git()
+ if not username or not password:
+ username, password = self._credentials_from_keychain(username)
</ins><span class="cx">
</span><span class="cx"> if not username:
</span><span class="cx"> username = user.prompt("%s login: " % self.host)
</span><span class="cx">
</span><del>- if username and not password and self._keyring:
</del><ins>+ if username and not password and self._keyring and use_stored_credentials:
</ins><span class="cx"> try:
</span><span class="cx"> password = self._keyring.get_password(self.host, username)
</span><span class="cx"> except:
</span></span></pre></div>
<a id="trunkToolsScriptswebkitpycommonnetcredentials_unittestpy"></a>
<div class="modfile"><h4>Modified: trunk/Tools/Scripts/webkitpy/common/net/credentials_unittest.py (186926 => 186927)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/Scripts/webkitpy/common/net/credentials_unittest.py 2015-07-17 03:59:22 UTC (rev 186926)
+++ trunk/Tools/Scripts/webkitpy/common/net/credentials_unittest.py 2015-07-17 03:59:31 UTC (rev 186927)
</span><span class="lines">@@ -207,3 +207,31 @@
</span><span class="cx"> # FIXME: Using read_credentials here seems too broad as higher-priority
</span><span class="cx"> # credential source could be affected by the user's environment.
</span><span class="cx"> self.assertEqual(credentials.read_credentials(FakeUser), ("test@webkit.org", "NOMNOMNOM"))
</span><ins>+
+ def test_do_not_use_stored_credentials(self):
+ class MockKeyring(object):
+ def get_password(self, host, username):
+ raise AssertionError("Should not read from keyring.")
+
+ class FakeCredentials(MockedCredentials):
+ def _credentials_from_keychain(self, username):
+ raise AssertionError("Should not read from keychain.")
+
+ def _credentials_from_environment(self):
+ raise AssertionError("Should not read from environment.")
+
+ def _offer_to_store_credentials_in_keyring(self, username, password):
+ pass
+
+ class FakeUser(MockUser):
+ @classmethod
+ def prompt(cls, message, repeat=1, raw_input=raw_input):
+ return "test@webkit.org"
+
+ @classmethod
+ def prompt_password(cls, message, repeat=1, raw_input=raw_input):
+ return "NOMNOMNOM"
+
+ with _TemporaryDirectory(suffix="not_a_git_repo") as temp_dir_path:
+ credentials = FakeCredentials("fake.hostname", cwd=temp_dir_path, keyring=MockKeyring())
+ self.assertEqual(credentials.read_credentials(FakeUser, use_stored_credentials=False), ("test@webkit.org", "NOMNOMNOM"))
</ins></span></pre>
</div>
</div>
</body>
</html>