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

public changes for passkey support #11546

Draft
wants to merge 13 commits into
base: passkey-main
Choose a base branch
from

Conversation

Xiaoshouzi-gh
Copy link
Contributor

API proposal for passkey changes.

@google-oss-bot
Copy link

2 Warnings
⚠️ Did you forget to add a changelog entry? (Add #no-changelog to the PR description to silence this warning.)
⚠️ New public headers were added, did you remember to add them to the umbrella header?

Generated by 🚫 Danger

@github-actions
Copy link
Contributor

github-actions bot commented Jul 13, 2023

Apple API Diff Report

Commit: 110b526
Last updated: Wed Aug 23 15:43 PDT 2023
View workflow logs & download artifacts


FirebaseAuth

Classes

FIRUser
[ADDED] -startPasskeyEnrollmentWithName:completion:
Swift:
+  func startPasskeyEnrollment ( with name : String ?) async throws -> UnsafeMutablePointer < Int32 >
Objective-C:
+  - ( void ) startPasskeyEnrollmentWithName :( nullable NSString * ) name completion :( nullable void ( ^ )( int * _Nullable , NSError * _Nullable )) completion ;
[ADDED] enrolledPasskeys
Swift:
+  var enrolledPasskeys : [ PasskeyInfo ] { get }
Objective-C:
+  @property ( nonatomic , readonly ) API_UNAVAILABLE ( watchos ) NSArray < FIRPasskeyInfo *> * enrolledPasskeys ;
[ADDED] -finalizePasskeyEnrollmentWithPlatformCredential:completion:
Swift:
+  func finalizePasskeyEnrollment ( with platformCredential : Any ! ) async throws -> AuthDataResult
Objective-C:
+  - ( void ) finalizePasskeyEnrollmentWithPlatformCredential :( id ) platformCredential completion : ( nullable void ( ^ )( FIRAuthDataResult * _Nullable , NSError * _Nullable )) completion ;
[ADDED] -unenrollPasskeyWithCredentialID:completion:
Swift:
+  func unenrollPasskey ( with credentialID : String ) async throws
Objective-C:
+  - ( void ) unenrollPasskeyWithCredentialID :( nonnull NSString * ) credentialID completion :( nullable void ( ^ )( NSError * _Nullable )) completion ;
[ADDED] FIRPasskeyInfo
[ADDED] FIRPasskeyInfo
Swift:
+  class PasskeyInfo : NSObject
+    var name : String { get }
+    var credentialID : String { get }
Objective-C:
+  @interface FIRPasskeyInfo : NSObject
+    @property ( nonatomic , readonly ) NSString * _Nonnull name ;
+    @property ( nonatomic , readonly ) NSString * _Nonnull credentialID ;
FIRAuth
[ADDED] -startPasskeySignInWithCompletion:
Swift:
+  func startPasskeySignIn () async throws -> UnsafeMutablePointer < Int32 >
Objective-C:
+  - ( void ) startPasskeySignInWithCompletion : ( nullable void ( ^ )( int * _Nullable , NSError * _Nullable )) completion ;
[ADDED] -finalizePasskeySignInWithPlatformCredential:completion:
Swift:
+  func finalizePasskeySignIn ( with platformCredential : Any ! ) async throws -> FIRAuthDataResult
Objective-C:
+  - ( void ) finalizePasskeySignInWithPlatformCredential :( id ) platformCredential completion : ( nullable void ( ^ )( FIRAuthDataResult * _Nullable , NSError * _Nullable )) completion ;

@paulb777
Copy link
Member

will this PR address #11548?

@@ -17,6 +17,7 @@
#import <AvailabilityMacros.h>
#import <Foundation/Foundation.h>

#import <AuthenticationServices/AuthenticationServices.h>
Copy link
Member

Choose a reason for hiding this comment

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

@paulb777, should types from this framework instead be forward declared within the header?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, and wrapped in #if TARGET_OS_IOS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can you elaborate on the forward declaration part?

another question on TARGET_OS_IOS, these APIs will run on mac os, ios and tv os (but not watch OS).
I was thinking using
if TARGET_OS_IOS || TARGET_OS_TV || TARGET_OS_MAC. However going thru this doc, I have the assumption that TARGET_OS_MAC will include all IOS, TVOS and WATCHOS, can you suggest?

Copy link
Member

Choose a reason for hiding this comment

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

can you elaborate on the forward declaration part?

So line 20 should be moved to the implementation file. And in its place should be forward declarations for the types used from the AuthenticationServices module. The diff should look like:

Suggested change
#import <AuthenticationServices/AuthenticationServices.h>
@class ASAuthorizationPlatformPublicKeyCredentialAssertion;
@class ASAuthorizationPlatformPublicKeyCredentialAssertionRequest;

another question on TARGET_OS_IOS, these APIs will run on mac os, ios and tv os (but not watch OS).
I was thinking using
if TARGET_OS_IOS || TARGET_OS_TV || TARGET_OS_MAC. However going thru this doc, I have the assumption that TARGET_OS_MAC will include all IOS, TVOS and WATCHOS, can you suggest?

TARGET_OS_MAC might be too broad. Something like the following should specifically get all platforms except watchOS:

#if TARGET_OS_IOS || TARGET_OS_TV || TARGET_OS_OSX || TARGET_OS_MACCATALYST

#endif

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you Nick. After trying the suggestion we synced offline, it seems importing in the .m file still cause me compiler errors. Can you help me take a look where I might did wrong? (CI is also failing for the same issue)

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

Successfully merging this pull request may close these issues.

None yet

4 participants