Closed madogiwa0124 closed 2 years ago
This seems reasonable to me. I'm apt to approve and merge. @madogiwa0124 have you tried your fork in one of your own applications to ensure there are no downsides to this approach?
Thank you for jumping on this.
@rickychilcott
Thanks for your comment!
I just tried forking on my application and it seems to be working fine. Here is what I actually did.
Thank you both for helping out. Don't forget a changelog entry if you decide to include this, @rickychilcott 😊
My review on this one is good, but let me spin up a test project here and merge/changlog/bump if all goes well.
I bumped to version 0.11.0 and added a changelog entry in 74e753068309a744a28e9432e2681a00178eeb0b. @mikker I'm not sure I have the ability to release the gem. If that's something you want to give to me, great. Otherwise, I wonder if we could work up a github action flow to possibly auto-publish.
https://andrewm.codes/blog/automating-ruby-gem-releases-with-github-actions/ could be a good read and an hour of work to save pain for the future :)
You can absolutely be a maintainer on RubyGems. Appreciate all your help here very much. Do I just use the e-mail address that I already have?
Same email. I couldn't readily find that email communication, so... I commented here. Thanks!
It is recommended to reset the session at sign in for protect from session fixation attacks. So reseted session in
Passwordless::ControllerHelpers#sign_in
.ex.
Sorcery::Controller::InstanceMethods#login
in Sorceryhttps://github.com/Sorcery/sorcery/blob/6fdc703416b3ff8d05708b05d5a8228ab39032a5/lib/sorcery/controller.rb#L52-L56
How about this PR?
fixed https://github.com/mikker/passwordless/issues/107