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.

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

App Check: optimize auto-refresh #8232

Merged
merged 51 commits into from
Jun 11, 2021
Merged

App Check: optimize auto-refresh #8232

merged 51 commits into from
Jun 11, 2021

Conversation

maksymmalyhin
Copy link
Contributor

@maksymmalyhin maksymmalyhin commented Jun 10, 2021

  • b/179721239: auto-refresh App Check token earlier than expiration to minimize probability of a request being blocked on the token refresh (which is a relatively expensive operation)

maksymmalyhin and others added 30 commits April 28, 2021 16:54
* App Attest provider: attestation sequence (#761)

* App Attest draft WIP

* FIRAppAttestProvider initializers

* ./scripts/style.sh

* FIRAppAttestProvider implementation draft

* Basic FIRAppAttestProviderTests and fixes

* style

* testGetTokenWhenAppAttestIsNotSupported

* More FIRAppAttestProviderTests

* Cleanup

* Remove unused file

* Availability annotations on DCAppAttestService category.

* Guard FIRAppAttestProvider with #if TARGET_OS_IOS

* Formatting

* Fix SPM

* app_check.yaml: Add diagnostics SPM builds

* fix yaml

* Fix Firebase-Package scheme bad merge

* Fix typo

* FIRAppAttestProvider: hide default init

* FIRAppAttestKeyIDStorage: methods placeholders

* Comments

* Fix updated block definition
* Implement FIRAppAttestKeyIDStorage

* Add FIRAppAttestKeyIDStorageTests

* Review [Draft]

* Style

* Docs updates

* Docs updates 2

* Review [Draft] 2

* Improve tests

* Improve test readability

* Improve test readability 2
* Handshake adjustments (WIP)

* Introduce FIRAppAttestProviderState

* WIP: calculate attestation state

* WIP: calculate attestation state 2

* formatting

* Comments and moving code around

* Fix init in tests

* Fix state calculation flow

* Cleanup state calculation and fix tests.

* Cleanup and fixes.

* Comments

* formatting

* Fix import

* Typo fixes and additional comments

* FIRAppAttestInitialHandshakeResponse API

* Cleanup state calculation using FBLPromiseAwait

* Cleanup

* style
* Update comments

* FIRAppAttestArtifactStorage implementation and tests

* Fix init

* API docs

* Clean up storage in tests

* Comments

* Disable Keychain dependent tests for SPM
* Initial implementation

* Parse response body for challenge and stub test cases

* Review [Draft]

* Avoid encoding challenge again

* Add tests

* Revert "Avoid encoding challenge again" and add TODO

This reverts commit 69eb00d.

* Document tests; Add test

* Tests: Add URL validation check

* Review
* App Attest provider API integration WIP

* update tests

* Draft attestation response parsing

* Attestation request draft

* style

* AppAttest Attestation API tests draft

* Error cases tests

* style

* Cleanup and API docs

* Merge fix

* Fix OCMock imports

* Fix nullability modifier

* Formatting

* comments
* calculatre sha256 of random challenge for attestation

* Test app adjustments

* cleanup

* use trailing closures in the test app
* Implement assertion exchange

* Tweak existing tests

* Add tests

* Rename JSON to better match gRPC  message

* Add HTTPBody helper

* Review

* Review 2

* Review 3
* App Attest assertion workflow draft

* send request

* assertion flow tests

* style
* Update artifact storage API and tests

* Artifact storage implementation update

* Save artifact for a key ID

* Style

* typos
* App Attest multiple get token method invocation tests

* Ensure a single App Attest handshake sequence at the time

* FIRAppCheckTests: get token request merging tests

* FIRAppCheck: Ensure a single get token operation at the time

* formatting

* Test new request after merged requests

* Release finished operation promise

* Style

* Typos

* typo

* Request merging tests for error cases

* formatting
* Remove/update outdated TODOs

* [WIP] Attestation rejection handling draft

* style

* retry tests draft

* reset key ID before retry

* Reset attestation

* test error and fixes

* style

* More details in the name

* Some debug logging

* style

* Use specific codes for log messages

* style
@maksymmalyhin maksymmalyhin changed the title Mm/appcheck refresh App Check: optimize auto-refresh Jun 10, 2021
@maksymmalyhin maksymmalyhin marked this pull request as ready for review June 10, 2021 23:06
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.

Deferring to @ncooke3 for approval

FirebaseAppCheck/CHANGELOG.md Show resolved Hide resolved
Copy link
Member

@ncooke3 ncooke3 left a comment

Choose a reason for hiding this comment

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

LGTM Added a few minor comments.

@maksymmalyhin
Copy link
Contributor Author

Would you like to take another look? Note, I'm enabling auto-merge, so once PR is approved it will be merged.

@maksymmalyhin maksymmalyhin enabled auto-merge (squash) June 11, 2021 19:44
@maksymmalyhin maksymmalyhin merged commit 2ec4dbd into master Jun 11, 2021
@maksymmalyhin maksymmalyhin deleted the mm/appcheck-refresh branch June 11, 2021 21:35
@ncooke3
Copy link
Member

ncooke3 commented Jun 11, 2021

Looked great! and the refresh logic was easy to follow 💯

@firebase firebase locked and limited conversation to collaborators Jul 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants