Skip to content

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

Merged
bojeil-google merged 4 commits into
masterfrom
verify-token-check-disabled
Aug 4, 2021
Merged

bojeil-google merged 4 commits into
masterfrom
verify-token-check-disabled

Conversation

@bojeil-google

Copy link
Copy Markdown
Contributor

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.

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.

@lahirumaramba lahirumaramba left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! LGTM!

@bojeil-google

Copy link
Copy Markdown
Contributor Author

Thanks! LGTM!

Thanks for the quick review. we are planning to fix this in Node.js, Java and Golang too.

@lahirumaramba

Copy link
Copy Markdown
Member

Added @egilmorez for docs changes. Thanks!

Comment thread firebase_admin/_auth_client.py Resolved

@hiranya911 hiranya911 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks mostly good. Just had one suggestion about the base type for the new UserDisabledError class.

Comment thread firebase_admin/_auth_utils.py Resolved
Comment thread tests/test_token_gen.py Resolved

@hiranya911 hiranya911 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a suggestion.

Comment thread tests/test_token_gen.py Resolved

@egilmorez egilmorez left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doc strings look good, thanks!

@bojeil-google bojeil-google removed their assignment Aug 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants