Differential / accounts-entry

Meteor sign up and sign in pages
http://github.differential.com/accounts-entry/
MIT License
305 stars 189 forks source link

Password/Email restrictions are enforced at the view layer #64

Open RobertLowe opened 10 years ago

RobertLowe commented 10 years ago

Password, and email validators shouldn't validate inside the the signUp.coffee, or at least it should be configurable.

Ideally the model layer in meteor would allow us to define something like:

Meteor.users.allow.insert = ()->
  if this.email != "win"
     throw new Error("view could handle this instead")

Perhaps it's possible to piggy back on autoforms validations.

Or something else, my point is that it's odd to have rules for this in one place but not globally applied.

Cheers

PS: Sorry for all the issues today D:

queso commented 10 years ago

I doubt we will want to piggyback on autoform at this point, as we are working on something much cleaner, imo - simple-forms.

But I do agree, we need to move the validators somewhere else. This is also an issue with signupCode too, I think?

RobertLowe commented 10 years ago

Re: autoform, Yea, I'm not a fan of afFieldLabel or superLongNames. Simple-form needs some TLC, it's just a handful of helpers right now. I've used simple-form & formtastic in the past, so, I'd love to see something with some feature parity.

But yea, I think there is the same issue for signupCode.

queso commented 10 years ago

Is your code in the first comment, legit? Or were you just taking a stab at an idea there? We should really try to work through this, as I tend to agree. We just hit a snag with #84 that would be solved by moving this to the model layer.

RobertLowe commented 10 years ago

I was using it as an example.

I think the best solution would be to piggyback on the allow, deny policies. The downside is the round-trip cost, but that might be negligible as a intermediate solution.

RobertLowe commented 10 years ago

Also thinking on this a bit more, touching the polices might be a bad idea in some cases. So the behaviour should be entirely optional.

AccountsEntry.configure({
   injectPolicies: false # defaults true
})

Something like that for cases where an application developer may not want their policies altered or needs to implement custom validators. There should be standard errors thrown for things like: InvalidPassword, MissingEmail, MissingPassword, AlreadyExists. Which the template could inherently handle even if overridden with custom validators/polices.

Looking at the accounts-base & accounts-password package, most validation are handle inside Meteor.methods, which doesn't feel right either.

queso commented 10 years ago

Well, at least in Meteor.methods, we are DRY the code up to one reusable spot. My worry here is that you have to use async callbacks if you want to have a return value from the method call. I guess maybe that is ok since we will be keeping the code contained in this package.