dani-garcia / vaultwarden

Unofficial Bitwarden compatible server written in Rust, formerly known as bitwarden_rs
GNU Affero General Public License v3.0
38.47k stars 1.87k forks source link

Bug: Signups_domains_whitelist does not wrk #898

Closed Brainscrewer closed 4 years ago

Brainscrewer commented 4 years ago

Subject of the issue

The 'Allow signups only from this list of comma-separated domains'-feature does not work as intended. If you only have added '@company.com' as allowed e-mailadress you can still register with @gmail.com for example.

Your environment

Steps to reproduce

Default configuration.

Expected behaviour

@company.com should only be able to register if configured.

Actual behaviour

Any e-mailaddress can be used to register.

Relevant logs

tomuta commented 4 years ago

I suspect this is a configuration issue. Domain names do not contain an '@' symbol. It is not an email address whitelist. See the examples in the documentation:

https://github.com/dani-garcia/bitwarden_rs/blob/70f3ab8ec3d6ccfd8ec8c71c888459de484d9b43/.env.template#L113-L115

Can you share the exact value you're using for SIGNUPS_DOMAINS_WHITELIST?

jjlin commented 4 years ago

@Brainscrewer Take a look at https://github.com/dani-garcia/bitwarden_rs/wiki/Disable-registration-of-new-users.

Brainscrewer commented 4 years ago

@tomuta @jjlin thanks for your reply. My config looks the following:

SIGNUPS_ALLOWED | false INVITATIONS_ALLOWED | false SIGNUPS_DOMAINS_WHITELIST | company.nl

Even with these values set, it's still possible to register an account using a Gmail-account.

jjlin commented 4 years ago

@Brainscrewer Check whether you have a config.json file in your data dir. The values in this file override the values of any corresponding env vars you have set.

Brainscrewer commented 4 years ago

@jjlin Oh wow, thanks for poiting that out! Only after you poiting that out I actually found out that information is posted on the admin-page as well, doh. After changing the values to the correct values on the admin panel everything seems to be working. Closing this issue.

tbluemel commented 4 years ago

It feels like this should really be fixed. At a minimum I would expect either warnings or an startup error if bitwarden detects mismatches between config.json and the environment variables. Silently preferring one over the other may have severe security complications, e.g. the administrator thought they fixed a bad setup, but the change didn't actually get applied.

I feel like we either should re-open this bug or create a new one, and add code to check for configuration mismatch and reject running, or at a minimum log a big fat warning.

mqus commented 4 years ago

I agree and want to add that a similar warning should be at the top of .env.template, too.

Brainscrewer commented 4 years ago

Some additional checking and/or more explicit warnings would be nice, yes.