search

silverstripe / silverstripe-mfa

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

NEW Disable MFA in dev environments by default #419

Closed chillu closed 3 years ago

chillu commented 3 years ago

I can't see how that's a good default for anyone, especially since it also applies to the SS_DEFAULT_ADMIN users which are very common for local development.

While it's possible for devs to opt-out of this already, the defaults are the wrong way around in my view.

stevie-mayhew commented 3 years ago

I actually disagree with this - you should dogfood your own environment. Unsure how many people use "dev" in staging environments and UAT, and what impact this would have so it seems "bad" (happy to be overridden)

Cheddam commented 3 years ago

I agree with @stevie-mayhew - perhaps adding the option to disable it via an environment variable would be a better approach? This would retain the ability to switch it off "by default" via your local .env file.

chillu commented 3 years ago

Unsure how many people use "dev" in staging environments and UAT

I don't think that's a strong reason against this change, the roles of the different environment types are pretty clear. You'll have other security issues if you mix dev mode in environments which are more "sensitive" than your localhost.

Thanks for the quick review! I still think that the value of having MFA on localhost is close to zero, and has already wasted countless dev hours to figure out how to disable it. But my concern is also addressed through an environment variable, or rather making it more obvious on how to disable it (rather than on page six of a doc with a relatively obscure name). I've opened a new PR with this approach: https://github.com/silverstripe/silverstripe-mfa/pull/420