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

Profiling password.mangling #233

Open
stuartpb opened this issue Feb 17, 2017 · 8 comments
Open

Profiling password.mangling #233

stuartpb opened this issue Feb 17, 2017 · 8 comments
Milestone

Comments

@stuartpb
Copy link
Member

I've been using password.classes.required[0].classes[0]: nonspaces for a while to represent sites that reject passwords with spaces in them, but the truth is, many to most sites are probably just mangling passwords by trimming them (the test: if it rejects eight spaces, does it reject seven spaces and an a?)

Consequently, you can do this test in password input, too.

I'm not sure how extensively I've tested this, but FluxBB definitely does trimming on all its password fields.

@stuartpb
Copy link
Member Author

stuartpb commented Feb 17, 2017

I was thinking this'd be password.mangling: [trim] (with maybe value or contents jammed in there), but now that I think about it, this should really be a property of form('s child fields), or at least a sibling of it, since there are definitely sites out there that mangle one but not the other.

@stuartpb
Copy link
Member Author

Also, I try not to be judgmental in field names, but seriously, if you modify a password value in any way, it's mangling. If a crypt function truncates a key, it's objectively mangling it - the same criteria applies for passwords.

@stuartpb
Copy link
Member Author

Better: make it a mangle object, with trim being an enum of start, end, or (most likely) both.

This also opens the door for, say, mangle.truncate.

@stuartpb
Copy link
Member Author

Maybe munge, or alter.

@stuartpb stuartpb added this to the future milestone Feb 20, 2017
@stuartpb
Copy link
Member Author

As much as I want to have this documented (I LOVE the idea of profiling truncation as a separate issue from maximum length), it needs some more discussion before it gets integrated.

The biggest unanswered question for me right now is "what about when not all parts of the site mangle passwords the same way?" That's a hard thing to keep track of, testing-wise, but it's still a salient issue - if only registration or only settings or only reset trim my password value, then I'm going to end up broken if I treat it as trimmed and it was entered through one of the other paths.

@stuartpb
Copy link
Member Author

stuartpb commented Feb 20, 2017

I'm thinking about having a transformations object (list?) that can apply to password as well as any form field that might have further transformations, but... egh.

Maybe password.value just needs trim.space and truncated.length as fields directly, and then this kind of inconsistency can be attached as caveats?

@stuartpb
Copy link
Member Author

Also, re: #233 (comment) - after noticing that Google disallows spaces on the ends of passwords (probably trimming on inputs for login), I'm thinking that maybe trimming isn't evil - or at least, not when combined with measures so that this is never silently mutating the password the user wanted to use (as #251 may document). I could see a user copy-pasting a password from some email, not noticing they highlighted a space, then wondering why login mysteriously isn't working.

@stuartpb
Copy link
Member Author

Truncation is definitely wrong, though. If you don't want to work with passwords past a certain length, just call it the maximum length, don't silently drop parts.

This was referenced Feb 21, 2017
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

1 participant