Update register response with transports field #80
Conversation
There was a problem hiding this comment.
Thanks for the PR. I have added 3 points that we have to address to make the change working. Changes like these are quite hard right now because we 1) have no automatic tests and 2) the code for code generation is not yet documented so sorry for that :)
To test your changes I would recommend the following:
- install melos (https://pub.dev/packages/melos) => very helpful if you work with mono repos
- run
melos bootstrap=> this will allow you to use your local changes in the example of the passkeys package - run the passkeys example => packages/passkeys/passkeys/example/lib/main.dart
FYI: We are planning to improve the passkeys package quite a bit as it is missing quite some fields from the webauthn specification now. We aim to do that in the upcoming two weeks. I will make sure to document also how to test and add a CONTRIBUTING.md. Ideally, we'll than add a few tests then :)
| .setRawId(json.getString("rawId")) | ||
| .setClientDataJSON(response.getString("clientDataJSON")) | ||
| .setAttestationObject(response.getString("attestationObject")) | ||
| .setTransports(json.getJSONArray("transports").toList()).build()); |
There was a problem hiding this comment.
Two comments here:
- You need to regenerate passkeys_android/Messages.java => you can do that by running
dart run pigeon --input pigeons/messages.dartfrom the passkeys_android directory - JSONArray does not have a toList() method AFIAK => we would probably need something like this:
JSONArray transports = response.getJSONArray("transports");
List<String> typedTransports = new ArrayList<>();
for (int i = 0; i < transports.length(); i++) {
typedTransports.add(transports.getString(i));
}
There was a problem hiding this comment.
Okok good. I added the suggestion in the Java/Swift code, however my environment isn't fully set for Java/Gradle and Swift so I dont have intellisense (I only develop for Web lately). Please double check if possible.
| @@ -117,6 +118,9 @@ class RegisterResponse { | |||
|
|
|||
| /// The attestation object | |||
| final String attestationObject; | |||
|
|
|||
| /// The supported transports for the authenticator | |||
| final List<String> transports; | |||
There was a problem hiding this comment.
For pigeon (the library we use to generate the glue code between Flutter and native code) every generic type has to be nullable => we have to use final List<String?> transports here and update line 90 in passkeys_android.dart like this r.transports.whereType<String>().toList().
| }); | ||
|
|
||
| final String id; | ||
| final String rawId; | ||
| final String clientDataJSON; | ||
| final String attestationObject; | ||
| final List<String> transports; |
There was a problem hiding this comment.
Similar as with passkeys_android we have to use List<String?> here => you also have to set this new transport field in RegisterController.swift then.
There was a problem hiding this comment.
Done, although please check the Swift code if possible.
PR for issue #79.
Made this PR very quickly and couldn't find contribution guidelines. I did modify the auto-gen'd file by hand too. Please let me know how to proceed with this to make it clean and ensure it doesn't break anything.
Thanks!