-
-
Notifications
You must be signed in to change notification settings - Fork 964
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: Do not remove all credentials when remove all security keys #2233
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2233 +/- ##
==========================================
+ Coverage 75.84% 75.87% +0.03%
==========================================
Files 297 297
Lines 15905 15909 +4
==========================================
+ Hits 12063 12071 +8
+ Misses 2984 2979 -5
- Partials 858 859 +1
Continue to review full report at Codecov.
|
d0cd751
to
00a906e
Compare
selfservice/flow/settings/hook.go
Outdated
@@ -223,6 +226,12 @@ func (e *HookExecutor) PostSettingsHook(w http.ResponseWriter, r *http.Request, | |||
return nil | |||
} | |||
|
|||
x.ContentNegotiationRedirection(w, r, ctxUpdate.GetIdentityToUpdate().CopyWithoutCredentials(), e.d.Writer(), returnTo.String()) | |||
i, ok := ctxUpdate.GetIdentityToUpdate() |
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.
Could you please add a test for these checks? :)
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 check is useless because there is ctxUpdate.UpdateIdentity(i)
at line 166. So I removed these.
Signed-off-by: sawadashota <[email protected]>
Signed-off-by: sawadashota <[email protected]>
Signed-off-by: sawadashota <[email protected]>
…#2233) Co-authored-by: aeneasr <[email protected]>
Related issue(s)
Fix bug removing all credentials when remove all security keys.
Related: #2181
Checklist
introduces a new feature.
contributing code guidelines.
vulnerability. If this pull request addresses a security. vulnerability, I
confirm that I got green light (please contact
[email protected]) from the maintainers to push
the changes.
works.
Further Comments
I'm sorry my previous PR #2181 fixes a bug but causes new bug.