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

Package seems to be ignoring validateloginattempt #22

Open
mitar opened this issue Apr 18, 2016 · 9 comments
Open

Package seems to be ignoring validateloginattempt #22

mitar opened this issue Apr 18, 2016 · 9 comments

Comments

@mitar
Copy link
Contributor

mitar commented Apr 18, 2016

http://docs.meteor.com/#/full/accounts_validateloginattempt

Even if callback is returning false, user is still logged in.

@mitar
Copy link
Contributor Author

mitar commented Apr 18, 2016

In short, callback is not called.

@kentonv
Copy link
Member

kentonv commented Apr 21, 2016

This does appear to be a bug.

That said, what is the use case for this on Sandstorm?

@mitar
Copy link
Contributor Author

mitar commented Apr 21, 2016

@kentonv
Copy link
Member

kentonv commented Apr 21, 2016

OK, so you're trying to disable non-Sandstorm login mechanisms when Sandstorm is enabled.

I actually think we should build this into accounts-sandstorm. Traditionally we have told people that they should build with a different package set when building for Sandstorm vs. outside, but we've noticed some projects forget to do this. Perhaps accounts-sandstorm should automatically install a login validator function that rejects everything. Since we now explicitly check for Sandstorm, it's safe to do this.

@mitar
Copy link
Contributor Author

mitar commented Apr 21, 2016

Yes. I think quite some functionalities which I had to implement in my app should be moved out into some Meteor-Sandstorm integration.

But this is just my use case. I can imagine people use validateLoginAttempt to log login attempts.

@kentonv
Copy link
Member

kentonv commented Apr 21, 2016

OK, I implemented "highlander mode" and published version 0.3.0, so you can remove your login validator.

I looked briefly at how to call the login validator callback, but this seems awkward. accounts-sandstorm doesn't actually use the Meteor Accounts framework as it isn't a great fit, and the validation hooks are buried pretty deep in there.

I'm open to a pull request but given that there's seemingly no immediate need I'm not going to work on this any further for now.

@mitar
Copy link
Contributor Author

mitar commented Apr 21, 2016

So cool! Thanks!

@mitar
Copy link
Contributor Author

mitar commented Apr 21, 2016

For the same reason, this also does not call: http://docs.meteor.com/#/full/accounts_onlogin

@mitar
Copy link
Contributor Author

mitar commented Apr 21, 2016

Pull request done.

While working on this I discovered that more importantly onLogin and onLoginFailure hooks were also not called.

This now calls all that and nicely integrates with Meteor accounts.

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

No branches or pull requests

2 participants