fleetdm / fleet

Open-source platform for IT, security, and infrastructure teams. (Linux, macOS, Chrome, Windows, cloud, data center)
https://fleetdm.com
Other
3.1k stars 427 forks source link

Email 2FA verification for admins logging into Fleet w/o SSO #22078

Open noahtalerman opened 1 month ago

noahtalerman commented 1 month ago

Goal

User story
As a security team who is checking out the IT team's new tool,
I want us to be able to use email verification for our “break glass” account in Fleet
so that I can feel confident that we're following security best practices.

Objective

Customer promises + renewal requests

Original request

Context

Changes

Includes updates to creating and editing users, the invitation flow, and introduces a new (optional) magic link (2FA) experience.

Product

Engineering

ℹ️  Please read this issue carefully and understand it. Pay special attention to UI wireframes, especially "dev notes".

QA

Risk assessment

Manual testing steps

  1. Step 1
  2. Step 2
  3. Step 3

Testing notes

Confirmation

  1. [ ] Engineer (@____): Added comment to user story confirming successful completion of QA.
  2. [ ] QA (@____): Added comment to user story confirming successful completion of QA.
noahtalerman commented 1 month ago

Hey @sharon-fdm! Who would be a good engineer for @randy-fleet to partner w/ while designing this one?

sharon-fdm commented 1 month ago

All our BE engineers can help, but @lucasmrod did several tasks in/around this area in the past.

noahtalerman commented 1 month ago

FYI @randy-fleet ^^

randy-fleet commented 1 month ago

@noahtalerman I added a note about reference doc changes needed, and noted where there are no changes needed for Activity and Permissions, but am unsure if there will be CLI/YAML/API changes. Can you help clarify there?

noahtalerman commented 1 month ago

@randy-fleet thanks!

There are no YAML changes but I think we do want to make CLI changes: add a new flag to the fleetctl user create command. Can you please help come up w/ a proposed name for this flag?

fleetctl is Fleet's CLI tool. I suggest following the guide here to download it so you can play around and see the existing options.

To see all options (flags) available for fleetctl user create you can run fleetctl user create -h (-h for help) in your Terminal. This is what the output should look like:

Screenshot 2024-09-24 at 4 20 46 PM
noahtalerman commented 1 month ago
  • [ ] REST API changes: TODO: Update the existing user API endpoints
  • [ ] Reference documentation changes: TODO: Need to add optional 2FA setting in Users API docs

@sharon-fdm this story is almost ready for specs.

To alleviate some design capacity and move quickly, I think we'll want the engineering team's help designing the API changes and updating the API docs.

To track this, I moved these checkboxes (above and in the issue description) to the engineering section.

Please let me know if you have questions/concerns.

noahtalerman commented 1 month ago

FYI @randy-fleet ^

sharon-fdm commented 1 month ago

@noahtalerman, no problem, we will try to allocate some cycles for this soon.

cc: @lucasmrod

sharon-fdm commented 1 month ago

@noahtalerman, @lucasmrod is assigned to help with the API design and will get to it after some P2 work.

randy-fleet commented 1 month ago

@noahtalerman I got fleetctl up and running and proposed the new flag on the ticket - thanks!

noahtalerman commented 1 month ago

Thanks @randy-fleet!

I think we want to be more explicit than 2fa. Also, not sure about 2FA. I've also seen "MFA" a lot when Googling / talking to folks. Maybe something like --email-verification? or --email-2fa? or --email-mfa?

Right now, as someone creating a user w/ fleetctl, I'm not sure what --2fa means exactly. What does this mean for the user I'm creating?

We could solve this by explaining that it's email in the description. But what if we add an option for 2FA via authenticator app later? I think we want to leave the door open for another explicit flag.

Also, note on an interesting design pattern for command line tools: command line tools often have a double dash -- before the flag (option) like --email-verification. Commands themselves don't have the -- like the user command in fleetctl create user.

noahtalerman commented 1 month ago

@rachaelshaw if you have time, and Lucas hasn't gotten to API design, I would pick this one up after you get through API design review.

rachaelshaw commented 1 month ago

@noahtalerman @lucasmrod API design PR here, added you both as reviewers. I kept "email" out of the new key name and just called it two_factor_authentication_enabled, so we have room to add other means of 2fa in the future by adding an additional key e.g. two_factor_authentication_type.

rachaelshaw commented 1 month ago

@randy-fleet @noahtalerman noticed one potential issue in the designs that I wasn't sure y'all had talked about: we're specifying that the email should be updated to say "Hello {First name}", but the form asks for a user's full name as one field. Inferring first/last names from full names can be tricky since you can't always rely on the location of spaces (e.g. "Mary Jane Van der Henst") so unless we already have code for handling that somewhere in the product, it may make sense to just do "Hello {Full name}".

noahtalerman commented 1 month ago

@rachaelshaw great catch! I think let's go w/ what's simple for now. Sounds like that's full name.

I updated the Figma here: Screenshot 2024-10-01 at 1 50 52 PM

cc @randy-fleet

lucasmrod commented 1 month ago

API design PR https://github.com/fleetdm/fleet/pull/22526, added you both as reviewers. I kept "email" out of the new key name and just called it two_factor_authentication_enabled, so we have room to add other means of 2fa in the future by adding an additional key e.g. two_factor_authentication_type.

Looks good!

We still need the API changes for the /login endpoint, right?:

  1. /login to not return token when 2FA is enabled for the user (or maybe return some flag that this is a 2FA user so that the UI can render the correct dialog).
  2. /login to accept some random token generated by Fleet (that links to the session created in step 1).
noahtalerman commented 1 month ago

cc @rachaelshaw ^^

sharon-fdm commented 1 month ago

We will be able to estimate essuming @rachaelshaw will add the "token" field to the Login API as required by @lucasmrod. (Moving to Specified column)

noahtalerman commented 4 weeks ago

Hey @zayhanlon heads up, this user story didn't make it into the upcoming engineering sprint because we didn't get it estimated in time.

It's still prioritized. We left it on the drafting board so that it can be pulled into the next engineering sprint.

rachaelshaw commented 3 weeks ago

@noahtalerman @randy-fleet did we spec out what the user sees when the magic link is expired? Here's what we show for expired links in the default Sails app: https://github.com/fleetdm/fleet/blob/main/website/views/498.ejs Maybe we should add a similar kind of error page if we don't have one already? (Or are we planning to just redirect to the login page?)

noahtalerman commented 3 weeks ago

did we spec out what the user sees when the magic link is expired?

Hey @rachaelshaw great catch. I don't think we did.

I added a note to today's design review to discuss.

randy-fleet commented 3 weeks ago

@rachaelshaw Thanks for highlighting! Do you happen to have a screenshot, or something that would show me how this current experience looks?

rachaelshaw commented 2 weeks ago

@randy-fleet worked out the error page copy w/ Noah the other day and added to the design. (Modeled after this one.)

noahtalerman commented 2 weeks ago

We want this to be a Fleet Premium feature. I updated the pricing page in the reference docs PR here (accidentally committed directly to the PR).

@rachaelshaw can you please check to see if we need to update Figma and the reference docs PRs and make changes if needed?

Can you also please take on the CLI changes? I'm not sure what the pattern is for calling out premium only flags. Do we hide them? Or show some "premium only" message.

cc @sharon-fdm

rachaelshaw commented 2 weeks ago

@noahtalerman follow-up for API docs: https://github.com/fleetdm/fleet/commit/4a373b061440504ee20d1c84af045380807a8a55 (I also accidentally committed directly to the branch 🤪)

Also updated dev notes in Figma to clarify the new field is only for Fleet Premium.