Skip to content

Add limited use token to FirAppeCheck Interface#11086

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.

Sign up for GitHub

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

Jump to bottom
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

@aiwenisevanaiwenisevan commented Apr 6, 2023•
edited
Loading

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

@aiwenisevanaiwenisevan marked this pull request as ready for review April 13, 2023 21:35
ncooke3
ncooke3 reviewed Apr 14, 2023
View reviewed changes
Copy link
Member

@ncooke3ncooke3 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
ContributorAuthor

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!

@aiwenisevanaiwenisevan changed the base branch from AppCheck-Nonce to masterApril 17, 2023 16:46
ncooke3
ncooke3 approved these changes Apr 17, 2023
View reviewed changes
Copy link
Member

@ncooke3ncooke3 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!

@ncooke3
Copy link
Member

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.

andrewheard
andrewheard approved these changes Apr 17, 2023
View reviewed changes
Copy link
Contributor

@andrewheardandrewheard 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

@google-oss-bot
Copy link

google-oss-bot commented Apr 17, 2023•
edited
Loading

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
@aiwenisevanaiwenisevan merged commit 87b07e2 into masterApr 17, 2023
@aiwenisevanaiwenisevan deleted the facNONCE branch April 17, 2023 20:56
@paulb777paulb777 added this to the 10.9.0 - M131 milestone Apr 18, 2023
@firebasefirebase locked and limited conversation to collaborators May 18, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api: appcheckpublic-api-change
7 participants
@aiwenisevan@ncooke3@google-oss-bot@andrewheard@rosalyntan@weixifan@paulb777
close