Email address length limited to 50 characters

[Mart124]

Hi,

Email address length is currently limited to 50 characters. Which unfortunately is too short in some cases, with long user names and long domain names...

Could we then think about increasing it please ?

Thank you 👍

[clayadams5226]

Hey @Mart124 , we use GitHub issues as a place to track bugs and other development related issues. The Bitwarden Community Forums has a section for submitting, voting for, and discussing product feature requests like this one.

Please sign up on our forums and search to see if this request already exists. If so, you can vote for it and contribute to any discussions about it. If not, you can re-create the request there so that it can be properly tracked.

This issue will now be closed. Thanks!

[Mart124]

@clayadams5226 @cscharf this sounds more like a bug than a feature request, as we can't register users with email address longer than 50 characters... Could you then please re-open this bug report please ? Many thanks for your support 👍

[cscharf]

@Mart124 , makes sense, considering https://tools.ietf.org/html/rfc5321#section-4.5.3.1.3 we'll re-open this and add it to the backlog. Thanks for the follow up.

[djsmith85]

@Mart124 @cscharf What would the new maximum length for an email address be? There seems to be quite a bit of confusion if it's 256 or 320 characters? I've already identified which files would need to be changed and wouldn't mind creating a PR for this issue.

[cscharf]

@djsmith85 , should be 256 octets per the spec and may require UI validation updates in many other repos as well.

[djsmith85]

@cscharf I've modified the sql files accordingly. I hope atleast. I obviously would like to test my changes and see them applied to a local database before issuing a PR. Maybe even some further testing after that. I'm unfortunately running into some issues with the deployment.

1: Display schema changes via sqlproj and local mssql docker image retrieved via bitwarden,ps1 -install I get a ton of changes using the schema compare in VS2019, especially changes I didn't make. The script that gets generate is definitely overkill for what I originally wanted to achieve.

2. Running the build.sh from the Setup project to create a new version of the Setup-container. Running a bitwarden.ps1 -install against that does not apply any changes to the DB I currently have no migragtion sql script in the migrator project as I had hoped to generate one via the schema compare

Am I missing something regarding a local deploy of my sql-scripts change to test them via the provided docker images and build scripts.

It honestly took me a while to stumble onto the "Setting up a Dev enviroment" in the CONTRIBUTING.MD... duh

I'd really appreciate some help and if I really misunderstood something I'd for sure try and update the docs accordingly so no-one else runs into my noobish mistakes.

P.S.: I know this is probably not a high priority issue, as for the majority, this is rather an edge case, but thought I'd have a stab at it, to get more familiar with the codebase and the deployment process

[cscharf]

Am I missing something regarding a local deploy of my sql-scripts change to test them via the provided docker images and build scripts.

Nope, currently we all manually run the migrator SQL files against our local running database when doing development. Initial setup is something I believe @MGibson1 scripted or created a utility for in order to get everything loaded in quickly. All scripts should be re-runnable so it's generally no harm, no foul when doing local development.

You will definitely need to create a migration script and follow the pattern there, that is how the self-hosted installations update the schema in target systems and we run those manually after checks, etc. in our environment (have to be more careful, index update/rebuild on a table with 2B+ rows gets nasty for example 😉 ), please don't use VS2019 schema compare, it's an awesome tool (one I love when I'm doing dev on Windows), but we hand-craft our update scripts to keep them clean and succinct; you'll need exists/drop/re-create for sprocs/functions and table column additions, etc. you have to be careful. Schema compare won't generate re-runnable/safe scripts at all and we can't use !CMD mode through the migrator anyway.

[MGibson1]

Not much to add to that. The script @cscharf is talking about is this:

MIGRATE_DIRECTORY="/mnt/DbScripts"
SERVER="localhost"
DATABASE="vault_dev"
USER="sa"
PASSWD="YOUR_SA_USER_PASSWORD"

for f in `ls -v $MIGRATE_DIRECTORY/*.sql`; do
  /opt/mssql-tools/bin/sqlcmd -S $SERVER -d $DATABASE -U $USER -P $PASSWD -I -i $f
done;

Only tested on Mac so far, but that sqlcmd command either exists or has an analog in Windows. All it really does is run through the migration scripts in a alphanumerically sorted order (which is date-sorted by our naming convention).

Nothing advanced but it's a lot better than applying the migrations one by one manually in my eyes.

[djsmith85]

@cscharf @MGibson1 Thanks a lot for the quick response.

I had just thought that generating the change scripts would've been really cool. I get your point though regarding the nasty side effects and sometimes unnecessary script parts of those generators. I've just finished handcrafting the migration scripts and will test them.

@MGibson1 That little snippet is really helpful, as I would've probably just applied them manually one by one. Thanks again.

I had hoped to get a somewhat streamlined local build/deploy process with docker running, but it has been quite a setup with a lot of trial and error. Once done with this, I might have another look at it and create some scripts or a guide from my experience.

I hope to get the changes done towards or on the weekend and will submit a PR-Draft.

[cscharf]

Thanks @djsmith85 for the contrib, this is headed to the QA team now that it has been merged!

[Mart124]

As the OP, many thanks to all the contributors, @djsmith85 @cscharf 👍 Absolutely perfect :)

[cscharf]

Hi @djsmith85 , thanks again for the contrib. Hate to do this because I missed it during review and I'm kicking myself for it, but the data column size is only 50% of the battle, the View Models used for requests to the API also need to be updated, for example, https://github.com/bitwarden/server/blob/master/src/Core/Models/Api/Request/Accounts/EmailRequestModel.cs#L10 has model validation attributes that also limit the size, so this failed QA and I was scratching my head until I looked and noted those. Sorry man.

Re-opening this for now if you wanted to tackle another PR for it, otherwise we'll have to add to our backlog to complete.

[djsmith85]

@cscharf No worries. If getting it done right, means re-opening, I'm down.

I honestly didn't think about any model validator attributes, when making the initial changes. So also shame on me :D

I've created a draft PR showing the changes I've found/made so far. I'll have a closer look tomorrow as it's getting late.

Thank you for checking/testing thoroughly and I would appreciate a review, once I've finished the PR.

---

Regarding the UI validation changes you proposed earlier in this issue. I've identified a couple of spots across the repos and had a look at https://www.regular-expressions.info/email.html for a possible regex. Currently the codebase mostly only checks for the existance of an '@'. Might be a universal method in jslib which can be reused in all clients to validate emails.

[djsmith85]

@cscharf I've had another look thru my changes in #1242 and they are ready for review. I hope I didn't miss anything this time.

Also found another issue in the directory-connector and created a PR for it.