fix(auth): check if user disabled on check_revoked (#565)

* fix(auth): check if user disabled on check_revoked

When `verify_session_cookie` or `verify_id_token` is called with
`check_revoked` set to `True` we should also check if the user is
disabled.

If disabled the `UserDisabledError` is raised.

Author: bojeil-google

firebase_admin/_auth_client.py

@@ -100,7 +100,8 @@ def verify_id_token(self, id_token, check_revoked=False):
 
         Args:
             id_token: A string of the encoded JWT.
-            check_revoked: Boolean, If true, checks whether the token has been revoked (optional).
+            check_revoked: Boolean, If true, checks whether the token has been revoked or
+                the user disabled (optional).
 
         Returns:
             dict: A dictionary of key-value pairs parsed from the decoded JWT.
@@ -115,6 +116,8 @@ def verify_id_token(self, id_token, check_revoked=False):
                 this ``Client`` instance.
             CertificateFetchError: If an error occurs while fetching the public key certificates
                 required to verify the ID token.
+            UserDisabledError: If ``check_revoked`` is ``True`` and the corresponding user
+                record is disabled.
         """
         if not isinstance(check_revoked, bool):
             # guard against accidental wrong assignment.
@@ -129,7 +132,8 @@ def verify_id_token(self, id_token, check_revoked=False):
                     'Invalid tenant ID: {0}'.format(token_tenant_id))
 
         if check_revoked:
-            self._check_jwt_revoked(verified_claims, _token_gen.RevokedIdTokenError, 'ID token')
+            self._check_jwt_revoked_or_disabled(
+                verified_claims, _token_gen.RevokedIdTokenError, 'ID token')
         return verified_claims
 
     def revoke_refresh_tokens(self, uid):
@@ -720,7 +724,9 @@ def list_saml_provider_configs(
         """
         return self._provider_manager.list_saml_provider_configs(page_token, max_results)
 
-    def _check_jwt_revoked(self, verified_claims, exc_type, label):
+    def _check_jwt_revoked_or_disabled(self, verified_claims, exc_type, label):
         user = self.get_user(verified_claims.get('uid'))
+        if user.disabled:
+            raise _auth_utils.UserDisabledError('The user record is disabled.')
         if verified_claims.get('iat') * 1000 < user.tokens_valid_after_timestamp:
             raise exc_type('The Firebase {0} has been revoked.'.format(label))

firebase_admin/_auth_utils.py

@@ -385,6 +385,15 @@ def __init__(self, message, cause=None, http_response=None):
         exceptions.NotFoundError.__init__(self, message, cause, http_response)
 
 
+class UserDisabledError(exceptions.InvalidArgumentError):
+    """An operation failed due to a user record being disabled."""
+
+    default_message = 'The user record is disabled'
+
+    def __init__(self, message, cause=None, http_response=None):
+        exceptions.InvalidArgumentError.__init__(self, message, cause, http_response)
+
+
 _CODE_TO_EXC_TYPE = {
     'CONFIGURATION_NOT_FOUND': ConfigurationNotFoundError,
     'DUPLICATE_EMAIL': EmailAlreadyExistsError,

firebase_admin/auth.py

@@ -62,6 +62,7 @@
     'TokenSignError',
     'UidAlreadyExistsError',
     'UnexpectedResponseError',
+    'UserDisabledError',
     'UserImportHash',
     'UserImportResult',
     'UserInfo',
@@ -135,6 +136,7 @@
 TokenSignError = _token_gen.TokenSignError
 UidAlreadyExistsError = _auth_utils.UidAlreadyExistsError
 UnexpectedResponseError = _auth_utils.UnexpectedResponseError
+UserDisabledError = _auth_utils.UserDisabledError
 UserImportHash = _user_import.UserImportHash
 UserImportResult = _user_import.UserImportResult
 UserInfo = _user_mgt.UserInfo
@@ -198,7 +200,8 @@ def verify_id_token(id_token, app=None, check_revoked=False):
     Args:
         id_token: A string of the encoded JWT.
         app: An App instance (optional).
-        check_revoked: Boolean, If true, checks whether the token has been revoked (optional).
+        check_revoked: Boolean, If true, checks whether the token has been revoked or
+            the user disabled (optional).
 
     Returns:
         dict: A dictionary of key-value pairs parsed from the decoded JWT.
@@ -210,6 +213,8 @@ def verify_id_token(id_token, app=None, check_revoked=False):
         RevokedIdTokenError: If ``check_revoked`` is ``True`` and the ID token has been revoked.
         CertificateFetchError: If an error occurs while fetching the public key certificates
             required to verify the ID token.
+        UserDisabledError: If ``check_revoked`` is ``True`` and the corresponding user
+            record is disabled.
     """
     client = _get_client(app)
     return client.verify_id_token(id_token, check_revoked=check_revoked)
@@ -246,7 +251,8 @@ def verify_session_cookie(session_cookie, check_revoked=False, app=None):
 
     Args:
         session_cookie: A session cookie string to verify.
-        check_revoked: Boolean, if true, checks whether the cookie has been revoked (optional).
+        check_revoked: Boolean, if true, checks whether the cookie has been revoked or the
+            user disabled (optional).
         app: An App instance (optional).
 
     Returns:
@@ -259,12 +265,15 @@ def verify_session_cookie(session_cookie, check_revoked=False, app=None):
         RevokedSessionCookieError: If ``check_revoked`` is ``True`` and the cookie has been revoked.
         CertificateFetchError: If an error occurs while fetching the public key certificates
             required to verify the session cookie.
+        UserDisabledError: If ``check_revoked`` is ``True`` and the corresponding user
+            record is disabled.
     """
     client = _get_client(app)
     # pylint: disable=protected-access
     verified_claims = client._token_verifier.verify_session_cookie(session_cookie)
     if check_revoked:
-        client._check_jwt_revoked(verified_claims, RevokedSessionCookieError, 'session cookie')
+        client._check_jwt_revoked_or_disabled(
+            verified_claims, RevokedSessionCookieError, 'session cookie')
     return verified_claims
 
 

integration/test_auth.py

@@ -569,6 +569,24 @@ def test_verify_id_token_revoked(new_user, api_key):
     claims = auth.verify_id_token(id_token, check_revoked=True)
     assert claims['iat'] * 1000 >= user.tokens_valid_after_timestamp
 
+def test_verify_id_token_disabled(new_user, api_key):
+    custom_token = auth.create_custom_token(new_user.uid)
+    id_token = _sign_in(custom_token, api_key)
+    claims = auth.verify_id_token(id_token, check_revoked=True)
+
+    # Disable the user record.
+    auth.update_user(new_user.uid, disabled=True)
+    # Verify the ID token without checking revocation. This should
+    # not raise.
+    claims = auth.verify_id_token(id_token, check_revoked=False)
+    assert claims['sub'] == new_user.uid
+
+    # Verify the ID token while checking revocation. This should
+    # raise an exception.
+    with pytest.raises(auth.UserDisabledError) as excinfo:
+        auth.verify_id_token(id_token, check_revoked=True)
+    assert str(excinfo.value) == 'The user record is disabled.'
+
 def test_verify_session_cookie_revoked(new_user, api_key):
     custom_token = auth.create_custom_token(new_user.uid)
     id_token = _sign_in(custom_token, api_key)
@@ -591,6 +609,24 @@ def test_verify_session_cookie_revoked(new_user, api_key):
     claims = auth.verify_session_cookie(session_cookie, check_revoked=True)
     assert claims['iat'] * 1000 >= user.tokens_valid_after_timestamp
 
+def test_verify_session_cookie_disabled(new_user, api_key):
+    custom_token = auth.create_custom_token(new_user.uid)
+    id_token = _sign_in(custom_token, api_key)
+    session_cookie = auth.create_session_cookie(id_token, expires_in=datetime.timedelta(days=1))
+
+    # Disable the user record.
+    auth.update_user(new_user.uid, disabled=True)
+    # Verify the session cookie without checking revocation. This should
+    # not raise.
+    claims = auth.verify_session_cookie(session_cookie, check_revoked=False)
+    assert claims['sub'] == new_user.uid
+
+    # Verify the session cookie while checking revocation. This should
+    # raise an exception.
+    with pytest.raises(auth.UserDisabledError) as excinfo:
+        auth.verify_session_cookie(session_cookie, check_revoked=True)
+    assert str(excinfo.value) == 'The user record is disabled.'
+
 def test_import_users():
     uid, email = _random_id()
     user = auth.ImportUserRecord(uid=uid, email=email)

snippets/auth/index.py

@@ -150,6 +150,9 @@ def verify_token_uid_check_revoke(id_token):
     except auth.RevokedIdTokenError:
         # Token revoked, inform the user to reauthenticate or signOut().
         pass
+    except auth.UserDisabledError:
+        # Token belongs to a disabled user record.
+        pass
     except auth.InvalidIdTokenError:
         # Token is invalid
         pass
@@ -1027,6 +1030,9 @@ def verify_id_token_and_check_revoked_tenant(tenant_client, id_token):
     except auth.RevokedIdTokenError:
         # Token revoked, inform the user to reauthenticate or signOut().
         pass
+    except auth.UserDisabledError:
+        # Token belongs to a disabled user record.
+        pass
     except auth.InvalidIdTokenError:
         # Token is invalid
         pass

tests/test_token_gen.py

@@ -208,6 +208,19 @@ def revoked_tokens():
     mock_user['users'][0]['validSince'] = str(int(time.time())+100)
     return json.dumps(mock_user)
 
+@pytest.fixture(scope='module')
+def user_disabled():
+    mock_user = json.loads(testutils.resource('get_user.json'))
+    mock_user['users'][0]['disabled'] = True
+    return json.dumps(mock_user)
+
+@pytest.fixture(scope='module')
+def user_disabled_and_revoked():
+    mock_user = json.loads(testutils.resource('get_user.json'))
+    mock_user['users'][0]['disabled'] = True
+    mock_user['users'][0]['validSince'] = str(int(time.time())+100)
+    return json.dumps(mock_user)
+
 
 class TestCreateCustomToken:
 
@@ -471,6 +484,23 @@ def test_revoked_token_check_revoked(self, user_mgt_app, revoked_tokens, id_toke
             auth.verify_id_token(id_token, app=user_mgt_app, check_revoked=True)
         assert str(excinfo.value) == 'The Firebase ID token has been revoked.'
 
+    @pytest.mark.parametrize('id_token', valid_tokens.values(), ids=list(valid_tokens))
+    def test_disabled_user_check_revoked(self, user_mgt_app, user_disabled, id_token):
+        _overwrite_cert_request(user_mgt_app, MOCK_REQUEST)
+        _instrument_user_manager(user_mgt_app, 200, user_disabled)
+        with pytest.raises(auth.UserDisabledError) as excinfo:
+            auth.verify_id_token(id_token, app=user_mgt_app, check_revoked=True)
+        assert str(excinfo.value) == 'The user record is disabled.'
+
+    @pytest.mark.parametrize('id_token', valid_tokens.values(), ids=list(valid_tokens))
+    def test_check_disabled_before_revoked(
+            self, user_mgt_app, user_disabled_and_revoked, id_token):
+        _overwrite_cert_request(user_mgt_app, MOCK_REQUEST)
+        _instrument_user_manager(user_mgt_app, 200, user_disabled_and_revoked)
+        with pytest.raises(auth.UserDisabledError) as excinfo:
+            auth.verify_id_token(id_token, app=user_mgt_app, check_revoked=True)
+        assert str(excinfo.value) == 'The user record is disabled.'
+
     @pytest.mark.parametrize('arg', INVALID_BOOLS)
     def test_invalid_check_revoked(self, user_mgt_app, arg):
         _overwrite_cert_request(user_mgt_app, MOCK_REQUEST)
@@ -485,6 +515,14 @@ def test_revoked_token_do_not_check_revoked(self, user_mgt_app, revoked_tokens,
         assert claims['admin'] is True
         assert claims['uid'] == claims['sub']
 
+    @pytest.mark.parametrize('id_token', valid_tokens.values(), ids=list(valid_tokens))
+    def test_disabled_user_do_not_check_revoked(self, user_mgt_app, user_disabled, id_token):
+        _overwrite_cert_request(user_mgt_app, MOCK_REQUEST)
+        _instrument_user_manager(user_mgt_app, 200, user_disabled)
+        claims = auth.verify_id_token(id_token, app=user_mgt_app, check_revoked=False)
+        assert claims['admin'] is True
+        assert claims['uid'] == claims['sub']
+
     @pytest.mark.parametrize('id_token', INVALID_JWT_ARGS.values(), ids=list(INVALID_JWT_ARGS))
     def test_invalid_arg(self, user_mgt_app, id_token):
         _overwrite_cert_request(user_mgt_app, MOCK_REQUEST)
@@ -622,6 +660,29 @@ def test_revoked_cookie_does_not_check_revoked(self, user_mgt_app, revoked_token
         _instrument_user_manager(user_mgt_app, 200, revoked_tokens)
         self._assert_valid_cookie(cookie, app=user_mgt_app, check_revoked=False)
 
+    @pytest.mark.parametrize('cookie', valid_cookies.values(), ids=list(valid_cookies))
+    def test_disabled_user_check_revoked(self, user_mgt_app, user_disabled, cookie):
+        _overwrite_cert_request(user_mgt_app, MOCK_REQUEST)
+        _instrument_user_manager(user_mgt_app, 200, user_disabled)
+        with pytest.raises(auth.UserDisabledError) as excinfo:
+            auth.verify_session_cookie(cookie, app=user_mgt_app, check_revoked=True)
+        assert str(excinfo.value) == 'The user record is disabled.'
+
+    @pytest.mark.parametrize('cookie', valid_cookies.values(), ids=list(valid_cookies))
+    def test_check_disabled_before_revoked(
+            self, user_mgt_app, user_disabled_and_revoked, cookie):
+        _overwrite_cert_request(user_mgt_app, MOCK_REQUEST)
+        _instrument_user_manager(user_mgt_app, 200, user_disabled_and_revoked)
+        with pytest.raises(auth.UserDisabledError) as excinfo:
+            auth.verify_session_cookie(cookie, app=user_mgt_app, check_revoked=True)
+        assert str(excinfo.value) == 'The user record is disabled.'
+
+    @pytest.mark.parametrize('cookie', valid_cookies.values(), ids=list(valid_cookies))
+    def test_disabled_user_does_not_check_revoked(self, user_mgt_app, user_disabled, cookie):
+        _overwrite_cert_request(user_mgt_app, MOCK_REQUEST)
+        _instrument_user_manager(user_mgt_app, 200, user_disabled)
+        self._assert_valid_cookie(cookie, app=user_mgt_app, check_revoked=False)
+
     @pytest.mark.parametrize('cookie', INVALID_JWT_ARGS.values(), ids=list(INVALID_JWT_ARGS))
     def test_invalid_args(self, user_mgt_app, cookie):
         _overwrite_cert_request(user_mgt_app, MOCK_REQUEST)