Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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