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

Add limited use token to FirAppeCheck Interface #11086

Merged
merged 21 commits into from
Apr 17, 2023
Merged

Add limited use token to FirAppeCheck Interface #11086

merged 21 commits into from
Apr 17, 2023

Conversation

aiwenisevan
Copy link
Contributor

@aiwenisevan aiwenisevan commented Apr 6, 2023

Discussion:
Added LimitedUseTokenWithCompletion to FIRAppCheck interface, and added unit tests accordingly.
Added function in FIRAppCheckTestApp to test limited-use token is obtained.

Tests:
Passed all unit tests and tested with AppCheckTestApp

Api Changes:
refer to this approved api proposal

@aiwenisevan aiwenisevan marked this pull request as ready for review April 13, 2023 21:35
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.

Thanks @aiwenisevan! Leaving a first round pass review. Mainly just documentation nits.

@aiwenisevan
Copy link
Contributor Author

Not able to use scripts/style.sh to format AppDelegate.swift file, and thus failing the style checks. Also tried swift-format to format the file, doesn't work either. Does anyone know how I can fix this? Thanks!

@aiwenisevan aiwenisevan changed the base branch from AppCheck-Nonce to master April 17, 2023 16:46
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.

Couple nits to address but LGTM– thanks!

FirebaseAppCheck/Sources/Core/FIRAppCheck.m Outdated Show resolved Hide resolved
FirebaseAppCheck/Tests/Unit/Core/FIRAppCheckTests.m Outdated Show resolved Hide resolved
FirebaseAppCheck/Tests/Unit/Core/FIRAppCheckTests.m Outdated Show resolved Hide resolved
FirebaseAppCheck/Tests/Unit/Core/FIRAppCheckTests.m Outdated Show resolved Hide resolved
@ncooke3
Copy link
Member

ncooke3 commented Apr 17, 2023

Oh, and please add an entry for the feature in the FirebaseAppCheck/CHANGELOG.md. You can use # Unreleased as the section header and use the [added] specifier for the entry.

Copy link
Contributor

@andrewheard andrewheard left a comment

Choose a reason for hiding this comment

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

LGTM! My suggestions are just nits

FirebaseAppCheck/CHANGELOG.md Outdated Show resolved Hide resolved
@google-oss-bot
Copy link

google-oss-bot commented Apr 17, 2023

Size Report 1

Affected Products

  • FirebaseAppCheck

    TypeBase (756451e)Merge (757c122)Diff
    CocoaPods?-51.5 kB? (?)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/EaSR8YAHZ0.html

@aiwenisevan aiwenisevan merged commit 87b07e2 into master Apr 17, 2023
32 checks passed
@aiwenisevan aiwenisevan deleted the facNONCE branch April 17, 2023 20:56
@paulb777 paulb777 added this to the 10.9.0 - M131 milestone Apr 18, 2023
@firebase firebase locked and limited conversation to collaborators May 18, 2023
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

7 participants