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

Increase max size for passwords - Solving the "E-Mail address already in use" Error #129

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

bioluks
Copy link

@bioluks bioluks commented Mar 26, 2024

Hi,

New installs are unable to register for accounts out of the box, I weren't able to do enough testing to see if it's related to password length (my one was about 40 characters). After clicking 'Create Account' it would always end up telling me "E-Mail address already in use"? I just launched the instance? Looking at issue #88 I could quickly verify it was related to the password length limiter in the code as @357up said.

Looking at the code only changing the number from 60 to 254 does the job just fine. Temporary solutions include:
for PostgreSQL databases:

ALTER TABLE users ALTER COLUMN password TYPE varchar(254) ALTER COLUMN password SET NOT NULL;

and for MySQL:

ALTER TABLE `users` MODIFY `password` VARCHAR(254) NOT NULL;

@@ -8,7 +8,7 @@ export default class extends BaseSchema {
table.increments();
table.string('username', 80).notNullable();
table.string('email', 254).notNullable().unique();
table.string('password', 60).notNullable();
table.string('password', 254).notNullable();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi!

Thank you for raising this PR. I haven't tested this yet (been having some personal issues that prevent me from reviewing and testing stuff)....

Nevertheless, we shouldn't modify migrations file, we should rather create a new one and modify that field to allow up to 254. Would you be able to create a new migration following adonisjs guides and change this PR accordingly?

Thank you so much in advance!

@SpecialAro SpecialAro assigned SpecialAro and bioluks and unassigned SpecialAro Apr 2, 2024
@KiARC
Copy link

KiARC commented Nov 4, 2024

I did fix this and make a PR here but it doesn't seem to have been seen yet, @SpecialAro would you like me to make a new PR on this repo?

@SpecialAro
Copy link
Member

@bioluks instead of making a new PR can you push the changes to this one instead? Thank you!

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

Successfully merging this pull request may close these issues.

3 participants