Skip to content
This repository has been archived by the owner on Oct 20, 2021. It is now read-only.

Include MFA modules #35

Closed
3 tasks done
brynwhyman opened this issue Mar 9, 2020 · 12 comments
Closed
3 tasks done

Include MFA modules #35

brynwhyman opened this issue Mar 9, 2020 · 12 comments

Comments

@brynwhyman
Copy link

brynwhyman commented Mar 9, 2020

Overview

With the next minor release of CWP, the multi-factor authentication module suite should be included for all new sites built with the cwp-installer.

What's in the MFA module suite?

Acceptance Criteria

  • Take the MFA and TOTP modules out of kitchen-sink (although leave the WebAuthn module
  • The next minor release of CWP includes the following modules in the cwp-installer module: Silverstripe MFA and TOTP
  • The test for the MFA module are considered and placed in the appropriate module for CI, i.e stay in kitchen-sink and/or move to cwp-installer?
  • The change log for the associated CWP release includes clear guidance on:
    • What MFA functionality will be included
    • Direct readers to documentation on getting the modules running on a local development environment

Notes

Pull Requests

@chillu
Copy link
Member

chillu commented Mar 15, 2020

@brynwhyman Is this a duplicate of the Epic: MFA is in CWP by default?

@brynwhyman
Copy link
Author

Is this a duplicate of the Epic: MFA is in CWP by default?

@chillu no, https://github.com/silverstripeltd/product-issues/issues/153 was created as an internal epic to capture all related work under one card.

@brynwhyman
Copy link
Author

I'd like to suggest that we leave out the Security token option as what's included in the CWP module by default. There's a number of edge cases and potential for havoc without the site owner and development agency taking the time to understand these before requesting this option, including:

  • Security key cannot be used over different domains connected to the same CMS (accessing a subsite /admin URL over a different top-level domain)
  • Security key needs to be reset for all users following a snapshot restore between different Silverstripe platform environments (where the top-level domain has changed). E.g. promoting UAT content to Prod.
  • Security key needs to be reset for all users following a general domain change for the site (company-name.com to new-company-name.com)

There's another open issue suggesting to disable the security token functionality if the subsites module is installed, but that's not covering all the edge cases. I believe something like this should still happen, but it shouldn't block sites getting access to the much more popular and accessible MFA method, TOTP.

@brynwhyman
Copy link
Author

@Cheddam has noted that there's at least one issue in the login-forms module that we'll want to look at as part of this issue. Will add to our internal epic.

@brynwhyman
Copy link
Author

We're talking about a few different options to actually make this happen. Should we:

  • add it to cwp-recipe-core
  • add it to cwp-recipe-cms
  • add it to cwp-installer (the upgrade instructions that's provided in each release references the modules in the cwp-installer). Noting that this will probably only serve new site builds

Or:

  • We keep it as effectively 'opt-in' as it's own module set and push it hard in the release documentation.

@brynwhyman brynwhyman changed the title Include MFA modules in this recipe Include MFA modules May 13, 2020
@chillu
Copy link
Member

chillu commented Jun 3, 2020

Bryn mentioned internally that the current inclination is to leave it out of the recipes, which would introduce silverstripe/mfa and silverstripe/login-forms on minor upgrades. Reasons:

  • While MFA can stay disabled by default, login-forms will be active. Note that we've added login-forms in silverstripe/installer (outside of cwp) rather than the recipes, so it won't be automatically included on upgrades there either.
  • With login-forms active by default, it'll override any custom login form styling. That's kind of the point of this module (and necessitated by the complex styling required during the MFA flows), but also might not be desired by customers
  • login-forms might also mess with any custom SSO flows
  • mfa is not compatible with LDAP, RealMe or SAML based logins

So in summary, I recommend that we use the third option described here:

add it to cwp-installer (the upgrade instructions that's provided in each release references the modules in the cwp-installer). Noting that this will probably only serve new site builds

Also, the installer would only include the TOTP MFA method, not the WebAuthn method since that requires a bit more thought by developers (subsites, multi env usage). This should be outlined in the upgrade docs though, to ensure that WebAuthn is actually considered. It's far more secure than TOTP (lower phishing potential, domain verification built in)

@chillu
Copy link
Member

chillu commented Jun 3, 2020

I'm also advocating for the same approach in core: Add this to the installer rather than recipes. silverstripe/silverstripe-installer#280

@brynwhyman
Copy link
Author

Thanks for the comments @chillu, I agree that pushing it through the CWP-installer is still a good thing to do.

Do you have any ideas on how to ensure that Developers get the TOTP encryption key .env variable? It's on the CWP platform by default so a deployed site would be covered, but can we only rely on documentation to ensure Developers are aware of this?

@brynwhyman brynwhyman transferred this issue from silverstripe/cwp Jun 3, 2020
@brynwhyman
Copy link
Author

I've moved this issue to the cwp-installer repository and update the description to focus on getting the modules added to this repo and some related changelog notes.

Other points around futher documentation will be captured in this issue: silverstripe/cwp#269

@chillu
Copy link
Member

chillu commented Jun 3, 2020

Do you have any ideas on how to ensure that Developers get the TOTP encryption key .env variable?

Within the scope of CWP, it's a default on Revera, and hopefully soon a default on AWS. Until that point, I think we need to add something to https://www.cwp.govt.nz/developer-docs/en/2/getting_started/

@brynwhyman
Copy link
Author

Alright, we'll have that covered in silverstripe/cwp#269

@brynwhyman brynwhyman added this to the Sprint 61 milestone Jun 8, 2020
bergice added a commit to creative-commoners/cwp-recipe-kitchen-sink that referenced this issue Jun 9, 2020
bergice added a commit to creative-commoners/cwp-installer that referenced this issue Jun 9, 2020
This was referenced Jun 9, 2020
@brynwhyman
Copy link
Author

The change log for the associated CWP release includes clear guidance on:

I think this AC can be removed, it's already covered in: silverstripe/cwp#269

@dnsl48 dnsl48 self-assigned this Jun 10, 2020
bergice added a commit to creative-commoners/cwp-installer that referenced this issue Jun 11, 2020
bergice added a commit to creative-commoners/cwp-recipe-kitchen-sink that referenced this issue Jun 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants