Skip to content

Commit

Permalink
fix(auth): check if user disabled on check_revoked (#565)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
bojeil-google authored Aug 4, 2021
1 parent fb64981 commit 0e35c9a
Show file tree
Hide file tree
Showing 6 changed files with 133 additions and 6 deletions.
12 changes: 9 additions & 3 deletions firebase_admin/_auth_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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):
Expand Down Expand Up @@ -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))
9 changes: 9 additions & 0 deletions firebase_admin/_auth_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
15 changes: 12 additions & 3 deletions firebase_admin/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
'TokenSignError',
'UidAlreadyExistsError',
'UnexpectedResponseError',
'UserDisabledError',
'UserImportHash',
'UserImportResult',
'UserInfo',
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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)
Expand Down Expand Up @@ -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:
Expand All @@ -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


Expand Down
36 changes: 36 additions & 0 deletions integration/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
6 changes: 6 additions & 0 deletions snippets/auth/index.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
61 changes: 61 additions & 0 deletions tests/test_token_gen.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 0e35c9a

Please sign in to comment.