Skip to content

Add generation of limited-use tokens with a TTL of 5 mins#11

Merged
andrewheard merged 10 commits intomainfrom
ah/fac-1-use-providers
Aug 22, 2023
Merged

Add generation of limited-use tokens with a TTL of 5 mins#11
andrewheard merged 10 commits intomainfrom
ah/fac-1-use-providers

Conversation

@andrewheard
Copy link
Collaborator

@andrewheard andrewheard commented Aug 14, 2023

Added support for generating App Check tokens with a TTL of 5 minutes when requesting limited-use tokens. This extends upon firebase/firebase-ios-sdk#11086, which added the ability to request limited-use tokens that aren't cached.

This is implemented by adding a getLimitedUseTokenWithCompletion: method to the GACAppCheckProvider protocol. When requesting a limited-use token through the public API limitedUseTokenWithCompletion:, the new GACAppCheckProvider method getLimitedUseTokenWithCompletion: will be called instead of the existing getTokenWithCompletion: method.

TODOs:

  • Add getLimitedUseTokenWithCompletion: to GACAppCheckProvider
  • Call getLimitedUseTokenWithCompletion: from GACAppCheck in limitedUseTokenWithCompletion:
  • Implement getLimitedUseTokenWithCompletion: in providers:
    • GACAppAttestProvider
    • GACAppCheckDebugProvider
    • GACDeviceCheckProvider
  • Update existing tests to ensure existing behaviour is unchanged
  • Add new tests to check limited-use methods set limitedUse as YES

Comment on lines -215 to -221
if (self.ongoingGetTokenOperation == nil) {
// Kick off a new handshake sequence only when there is not an ongoing
// handshake to avoid race conditions.
self.ongoingGetTokenOperation =
[self createGetTokenSequenceWithBackoffPromise]

// Release the ongoing operation promise on completion.
.then(^GACAppCheckToken *(GACAppCheckToken *token) {
self.ongoingGetTokenOperation = nil;
return token;
})
.recover(^NSError *(NSError *error) {
self.ongoingGetTokenOperation = nil;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Previously, self.ongoingGetTokenOperation was read on the self.queue dispatch queue but mutated on the main thread in .then(...) and .recover(...) (the main thread is the default when a queue is not specified) -- this could potentially result in a synchronization issue.

@andrewheard andrewheard force-pushed the ah/fac-1-use-providers branch from 70dede4 to 86321c3 Compare August 21, 2023 20:36
@andrewheard andrewheard marked this pull request as ready for review August 21, 2023 21:10
Copy link
Member

@paulb777 paulb777 left a comment

Choose a reason for hiding this comment

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

Wow! so much code for parameter propagation.

@andrewheard
Copy link
Collaborator Author

Wow! so much code for parameter propagation.

@paulb777 I know, right! That's why I took the shortcut in 3bbea5f until now 😛

@andrewheard
Copy link
Collaborator Author

cc: @mdmathias Once this PR is merged I can push a new version of the AppCheckCore pod and then you can remove this line https://github.com/google/GoogleSignIn-iOS/blob/18b5fbe51adcef2d38d539f1693adbf15875bb1c/GoogleSignIn/Sources/GIDAppCheck/Implementations/GIDAppCheck.m#L165 (since you're already calling getLimitedUseTokenWithCompletion: it should behave exactly the same as before).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants