Re "implement in php" - AFAICT, there's no built-in support for websockets in php, but I found at least 2 pure-php implementations out there that might work.

Possible ways to reduce risk for future issues:

• add a Setup Check that runs npm audit
• remove node, use php-based websocket implementation

In D25935#25149, @Cigaryno wrote:

Why would a cancel URI be needed?

In D25935#25135, @aklapper wrote:

After these steps I get Unhandled Exception ("Exception"): This transaction group requires MFA to apply, but the Editor was not configured with a Cancel URI. This workflow can not perform an MFA check.

Why would a cancel URI be needed? Do you know a Cancel URI for an app with something that prompts for MFA (ie. exposing Passphrases, empowering users, signing comments with MFA, managing your VCS password and SSH keys)

In D25935#25144, @Cigaryno wrote:

In D25935#25135, @aklapper wrote:

Which "an application" exactly?

Any application were canUninstall is not set to false (thus not a required application).

That's what I tested (as the Files application can be uninstalled). Which exact application(s) did you test?
I'm surprised that you did not run into the same problem as I did described in my last comment...maybe it's related to not being an admin?

In D25935#25135, @aklapper wrote:

Which "an application" exactly?

Any application were canUninstall is not set to false (thus not a required application).

As which type of user?

A user with the Can Configure Application capability (by default admins).

Clear Test Plans with URIs are welcome - the less others need to think "how/where to do that" the easier gets testing.

I don't know if what I done on D25935 is correct, but as usual, Request Changes if I did it wrong.
I relied on the code on PhabricatorUserEmpowerTransaction to try to make PhabricatorApplicationUninstallTransaction require MFA if enabled.

Make lint happy

Let’s do it

If there are no objections I would be happy to accept the diff. @speck are your concerns addressed or should we continue discussion / consider other options?

In T15965#20144, @valerio.bozzolan wrote:

What is changing is, that unverified email will not match your unverified email as default, so that should need these 2 clicks manual configs (or, find a way to verify the email)

Yep, manually setting your unverified (and not verifiable) email would still be possible 👍 just two clicks are needed from this kind of pages:

Another edge case: Most of my contributions to Phorge happened as part of my work for Wikimedia. Those commits are under an email address that I no longer have access to, since I am no longer employed at the Wikimedia Foundation.

(I cannot edit this task lol - I would like to add Spam mitigation tag to keep an additional eye on these nice things)

Take for example this commit that has a default (empty) identity:

"Steal credit" might actually lead to a real issue: If a new user can get themselves identified as an old, trusted, user based on commit history, their changes might not be checked as rigorously by the rest of the team - similar to the XZ Utils backdoor issue, only faster.

Limitation: to steal a commit identity, it must be the default. Sorry I forgot to say.

In T15965#20052, @speck wrote:

What can a malicious user accomplish by claiming unverified email for commits?

What can a malicious user accomplish by claiming unverified email for commits? The idea outlined here sounds right but I’d like to understand what potential harm could be done on its current state, and also whether there’s any legitimate use case for the current behavior.

Adding @aklapper as subscriber in this security issue since I trust this user (unclear if this should be flagged as security thought, feel free to open)

CVE-2017-5223, CVE-2018-19296 and CVE-2020-36326:

CVE-2021-34551:
This one requires passing user-provided input as a filename to the "setLanguage" method; We don't call that method.

First pass, these one do not apply to us (and some of them do not apply to anyone at all):

(I also cannot see T15665)