Added option clockSkewInSeconds to allow setting clock_skew_in_seconds parameter for token verification

[fschaeck]

Adding the optional parameter clock_skew_in_seconds=60 to the call to google.oauth2.id_token.verify_token now allows for the token-issuing server's clock to be off by up to a minute without the token becoming invalid due to a 'issued-at-time' timestamp that is in the future.

Discussion

Issue #624

Testing

No additional tests required.

API Changes

n/a

[google-cla[bot]]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[DanielJerrehian]

Hi, I wanted to follow up on this issue and ask when it will be merged?

[fschaeck]

I changed the pull request a little. The parameter clock_skew_in_seconds for the Google token verification API call is no longer hard-coded with 60 seconds but can be set as an App option. If that option is not set, a default of 0 is used.

This makes all uses of firebase_admin work the same way as before. So the change won't introduce a change of behaviour for those existing applications. But users that need the clock skew setting can add an option to their App and overwrite the default of 0 that way.

I think that is the better way to introduce using the clock_skew_in_seconds parameter of the Google API.

[Nakachi-S]

Why is this PR not merge? I am facing this problem too.

[lahirumaramba]

Thank you @fschaeck for your contribution! The changes to App options require an internal API review and might take a while.

Hi, @prameshj, @renkelvin let me know your thoughts on this, I can initiate the internal changes. Thank you!

[Yudai-Saito]

@lahirumaramba @fschaeck Hello. I hope this PR will be merged because I had the same problem with the same bug.

By the way, we have a similar bug with this method.

auth.verify_session_cookie(session, check_revoked=True)

I solved it this way.

session_cookie_request = requests.Request()
CERTS_URL = "https://www.googleapis.com/identitytoolkit/v3/relyingparty/publicKeys"
id_token.verify_token(session_token=session, request=session_cookie_request, certs_url=CERTS_URL, clock_skew_in_seconds=10)

[ryanlinnane]

@fschaeck are you able to merge this now? Subscribing.

[smartexpert]

appreciate if this could be merged

[lahirumaramba]

Thank you for your patience everyone. We just completed the internal API review and we think a better approach would be to add a new optional clock_skew_seconds parameter to verify_id_token() and verify_session_cookie() APIs.

API:

def verify_id_token(id_token, app=None, check_revoked=False, clock_skew_seconds=0):

def verify_session_cookie(session_cookie, check_revoked=False, app=None, clock_skew_seconds=0):

Usage:

import firebase_admin
from firebase_admin import auth

app = firebase_admin.initialize_app()

decoded_token = auth.verify_id_token(id_token, clock_skew_seconds=60)
decoded_claims = auth.verify_session_cookie(session_cookie, check_revoked=True, clock_skew_seconds=10)

@fschaeck would you be able to make the changes? Thank you!

[barel-mishal]

Hi mean while this need to be changed what the work around? could not figure out yet how to skew my clock with node js

[ryanlinnane]

Hi mean while this need to be changed what the work around? could not figure out yet how to skew my clock with node js

Fork the fix and install the git repo with pip works for now.

[fschaeck]

@fschaeck would you be able to make the changes? Thank you!

@lahirumaramba

You do realize, that this means to change the signatures of methods TokenVerifier.verfify_id_token, TokenVerifier.verify_session_cookie and _JWTVerifier.verify from firebase_admin/_token_gen.py by adding the optional parameter clock_skew_seconds there as well, right?

Since TokenVerifier and _JWTVerifier are classes (doubly) abstracting the token verification and I was not able to assess, if there are any other implementations that might be used here, I chose the way of the additional app property instead of changing the signature of those methods, because other such abstracting classes may not understand the additional parameter and then apps using this optional parameter all the sudden stop working.

If you okay THOSE change as well, I will change this pull request right the way!

[beef-stek]

verify_token

Can you please go into more detail on your fix? What class is id_token? Thanks.

This bug has been annoying as it would happen randomly. Please merge this as soon as possible! THANKS GOOGLE.

[fschaeck]

@lahirumaramba Any news here?

[lahirumaramba]

You do realize, that this means to change the signatures of methods TokenVerifier.verfify_id_token, TokenVerifier.verify_session_cookie and _JWTVerifier.verify from firebase_admin/_token_gen.py by adding the optional parameter clock_skew_seconds there as well, right?

Since TokenVerifier and _JWTVerifier are classes (doubly) abstracting the token verification and I was not able to assess, if there are any other implementations that might be used here, I chose the way of the additional app property instead of changing the signature of those methods, because other such abstracting classes may not understand the additional parameter and then apps using this optional parameter all the sudden stop working.

If you okay THOSE change as well, I will change this pull request right the way!

Hi @fschaeck thank you for being so thoughtful about this change. Since _JWTVerifier and TokenVerifier are internal classes and not exposed in the public API surface, we are safe to change the method signature by adding the optional parameter clock_skew_seconds. Please add unit tests (and integration tests) where possible. Thanks!

[karkir0003]

@lahirumaramba when will this pr get reviewed/approved to be merged? this change would be so nice to have for firebase

[theobouwman]

Any updates on this? Or how to avoid it?

[jonathanedey]

Addressed in #714