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

supporting empty allow credentials list (during authentication) #305

Closed
wants to merge 1 commit into from
Closed

supporting empty allow credentials list (during authentication) #305

wants to merge 1 commit into from

Conversation

bmd007
Copy link

@bmd007 bmd007 commented Jun 24, 2023

Suggesting this PR as a fix fir the issues discussed in #300.
Basically, allowCredential being an empty optional is not enough for username less flow.
It should be an empty list.
Fundamentally, a list wrapped by an optional is a bad idea but itself. But solving that issue would have taken too much time.

@bmd007 bmd007 mentioned this pull request Jun 24, 2023
@emlun
Copy link
Member

emlun commented Jun 26, 2023

As noted in #300 (comment), I see no issue with the code as is. AssertionRequest.toCredentialsCreateJson() omits empty optional values from the serialization result, and the default value of allowCredentials in the WebAuthn API is []. This is working as intended.

@emlun emlun closed this Jun 26, 2023
@bmd007
Copy link
Author

bmd007 commented Jul 3, 2023

As noted in #300 (comment), I see no issue with the code as is. AssertionRequest.toCredentialsCreateJson() omits empty optional values from the serialization result, and the default value of allowCredentials in the WebAuthn API is []. This is working as intended.

the java code, as I pointed to the line, results in a null instead of []. Please simply debug the flow. Not a complicated thing to try.

@emlun
Copy link
Member

emlun commented Jul 4, 2023

I did attempt to debug the flow, but found no issue like what you describe (though I did find a different one, see #300 (comment)). What results in null?

@bmd007
Copy link
Author

bmd007 commented Jul 12, 2023

I did attempt to debug the flow, but found no issue like what you describe (though I did find a different one, see #300 (comment)). What results in null?

please check my comments on the corresponding issue. I have provided screenshots and possible theories

@emlun
Copy link
Member

emlun commented Jul 24, 2023

Thank you, I've replied in #300.

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

Successfully merging this pull request may close these issues.

2 participants