[34253] - Add AuthorizationControllerWrapper & Unit Test for TwilioPasskey iOS. #21

AlejandroOrozco · 2024-04-17T22:16:46Z

Ticket

[34253]

Github Issue

ISSUE-

Description

Add AuthorizationControllerWrapper & Unit Test for TwilioPasskey iOS.

Commit message

[34253] - Add AuthorizationControllerWrapper & Unit Test for TwilioPasskey iOS.

Screenshot

Testing

Added unit tests
Ran unit tests successfully
Added documentation for public APIs and/or Wiki

authid-sdk-ci · 2024-04-18T20:13:40Z

shared/src/iosTest/kotlin/com/twilio/passkeys/TwilioPasskeyTest.kt

+      // Then
+      when (result) {
+        is CreatePasskeyResult.Success -> {
+          fail("It shouldn't succeed")


⚠️ Line detected, which is longer than the defined maximum line length in the code style.

looks like a false positive

…wilioPasskey iOS.

yafuquen · 2024-04-18T21:06:35Z

iosApp/iosApp/Core/AuthenticationManager.swift

@@ -84,7 +84,7 @@ class AuthenticationManager: NSObject, ObservableObject {
    ) {
        Task {
            do {
-                let result = try await worker.registrationVerification(request: .init(
+                _ = try await worker.registrationVerification(request: .init(


Is there something that can be checked in the verification response?

Yeah definitely makes sense, I added a validation to check if the response was verified by the backend. Thank you.

yafuquen · 2024-04-18T22:12:47Z

shared/src/iosTest/kotlin/com/twilio/passkeys/mocks/PayloadMocks.kt

+      "name": "Example"
+    },
+    "user": {
+      "id": "WUU0ZmQzYWFmNGU0NTMyNGQwZjNlMTM0NjA3YjIxOTEyYg",


This can be flaged as a secret, could you please change them to a placeholder?

Definitely 😅, added Placeholder prefix just to avoid any confusion. But apart from that the length must be exact. Thank you

yafuquen · 2024-04-18T22:14:10Z

shared/src/iosMain/kotlin/com/twilio/passkeys/AuthorizationControllerWrapper.kt

+ * @property deviceUtils The utility class for device-related operations.
+ */
+class AuthorizationControllerWrapper : IAuthorizationControllerWrapper {
+  private var authController: ASAuthorizationController? = null


Could this be a lateinit var?

Done, seems way clear with that one. Thank you

authid-sdk-ci · 2024-04-23T16:49:39Z

	1 Warning
⚠️	Big PR

Generated by 🚫 Danger

authid-sdk-ci · 2024-04-23T16:52:37Z

1 Warning

⚠️

Size impact

ABI	APK Size Impact
x86	2MB
x86_64	2MB
armeabi-v7a	2MB
arm64-v8a	2MB
universal	2MB

Generated by 🚫 Danger

authid-sdk-ci · 2024-04-23T16:53:03Z

🧛 shared module Code Coverage: `96.16%`

Coverage of Modified Files:

File	Coverage
`TwilioPasskey.kt`	`97.38%`

Modified Files Not Found In Coverage Report:

AuthenticationManager.swift
AuthorizationControllerWrapper.kt
AuthorizationControllerWrapperMock.kt
PayloadMocks.kt
TwilioPasskeyTest.kt

Codebase cunningly covered by count Shroud 🧛

Generated by 🚫 Danger

authid-sdk-ci · 2024-04-23T16:53:44Z

	1 Warning
⚠️	IPA size: 1.9 MB App size: 6.6 MB TwilioPasskeys.framework: 5.5 MB, Impact: 84.2%

Generated by 🚫 Danger

yafuquen · 2024-04-30T21:12:20Z

shared/src/iosMain/kotlin/com/twilio/passkeys/TwilioPasskey.kt

 */
-actual class TwilioPasskey private constructor(
+actual class TwilioPasskey internal constructor(


Can mapToTwilioException be part of this class instead of the wrapper? As it is a TwilioException, this could be more related to this class and the wrapper can pass the original error

yafuquen · 2024-04-30T21:14:17Z

shared/src/iosTest/kotlin/com/twilio/passkeys/mocks/PayloadMocks.kt

+      "name": "Example"
+    },
+    "user": {
+      "id": "PLACEHOLDERIDWUU0ZmQzYWFmNGU0NTMyNGQwZjNlMTM0N",


Can this be something like PLACEHOLDERID without the letters? Or can the letters be ABCD.... instead of WUU0ZmQzY.....

The size of the userId is being validated, so it requires to have the same size, maybe we can try with PLACEHOLDERID and spaces wdyt?

So, we can complete the values with ABCDEFG......., so it will be added as the alphabet (known order)

yafuquen · 2024-04-30T21:14:34Z

shared/src/iosTest/kotlin/com/twilio/passkeys/mocks/PayloadMocks.kt

+      "name": "user1",
+      "displayName": "User One"
+    },
+    "challenge": "WUYwNDhkMWE3ZWMzYTJhNjk3MDA1OWMyNzY2YmJjN2UwZg",


Same as above, this can be flagged as a secret

yafuquen · 2024-04-30T21:14:50Z

shared/src/iosTest/kotlin/com/twilio/passkeys/mocks/PayloadMocks.kt

+const val TIMEOUT = 600000L
+val keyCredential =
+  KeyCredential(
+    id = "6ySmhJd6qGUMCthiqszyb4Od4U6TFn0v3DLz-1EZrNQ",


This can be flagged as a secret

Same for values below RAW_ID, ATTESTATION_OBJECT, AUTHENTICATE_CHALLENGE, CLIENT_DATA_JSON_CREATE, SIGNATURE, USER_HANDLE. If we can use placeholder without letters will be better, if we need the values encoded or following a format, we should use dummy values so they are not considered secrets

yafuquen · 2024-04-30T21:24:11Z

shared/src/iosMain/kotlin/com/twilio/passkeys/AuthorizationControllerWrapper.kt

+        controller: ASAuthorizationController,
+        didCompleteWithAuthorization: ASAuthorization,
+      ) {
+        val credentialRegistration = didCompleteWithAuthorization.credential as ASAuthorizationPlatformPublicKeyCredentialRegistration


Is there a way to pass the credentialRegistration to TwilioPasskey and applying the logic below in TwilioPasskey? AuthorizationControllerWrapper could be in charge of completing the interaction / needed code for iOS authorization, but some logic can be kept in TwilioPasskey, so it can be tested

No unfortunately that ASAuthorizationPlatformPublicKeyCredentialRegistration contains a couple of Objects that can't be initialized outside of Apple private logic. So It won't be possible to create that object from Scratch to use it as a mock.

AlejandroOrozco self-assigned this Apr 17, 2024

AlejandroOrozco requested a review from a team as a code owner April 17, 2024 22:16

AlejandroOrozco force-pushed the aorozcobuiles/ACCSEC-34253/PasskeysWrappersForUnitTests branch from e80eb42 to de93987 Compare April 18, 2024 20:10

authid-sdk-ci reviewed Apr 18, 2024

View reviewed changes

AlejandroOrozco requested a review from authid-sdk-ci April 18, 2024 20:24

[ACCSEC-34253] - Add AuthorizationControllerWrapper & Unit Test for T…

375997d

…wilioPasskey iOS.

AlejandroOrozco force-pushed the aorozcobuiles/ACCSEC-34253/PasskeysWrappersForUnitTests branch from de93987 to 375997d Compare April 18, 2024 20:27

yafuquen reviewed Apr 18, 2024

View reviewed changes

ferazoguerrero approved these changes Apr 23, 2024

View reviewed changes

[ACCSEC-34253] - Address Comments

6a5e387

AlejandroOrozco requested a review from yafuquen April 25, 2024 14:41

yafuquen requested a review from SergioFierro April 30, 2024 21:05

yafuquen reviewed Apr 30, 2024

View reviewed changes

SergioFierro approved these changes May 6, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[34253] - Add AuthorizationControllerWrapper & Unit Test for TwilioPasskey iOS. #21

[34253] - Add AuthorizationControllerWrapper & Unit Test for TwilioPasskey iOS. #21

AlejandroOrozco commented Apr 17, 2024

authid-sdk-ci Apr 18, 2024

ferazoguerrero Apr 23, 2024

yafuquen Apr 18, 2024

AlejandroOrozco Apr 23, 2024

yafuquen Apr 18, 2024

AlejandroOrozco Apr 23, 2024

yafuquen Apr 18, 2024

AlejandroOrozco Apr 23, 2024

authid-sdk-ci commented Apr 23, 2024

authid-sdk-ci commented Apr 23, 2024

Size impact

authid-sdk-ci commented Apr 23, 2024

authid-sdk-ci commented Apr 23, 2024

yafuquen Apr 30, 2024

yafuquen Apr 30, 2024

AlejandroOrozco Apr 30, 2024

yafuquen Apr 30, 2024

yafuquen Apr 30, 2024

yafuquen Apr 30, 2024

yafuquen Apr 30, 2024

yafuquen Apr 30, 2024

AlejandroOrozco Apr 30, 2024

[34253] - Add AuthorizationControllerWrapper & Unit Test for TwilioPasskey iOS. #21

Are you sure you want to change the base?

[34253] - Add AuthorizationControllerWrapper & Unit Test for TwilioPasskey iOS. #21

Conversation

AlejandroOrozco commented Apr 17, 2024

Ticket

Github Issue

Description

Commit message

Screenshot

Testing

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

authid-sdk-ci commented Apr 23, 2024

authid-sdk-ci commented Apr 23, 2024

Size impact

authid-sdk-ci commented Apr 23, 2024

🧛 shared module Code Coverage: 96.16%

Coverage of Modified Files:

Modified Files Not Found In Coverage Report:

authid-sdk-ci commented Apr 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

🧛 shared module Code Coverage: `96.16%`