search

hoodiehq / hoodie-account-client

:dog: Account client API for the browser
https://hoodiehq.github.io/hoodie-account-client
Apache License 2.0
11 stars 24 forks source link

hoodie.account.signUp({username}) does not lowercase username #115

Open gr2m opened 8 years ago

gr2m commented 8 years ago

@rmehner is already working on it

MattCed commented 8 years ago

Noticed a related problem. If I have a user {username: "www", password: "www} and then do hoodie.account.update({username: "Cap"}).then....

I can no longer sign in. Not with www, cap or Cap

gr2m commented 8 years ago

thanks Matt!

Sticksword commented 7 years ago

Asked in Slack but will also leave the message here for reference.

I’m looking at the sign-up.js file in hoodie-account-client/lib. Can someone explain what state.validate.call(this, options) means? In the hoodie-tracker-app, the signUp method is called with only one parameter but here, it’s exported with 2 parameters. As far as I can tell for #115 (https://github.com/hoodiehq/hoodie-account-client/issues/115), if the validate.call doesn’t lower case the username/password, then the logic is not there to begin with and needs to be implemented.

gr2m commented 7 years ago

By default, the validate function does nothing. See docs here: https://github.com/hoodiehq/hoodie-account-client#accountvalidate

The idea is that you can can pass in options.validate when initialising the account API to enforce app-specific rules for your app’s account, e.g. you could enforce that all usernames are email addresses.

This issue is about the problem that username must always be lowercase to avoid authentication issues. For that, the serialise util method should check if attributes.username is a string and if it is, it should lowercase it.

Do you want to try implementing it? For tests, you can adapte the sign in integration test to use upper case letters in username, and make another similar one for sign up and account. Also adapt the update test where we change the username to make sure we send a lowercase there, too.