Adjust login to conform to UX specification

docs/upload-your-snap.md

@@ -22,8 +22,8 @@ store on your behalf:
     Enter your Ubuntu One SSO credentials.
     Email: me@example.com
     Password:
-    One-time password (just press enter if you don't use two-factor authentication):
-    Authenticating against Ubuntu One SSO.
+    Second-factor auth:
+
     Login successful.
 
 These credentials will remain valid until you revoke them, which you can do

integration_tests/__init__.py

@@ -111,11 +111,9 @@ def login(self, email=None, password=None, expect_success=True):
         process.sendline(email)
         process.expect_exact('Password: ')
         process.sendline(password)
-        process.expect_exact(
-            "One-time password (just press enter if you don't use two-factor "
-            "authentication): ")
-        process.sendline('')
-        process.expect_exact('Authenticating against Ubuntu One SSO.')
+        if expect_success:
+            process.expect_exact(
+                'We strongly recommend enabling multi-factor authentication:')
         result = 'successful' if expect_success else 'failed'
         process.expect_exact('Login {}.'.format(result))
 
@@ -146,12 +144,9 @@ def register_key(self, key_name, email=None, password=None,
         process.sendline(email)
         process.expect_exact('Password: ')
         process.sendline(password)
-        process.expect_exact(
-            "One-time password (just press enter if you don't use two-factor "
-            "authentication): ")
-        process.sendline('')
-        process.expect_exact('Authenticating against Ubuntu One SSO.')
         if expect_success:
+            process.expect_exact(
+                'We strongly recommend enabling multi-factor authentication:')
             process.expect_exact('Login successful.')
             process.expect(
                 r'Done\. The key "{}" .* may be used to sign your '

snapcraft/_store.py

@@ -51,20 +51,26 @@ def _login(store, acls=None, save=True):
     print('Enter your Ubuntu One SSO credentials.')
     email = input('Email: ')
     password = getpass.getpass('Password: ')
-    one_time_password = input(
-        'One-time password (just press enter if you don\'t use two-factor '
-        'authentication): ')
 
-    logger.info('Authenticating against Ubuntu One SSO.')
     try:
-        store.login(
-            email, password, one_time_password=one_time_password, acls=acls,
-            save=save)
+        try:
+            store.login(email, password, acls=acls, save=save)
+            print()
+            logger.info(
+                'We strongly recommend enabling multi-factor authentication: '
+                'https://help.ubuntu.com/community/SSO/FAQs/2FA')
+        except storeapi.errors.StoreTwoFactorAuthenticationRequired:
+            one_time_password = input('Second-factor auth: ')
+            store.login(
+                email, password, one_time_password=one_time_password,
+                acls=acls, save=save)
     except (storeapi.errors.InvalidCredentialsError,
             storeapi.errors.StoreAuthenticationError):
+        print()
         logger.info('Login failed.')
         return False
     else:
+        print()
         logger.info('Login successful.')
         return True
 

snapcraft/storeapi/__init__.py

@@ -24,13 +24,14 @@
 from threading import Thread
 from queue import Queue
 
-import pymacaroons
-import requests
 from progressbar import (
     AnimatedMarker,
     ProgressBar,
     UnknownLength,
 )
+import pymacaroons
+import requests
+from simplejson.scanner import JSONDecodeError
 
 import snapcraft
 from snapcraft import config
@@ -212,7 +213,7 @@ def _is_downloaded(self, path, expected_sha512):
 
 
 class SSOClient(Client):
-    """The Single Sign On server deals with authentification.
+    """The Single Sign On server deals with authentication.
 
     It is used directly or indirectly by other servers.
 
@@ -232,11 +233,20 @@ def get_unbound_discharge(self, email, password, one_time_password,
             'tokens/discharge', data=json.dumps(data),
             headers={'Content-Type': 'application/json',
                      'Accept': 'application/json'})
+        try:
+            response_json = response.json()
+        except JSONDecodeError:
+            response_json = {}
         if response.ok:
-            return response.json()['discharge_macaroon']
+            return response_json['discharge_macaroon']
         else:
-            raise errors.StoreAuthenticationError(
-                'Failed to get unbound discharge: '.format(response.text))
+            if (response.status_code == requests.codes.unauthorized and
+                any(error.get('code') == 'twofactor-required'
+                    for error in response_json.get('error_list', []))):
+                raise errors.StoreTwoFactorAuthenticationRequired()
+            else:
+                raise errors.StoreAuthenticationError(
+                    'Failed to get unbound discharge: '.format(response.text))
 
 
 class SnapIndexClient(Client):

snapcraft/storeapi/errors.py

@@ -59,6 +59,12 @@ def __init__(self, message):
         super().__init__(message=message)
 
 
+class StoreTwoFactorAuthenticationRequired(StoreAuthenticationError):
+
+    def __init__(self):
+        super().__init__("Two-factor authentication required.")
+
+
 class StoreAccountInformationError(StoreError):
 
     fmt = 'Error fetching account information from store: {error}'

snapcraft/tests/fake_servers.py

@@ -216,6 +216,10 @@ def _handle_tokens_discharge_request(self):
             ('otp' not in data or
              data['otp'] == 'test correct one-time password')):
             self._send_tokens_discharge(data)
+        elif data['password'] == 'test requires 2fa':
+            self._send_twofactor_required_error()
+        elif data['password'] == 'test 401 invalid json':
+            self._send_401_invalid_json()
         else:
             self._send_invalid_credentials_error()
 
@@ -229,14 +233,33 @@ def _send_tokens_discharge(self, data):
         self.wfile.write(
             json.dumps(response).encode())
 
+    def _send_twofactor_required_error(self):
+        self.send_response(401)
+        self.send_header('Content-Type', 'application/json')
+        self.end_headers()
+        response = {
+            'error_list': [{
+                'code': 'twofactor-required',
+                'message': '2-factor authentication required.',
+            }],
+        }
+        self.wfile.write(json.dumps(response).encode())
+
+    def _send_401_invalid_json(self):
+        self.send_response(401)
+        self.send_header('Content-Type', 'application/json')
+        self.end_headers()
+        self.wfile.write(b'invalid{json')
+
     def _send_invalid_credentials_error(self):
         self.send_response(401)
         self.send_header('Content-Type', 'application/json')
         self.end_headers()
         response = {
-            'code': 'INVALID_CREDENTIALS',
-            'message': 'Provided email/password is not correct.',
-            'message': 'Provided email/password is not correct.',
+            'error_list': [{
+                'code': 'invalid-credentials',
+                'message': 'Provided email/password is not correct.',
+            }],
         }
         self.wfile.write(json.dumps(response).encode())
 

snapcraft/tests/store/test_store_client.py

@@ -61,6 +61,12 @@ def test_failed_login_with_wrong_password(self):
 
         self.assertTrue(config.Config().is_empty())
 
+    def test_failed_login_requires_one_time_password(self):
+        with self.assertRaises(errors.StoreTwoFactorAuthenticationRequired):
+            self.client.login('dummy email', 'test requires 2fa')
+
+        self.assertTrue(config.Config().is_empty())
+
     def test_failed_login_with_wrong_one_time_password(self):
         with self.assertRaises(errors.StoreAuthenticationError):
             self.client.login(
@@ -70,6 +76,12 @@ def test_failed_login_with_wrong_one_time_password(self):
 
         self.assertTrue(config.Config().is_empty())
 
+    def test_failed_login_with_invalid_json(self):
+        with self.assertRaises(errors.StoreAuthenticationError):
+            self.client.login('dummy email', 'test 401 invalid json')
+
+        self.assertTrue(config.Config().is_empty())
+
 
 class DownloadTestCase(tests.TestCase):
 

snapcraft/tests/test_commands_login.py

@@ -34,7 +34,7 @@ def setUp(self):
         self.useFixture(self.fake_logger)
 
         patcher = mock.patch('builtins.input')
-        patcher.start()
+        self.mock_input = patcher.start()
         self.addCleanup(patcher.stop)
 
         patcher = mock.patch('builtins.print')
@@ -45,36 +45,56 @@ def setUp(self):
         patcher.start()
         self.addCleanup(patcher.stop)
 
-    def test_successful_login(self):
-        with mock.patch.object(storeapi.StoreClient, 'login'):
-            # no exception raised.
-            main(['login'])
+    @mock.patch.object(storeapi.StoreClient, 'login')
+    def test_successful_login(self, mock_login):
+        self.mock_input.return_value = 'user@example.com'
+
+        # no exception raised.
+        main(['login'])
 
+        self.mock_input.assert_called_once_with('Email: ')
+        mock_login.assert_called_once_with(
+            'user@example.com', mock.ANY, acls=None, save=True)
         self.assertEqual(
-            'Authenticating against Ubuntu One SSO.\n'
+            'We strongly recommend enabling multi-factor authentication: '
+            'https://help.ubuntu.com/community/SSO/FAQs/2FA\n'
             'Login successful.\n',
             self.fake_logger.output)
 
-    def test_failed_login_with_invalid_credentials(self):
-        with mock.patch.object(
-                storeapi.StoreClient, 'login') as mock_login:
-            mock_login.side_effect = storeapi.errors.InvalidCredentialsError(
-                'error')
-            main(['login'])
+    @mock.patch.object(storeapi.StoreClient, 'login')
+    def test_successful_login_with_2fa(self, mock_login):
+        self.mock_input.side_effect = ('user@example.com', '123456')
+        mock_login.side_effect = [
+            storeapi.errors.StoreTwoFactorAuthenticationRequired(), None]
 
-        self.assertEqual(
-            'Authenticating against Ubuntu One SSO.\n'
-            'Login failed.\n',
-            self.fake_logger.output)
+        # no exception raised.
+        main(['login'])
 
-    def test_failed_login_with_store_authentication_error(self):
-        with mock.patch.object(
-                storeapi.StoreClient, 'login') as mock_login:
-            mock_login.side_effect = storeapi.errors.StoreAuthenticationError(
-                'error')
-            main(['login'])
+        self.assertEqual(2, self.mock_input.call_count)
+        self.mock_input.assert_has_calls([
+            mock.call('Email: '), mock.call('Second-factor auth: ')])
+        self.assertEqual(2, mock_login.call_count)
+        mock_login.assert_has_calls([
+            mock.call('user@example.com', mock.ANY, acls=None, save=True),
+            mock.call(
+                'user@example.com', mock.ANY, one_time_password='123456',
+                acls=None, save=True)])
+        self.assertEqual('Login successful.\n', self.fake_logger.output)
 
-        self.assertEqual(
-            'Authenticating against Ubuntu One SSO.\n'
-            'Login failed.\n',
-            self.fake_logger.output)
+    @mock.patch.object(storeapi.StoreClient, 'login')
+    def test_failed_login_with_invalid_credentials(self, mock_login):
+        mock_login.side_effect = storeapi.errors.InvalidCredentialsError(
+            'error')
+
+        main(['login'])
+
+        self.assertEqual('Login failed.\n', self.fake_logger.output)
+
+    @mock.patch.object(storeapi.StoreClient, 'login')
+    def test_failed_login_with_store_authentication_error(self, mock_login):
+        mock_login.side_effect = storeapi.errors.StoreAuthenticationError(
+            'error')
+
+        main(['login'])
+
+        self.assertEqual('Login failed.\n', self.fake_logger.output)

snapcraft/tests/test_commands_register_key.py

@@ -114,19 +114,20 @@ def test_register_key_successfully(self, mock_installed, mock_input,
         mock_input.side_effect = ['sample.person@canonical.com', '123456']
         mock_getpass.return_value = 'secret'
         mock_check_output.side_effect = mock_snap_output
+        mock_login.side_effect = [
+            storeapi.errors.StoreTwoFactorAuthenticationRequired(), None]
         mock_get_account_information.return_value = {'account_id': 'abcd'}
 
         main(['register-key', 'default'])
 
         self.assertEqual(
-            'Authenticating against Ubuntu One SSO.\n'
             'Login successful.\n'
             'Registering key ...\n'
             'Done. The key "default" ({}) may be used to sign your '
             'assertions.\n'.format(get_sample_key('default')['sha3-384']),
             self.fake_logger.output)
 
-        mock_login.assert_called_once_with(
+        mock_login.assert_called_with(
             'sample.person@canonical.com', 'secret',
             one_time_password='123456', acls=['modify_account_key'],
             save=False)
@@ -286,7 +287,7 @@ def test_register_key_select_key(self, mock_installed, mock_input,
                                      mock_login, mock_get_account_information,
                                      mock_register_key):
         mock_installed.return_value = True
-        mock_input.side_effect = ['x', '2', 'sample.person@canonical.com', '']
+        mock_input.side_effect = ['x', '2', 'sample.person@canonical.com']
         mock_getpass.return_value = 'secret'
         mock_check_output.side_effect = mock_snap_output
         mock_get_account_information.return_value = {'account_id': 'abcd'}
@@ -308,7 +309,8 @@ def test_register_key_select_key(self, mock_installed, mock_input,
             [mock.call('Key number: '), mock.call('Key number: ')])
 
         self.assertEqual(
-            'Authenticating against Ubuntu One SSO.\n'
+            'We strongly recommend enabling multi-factor authentication: '
+            'https://help.ubuntu.com/community/SSO/FAQs/2FA\n'
             'Login successful.\n'
             'Registering key ...\n'
             'Done. The key "another" ({}) may be used to sign your '