_enable_email_2fa default not set as expected #449

New Issue

OVERLORD · 2026-02-04T20:36:22+03:00

OVERLORD commented

2026-02-04 20:36:22 +03:00

Originally created by @vplme on GitHub (Oct 16, 2019).

The config option for _enable_email_2fa:

 _enable_email_2fa:      bool,   true,   auto,    |c| c._enable_smtp && c.smtp_host.is_some();

means that when SMTP is enabled (default true) and the SMTP_HOST is filled, 2FA email should also be enabled. This is not the case for me.

If I move the Email 2FA config block to after the SMTP config block it does work as expected.
i.e. this works:

....
    /// Password
        smtp_password:          Pass,   true,   option;
        /// Json form auth mechanism |> Defaults for ssl is "Plain" and "Login" and nothing for non-ssl connections. Possible values: ["Plain", "Login", "Xoauth2"]
        smtp_auth_mechanism:    String, true,   option;
    },

      /// Email 2FA Settings
    email_2fa: _enable_email_2fa {
        /// Enabled |> Disabling will prevent users from setting up new email 2FA and using existing email 2FA configured
        _enable_email_2fa:      bool,   true,   auto,    |c| c._enable_smtp && c.smtp_host.is_some();
        /// Token number length |> Length of the numbers in an email token. Minimum of 6. Maximum is 19.
        email_token_size:       u32,    true,   def,      6;
        /// Token expiration time |> Maximum time in seconds a token is valid. The time the user has to open email client and copy token.
        email_expiration_time:  u64,    true,   def,      600;
        /// Maximum attempts |> Maximum attempts before an email token is reset and a new email will need to be sent
        email_attempts_limit:   u64,    true,   def,      3;
    },

Branch: https://github.com/dani-garcia/bitwarden_rs/compare/master...vverst:enable-2fa-email

I'm not familiar with macros but I'm assuming it evaluates the closure before the SMTP config block has been configured and stores that result.

Should we just move the configuration or is there a nicer solution? Maybe evaluating the closures once the initial configuration has been set?

Originally created by @vplme on GitHub (Oct 16, 2019). The config option for `_enable_email_2fa`: ```rust _enable_email_2fa: bool, true, auto, |c| c._enable_smtp && c.smtp_host.is_some(); ``` means that when SMTP is enabled (default true) and the SMTP_HOST is filled, 2FA email should also be enabled. This is not the case for me. If I move the Email 2FA config block to after the SMTP config block it does work as expected. i.e. this works: ```rust .... /// Password smtp_password: Pass, true, option; /// Json form auth mechanism |> Defaults for ssl is "Plain" and "Login" and nothing for non-ssl connections. Possible values: ["Plain", "Login", "Xoauth2"] smtp_auth_mechanism: String, true, option; }, /// Email 2FA Settings email_2fa: _enable_email_2fa { /// Enabled |> Disabling will prevent users from setting up new email 2FA and using existing email 2FA configured _enable_email_2fa: bool, true, auto, |c| c._enable_smtp && c.smtp_host.is_some(); /// Token number length |> Length of the numbers in an email token. Minimum of 6. Maximum is 19. email_token_size: u32, true, def, 6; /// Token expiration time |> Maximum time in seconds a token is valid. The time the user has to open email client and copy token. email_expiration_time: u64, true, def, 600; /// Maximum attempts |> Maximum attempts before an email token is reset and a new email will need to be sent email_attempts_limit: u64, true, def, 3; }, ``` Branch: https://github.com/dani-garcia/bitwarden_rs/compare/master...vverst:enable-2fa-email I'm not familiar with macros but I'm assuming it evaluates the closure before the SMTP config block has been configured and stores that result. Should we just move the configuration or is there a nicer solution? Maybe evaluating the closures once the initial configuration has been set?

OVERLORD closed this issue

2026-02-04 20:36:25 +03:00

OVERLORD commented

2026-02-04 20:36:32 +03:00

@dani-garcia commented on GitHub (Oct 16, 2019):

Yeah the options are parsed in order so if the closure is placed before the settings it references it won't work.

I think for now swapping the position of the config blocks is fine. We could do as you say and run two passes through the configuration but it would complicate the code without much benefit (as long as all the auto settings are placed after the fields they reference it won't have any effect).

Can you make a PR to swap the blocks?

@dani-garcia commented on GitHub (Oct 16, 2019): Yeah the options are parsed in order so if the closure is placed before the settings it references it won't work. I think for now swapping the position of the config blocks is fine. We could do as you say and run two passes through the configuration but it would complicate the code without much benefit (as long as all the auto settings are placed after the fields they reference it won't have any effect). Can you make a PR to swap the blocks?

OVERLORD commented

2026-02-04 20:36:38 +03:00

@vplme commented on GitHub (Oct 16, 2019):

Ok. I made a PR.

@vplme commented on GitHub (Oct 16, 2019): Ok. I made a PR.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/vaultwarden#449

_enable_email_2fa default not set as expected #449

`_enable_email_2fa` default not set as expected #449