Differential D20045

Add a warning about MFA requirements to edit forms
ClosedPublic
Actions

Authored by epriestley on Jan 28 2019, 6:29 PM.

Details

Reviewers

amckinley

Maniphest Tasks

T13242: 2019 Week 5 Bonus Content

Commits

rP2374c92544e0: Add a warning about MFA requirements to edit forms

Summary

Depends on D20044. Ref T13242. Similar to D20044, add reminder text to edit forms.

It would be nice to "workflow" these so the MFA flow happens inline, but Maniphest's inline edit behavior currently conflicts with this. Set it aside for now since the next workboards iteration (triggers) is probably a good opportunity to revisit it.

Test Plan

Screen Shot 2019-01-28 at 10.27.40 AM.png (329×699 px, 26 KB)

Diff Detail

Repository

rP Phabricator

Branch

mfab2

Lint

Lint Passed

Severity	Location	Code	Message
Advice	src/applications/transactions/editengine/PhabricatorEditEngine.php:1310	XHP16	TODO Comment

Unit

Tests Passed

Build Status

Buildable 21724
Build 29639: Run Core Tests
Build 29638: arc lint + arc unit

Event Timeline

epriestley created this revision.Jan 28 2019, 6:29 PM

Harbormaster completed remote builds in B21724: Diff 47870.Jan 28 2019, 6:31 PM

epriestley requested review of this revision.Jan 28 2019, 6:31 PM

This change looks good, but what exactly does it accomplish? I feel like this would be useful if a user was trying to take an action that required MFA, but their account had no MFA factors configured, but is such a thing even possible? Otherwise this feels like a message that we train users to ignore whenever they see it.

Oh, there's more context in D20044, hang on.

Ok I'm on board.

This revision is now accepted and ready to land.Jan 29 2019, 2:38 AM

Yeah, D20044 is the better version of this.

This is just a hint like "go dig your phone out of your bag before writing your entire memoir into this box". The warnings in D20044 are more substantive/useful.

Closed by commit rP2374c92544e0: Add a warning about MFA requirements to edit forms (authored by epriestley). · Explain WhyJan 30 2019, 2:21 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Path

Size

src/

applications/

transactions/

editengine/

PhabricatorEditEngine.php

27 lines

Diff 47870

View Options

src/applications/transactions/editengine/PhabricatorEditEngine.php

Add a warning about MFA requirements to edit formsClosedPublicActions