Updating validation and test cases to be independent of top level state as that enables/disables all multi factor enrollments

pragatimodi · pragatimodi · commit aa3193fa7159 · 2023-01-19T16:41:53.000-08:00
diff --git a/firebase_admin/_auth_utils.py b/firebase_admin/_auth_utils.py
@@ -330,9 +330,7 @@ def validate_mfa_config(mfa_config):
     mfa_config_payload['state'] = state
 
     # validate factor_ids if MFA is enabled
-    if state == 'ENABLED':
-        if 'factorIds' not in mfa_config:
-            raise ValueError('multiFactorConfig.factorIds must be defined')
+    if 'factorIds' in mfa_config:
         factor_ids = mfa_config['factorIds']
         if not isinstance(factor_ids, list) or not factor_ids:
             raise ValueError(
diff --git a/tests/test_project_config_mgt.py b/tests/test_project_config_mgt.py
@@ -144,12 +144,6 @@ def test_update_project_invalid_mfa_config_state(self, project_config_mgt_app, s
             project_config_mgt.update_project(mfa=mfa_config, app=project_config_mgt_app)
         assert str(excinfo.value).startswith('multiFactorConfig.state must be either "ENABLED" or "DISABLED"')
 
-    def test_update_project_undefined_mfa_config_factor_ids_enabled_state(self, project_config_mgt_app):
-        mfa_config = {'state':'ENABLED'}
-        with pytest.raises(ValueError) as excinfo:
-            project_config_mgt.update_project(mfa=mfa_config, app=project_config_mgt_app)
-        assert str(excinfo.value).startswith('multiFactorConfig.factorIds must be defined')
-
     @pytest.mark.parametrize('factor_ids', [True, False, 1, 0, 'foo', {}, dict(), tuple(), list()])
     def test_invalid_mfa_config_factor_ids_type(self, factor_ids, project_config_mgt_app):
         mfa_config = {'state': 'ENABLED', 'factorIds': factor_ids}
@@ -298,9 +292,10 @@ def test_update_project_valid_mfa_configs(self, project_config_mgt_app):
              mfa=mfa_config_state_disabled,
             app=project_config_mgt_app)
 
-        mfa_config_state_disabled.pop('factorIds')
         _assert_project(project)
-        mask = ['mfa.providerConfigs','mfa.state']
+        mfa_config_state_disabled['enabledProviders'] = mfa_config_state_disabled['factorIds']
+        mfa_config_state_disabled.pop('factorIds')
+        mask = ['mfa.enabledProviders','mfa.providerConfigs','mfa.state']
         self._assert_request(recorder, {
             'mfa': mfa_config_state_disabled
         }, mask)
diff --git a/tests/test_tenant_mgt.py b/tests/test_tenant_mgt.py
@@ -304,12 +304,6 @@ def test_invalid_mfa_config_state(self, tenant_mgt_app, state):
             tenant_mgt.create_tenant(display_name='test', mfa_config=mfa_config, app=tenant_mgt_app)
         assert str(excinfo.value).startswith('multiFactorConfig.state must be either "ENABLED" or "DISABLED"')
 
-    def test_undefined_mfa_config_factor_ids_enabled_state(self, tenant_mgt_app):
-        mfa_config = {'state':'ENABLED'}
-        with pytest.raises(ValueError) as excinfo:
-            tenant_mgt.create_tenant(display_name='test', mfa_config=mfa_config, app=tenant_mgt_app)
-        assert str(excinfo.value).startswith('multiFactorConfig.factorIds must be defined')
-
     @pytest.mark.parametrize('factor_ids', [True, False, 1, 0, 'foo', {}, dict(), tuple(), list()])
     def test_invalid_mfa_config_factor_ids_type(self, tenant_mgt_app, factor_ids):
         mfa_config = {'state': 'ENABLED', 'factorIds': factor_ids}
@@ -473,8 +467,9 @@ def test_create_tenant_valid_mfa_configs(self, tenant_mgt_app):
             display_name='test', mfa_config=mfa_config_state_disabled,
             app=tenant_mgt_app)
 
-        mfa_config_state_disabled.pop('factorIds')
         _assert_tenant(tenant)
+        mfa_config_state_disabled['enabledProviders'] = mfa_config_state_disabled['factorIds']
+        mfa_config_state_disabled.pop('factorIds')
         self._assert_request(recorder, {
             'displayName': 'test',
             'mfaConfig': mfa_config_state_disabled
