silverstripe / silverstripe-mfa

MultiFactor Authentication for Silverstripe CMS
BSD 3-Clause "New" or "Revised" License
11 stars 25 forks source link

Disable MFA by default, enable via CMS setting #359

Closed chillu closed 5 years ago

chillu commented 5 years ago

Installing SilverStripe modules is a developer activity, and often decided by developers where website owners don't understand the complexities of composer and dependency management. This often leads to modules not being installed by risk adverse devs, and website owners misunderstanding what they're getting e.g. in a recipe update ("I thought this was included by default"). Given how expensive any composer update can be once you factor in regression testing and vendor engagements, they're unlikely to happen often.

In the case of MFA, this means a lower adoption unless the module is installed in many codebases already, and the decision to use it can be taken by the website owner (not a developer). Currently, the opt-in happens on a dev level via a composer require, and the modules auto-register and enable. I'm proposing that we flip this around, and leave the installed modules disabled by default, with an option to enable them through a CMS setting. This way, the website owners might not even know they have the MFA module installed (through a recipe upgrade), but once they become aware of the security benefits, they can act without expensive dev time.

Context: This is currently blocking inclusion of the MFA module in default recipes on the CMS 4.x and CWP 2.x release lines, since we don't want to enable MFA on every site running a recipe upgrade (we just want to give them the option to do so without technical blockers).

By adding this to cwp/cwp-recipe-core rather than cwp/cwp-installer, we'll add this feature to more codebases over time. We also make it harder to remove this code from your own project codebase, although it's still possible. It's a tradeoff: Either we add it to cwp/cwp-installer for new projects and rely on devs and clued-up website owners to explicitly ask for the new dependency install on existing projects, or we get the dependency installed through a recipe upgrade and empower the website owners to enable it directly.

Note that I treat the three MFA modules as "one system" here. I believe that every website owner should be able to decide whether they want to allow TOTP, WebAuthn, or both. Installing silverstripe/mfa only is pointless without either of them, and I don't think it should be up to devs to decide that "TOTP is good enough". From a business perspective, website owners can still decide on which methods to allow, and website users can decide which ones suits their needs. If we leave this up to devs, it'll always be a "the least amount of code/features which has been explicitly asked for" :)

Outside of CWP, the TOTP method needs some configuration (base secret) to work, but it's already disabled if that's not present, so won't lead to a bad user experience.

michalkleiner commented 5 years ago

Just an aside note — not sure if this sentence conveys what the author intended:

I don't think it should be up to devs to decide that "TOTP is good enough".

Devs (or better, vendors) are there to advise business owners on the options, explain implications and discuss the possibilities, and then action them as best as possible. To me, as it's written now, it sort of feels like SS makes the decision and leaves vendors out of the equation.

brynwhyman commented 5 years ago

Can you clarify what the intended outcome of disabling it by default would be?

Bit of an essay disagreeing with this below, but TL;DR:

Current functionality will, on install, enable MFA as optional. This will present the MFA dialog to everyone the first time they log in (with the option to skip). To me this seems like a sensible default after a site upgrade which is clearly communicated will include MFA functionality.

Even if a developer doesn't read the change logs and updates the site, it would be pretty clear on UAT and discussions could be had before a site release. The above allows individuals to register MFA methods for their own account before it's enforced by a site owner. There's an included report that monitors uptake, and the registration flow gives sufficient guidance for those not familiar with MFA.

The MFA module bundles the login-forms module which will override any existing login page (Security.ss) template. This could be a detail that warrants the MFA functionality being installed but embargoed until further approval - but I'd expect this is something a development agency should be involved in, instead of a CMS admin enabling it and blindly breaking a custom login form. This discussion could easily be had at the time of a site upgrade.

However, if for whatever reason the dev agency doesn't think MFA is suitable for a site on upgrade, there is the opportunity to disable the login redirect to MFA via yaml, without having to alter their composer.json file.

Sorry for a bit of internal back and forth here, but during design and development we spent a good amount of time considering whether MFA should be enabled or disabled by default. After much deliberation we chose 'enabled but optional' as the most practical and secure default that gives individuals the option and CMS admins the responsibility for enforcing.

ScopeyNZ commented 5 years ago

I think the really important distinction here is required vs available. If MFA is installed and somehow it gets past the agency without them realising there's a change, it can technically be dismissed on first login and completely ignored. The security conscious individuals may choose to configure TOTP if they want though.

chillu commented 5 years ago

There's different levels of decision making:

Introducing the new login forms alongside MFA is making it more difficult to include it by default. Alright, I'm going to close this issue as "too hard". The responsibility remains on the Vendor, Dev and Website Owner roles to ensure that websites they operate include optional modules that make them secure.