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

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

@aiwenisevan aiwenisevan requested a review from rosalyntan April 6, 2023 22:49
@aiwenisevan aiwenisevan marked this pull request as ready for review April 13, 2023 21:35
@aiwenisevan aiwenisevan requested a review from ncooke3 April 14, 2023 18:44
ncooke3
ncooke3 reviewed Apr 14, 2023
View reviewed changes
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 requested a review from andrewheard April 17, 2023 15:47
@aiwenisevan aiwenisevan changed the base branch from AppCheck-Nonce to master April 17, 2023 16:46
ncooke3
ncooke3 approved these changes Apr 17, 2023
View reviewed changes
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!

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

andrewheard
andrewheard approved these changes Apr 17, 2023
View reviewed changes
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

@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

@aiwenisevan aiwenisevan merged commit 87b07e2 into master Apr 17, 2023
@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.
Reviewers

@andrewheard andrewheard andrewheard approved these changes

@ncooke3 ncooke3 ncooke3 approved these changes

@weixifan weixifan weixifan left review comments

@rosalyntan rosalyntan rosalyntan approved these changes

Assignees
No one assigned
Labels
api: appcheck public-api-change
Projects
None yet
Milestone
10.9.0 - M131
Development

Successfully merging this pull request may close these issues.
7 participants
@aiwenisevan @ncooke3 @google-oss-bot @andrewheard @rosalyntan @weixifan @paulb777