~sircmpwn/meta.sr.ht#52: 
Email validation skipped when updating profile

The registration tests the email length and uniqueness. But this is not enforced in profile_POST().

I feel like a lot of this validation might be better off in the ORM? It can often delegate these checks to the database which is nice for things like supporting bulk updates or ensuring uniqueness without race conditions by creating an index. At the moment, I think concurrent POSTs to /register might create users with the same email or username if those requests handlers do their uniqueness checks before the other handlers commit the session. (I could be wrong about that; I'm not real familiar with sqlalchemy.orm.)

Also, you can also register with the email foo@example.com; bar@example.com but I don't think it sends two emails so maybe that doesn't matter. And I can't think of a way in which either this or skipping email validation could be used to harm other users, so this is not super serious afaik.

Also you can kind of get around the uniqueness constraint on both fields with trailing whitespace. And zero width characters are permitted in usernames (and encode properly in the site's hyperlinks) but could maybe be a way for like a homograph attack or something?

There are different points in here; it's not real concise, sorry. The title of this todo is the only thing I think is a proper bug. Maybe I'm just being pedantic about the other things but I figure I'd mention them in case you have an opinion.

Status
RESOLVED FIXED
Submitter
~sqwishy
Assigned to
No-one
Submitted
1 year, 10 months ago
Updated
1 year, 6 months ago
Labels
No labels applied.

~sircmpwn 1 year, 10 months ago

I feel like a lot of this validation might be better off in the ORM? It can often delegate these checks to the database which is nice for things like supporting bulk updates or ensuring uniqueness without race conditions by creating an index. At the moment, I think concurrent POSTs to /register might create users with the same email or username if those requests handlers do their uniqueness checks before the other handlers commit the session. (I could be wrong about that; I'm not real familiar with sqlalchemy.orm.)

The Python logic is just to present a pretty error. There should be a unique constraint on the database, though. I'm not interested in adding more complex constraints at the database level, nor am I interested in increasing the complexity of the ORM layer.

The rest of the points are all valid, thanks. This should be improved.

Register here or Log in to comment, or comment via email.