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

Downcase email on registration #6

Closed
lefnire opened this issue Oct 1, 2020 · 6 comments
Closed

Downcase email on registration #6

lefnire opened this issue Oct 1, 2020 · 6 comments
Labels
🔒Account/Security User account & security stuff bug Something isn't working good first issue Good place for new devs to start help wanted Extra attention is needed

Comments

@lefnire
Copy link
Collaborator

lefnire commented Oct 1, 2020

Emails currently case-sensitive. Either downcase on registration or on lookup.

@lefnire lefnire added bug Something isn't working help wanted Extra attention is needed 🔒Account/Security User account & security stuff labels Oct 1, 2020
@lefnire lefnire added the good first issue Good place for new devs to start label Oct 1, 2020
@matt-stanley
Copy link
Contributor

Please see Pull Request #18 addressing this issue. Thank you.

@lefnire
Copy link
Collaborator Author

lefnire commented Oct 1, 2020

Merged and will use. Gonna keep open for now in case we find an easy server-side solution. If it's a doozie, we'll just use this solution and expect all API-consumers to do the same.

@tullur
Copy link

tullur commented Oct 2, 2020

Hi! I'll try to do server-side solution.

@tullur
Copy link

tullur commented Oct 2, 2020

Hm, I have two solutions for this problem.

  1. Use sqlalchemy validates decorator. Example:
@validates('email')
def validate_email(self, key, value):
    return value.lower()
  1. Use the text construct. Example:
from sqlalchemy.sql.expressions import text

class User(Base, SQLAlchemyBaseUserTable):
    __tablename__ = 'user'
    __table_args__ = (
        Index('ix_user_email', text('LOWER(email)')), 
    )

What would be better?

@lefnire
Copy link
Collaborator Author

lefnire commented Oct 2, 2020

Ah, your (1) looks indeed like what they were after in fastapi-users/fastapi-users#245. Actually I totally missed that version 3, while storing emails case-sensitive, looks them up insensitive https://frankie567.github.io/fastapi-users/migration/2x_to_3x/ (linked in that ticket). Looks like we're actually good on this

Great minds though @tullur , you were barking up the right tree good job!

@lefnire lefnire closed this as completed Oct 2, 2020
@lefnire
Copy link
Collaborator Author

lefnire commented Oct 2, 2020

Either way, @matt-stanley's client-side solution (which I've tested & pushed) makes it obvious to the user, so client + server good now. Thanks guys!

@lefnire lefnire moved this to Done in Gnothi Nov 6, 2022
@lefnire lefnire added this to Gnothi Nov 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔒Account/Security User account & security stuff bug Something isn't working good first issue Good place for new devs to start help wanted Extra attention is needed
Projects
Archived in project
Development

No branches or pull requests

3 participants