Skip to content

Conversation

colerogers
Copy link
Contributor

API Changes

const decoded = await firebase.getAuth()._verifyAuthBlockingToken(token, audience);

@colerogers colerogers requested a review from lahirumaramba April 7, 2022 16:17
Copy link
Member

@lahirumaramba lahirumaramba left a comment

Choose a reason for hiding this comment

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

Thank you, @colerogers !
Looks good so far. I left a few comments. Please run npm run api-extractor:local to fix the CI errors.

@colerogers colerogers marked this pull request as ready for review April 13, 2022 15:58
Copy link
Member

@lahirumaramba lahirumaramba left a comment

Choose a reason for hiding this comment

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

Thanks @colerogers ! Looks pretty good. Added a few more comments.

export function generateAuthBlockingToken(overrides?: object, claims?: object): string {
const options = _.assign({
audience: `https://us-central1-${projectId}.cloudfunctions.net/functionName`,
expiresIn: ONE_HOUR_IN_SECONDS,
Copy link
Member

Choose a reason for hiding this comment

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

Just confirming. Is 1 hour the intended expiry time or do we want a custom/shorter duration here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looked at the DD and changed it to ten minutes

.then((customToken) => {
return tokenVerifierSessionCookie._verifyAuthBlockingToken(customToken, false, undefined)
.should.eventually.be.rejectedWith(
'verifySessionCookie() expects a session cookie, but was given a custom token');
Copy link
Member

Choose a reason for hiding this comment

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

Does _verifyAuthBlockingToken() expects a session cookie here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh, I copied this test from verifyIdToken, removed it in the latest push


return tokenVerifierSessionCookie._verifyAuthBlockingToken(legacyCustomToken, false, undefined)
.should.eventually.be.rejectedWith(
'verifySessionCookie() expects a session cookie, but was given a legacy custom token');
Copy link
Member

Choose a reason for hiding this comment

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

Same here... why does _verifyAuthBlockingToken() expect a session cookie here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same - I copied this test from verifyIdToken, removed it in the latest push

Copy link
Member

@lahirumaramba lahirumaramba left a comment

Choose a reason for hiding this comment

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

LGTM!

@lahirumaramba lahirumaramba requested a review from prameshj April 19, 2022 18:16
@lahirumaramba
Copy link
Member

lahirumaramba commented Apr 19, 2022

Adding @Xiaoshouzi-gh for visibility

@lahirumaramba lahirumaramba added the release:stage Stage a release candidate label Apr 19, 2022
@lahirumaramba lahirumaramba removed the request for review from prameshj April 19, 2022 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:stage Stage a release candidate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants