Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(auth): check if the user is disabled on checkRevoked=true for verifyIdToken and verifySessionCookie #585

Merged
merged 2 commits into from
Aug 9, 2021
Merged

fix(auth): check if the user is disabled on checkRevoked=true for verifyIdToken and verifySessionCookie #585

merged 2 commits into from
Aug 9, 2021

Conversation

lsirac
Copy link
Contributor

@lsirac lsirac commented Aug 6, 2021 •
edited
Loading

When verifyIdToken or verifySessionCookie is called with checkRevoked set to true we also check if the user is disabled. If so, a FirebaseAuthException with the new AuthErrorCode USER_DISABLED is thrown.

hiranya911
hiranya911 suggested changes Aug 6, 2021
View reviewed changes
Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

Looks pretty good. Just a few minor nits pointed out.

src/main/java/com/google/firebase/auth/RevocationCheckDecorator.java Outdated Show resolved Hide resolved
src/test/java/com/google/firebase/auth/FirebaseAuthIT.java Outdated Show resolved Hide resolved
src/test/java/com/google/firebase/auth/FirebaseAuthIT.java Outdated Show resolved Hide resolved
src/test/java/com/google/firebase/auth/FirebaseAuthIT.java Outdated Show resolved Hide resolved
src/test/java/com/google/firebase/auth/FirebaseAuthTest.java Outdated Show resolved Hide resolved
@lsirac lsirac requested a review from hiranya911 August 7, 2021 23:13
hiranya911
hiranya911 approved these changes Aug 9, 2021
View reviewed changes
Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

Thanks. LGTM 👍

@hiranya911
Copy link
Contributor

@egilmorez PTAL the javadoc updates in AbstractFirebaseAuth and AuthErrorCode.

egilmorez
egilmorez reviewed Aug 9, 2021
View reviewed changes
Copy link
Contributor

@egilmorez egilmorez left a comment

Choose a reason for hiding this comment

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

LG, though a couple of comments don't fully make sense to me -- possibly my problem but please clarify if you can, thanks!

src/main/java/com/google/firebase/auth/AbstractFirebaseAuth.java Show resolved Hide resolved
src/main/java/com/google/firebase/auth/AbstractFirebaseAuth.java Show resolved Hide resolved
egilmorez
egilmorez approved these changes Aug 9, 2021
View reviewed changes
Copy link
Contributor

@egilmorez egilmorez left a comment

Choose a reason for hiding this comment

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

Thanks!

@lsirac lsirac merged commit 2d50628 into master Aug 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@egilmorez egilmorez egilmorez approved these changes

@hiranya911 hiranya911 hiranya911 approved these changes

Assignees

@egilmorez egilmorez

@lsirac lsirac

Labels
release-note
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lsirac @hiranya911 @egilmorez