Allow admins to disable the 2FA of users in subdomains

[winterhazel]

Description

Admins should be able to disable the 2FA of users that lost their second factor; however, the setupUserTwoFactorAuthentication API only allows them to disable the 2FA of users in the same domain. This means that, if the root admin wants to disable the 2FA of a user in ROOT/test, he must create a domain admin account in ROOT/test and log into that account to do this.

This PR addresses this issue by allowing admins to disable the 2FA of users in their subdomains.

Types of changes

• Breaking change (fix or feature that would cause existing functionality to change)
• New feature (non-breaking change which adds functionality)
• Bug fix (non-breaking change which fixes an issue)
• Enhancement (improves an existing feature and functionality)
• Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

• Major
• Minor

Bug Severity

• BLOCKER
• Critical
• Major
• Minor
• Trivial

Screenshots (if appropriate):

How Has This Been Tested?

I did the following tests in a local lab:

Number	Test	Previous result	New result	New result was expected (Y/N)?
1	Disable your own 2FA as the root admin	Success	Success	Y
2	Disable the 2FA of a user in ROOT as the root admin	Success	Success	Y
3	Disable the 2FA of a user in ROOT/test as the root admin	Exception	Success	Y
4	Disable your own 2FA as the domain admin of ROOT/test	Success	Success	Y
5	Disable the 2FA of a user in ROOT/test as the domain admin of ROOT/test	Success	Success	Y
6	Disable the 2FA of a user in ROOT/test/test2 as the domain admin of ROOT/test	Exception	Success	Y
7	Disable the 2FA of a domain admin of ROOT/test as another domain admin of ROOT/test	Success	Success	Y
8	Disable the 2FA of a domain admin of ROOT/test/test2 as the domain admin of ROOT/test	Exception	Success	Y
9	Disable the 2FA of a domain admin of ROOT as the domain admin of ROOT/test	Exception	Exception	Y
10	Disable the 2FA of a domain admin of ROOT/test3 as the domain admin of ROOT/test	Exception	Exception	Y
11	Disable your own 2FA as a non-administrator user	Success	Success	Y

[codecov]

Codecov Report

Merging #7870 (776dd8e) into 4.18 (19aacad) will decrease coverage by 0.01%.
Report is 1 commits behind head on 4.18.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               4.18    #7870      +/-   ##
============================================
- Coverage     13.02%   13.02%   -0.01%     
+ Complexity     9040     9039       -1     
============================================
  Files          2720     2720              
  Lines        257094   257092       -2     
  Branches      40092    40091       -1     
============================================
- Hits          33491    33489       -2     
- Misses       219398   219400       +2     
+ Partials       4205     4203       -2     

Files Changed	Coverage Δ
...c/main/java/com/cloud/user/AccountManagerImpl.java	20.47% <100.00%> (+0.02%)	⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[DaanHoogland]

In addition to @harikrishna-patnala 's comment, I wonder if scenario 11 is really what we want?
As an operator I want to force my users to use f2a.
scenario 11 negates that.
what do you think @winterhazel ?

[weizhouapache]

code lgtm

[weizhouapache]

In addition to @harikrishna-patnala 's comment, I wonder if scenario 11 is really what we want? As an operator I want to force my users to use f2a. scenario 11 negates that. what do you think @winterhazel ?

good point @DaanHoogland

@winterhazel can you test if mandate.user.2fa is set to true ?

[winterhazel]

In addition to @harikrishna-patnala 's comment, I wonder if scenario 11 is really what we want? As an operator I want to force my users to use f2a. scenario 11 negates that. what do you think @winterhazel ?

good point @DaanHoogland

@winterhazel can you test if mandate.user.2fa is set to true ?

@DaanHoogland @weizhouapache

You can force users to use 2FA by enabling the global setting mandate.user.2fa. If this setting is set to true, when a user disables his 2FA through the setupUserTwoFactorAuthentication API, he will need to reconfigure it next time he logins in order to proceed. I have tested it.

However, I think we can reconsider the behavior of this API to not allow users to disable their 2FA in the first place when this setting is enabled and return a message saying that 2FA is mandatory, since users may think the current behavior is a bug.

[harikrishna-patnala]

In addition to @harikrishna-patnala 's comment, I wonder if scenario 11 is really what we want? As an operator I want to force my users to use f2a. scenario 11 negates that. what do you think @winterhazel ?

good point @DaanHoogland
@winterhazel can you test if mandate.user.2fa is set to true ?

@DaanHoogland @weizhouapache

You can force users to use 2FA by enabling the global setting mandate.user.2fa. If this setting is set to true, when a user disables his 2FA through the setupUserTwoFactorAuthentication API, he will need to reconfigure it next time he logins in order to proceed. I have tested it.

However, I think we can reconsider the behavior of this API to not allow users to disable their 2FA in the first place when this setting is enabled and return a message saying that 2FA is mandatory, since users may think the current behavior is a bug.

I would say we can still allow disabling the 2FA just in case user wants to reset the 2FA settings to use other provider. To provide more visibility to the user we can pop up a warning message when user tries to disable 2FA saying something like "2FA needs to setup again since it is mandated by the admin"

[DaanHoogland]

In addition to @harikrishna-patnala 's comment, I wonder if scenario 11 is really what we want? As an operator I want to force my users to use f2a. scenario 11 negates that. what do you think @winterhazel ?

good point @DaanHoogland
@winterhazel can you test if mandate.user.2fa is set to true ?

@DaanHoogland @weizhouapache
You can force users to use 2FA by enabling the global setting mandate.user.2fa. If this setting is set to true, when a user disables his 2FA through the setupUserTwoFactorAuthentication API, he will need to reconfigure it next time he logins in order to proceed. I have tested it.
However, I think we can reconsider the behavior of this API to not allow users to disable their 2FA in the first place when this setting is enabled and return a message saying that 2FA is mandatory, since users may think the current behavior is a bug.

I would say we can still allow disabling the 2FA just in case user wants to reset the 2FA settings to use other provider. To provide more visibility to the user we can pop up a warning message when user tries to disable 2FA saying something like "2FA needs to setup again since it is mandated by the admin"

This makes sense @harikrishna-patnala , but would not have to be included in this PR. It seems like a separate improvement.

cc @weizhouapache @winterhazel ??

[weizhouapache]

In addition to @harikrishna-patnala 's comment, I wonder if scenario 11 is really what we want? As an operator I want to force my users to use f2a. scenario 11 negates that. what do you think @winterhazel ?

good point @DaanHoogland
@winterhazel can you test if mandate.user.2fa is set to true ?

@DaanHoogland @weizhouapache
You can force users to use 2FA by enabling the global setting mandate.user.2fa. If this setting is set to true, when a user disables his 2FA through the setupUserTwoFactorAuthentication API, he will need to reconfigure it next time he logins in order to proceed. I have tested it.
However, I think we can reconsider the behavior of this API to not allow users to disable their 2FA in the first place when this setting is enabled and return a message saying that 2FA is mandatory, since users may think the current behavior is a bug.

I would say we can still allow disabling the 2FA just in case user wants to reset the 2FA settings to use other provider. To provide more visibility to the user we can pop up a warning message when user tries to disable 2FA saying something like "2FA needs to setup again since it is mandated by the admin"

This makes sense @harikrishna-patnala , but would not have to be included in this PR. It seems like a separate improvement.

cc @weizhouapache @winterhazel ??

agree with @DaanHoogland

[weizhouapache]

code lgtm

[weizhouapache]

@blueorangutan package

[blueorangutan]

@weizhouapache a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

[blueorangutan]

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6826

[weizhouapache]

@blueorangutan test

[blueorangutan]

@weizhouapache a [SF] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

[winterhazel]

In addition to @harikrishna-patnala 's comment, I wonder if scenario 11 is really what we want? As an operator I want to force my users to use f2a. scenario 11 negates that. what do you think @winterhazel ?

good point @DaanHoogland
@winterhazel can you test if mandate.user.2fa is set to true ?

@DaanHoogland @weizhouapache
You can force users to use 2FA by enabling the global setting mandate.user.2fa. If this setting is set to true, when a user disables his 2FA through the setupUserTwoFactorAuthentication API, he will need to reconfigure it next time he logins in order to proceed. I have tested it.
However, I think we can reconsider the behavior of this API to not allow users to disable their 2FA in the first place when this setting is enabled and return a message saying that 2FA is mandatory, since users may think the current behavior is a bug.

I would say we can still allow disabling the 2FA just in case user wants to reset the 2FA settings to use other provider. To provide more visibility to the user we can pop up a warning message when user tries to disable 2FA saying something like "2FA needs to setup again since it is mandated by the admin"

This makes sense @harikrishna-patnala , but would not have to be included in this PR. It seems like a separate improvement.

cc @weizhouapache @winterhazel ??

This is a valid reason to allow users to disable their 2FA. I think @harikrishna-patnala's suggestion of providing more information to the user is great. We can handle it in a separate PR, though.

[blueorangutan]

[SF] Trillian test result (tid-7478)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 45243 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7870-t7478-kvm-centos7.zip
Smoke tests completed. 107 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_08_upgrade_kubernetes_ha_cluster	Failure	691.06	test_kubernetes_clusters.py

[DaanHoogland]

clgtm

[weizhouapache]

tested ok