firebase-admin-go icon indicating copy to clipboard operation
firebase-admin-go copied to clipboard

feat: Add App Check token verification

Open bamnet opened this issue 3 years ago • 12 comments

Add an appcheck module which provides the ability to verify tokens, closing #483.

Returns a token if valid and error otherwise, similar to the implementation in Node SDK. PTAL and let me know what you think!

Sample usage:

client, err := app.AppCheck(context.Background())
if err != nil {
	log.Fatalf("error getting AppCheck client: %v\n", err)
}

token := "..."
_, err := client.VerifyToken(token)
if err != nil {
	log.Fatalf("error verifying token: %v\n", err)
	return
}

bamnet avatar Feb 06 '22 05:02 bamnet

Thanks for kicking off those workflows! I fixed up the lint and go1.16 errors.

The go1.12 error is tricky, it looks like I'll need to rewrite the JWKS library, the general purpose one requires crypto/ed25519 which wasn't introduced until 1.13 [ref].

Alternatively, we could bump go.mod from 1.11 => 1.13, but I'm not sure how critical go1.11/1.12 is to this library (perhaps an artifact of supporting all GAE standard platforms).

bamnet avatar Feb 09 '22 02:02 bamnet

@bamnet I'd be happy to make a Go 1.12 compatible branch or fork of github.com/MicahParks/keyfunc, if you'd like.

However, there would be at least a couple of drawbacks:

  • The github.com/golang-jwt/jwt project could not be used. Neither the /v3 nor /v4 module support Go 1.12.
    • github.com/form3tech-oss/jwt-go could be used, but that's archived.
  • The exported error types wouldn't be available for unwrapping. (Since that was introduced in 1.13.)

MicahParks avatar Feb 09 '22 15:02 MicahParks

Thanks for the offer @MicahParks! I am slowly resigning to the fact that the best solution here is likely to roll our own mini-JWT/JWKS implementation here. After locally patching keyfunc I realized the bigger golang-jwt problem dependency problem and, if we're doing out own JWTs, it's not that much work to add some very specific JWKS support.

There's some code I can likely extract from the existing auth package provided here, which already rolls it's own very specific JWT code.

bamnet avatar Feb 09 '22 15:02 bamnet

Hi @bamnet Thank you so much for your contribution! This is on my radar and I plan to review the code this week. Thank you for fixing the lint errors.

Re: Go 1.12 compatibility: We prefer to stay aligned with the Google Cloud Go clients and it seems like the minimum requirement is still Go 1.11. Let me discuss this with the team and get back to you.

In the meantime, I found this auth0 go module that seems to support jwks. I didn't check to see if it works on 1.11 though.

Writing our own key-fetcher also sounds like a reasonable approach... please refer to the current implementation in auth/token_verifier.go to see if we can reuse/refactor components from there.

We need to get the public API approved (an internal process) before merging this change. I will get that process started.

Thank you for your patience!

lahirumaramba avatar Feb 09 '22 18:02 lahirumaramba

@bamnet Thank you for your patience! We have decided to update the minimum Go version to 1.15. See #485. Once that is merged, we should be able to continue with this PR.

lahirumaramba avatar Mar 04 '22 22:03 lahirumaramba

Woohoo, thanks for the update! I'll stay tuned for the go version bump.

bamnet avatar Mar 05 '22 05:03 bamnet

Woohoo, thanks for the update! I'll stay tuned for the go version bump.

@bamnet changes are now in the dev branch. I have updated the CI workflows as part of the version bump. Please rebase this PR with the dev branch and that will trigger the CI tests again. Thank you!

lahirumaramba avatar Mar 07 '22 20:03 lahirumaramba

Done, PTAL / kick off workflows!

bamnet avatar Mar 09 '22 05:03 bamnet

FYI - I've re-based with the latest dev branch to pull in the latest releases.

bamnet avatar Apr 11 '22 03:04 bamnet

Thank you @bamnet! The next step is to complete the internal review process for the new public API. I will start the process later this week. Please note that this might take a few weeks to complete. Once the API is approved, we can merge this PR. I will track the progress here. Thanks again for your patience.

lahirumaramba avatar Apr 11 '22 16:04 lahirumaramba

Hi @lahirumaramba, I just wanted to check in and see if there's anything I can do to move the API process forward? Happy to help fill out any documentation, etc.

bamnet avatar Jun 14 '22 02:06 bamnet

What is the status of this PR? Can this be merged? We really need this, can I help in any way?

wesselvanderlinden avatar Aug 04 '22 12:08 wesselvanderlinden

@lahirumaramba any progress? :)

wesselvanderlinden avatar Oct 06 '22 15:10 wesselvanderlinden

FYI - I've rebased with the latest dev branch to pull in all the work from the past 6 months. PTAL.

bamnet avatar Oct 21 '22 03:10 bamnet

Thanks for all the feedback. Incorporated ~all of it, LMK what I missed.

bamnet avatar Oct 21 '22 20:10 bamnet

Hi @kevinthecheung could we get a review on the reference docs, please? Thank you!

lahirumaramba avatar Oct 21 '22 21:10 lahirumaramba

Thanks for the review @kevinthecheung!

@lahirumaramba back over to you?

bamnet avatar Oct 26 '22 18:10 bamnet

Thank you, @bamnet ! We will include this feature in the next release of Admin Go SDK.

lahirumaramba avatar Oct 26 '22 19:10 lahirumaramba