Repository: girder/girder
Issue: Log in to hub.yt through Github fails

When trying to log into hub.yt using my github account I get the following message:

{
    "message": "Invalid CSRF token (state=\\"z7XYtLC6b63gUAThc0FcaEniJzqxagxsxA56BSPi9dLdqRIQmnCLKooft0fZc1tj.https://girder.hub.yt/#\\").",
    "type": "rest"
}

{
    "message": "AttributeError: AttributeError(\\"'NoneType' object has no attribute 'split'\\",)",
    "type": "internal"
}

How can I fix this or is this some bug?

Thanks,


Changed files:
plugins/oauth/server/providers/github.py, plugins/oauth/plugin_tests/oauth_test.py, tests/base.py, plugins/oauth/server/providers/bitbucket.py

diff --git a/plugins/oauth/plugin_tests/oauth_test.py b/plugins/oauth/plugin_tests/oauth_test.py
index 3de400827..970adf948 100644
--- a/plugins/oauth/plugin_tests/oauth_test.py
+++ b/plugins/oauth/plugin_tests/oauth_test.py
@@ -57,7 +57,7 @@ class OauthTest(base.TestCase):
             admin=True
         )
 
-        # Specifies which test account (typically "new" or "existing") a
+        # Specifies which test account (typically 'new' or 'existing') a
         # redirect to a provider will simulate authentication for
         self.accountType = None
 
@@ -73,17 +73,14 @@ class OauthTest(base.TestCase):
         login = ProviderBase._deriveLogin('hello#world#foo@mail.com', 'A', 'B')
         self.assertEqual(login, 'helloworldfoo')
 
-        login = ProviderBase._deriveLogin('hello.world@mail.com', 'A', 'B',
-                                          'user2')
+        login = ProviderBase._deriveLogin('hello.world@mail.com', 'A', 'B', 'user2')
         self.assertEqual(login, 'user2')
 
         # This should conflict with the saved admin user
-        login = ProviderBase._deriveLogin('rocky@phila.pa.us', 'Robert',
-                                          'Balboa', 'rocky')
+        login = ProviderBase._deriveLogin('rocky@phila.pa.us', 'Robert', 'Balboa', 'rocky')
         self.assertEqual(login, 'rocky1')
 
-    def _testOauth(self, providerInfo):
-        # Close registration to start off, and simulate a new user
+    def _testSettings(self, providerInfo):
         self.model('setting').set(SettingKey.REGISTRATION_POLICY, 'closed')
         self.accountType = 'new'
 
@@ -92,8 +89,7 @@ class OauthTest(base.TestCase):
             'key': PluginSettings.PROVIDERS_ENABLED,
             'value': []
         }
-        resp = self.request(
-            '/system/setting', user=self.adminUser, method='PUT', params=params)
+        resp = self.request('/system/setting', user=self.adminUser, method='PUT', params=params)
         self.assertStatusOk(resp)
 
         resp = self.request('/oauth/provider', exception=True, params={
@@ -110,12 +106,12 @@ class OauthTest(base.TestCase):
                 'value': [providerInfo['id']]
             }])
         }
-        resp = self.request(
-            '/system/setting', user=self.adminUser, method='PUT', params=params)
+        resp = self.request('/system/setting', user=self.adminUser, method='PUT', params=params)
         self.assertStatusOk(resp)
 
         resp = self.request('/oauth/provider', exception=True, params={
-            'redirect': 'http://localhost/#foo/bar'})
+            'redirect': 'http://localhost/#foo/bar'
+        })
         self.assertStatus(resp, 500)
 
         # Set up provider normally
@@ -133,13 +129,15 @@ class OauthTest(base.TestCase):
                 }
             ])
         }
-        resp = self.request(
-            '/system/setting', user=self.adminUser, method='PUT',
-            params=params)
+        resp = self.request('/system/setting', user=self.adminUser, method='PUT', params=params)
         self.assertStatusOk(resp)
         # No need to re-fetch and test all of these settings values; they will
         # be implicitly tested later
 
+    def _testOauth(self, providerInfo):
+        # Close registration to start off, and simulate a new user
+        self._testSettings(providerInfo)
+
         # Make sure that if no list param is passed, we receive the old format
         resp = self.request('/oauth/provider', params={
             'redirect': 'http://localhost/#foo/bar'
@@ -148,9 +146,7 @@ class OauthTest(base.TestCase):
         self.assertIsInstance(resp.json, dict)
         self.assertEqual(len(resp.json), 1)
         self.assertIn(providerInfo['name'], resp.json)
-        self.assertRegexpMatches(
-            resp.json[providerInfo['name']],
-            providerInfo['url_re'])
+        six.assertRegex(self, resp.json[providerInfo['name']], providerInfo['url_re'])
 
         # This will need to be called several times, to get fresh tokens
         def getProviderResp():
@@ -162,31 +158,24 @@ class OauthTest(base.TestCase):
             self.assertIsInstance(resp.json, list)
             self.assertEqual(len(resp.json), 1)
             providerResp = resp.json[0]
-            self.assertSetEqual(
-                set(six.viewkeys(providerResp)),
-                {'id', 'name', 'url'})
+            self.assertSetEqual(set(six.viewkeys(providerResp)), {'id', 'name', 'url'})
             self.assertEqual(providerResp['id'], providerInfo['id'])
             self.assertEqual(providerResp['name'], providerInfo['name'])
-            self.assertRegexpMatches(
-                providerResp['url'],
-                providerInfo['url_re'])
+            six.assertRegex(self, providerResp['url'], providerInfo['url_re'])
             redirectParams = urllib.parse.parse_qs(
                 urllib.parse.urlparse(providerResp['url']).query)
             csrfTokenParts = redirectParams['state'][0].partition('.')
-            token = self.model('token').load(
-                csrfTokenParts[0], force=True, objectId=False)
+            token = self.model('token').load(csrfTokenParts[0], force=True, objectId=False)
             self.assertLess(
                 token['expires'],
                 datetime.datetime.utcnow() + datetime.timedelta(days=0.30))
-            self.assertEqual(
-                csrfTokenParts[2],
-                'http://localhost/#foo/bar')
+            self.assertEqual(csrfTokenParts[2], 'http://localhost/#foo/bar')
             return providerResp
 
         # Try the new format listing
         getProviderResp()
 
-        # Try callback, for a non-existant provider
+        # Try callback, for a nonexistent provider
         resp = self.request('/oauth/foobar/callback')
         self.assertStatus(resp, 400)
 
@@ -202,9 +191,7 @@ class OauthTest(base.TestCase):
                 'error': 'some_custom_error',
             }, exception=True)
         self.assertStatus(resp, 502)
-        self.assertEqual(
-            resp.json['message'],
-            "Provider returned error: 'some_custom_error'.")
+        self.assertEqual(resp.json['message'], "Provider returned error: 'some_custom_error'.")
 
         # This will need to be called several times, to use fresh tokens
         def getCallbackParams(providerResp):
@@ -212,8 +199,7 @@ class OauthTest(base.TestCase):
             self.assertEqual(resp.status_code, 302)
             callbackLoc = urllib.parse.urlparse(resp.headers['location'])
             self.assertEqual(
-                callbackLoc.path,
-                r'/api/v1/oauth/%s/callback' % providerInfo['id'])
+                callbackLoc.path, r'/api/v1/oauth/%s/callback' % providerInfo['id'])
             callbackLocQuery = urllib.parse.parse_qs(callbackLoc.query)
             self.assertNotHasKeys(callbackLocQuery, ('error',))
             callbackParams = {
@@ -227,8 +213,7 @@ class OauthTest(base.TestCase):
         # Try callback, with incorrect CSRF token
         params = getCallbackParams(getProviderResp())
         params['state'] = 'something_wrong'
-        resp = self.request('/oauth/%s/callback' % providerInfo['id'],
-                            params=params)
+        resp = self.request('/oauth/%s/callback' % providerInfo['id'], params=params)
         self.assertStatus(resp, 403)
         self.assertTrue(
             resp.json['message'].startswith('Invalid CSRF token'))
@@ -239,79 +224,66 @@ class OauthTest(base.TestCase):
             params['state'].partition('.')[0], force=True, objectId=False)
         token['expires'] -= datetime.timedelta(days=1)
         self.model('token').save(token)
-        resp = self.request('/oauth/%s/callback' % providerInfo['id'],
-                            params=params)
+        resp = self.request('/oauth/%s/callback' % providerInfo['id'], params=params)
         self.assertStatus(resp, 403)
-        self.assertTrue(
-            resp.json['message'].startswith('Expired CSRF token'))
+        self.assertTrue(resp.json['message'].startswith('Expired CSRF token'))
 
         # Try callback, with a valid CSRF token but no redirect
         params = getCallbackParams(getProviderResp())
         params['state'] = params['state'].partition('.')[0]
-        resp = self.request('/oauth/%s/callback' % providerInfo['id'],
-                            params=params)
+        resp = self.request('/oauth/%s/callback' % providerInfo['id'], params=params)
         self.assertStatus(resp, 400)
-        self.assertTrue(
-            resp.json['message'].startswith('No redirect location'))
+        self.assertTrue(resp.json['message'].startswith('No redirect location'))
 
         # Try callback, with incorrect code
         params = getCallbackParams(getProviderResp())
         params['code'] = 'something_wrong'
-        resp = self.request('/oauth/%s/callback' % providerInfo['id'],
-                            params=params)
+        resp = self.request('/oauth/%s/callback' % providerInfo['id'], params=params)
         self.assertStatus(resp, 502)
 
         # Try callback, with real parameters from provider, but still for the
         # 'new' account
         params = getCallbackParams(getProviderResp())
-        resp = self.request('/oauth/%s/callback' % providerInfo['id'],
-                            params=params)
+        resp = self.request('/oauth/%s/callback' % providerInfo['id'], params=params)
         self.assertStatus(resp, 400)
         self.assertTrue(
-            resp.json['message'].startswith(
-                'Registration on this instance is closed.'))
+            resp.json['message'].startswith('Registration on this instance is closed.'))
 
         # This will need to be called several times, and will do a normal login
         def doOauthLogin(accountType):
             self.accountType = accountType
             params = getCallbackParams(getProviderResp())
-            resp = self.request('/oauth/%s/callback' % providerInfo['id'],
-                                params=params, isJson=False)
+            resp = self.request(
+                '/oauth/%s/callback' % providerInfo['id'], params=params, isJson=False)
             self.assertStatus(resp, 303)
-            self.assertEqual(resp.headers['Location'],
-                             'http://localhost/#foo/bar')
+            self.assertEqual(resp.headers['Location'], 'http://localhost/#foo/bar')
             self.assertTrue('girderToken' in resp.cookie)
 
-            resp = self.request('/user/me',
-                                token=resp.cookie['girderToken'].value)
+            resp = self.request('/user/me', token=resp.cookie['girderToken'].value)
+            user = resp.json
             self.assertStatusOk(resp)
             self.assertEqual(
-                resp.json['email'],
-                providerInfo['accounts'][accountType]['user']['email'])
+                user['email'], providerInfo['accounts'][accountType]['user']['email'])
             self.assertEqual(
-                resp.json['login'],
-                providerInfo['accounts'][accountType]['user']['login'])
+                user['login'], providerInfo['accounts'][accountType]['user']['login'])
             self.assertEqual(
-                resp.json['firstName'],
-                providerInfo['accounts'][accountType]['user']['firstName'])
+                user['firstName'], providerInfo['accounts'][accountType]['user']['firstName'])
             self.assertEqual(
-                resp.json['lastName'],
-                providerInfo['accounts'][accountType]['user']['lastName'])
+                user['lastName'], providerInfo['accounts'][accountType]['user']['lastName'])
+            return user
 
         # Try callback for the 'existing' account, which should succeed
-        doOauthLogin('existing')
+        existing = doOauthLogin('existing')
 
         # Try callback for the 'new' account, with open registration
         self.model('setting').set(SettingKey.REGISTRATION_POLICY, 'open')
-        doOauthLogin('new')
+        new = doOauthLogin('new')
 
         # Password login for 'new' OAuth-only user should fail gracefully
         newUser = providerInfo['accounts']['new']['user']
-        resp = self.request('/user/authentication',
-                            basicAuth='%s:mypasswd' % newUser['login'])
+        resp = self.request('/user/authentication', basicAuth='%s:mypasswd' % newUser['login'])
         self.assertStatus(resp, 400)
-        self.assertTrue(
-            resp.json['message'].startswith('You don\\'t have a password.'))
+        self.assertTrue(resp.json['message'].startswith('You don\\'t have a password.'))
 
         # Reset password for 'new' OAuth-only user should work
         self.assertTrue(base.mockSmtp.isMailQueueEmpty())
@@ -329,27 +301,28 @@ class OauthTest(base.TestCase):
         linkParts = link.split('/')
         userId = linkParts[-3]
         tokenId = linkParts[-1]
-        tempToken = self.model('token').load(
-            tokenId, force=True, objectId=False)
-        resp = self.request('/user/password/temporary/' + userId,
-                            method='GET', params={'token': tokenId})
+        tempToken = self.model('token').load(tokenId, force=True, objectId=False)
+        resp = self.request(
+            '/user/password/temporary/' + userId, method='GET', params={'token': tokenId})
         self.assertStatusOk(resp)
         self.assertEqual(resp.json['user']['login'], newUser['login'])
         # We should now be able to change the password
-        resp = self.request('/user/password',
-                            method='PUT', user=resp.json['user'], params={
-                                'old': tokenId,
-                                'new': 'mypasswd'})
+        resp = self.request(
+            '/user/password', method='PUT', user=resp.json['user'], params={
+                'old': tokenId,
+                'new': 'mypasswd'
+            })
         self.assertStatusOk(resp)
         # The temp token should get deleted on password change
         token = self.model('token').load(tempToken, force=True, objectId=False)
         self.assertEqual(token, None)
 
         # Password login for 'new' OAuth-only user should now succeed
-        resp = self.request('/user/authentication',
-                            basicAuth='%s:mypasswd' % newUser['login'])
+        resp = self.request('/user/authentication', basicAuth='%s:mypasswd' % newUser['login'])
         self.assertStatusOk(resp)
 
+        return existing, new
+
     @httmock.all_requests
     def mockOtherRequest(self, url, request):
         raise Exception('Unexpected url %s' % str(request.url))
@@ -366,8 +339,7 @@ class OauthTest(base.TestCase):
                 'key': PluginSettings.GOOGLE_CLIENT_SECRET,
                 'value': 'google_test_client_secret'
             },
-            'allowed_callback_re':
-                r'^http://127\\.0\\.0\\.1(?::\\d+)?/api/v1/oauth/google/callback$',
+            'allowed_callback_re': r'^http://127\\.0\\.0\\.1(?::\\d+)?/api/v1/oauth/google/callback$',
             'url_re': r'^https://accounts\\.google\\.com/o/oauth2/auth',
             'accounts': {
                 'existing': {
@@ -408,15 +380,9 @@ class OauthTest(base.TestCase):
         def mockGoogleRedirect(url, request):
             try:
                 params = urllib.parse.parse_qs(url.query)
-                self.assertEqual(
-                    params['response_type'],
-                    ['code'])
-                self.assertEqual(
-                    params['access_type'],
-                    ['online'])
-                self.assertEqual(
-                    params['scope'],
-                    ['profile email'])
+                self.assertEqual(params['response_type'], ['code'])
+                self.assertEqual(params['access_type'], ['online'])
+                self.assertEqual(params['scope'], ['profile email'])
             except (KeyError, AssertionError) as e:
                 return {
                     'status_code': 400,
@@ -425,9 +391,7 @@ class OauthTest(base.TestCase):
                     })
                 }
             try:
-                self.assertEqual(
-                    params['client_id'],
-                    [providerInfo['client_id']['value']])
+                self.assertEqual(params['client_id'], [providerInfo['client_id']['value']])
             except (KeyError, AssertionError) as e:
                 return {
                     'status_code': 401,
@@ -436,9 +400,8 @@ class OauthTest(base.TestCase):
                     })
                 }
             try:
-                self.assertRegexpMatches(
-                    params['redirect_uri'][0],
-                    providerInfo['allowed_callback_re'])
+                six.assertRegex(
+                    self, params['redirect_uri'][0], providerInfo['allowed_callback_re'])
                 state = params['state'][0]
                 # Nothing to test for state, since provider doesn't care
             except (KeyError, AssertionError) as e:
@@ -455,8 +418,7 @@ class OauthTest(base.TestCase):
             return {
                 'status_code': 302,
                 'headers': {
-                    'Location': '%s?%s' % (params['redirect_uri'][0],
-                                           returnQuery)
+                    'Location': '%s?%s' % (params['redirect_uri'][0], returnQuery)
                 }
             }
 
@@ -465,9 +427,7 @@ class OauthTest(base.TestCase):
         def mockGoogleToken(url, request):
             try:
                 params = urllib.parse.parse_qs(request.body)
-                self.assertEqual(
-                    params['client_id'],
-                    [providerInfo['client_id']['value']])
+                self.assertEqual(params['client_id'], [providerInfo['client_id']['value']])
             except (KeyError, AssertionError) as e:
                 return {
                     'status_code': 401,
@@ -476,15 +436,10 @@ class OauthTest(base.TestCase):
                     })
                 }
             try:
-                self.assertEqual(
-                    params['grant_type'],
-                    ['authorization_code'])
-                self.assertEqual(
-                    params['client_secret'],
-                    [providerInfo['client_secret']['value']])
-                self.assertRegexpMatches(
-                    params['redirect_uri'][0],
-                    providerInfo['allowed_callback_re'])
+                self.assertEqual(params['grant_type'], ['authorization_code'])
+                self.assertEqual(params['client_secret'], [providerInfo['client_secret']['value']])
+                six.assertRegex(
+                    self, params['redirect_uri'][0], providerInfo['allowed_callback_re'])
                 for account in six.viewvalues(providerInfo['accounts']):
                     if account['auth_code'] == params['code'][0]:
                         break
@@ -509,16 +464,13 @@ class OauthTest(base.TestCase):
         def mockGoogleApi(url, request):
             try:
                 for account in six.viewvalues(providerInfo['accounts']):
-                    if 'Bearer %s' % account['access_token'] == \\
-                            request.headers['Authorization']:
+                    if 'Bearer %s' % account['access_token'] == request.headers['Authorization']:
                         break
                 else:
                     self.fail()
 
                 params = urllib.parse.parse_qs(url.query)
-                self.assertSetEqual(
-                    set(params['fields'][0].split(',')),
-                    {'id', 'emails', 'name'})
+                self.assertSetEqual(set(params['fields'][0].split(',')), {'id', 'emails', 'name'})
             except AssertionError as e:
                 return {
                     'status_code': 401,
@@ -547,7 +499,7 @@ class OauthTest(base.TestCase):
             mockGoogleRedirect,
             mockGoogleToken,
             mockGoogleApi,
-            # Must keep "mockOtherRequest" last
+            # Must keep 'mockOtherRequest' last
             self.mockOtherRequest
         ):
             self._testOauth(providerInfo)
@@ -610,9 +562,7 @@ class OauthTest(base.TestCase):
                 params = urllib.parse.parse_qs(url.query)
                 # Check redirect_uri first, so other errors can still redirect
                 redirectUri = params['redirect_uri'][0]
-                self.assertEqual(
-                    params['client_id'],
-                    [providerInfo['client_id']['value']])
+                self.assertEqual(params['client_id'], [providerInfo['client_id']['value']])
             except (KeyError, AssertionError) as e:
                 return {
                     'status_code': 404,
@@ -621,14 +571,10 @@ class OauthTest(base.TestCase):
                     })
                 }
             try:
-                self.assertRegexpMatches(
-                    redirectUri,
-                    providerInfo['allowed_callback_re'])
+                six.assertRegex(self, redirectUri, providerInfo['allowed_callback_re'])
                 state = params['state'][0]
                 # Nothing to test for state, since provider doesn't care
-                self.assertEqual(
-                    params['scope'],
-                    ['user:email'])
+                self.assertEqual(params['scope'], ['user:email'])
             except (KeyError, AssertionError) as e:
                 returnQuery = urllib.parse.urlencode({
                     'error': repr(e),
@@ -636,8 +582,7 @@ class OauthTest(base.TestCase):
             else:
                 returnQuery = urllib.parse.urlencode({
                     'state': state,
-                    'code':
-                        providerInfo['accounts'][self.accountType]['auth_code']
+                    'code': providerInfo['accounts'][self.accountType]['auth_code']
                 })
             return {
                 'status_code': 302,
@@ -652,9 +597,7 @@ class OauthTest(base.TestCase):
             try:
                 self.assertEqual(request.headers['Accept'], 'application/json')
                 params = urllib.parse.parse_qs(request.body)
-                self.assertEqual(
-                    params['client_id'],
-                    [providerInfo['client_id']['value']])
+                self.assertEqual(params['client_id'], [providerInfo['client_id']['value']])
             except (KeyError, AssertionError) as e:
                 return {
                     'status_code': 404,
@@ -668,12 +611,9 @@ class OauthTest(base.TestCase):
                         break
                 else:
                     self.fail()
-                self.assertEqual(
-                    params['client_secret'],
-                    [providerInfo['client_secret']['value']])
-                self.assertRegexpMatches(
-                    params['redirect_uri'][0],
-                    providerInfo['allowed_callback_re'])
+                self.assertEqual(params['client_secret'], [providerInfo['client_secret']['value']])
+                six.assertRegex(
+                    self, params['redirect_uri'][0], providerInfo['allowed_callback_re'])
             except (KeyError, AssertionError) as e:
                 returnBody = json.dumps({
                     'error': repr(e),
@@ -693,13 +633,11 @@ class OauthTest(base.TestCase):
                 'content': returnBody
             }
 
-        @httmock.urlmatch(scheme='https', netloc='^api.github.com$',
-                          path='^/user$', method='GET')
+        @httmock.urlmatch(scheme='https', netloc='^api.github.com$', path='^/user$', method='GET')
         def mockGithubApiUser(url, request):
             try:
                 for account in six.viewvalues(providerInfo['accounts']):
-                    if 'token %s' % account['access_token'] == \\
-                            request.headers['Authorization']:
+                    if 'token %s' % account['access_token'] == request.headers['Authorization']:
                         break
                 else:
                     self.fail()
@@ -713,8 +651,7 @@ class OauthTest(base.TestCase):
             return json.dumps({
                 'id': account['user']['oauth']['id'],
                 'login': account['user']['login'],
-                'name': '%s %s' % (account['user']['firstName'],
-                                   account['user']['lastName'])
+                'name': '%s %s' % (account['user']['firstName'], account['user']['lastName'])
             })
 
         @httmock.urlmatch(scheme='https', netloc='^api.github.com$',
@@ -722,8 +659,7 @@ class OauthTest(base.TestCase):
         def mockGithubApiEmail(url, request):
             try:
                 for account in six.viewvalues(providerInfo['accounts']):
-                    if 'token %s' % account['access_token'] == \\
-                            request.headers['Authorization']:
+                    if 'token %s' % account['access_token'] == request.headers['Authorization']:
                         break
                 else:
                     self.fail()
@@ -751,7 +687,47 @@ class OauthTest(base.TestCase):
             mockGithubToken,
             mockGithubApiUser,
             mockGithubApiEmail,
-            # Must keep "mockOtherRequest" last
+            # Must keep 'mockOtherRequest' last
+            self.mockOtherRequest
+        ):
+            self._testOauth(providerInfo)
+
+        @httmock.urlmatch(scheme='https', netloc='^api.github.com$', path='^/user$', method='GET')
+        def mockGithubUserWithoutName(url, request):
+            try:
+                for account in six.viewvalues(providerInfo['accounts']):
+                    if 'token %s' % account['access_token'] == request.headers['Authorization']:
+                        break
+                else:
+                    self.fail()
+            except AssertionError as e:
+                return {
+                    'status_code': 401,
+                    'content': json.dumps({
+                        'message': repr(e)
+                    })
+                }
+            return json.dumps({
+                'id': account['user']['oauth']['id'],
+                'login': account['user']['login'],
+                'name': None
+            })
+
+        self.setUp()  # Call to reset everything so we can call _testOauth again
+
+        # If no name is provided, we expect to use the github login for both
+        providerInfo['accounts']['existing']['user']['lastName'] = 'rocky'
+        providerInfo['accounts']['existing']['user']['firstName'] = 'rocky'
+
+        providerInfo['accounts']['new']['user']['lastName'] = 'drago'
+        providerInfo['accounts']['new']['user']['firstName'] = 'drago'
+
+        with httmock.HTTMock(
+            mockGithubRedirect,
+            mockGithubToken,
+            mockGithubUserWithoutName,
+            mockGithubApiEmail,
+            # Must keep 'mockOtherRequest' last
             self.mockOtherRequest
         ):
             self._testOauth(providerInfo)
@@ -769,8 +745,7 @@ class OauthTest(base.TestCase):
                 'value': 'linkedin_test_client_secret'
             },
             'allowed_callback_re':
-                r'^http://127\\.0\\.0\\.1(?::\\d+)?'
-                r'/api/v1/oauth/linkedin/callback$',
+                r'^http://127\\.0\\.0\\.1(?::\\d+)?/api/v1/oauth/linkedin/callback$',
             'url_re': r'^https://www\\.linkedin\\.com/uas/oauth2/authorization',
             'accounts': {
                 'existing': {
@@ -811,12 +786,9 @@ class OauthTest(base.TestCase):
         def mockLinkedinRedirect(url, request):
             try:
                 params = urllib.parse.parse_qs(url.query)
-                self.assertEqual(
-                    params['client_id'],
-                    [providerInfo['client_id']['value']])
-                self.assertRegexpMatches(
-                    params['redirect_uri'][0],
-                    providerInfo['allowed_callback_re'])
+                self.assertEqual(params['client_id'], [providerInfo['client_id']['value']])
+                six.assertRegex(
+                    self, params['redirect_uri'][0], providerInfo['allowed_callback_re'])
             except (KeyError, AssertionError) as e:
                 return {
                     'status_code': 200,
@@ -825,12 +797,9 @@ class OauthTest(base.TestCase):
                     })
                 }
             try:
+                self.assertEqual(params['response_type'], ['code'])
                 self.assertEqual(
-                    params['response_type'],
-                    ['code'])
-                self.assertEqual(
-                    params['scope'][0].split(' '),
-                    ['r_basicprofile', 'r_emailaddress'])
+                    params['scope'][0].split(' '), ['r_basicprofile', 'r_emailaddress'])
                 state = params['state'][0]
                 # Nothing to test for state, since provider doesn't care
             except (KeyError, AssertionError) as e:
@@ -841,14 +810,12 @@ class OauthTest(base.TestCase):
             else:
                 returnQuery = urllib.parse.urlencode({
                     'state': state,
-                    'code':
-                        providerInfo['accounts'][self.accountType]['auth_code']
+                    'code': providerInfo['accounts'][self.accountType]['auth_code']
                 })
             return {
                 'status_code': 302,
                 'headers': {
-                    'Location': '%s?%s' % (params['redirect_uri'][0],
-                                           returnQuery)
+                    'Location': '%s?%s' % (params['redirect_uri'][0], returnQuery)
                 }
             }
 
@@ -856,23 +823,18 @@ class OauthTest(base.TestCase):
                           path='^/uas/oauth2/accessToken$', method='POST')
         def mockLinkedinToken(url, request):
             try:
-                self.assertEqual(request.headers['Content-Type'],
-                                 'application/x-www-form-urlencoded')
-                params = urllib.parse.parse_qs(request.body)
-                self.assertEqual(
-                    params['grant_type'],
-                    ['authorization_code'])
                 self.assertEqual(
-                    params['client_id'],
-                    [providerInfo['client_id']['value']])
+                    request.headers['Content-Type'], 'application/x-www-form-urlencoded')
+                params = urllib.parse.parse_qs(request.body)
+                self.assertEqual(params['grant_type'], ['authorization_code'])
+                self.assertEqual(params['client_id'], [providerInfo['client_id']['value']])
                 for account in six.viewvalues(providerInfo['accounts']):
                     if account['auth_code'] == params['code'][0]:
                         break
                 else:
                     self.fail()
-                self.assertRegexpMatches(
-                    params['redirect_uri'][0],
-                    providerInfo['allowed_callback_re'])
+                six.assertRegex(
+                    self, params['redirect_uri'][0], providerInfo['allowed_callback_re'])
             except (KeyError, AssertionError) as e:
                 return {
                     'status_code': 400,
@@ -882,9 +844,7 @@ class OauthTest(base.TestCase):
                     })
                 }
             try:
-                self.assertEqual(
-                    params['client_secret'],
-                    [providerInfo['client_secret']['value']])
+                self.assertEqual(params['client_secret'], [providerInfo['client_secret']['value']])
             except (KeyError, AssertionError) as e:
                 return {
                     'status_code': 401,
@@ -903,8 +863,7 @@ class OauthTest(base.TestCase):
         def mockLinkedinApi(url, request):
             try:
                 for account in six.viewvalues(providerInfo['accounts']):
-                    if 'Bearer %s' % account['access_token'] == \\
-                            request.headers['Authorization']:
+                    if 'Bearer %s' % account['access_token'] == request.headers['Authorization']:
                         break
                 else:
                     self.fail()
@@ -943,7 +902,7 @@ class OauthTest(base.TestCase):
             mockLinkedinRedirect,
             mockLinkedinToken,
             mockLinkedinApi,
-            # Must keep "mockOtherRequest" last
+            # Must keep 'mockOtherRequest' last
             self.mockOtherRequest
         ):
             self._testOauth(providerInfo)
@@ -1007,9 +966,7 @@ class OauthTest(base.TestCase):
                 params = urllib.parse.parse_qs(url.query)
                 # Check redirect_uri first, so other errors can still redirect
                 redirectUri = params['redirect_uri'][0]
-                self.assertEqual(
-                    params['client_id'],
-                    [providerInfo['client_id']['value']])
+                self.assertEqual(params['client_id'], [providerInfo['client_id']['value']])
             except (KeyError, AssertionError) as e:
                 return {
                     'status_code': 404,
@@ -1018,14 +975,10 @@ class OauthTest(base.TestCase):
                     })
                 }
             try:
-                self.assertRegexpMatches(
-                    redirectUri,
-                    providerInfo['allowed_callback_re'])
+                six.assertRegex(self, redirectUri, providerInfo['allowed_callback_re'])
                 state = params['state'][0]
                 # Nothing to test for state, since provider doesn't care
-                self.assertEqual(
-                    params['scope'],
-                    ['account'])
+                self.assertEqual(params['scope'], ['account'])
             except (KeyError, AssertionError) as e:
                 returnQuery = urllib.parse.urlencode({
                     'error': repr(e),
@@ -1034,8 +987,7 @@ class OauthTest(base.TestCase):
             else:
                 returnQuery = urllib.parse.urlencode({
                     'state': state,
-                    'code':
-                        providerInfo['accounts'][self.accountType]['auth_code']
+                    'code': providerInfo['accounts'][self.accountType]['auth_code']
                 })
             return {
                 'status_code': 302,
@@ -1050,9 +1002,7 @@ class OauthTest(base.TestCase):
             try:
                 self.assertEqual(request.headers['Accept'], 'application/json')
                 params = urllib.parse.parse_qs(request.body)
-                self.assertEqual(
-                    params['grant_type'],
-                    ['authorization_code'])
+                self.assertEqual(params['grant_type'], ['authorization_code'])
             except (KeyError, AssertionError) as e:
                 return {
                     'status_code': 400,
@@ -1067,12 +1017,9 @@ class OauthTest(base.TestCase):
                         break
                 else:
                     self.fail()
-                self.assertEqual(
-                    params['client_secret'],
-                    [providerInfo['client_secret']['value']])
-                self.assertRegexpMatches(
-                    params['redirect_uri'][0],
-                    providerInfo['allowed_callback_re'])
+                self.assertEqual(params['client_secret'], [providerInfo['client_secret']['value']])
+                six.assertRegex(
+                    self, params['redirect_uri'][0], providerInfo['allowed_callback_re'])
             except (KeyError, AssertionError) as e:
                 returnBody = json.dumps({
                     'error': repr(e),
@@ -1097,8 +1044,7 @@ class OauthTest(base.TestCase):
         def mockBitbucketApiUser(url, request):
             try:
                 for account in six.viewvalues(providerInfo['accounts']):
-                    if 'Bearer %s' % account['access_token'] == \\
-                            request.headers['Authorization']:
+                    if 'Bearer %s' % account['access_token'] == request.headers['Authorization']:
                         break
                 else:
                     self.fail()
@@ -1110,14 +1056,14 @@ class OauthTest(base.TestCase):
                     })
                 }
             return json.dumps({
-                "created_on": "2011-12-20T16:34:07.132459+00:00",
-                "uuid": account['user']['oauth']['id'],
-                "location": "Santa Monica, CA",
-                "links": {},
-                "website": "https://tutorials.bitbucket.org/",
-                "username": account['user']['login'],
-                "display_name": '%s %s' % (account['user']['firstName'],
-                                           account['user']['lastName'])
+                'created_on': '2011-12-20T16:34:07.132459+00:00',
+                'uuid': account['user']['oauth']['id'],
+                'location': 'Santa Monica, CA',
+                'links': {},
+                'website': 'https://tutorials.bitbucket.org/',
+                'username': account['user']['login'],
+                'display_name': '%s %s' % (
+                    account['user']['firstName'], account['user']['lastName'])
             })
 
         @httmock.urlmatch(scheme='https', netloc='^api.bitbucket.org$',
@@ -1125,8 +1071,7 @@ class OauthTest(base.TestCase):
         def mockBitbucketApiEmail(url, request):
             try:
                 for account in six.viewvalues(providerInfo['accounts']):
-                    if 'Bearer %s' % account['access_token'] == \\
-                            request.headers['Authorization']:
+                    if 'Bearer %s' % account['access_token'] == request.headers['Authorization']:
                         break
                 else:
                     self.fail()
@@ -1138,15 +1083,15 @@ class OauthTest(base.TestCase):
                     })
                 }
             return json.dumps({
-                "page": 1,
-                "pagelen": 10,
-                "size": 1,
-                "values": [{
+                'page': 1,
+                'pagelen': 10,
+                'size': 1,
+                'values': [{
                     'is_primary': True,
                     'is_confirmed': True,
                     'email': account['user']['email'],
                     'links': {},
-                    "type": "email"
+                    'type': 'email'
                 }]
             })
 
@@ -1155,7 +1100,7 @@ class OauthTest(base.TestCase):
             mockBitbucketToken,
             mockBitbucketApiUser,
             mockBitbucketApiEmail,
-            # Must keep "mockOtherRequest" last
+            # Must keep 'mockOtherRequest' last
             self.mockOtherRequest
         ):
             self._testOauth(providerInfo)
diff --git a/plugins/oauth/server/providers/bitbucket.py b/plugins/oauth/server/providers/bitbucket.py
index cd8b84cbd..1d39430c2 100644
--- a/plugins/oauth/server/providers/bitbucket.py
+++ b/plugins/oauth/server/providers/bitbucket.py
@@ -110,10 +110,8 @@ class Bitbucket(ProviderBase):
 
         login = resp.get('username', None)
 
-        names = resp.get('display_name', '').split()
-        firstName = names[0] if names else ''
-        lastName = names[-1] if len(names) > 1 else ''
+        names = (resp.get('display_name') or login).split()
+        firstName, lastName = names[0], names[-1]
 
-        user = self._createOrReuseUser(oauthId, email, firstName, lastName,
-                                       login)
+        user = self._createOrReuseUser(oauthId, email, firstName, lastName, login)
         return user
diff --git a/plugins/oauth/server/providers/github.py b/plugins/oauth/server/providers/github.py
index ea3f08c7b..c27a06da1 100644
--- a/plugins/oauth/server/providers/github.py
+++ b/plugins/oauth/server/providers/github.py
@@ -84,8 +84,7 @@ class GitHub(ProviderBase):
         # Get user's email address
         # In the unlikely case that a user has more than 30 email addresses,
         # this HTTP request might have to be made multiple times with pagination
-        resp = self._getJson(method='GET', url=self._API_EMAILS_URL,
-                             headers=headers)
+        resp = self._getJson(method='GET', url=self._API_EMAILS_URL, headers=headers)
         emails = [
             email.get('email')
             for email in resp
@@ -98,18 +97,13 @@ class GitHub(ProviderBase):
         email = emails[0]
 
         # Get user's OAuth2 ID, login, and name
-        resp = self._getJson(method='GET', url=self._API_USER_URL,
-                             headers=headers)
+        resp = self._getJson(method='GET', url=self._API_USER_URL, headers=headers)
         oauthId = resp.get('id')
         if not oauthId:
             raise RestException('GitHub did not return a user ID.', code=502)
 
-        login = resp.get('login', None)
+        login = resp.get('login', '')
+        names = (resp.get('name') or login).split()
+        firstName, lastName = names[0], names[-1]
 
-        names = resp.get('name', '').split()
-        firstName = names[0] if names else ''
-        lastName = names[-1] if len(names) > 1 else ''
-
-        user = self._createOrReuseUser(oauthId, email, firstName, lastName,
-                                       login)
-        return user
+        return self._createOrReuseUser(oauthId, email, firstName, lastName, login)
diff --git a/tests/base.py b/tests/base.py
index cb6b31a25..206d53b4a 100644
--- a/tests/base.py
+++ b/tests/base.py
@@ -454,6 +454,7 @@ class TestCase(unittest.TestCase, model_importer.ModelImporter):
             try:
                 response.json = json.loads(body)
             except Exception:
+                print(url)
                 print(body)
                 raise AssertionError('Did not receive JSON response')