-
Notifications
You must be signed in to change notification settings - Fork 339
Revoke refresh token #123
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
Revoke refresh token #123
Changes from 8 commits
55420df
f9f1ba3
ab14dfe
cdfffa9
40daf4e
82e4ef3
8f0695c
883fec0
c1ff5ed
11920d8
13a513f
91c8b4b
b56197e
a7db23c
e2b0c80
fe0011b
70cef7c
64c2e93
4025957
5138956
44a6bef
639b425
2bd5ec5
2f4e156
589fd10
0293bb5
393c1ab
e4fa1e7
d52c49e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,18 @@ | ||
# Unreleased | ||
- | ||
### Token revokaction | ||
- [added] The ['verify_id_token(...)'](https://firebase.google.com/docs/reference/admin/python/firebase_admin.auth#verify_id_token) | ||
method can now take an additional parameter `check_revoked`, when `True`, | ||
a further check will be performed to see if the token has been revoked. | ||
- [added] A new method ['auth.revoke_refresh_tokens(uid)'](https://firebase.google.com/docs/reference/admin/python/firebase_admin.auth#revoke_refresh_tokens) | ||
has been added to invalidate all tokens issued before the current second. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ...issued to a user. |
||
- [added] A new property `tokens_valid_after_time` has been added to the ['UserRecord'](https://firebase.google.com/docs/reference/admin/python/firebase_admin.auth#userrecord) | ||
|
||
# v2.8.0 | ||
|
||
### Initialization | ||
|
||
- [added] The [`initialize_app()`](https://firebase.google.com/docs/reference/admin/node/admin#.initializeApp) | ||
- [added] The [`initialize_app()`](https://firebase.google.com/docs/reference/admin/python/firebase_admin#initialize_app) | ||
method can now be invoked without any arguments. This initializes an app | ||
using Google Application Default Credentials, and other | ||
options loaded from the `FIREBASE_CONFIG` environment variable. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -113,6 +113,20 @@ def validate_photo_url(cls, photo_url): | |
except Exception: | ||
raise ValueError('Malformed photo URL: "{0}".'.format(photo_url)) | ||
|
||
@classmethod | ||
def validate_tokens_valid_after_time(cls, valid_since): | ||
if not isinstance(valid_since, six.string_types) or not valid_since: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How come this is a string? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the other timestamps in the UserMetadata are also returned as quoted strings. (passwordUpdate is a float in the response, but all other timestamps are quoted.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this a string? It should just accept a timestamp. |
||
raise ValueError( | ||
'Invalid time string for: "{0}". Tokens Valid After Time must be a non-empty ' | ||
'string.'.format(valid_since)) | ||
try: | ||
int(valid_since) | ||
except ValueError: | ||
raise ValueError('Malformed Tokens Valid After Time: "{0}" ' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "Invalid tokens_valid_after_time value: {0}" should suffice here |
||
'must be an interger.'.format(valid_since)) | ||
except Exception: | ||
raise ValueError('Malformed Tokens Valid After Time: "{0}".'.format(valid_since)) | ||
|
||
@classmethod | ||
def validate_disabled(cls, disabled): | ||
if not isinstance(disabled, bool): | ||
|
@@ -184,6 +198,7 @@ class UserManager(object): | |
'password' : _Validator.validate_password, | ||
'phoneNumber' : _Validator.validate_phone, | ||
'photoUrl' : _Validator.validate_photo_url, | ||
'validSince' : _Validator.validate_tokens_valid_after_time, | ||
} | ||
|
||
_CREATE_USER_FIELDS = { | ||
|
@@ -206,6 +221,7 @@ class UserManager(object): | |
'password' : 'password', | ||
'disabled' : 'disableUser', | ||
'custom_claims' : 'customAttributes', | ||
'tokens_valid_after_time' : 'validSince', | ||
} | ||
|
||
_REMOVABLE_FIELDS = { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -76,7 +76,7 @@ def create_custom_token(uid, developer_claims=None, app=None): | |
return token_generator.create_custom_token(uid, developer_claims) | ||
|
||
|
||
def verify_id_token(id_token, app=None): | ||
def verify_id_token(id_token, app=None, check_revoked=False): | ||
"""Verifies the signature and data for the provided JWT. | ||
|
||
Accepts a signed token string, verifies that it is current, and issued | ||
|
@@ -85,17 +85,27 @@ def verify_id_token(id_token, app=None): | |
Args: | ||
id_token: A string of the encoded JWT. | ||
app: An App instance (optional). | ||
check_revoked: check whether the token was revoked(optional). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A boolean indicating whether the token has been revoked (optional). |
||
|
||
Returns: | ||
dict: A dictionary of key-value pairs parsed from the decoded JWT. | ||
|
||
Raises: | ||
ValueError: If the JWT was found to be invalid, or if the App was not | ||
initialized with a credentials.Certificate. | ||
AuthError: If check_token is requested and the token was revoked. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If check_revoked is requested... |
||
""" | ||
token_generator = _get_auth_service(app).token_generator | ||
return token_generator.verify_id_token(id_token) | ||
|
||
verified_claims = token_generator.verify_id_token(id_token) | ||
if check_revoked: | ||
user = get_user(verified_claims.get('uid'), app) | ||
if verified_claims.get('iat') < user.tokens_valid_after_time: | ||
raise AuthError('ID_TOKEN_REVOKED', 'The Firebase ID token has been revoked.') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lets define a module-level constant for this (_ID_TOKEN_REVOKED). |
||
return verified_claims | ||
|
||
def revoke_refresh_tokens(uid, app=None): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add docstring. Also update the docstring of |
||
user_manager = _get_auth_service(app).user_manager | ||
user_manager.update_user(uid, tokens_valid_after_time=str(int(time.time()))) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unnecessary wrapping in a str IMO There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It works without it, but this conforms to the other timestamps (the golang json tags also wrap this field in quotes.) |
||
|
||
def get_user(uid, app=None): | ||
"""Gets the user data corresponding to the specified user ID. | ||
|
@@ -430,6 +440,16 @@ def disabled(self): | |
""" | ||
return bool(self._data.get('disabled')) | ||
|
||
@property | ||
def tokens_valid_after_time(self): | ||
"""Returns the time before which tokens are invalid. | ||
|
||
Returns: | ||
int: Timestap in seconds since the epoch. All tokens issued before that time | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typo: Timestap |
||
are considered revoked. | ||
""" | ||
return int(self._data.get('validSince', 0)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We seem to return 0 by default.
|
||
|
||
@property | ||
def user_metadata(self): | ||
"""Returns additional metadata associated with this user. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,6 +48,9 @@ | |
|
||
MOCK_GET_USER_RESPONSE = testutils.resource('get_user.json') | ||
MOCK_LIST_USERS_RESPONSE = testutils.resource('list_users.json') | ||
_REVOKED_TOKENS_USER = json.loads(MOCK_GET_USER_RESPONSE) | ||
_REVOKED_TOKENS_USER['users'][0]['validSince'] = str(int(time.time())+1) | ||
MOCK_GET_USER_REVOKED_TOKENS_RESPONSE = json.dumps(_REVOKED_TOKENS_USER) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps introduce a helper function here:
|
||
|
||
|
||
class AuthFixture(object): | ||
|
@@ -60,14 +63,12 @@ def __init__(self, name=None): | |
def create_custom_token(self, *args): | ||
if self.app: | ||
return auth.create_custom_token(*args, app=self.app) | ||
else: | ||
return auth.create_custom_token(*args) | ||
return auth.create_custom_token(*args) | ||
|
||
def verify_id_token(self, *args): | ||
def verify_id_token(self, *args, **kwargs): | ||
if self.app: | ||
return auth.verify_id_token(*args, app=self.app) | ||
else: | ||
return auth.verify_id_token(*args) | ||
return auth.verify_id_token(*args, app=self.app, **kwargs) | ||
return auth.verify_id_token(*args, **kwargs) | ||
|
||
def setup_module(): | ||
firebase_admin.initialize_app(MOCK_CREDENTIAL) | ||
|
@@ -160,7 +161,6 @@ def get_id_token(payload_overrides=None, header_overrides=None): | |
|
||
TEST_ID_TOKEN = get_id_token() | ||
|
||
|
||
class TestCreateCustomToken(object): | ||
|
||
valid_args = { | ||
|
@@ -244,8 +244,47 @@ def test_valid_token(self, authtest, id_token): | |
assert claims['admin'] is True | ||
assert claims['uid'] == claims['sub'] | ||
|
||
@pytest.mark.parametrize('id_token', invalid_tokens.values(), | ||
ids=list(invalid_tokens)) | ||
@pytest.mark.parametrize('id_token', valid_tokens.values(), ids=list(valid_tokens)) | ||
def test_valid_token_check_revoked(self, user_mgt_app_for_verify_token, id_token): | ||
_instrument_user_manager(user_mgt_app_for_verify_token, 200, MOCK_GET_USER_RESPONSE) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also need a test case for when the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is MOCK_GET_USER_REVOKED_TOKENS_RESPONSE |
||
claims = auth.verify_id_token(id_token, app=user_mgt_app_for_verify_token, | ||
check_revoked=True) | ||
assert claims['admin'] is True | ||
assert claims['uid'] == claims['sub'] | ||
|
||
@pytest.mark.parametrize('id_token', valid_tokens.values(), ids=list(valid_tokens)) | ||
def test_revoked_token_check_revoked(self, user_mgt_app_for_verify_token, id_token): | ||
_instrument_user_manager(user_mgt_app_for_verify_token, 200, | ||
MOCK_GET_USER_REVOKED_TOKENS_RESPONSE) | ||
|
||
with pytest.raises(auth.AuthError) as excinfo: | ||
auth.verify_id_token(id_token, app=user_mgt_app_for_verify_token, | ||
check_revoked=True) | ||
|
||
assert excinfo.value.code == 'ID_TOKEN_REVOKED' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Check against the constant once you have it |
||
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_revoked_token_do_not_check_revoked(self, user_mgt_app_for_verify_token, id_token): | ||
_instrument_user_manager(user_mgt_app_for_verify_token, 200, | ||
MOCK_GET_USER_REVOKED_TOKENS_RESPONSE) | ||
claims = auth.verify_id_token(id_token, app=user_mgt_app_for_verify_token, | ||
check_revoked=False) | ||
assert claims['admin'] is True | ||
assert claims['uid'] == claims['sub'] | ||
|
||
def test_revoke_refresh_token(self, user_mgt_app): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add test case for setting this via There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the revoke in turn calls update_user, do we want to explicitly do this with update user? |
||
_, recorder = _instrument_user_manager(user_mgt_app, 200, '{"localId":"testuser"}') | ||
before_time = time.time() | ||
auth.revoke_refresh_tokens('testuser', app=user_mgt_app) | ||
after_time = time.time() | ||
|
||
request = json.loads(recorder[0].body.decode()) | ||
assert request['localId'] == 'testuser' | ||
assert int(request['validSince']) >= int(before_time) | ||
assert int(request['validSince']) <= int(after_time) | ||
|
||
@pytest.mark.parametrize('id_token', invalid_tokens.values(), ids=list(invalid_tokens)) | ||
def test_invalid_token(self, authtest, id_token): | ||
with pytest.raises(ValueError): | ||
authtest.verify_id_token(id_token) | ||
|
@@ -287,6 +326,13 @@ def user_mgt_app(): | |
yield app | ||
firebase_admin.delete_app(app) | ||
|
||
@pytest.fixture(scope='module') | ||
def user_mgt_app_for_verify_token(): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we just modify the existing |
||
app = firebase_admin.initialize_app(testutils.MockCredential(), name='userMgtVer', | ||
options={'projectId': 'mock-project-id'}) | ||
yield app | ||
firebase_admin.delete_app(app) | ||
|
||
def _instrument_user_manager(app, status, payload): | ||
auth_service = auth._get_auth_service(app) | ||
user_manager = auth_service.user_manager | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...method now accepts an optional
check_revoked
argument. When set toTrue
an additional check is performed to see...