-
Notifications
You must be signed in to change notification settings - Fork 977
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
fix: use correct names in WebAuthN dialogs #3215
Conversation
Not really sure how we can test this properly. If you have an idea, let me know. |
|
||
kratos-selfservice-ui-node: | ||
environment: | ||
- KRATOS_BROWSER_URL=http://localhost:4433/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This overrides the variable from the standalone quickstart with localhost
instead of the local IP. WebAuthN only works on domains, not IPs so this is important to get this to work.
Codecov Report
@@ Coverage Diff @@
## master #3215 +/- ##
==========================================
- Coverage 77.89% 77.88% -0.01%
==========================================
Files 320 320
Lines 20451 20451
==========================================
- Hits 15931 15929 -2
- Misses 3317 3318 +1
- Partials 1203 1204 +1
... and 1 file with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
@@ -356,12 +356,12 @@ func (s *Strategy) PopulateSettingsMethod(r *http.Request, id *identity.Identity | |||
} | |||
} | |||
|
|||
web, err := s.newWebAuthn(r.Context()) | |||
web, err := webauthn.New(s.d.Config().WebAuthnConfig(r.Context())) | |||
if err != nil { | |||
return err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since webauthn.New
probably doesn't return errors with stack traces, it would be difficult to find the error if it happened here. So please make sure to wrap them:
return errors.WithStack(err)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, makes sense. Done.
Fixes #3110
Related issue(s)
Checklist
introduces a new feature.
contributing code guidelines.
vulnerability. If this pull request addresses a security vulnerability, I
confirm that I got the approval (please contact
[email protected]) from the maintainers to push
the changes.
works.
Further Comments