firebase / firebase-admin-go

Firebase Admin Go SDK

Apache License 2.0

1.13k stars 244 forks source link

feat(auth): Add token verification logic for emulator mode #419

Closed maku693 closed 3 years ago

maku693 commented 3 years ago

Discussion

409

This PR adds token verification with auth emulator for auth client, in addition to the changes introduced at #414.

maku693 commented 3 years ago

We should add integration tests for this PR, like Node.js SDK implementation. https://github.com/firebase/firebase-admin-node/pull/1148

But I'm not sure how much should we add tests for this feature, and how to setup CI. any suggestions?

maku693 commented 3 years ago

Thank you for reviewing! I've fixed the points you've commented.

robinmitra commented 3 years ago

Would be awesome to get this merged in if it all looks good, as this issue is breaking my local development flow.

maku693 commented 3 years ago

Hi @hiranya911 and @yuchenshi, do you have any plans to release this in the near future?

hiranya911 commented 3 years ago

I've merged the PR. It will be included in the next release (possibly in another week or so).

zkrzyzanowski commented 3 years ago

Hi all, Thank you to those that worked on this and got this merged in! Is there an ETA on when the release with this in it might happen? It'd be great to be able to use the admin sdk with the local emulator, as this is breaking some of my dev workflow at the moment. Any other suggestions would be appreciated if there's a workaround.

Thank you!

hiranya911 commented 3 years ago

Will try and get it released this week.

saulortega commented 1 year ago

Hi.

@maku693 @hiranya911

Why when using emulator it ALWAYS checks if token was revoked? It makes no sense to me.

maku693 commented 1 year ago

@saulortega

I just implemented similar logic based on the Node.js implementation as a reference, so I don't have an exact understanding of the reasons.

Looking at the Node.js implementation, using checkRevoked || c.isEmulator seems to more accurately replicate the original implementation, but since it's using ||, either option appears to be valid.