Warning: Can't synchronize with repository "(default)" (Unsupported version control system "svn": No module named svn). Look in the Trac log for more information.

Ticket #831 (closed enhancement: wontfix)

Opened 13 years ago

Last modified 12 years ago

Add form validation for identity_from_login

Reported by: jvanasco@… Owned by: anonymous
Priority: normal Milestone: 1.0b1
Component: Identity Version: 0.9a5
Severity: normal Keywords:
Cc:

Description

identity_from_login doesn't currently have any sort of form validation

it would be nice to be able to do put in a form validation into it, other than just return the 'invalid credential' error

ie:

notnull for username/password min/max for username/password custom validator for username ( ie: isEmailAddress )

all of those things are in the widgets api, we just need to somehow hook them in by deciding how to define the validators in app.cfg, and where to log the error

Change History

comment:1 Changed 13 years ago by godoy

What is preventing you from declaring a validators.Schema and validating it in your controller -- as it should be -- instead of putting unnecessary configuration options inside the configuration file?

Lets keep each thing in its place.

comment:2 Changed 13 years ago by jvanasco@…

  1. What is preventing you from declaring a validators.Schema and validating it in your controller?
  1. turbogears.

The function call to attempt login doesn't happen in the contoller, it happens in some magical place before anything is dispatched to it, and there are no hook functions that i could extrapolate from visit.api

comment:3 Changed 13 years ago by alberto

The function call to atempt login takes place in Identity's CP filter which is "outside" all the validation/error_handling done by decorators. I'm sure you could hack on this filter to do manual validation using a Schema but you'd have to take care to prepopulate CP.request.valdation_errors with values for the login form (which you'd have to implement with widgets) to display. Tak a loog at turbogears.controllers.validate to see ow it's done.

Just for the record, which kind of extra validation would you like to perform on the user_name, password tuple? Isn't "authorized" or "non-authorized" enough? Alberto

comment:4 Changed 13 years ago by jvanasco@…

in the current implementation, it'll try to authenticate against the db on every attempt, which i think is wasteful if someone supplies informaiton that you know could never authorize an account.

there are a ton of better ways to handle authentication validation: if someone enters in no username, it should mark that as empty and not go against the db. if someone enters in a 4char password and you have a minimum of 6 chars, it should say "password is too short". if your username is must be 8+chars or an email address , it should say as much.

considing how many dozens of logins and passwords people have today, hints like that help them remember which they should be using.

comment:5 Changed 13 years ago by alberto

I think the easiest (and most appropiate) way to perform this kind of validation would be by overriding validate_identity in a custom id provider. Just do your checks before hitting the DB (before user= user_class.by_user_name( user_name ))

You won't be able to send helpful messages to the user informing her why the credential checks failed, but this is good IMO as you don't need/want to leak this info to a potential malicious user.

Should we close this?

comment:6 Changed 13 years ago by alberto

Correction to my comment: You *can* show helpful messages informing why the checks failed in case you really need to, it's rather hacky and prone to break if TG internaals change implementation details, but.. a previous comment explains how.

comment:7 Changed 13 years ago by jvanasco@…

what if we provide a hook function for validation in validate_identity that a user can define if wanted.

i never give errors like 'this password doesn't match this account' - it allows for email harvesting. but i think saying 'you didn't enter anything in here' , 'these characters are invalid for accountnames' , and 'this is way too short' is requisite for a webapp - most general internet users are exceedingly stupid and need to be pushed a bit towards providing the right info.

comment:8 Changed 13 years ago by godoy

I dunno... I prefer doing that on my own code, using some mean to also verify how strong / weak is the password. There are lots of JavaScript? functions available. After all, having a minimum of 8 chars and accepting "12345678" or "abcdefgh" or even "aaaaaaaa" is not better than accepting a minimum of four and using "@!çã"...

And I *always* loved sites that demand a minimum of "n" digits but doesn't say that in the login form -- they already said that in the registration form or invitation or welcome message... -- so that somebody doesn't have a clue on how many chars they should start attacking passwords and usernames.

The minimum that is said about password length, valid usernames, etc. the better.

GMail, for example, just says "Username and password do not match. (You provided jgodoy)". Orkut, one of the most used web applications today, also have the same message and doesn't provide the username I tried logging in with. So, the "requisite for a web app" doesn't seem to apply. Yahoo! Groups and MSN do almost the same here, again contradicting your requisites.

One must clearly separate login interface from user creation interface, password setting interface, etc.

I also have a hidden div with a help message explaining some details -- not too much -- labeled "Help" on all of my forms, including the login form. This might also be a better solution to the problem.

I always tell people to not trying to solve user education/conscientization problems with technical workarounds. It will fail sometime and users will be deceived because they weren't told how to do it the right way before.

comment:9 Changed 13 years ago by jorge.vargas

I agree with godoy here.

most of the checks that want to be plugin are application specific, in fact I'll say they are programmer specific cause people tend to do the same type of things.

As for the possible things here I'll say that the only one that should go into TG is to send out an error based on empty user and/or password.

Other then that if you want to "check it before it goes to the db" go with JS validations since all the things you sugggested are not critical.

I'm tempted to close this as wontfix.

comment:10 Changed 13 years ago by elvelind

  • Status changed from new to closed
  • Resolution set to wontfix

And I will close it as won't fix. If you really want to give the user info on what part of the credentials were wrong, use JS validation or a custom provider.

Note: See TracTickets for help on using tickets.