ssc-spc-ccoe-cei / azure-guardrails-solution-accelerator

This implementation automates reporting to verify compliance with GC Cloud Guardrails. SSC and TBS review the results. Cette mise en œuvre automatise la production de rapports afin de vérifier la conformité aux mesures de sécurité infonuagique du GC. SPC et SCT examinent les résultats.
Other
11 stars 4 forks source link

[Bug Fix] GR1 & Gr3 - list the UPNs having non-enabled MFA #89

Closed dutt0 closed 9 months ago

dutt0 commented 9 months ago

Overview/Summary

This pull request fixes the reporting bug in 163gccspm for GR1 & Gr3 GA MFA UPN list feature.

This PR fixes/adds/changes/removes

In total, there should be 4 types of authentication methods to consider:

  1. microsoft.graph.microsoftAuthenticatorAuthenticationMethod

  2. microsoft.graph.phoneAuthenticationMethod

  3. microsoft.graph.passwordAuthenticationMethod

  4. microsoft.graph.emailAuthenticationMethod

This PR has the fix for the MFA authentication by including two more authentication methods that users may have. Since dev tenant users did not have password and email authentication methods, this issue was not prominent in that tenant. However, the users in Test tenant carries these MFA. This pull request has the fix for this use case.

Testing Evidence

image

As part of this Pull Request I have

dutt0 commented 9 months ago

@alalvi00 As you mentioned yesterday, you have #microsoft.graph.passwordAuthenticationMethod and #microsoft.graph.AuthenticatorAuthenticationMethod and both are enabled for MFA. This logic goes by that perspective.

alalvi00 commented 9 months ago

@alalvi00 As you mentioned yesterday, you have #microsoft.graph.passwordAuthenticationMethod and #microsoft.graph.AuthenticatorAuthenticationMethod and both are enabled for MFA. This logic goes by that perspective.

Thats fine but a user will also pass if they only have password and email and I have tested this in the test tenant

dutt0 commented 9 months ago

⁠Just for the reference in localexecution this is what I see for $comments and the ComplianceStatus is also false. Thanks for your testing and giving reviews. I will discuss the logic in tomorrow's meeting and thereafter finalize the PR

Testcase 1 : -<testuser's upn>@xxxxxxxxx.onmicrosoft.com -< ila's upn>@xxxxxxxxx.onmicrosoft.com

Testcase 2 : -<testuser's upn>@xxxxxxxxx.onmicrosoft.com -<Ali's upn>@xxxxxxxxx.onmicrosoft.com

image

dutt0 commented 9 months ago

Thanks for your reviews @alalvi00 @singhgss. I am refactoring this function. Cancelling this PR. I will raise another PR later