Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Warn instead of hard error when validating u2f facets #12826

Merged
merged 4 commits into from
May 23, 2022
Merged

Conversation

xacrimon
Copy link
Contributor

@xacrimon xacrimon commented May 23, 2022

Note, this will still hard error if the u2f facet is of a non URL format and cannot correctly be parsed but won't error on a differing domain or a missing https scheme.

Do we have a solid way to check for emitted log messages? A bit tricky to test now since we don't have a hard error to check.

Copy link
Contributor

@codingllama codingllama left a comment

Choose a reason for hiding this comment

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

Thanks for the quick change.

I don't think we have a great story for how to test this, barring manual testing. Maybe keep a test so we know the ifs are exercised but won't lead to failures? (It's a bit weird, I know.)

@xacrimon
Copy link
Contributor Author

@codingllama might be the best we can do, fixed

@xacrimon xacrimon requested a review from codingllama May 23, 2022 14:28
@xacrimon xacrimon enabled auto-merge (squash) May 23, 2022 16:24
@xacrimon xacrimon merged commit 50f3402 into branch/v9 May 23, 2022
@webvictim webvictim mentioned this pull request Jun 8, 2022
@zmb3 zmb3 deleted the joel/u2f-warn branch April 26, 2023 21:32
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants