Repository: Azure/azure-cli

Issue:
Login: do not set default on disabled subscriptions

Discussion:
codecov-io: # [Codecov](https://codecov.io/gh/Azure/azure-cli/pull/2575?src=pr&el=h1) Report
> Merging [#2575](https://codecov.io/gh/Azure/azure-cli/pull/2575?src=pr&el=desc) into [master](https://codecov.io/gh/Azure/azure-cli/commit/3b6324d00353405e547723056bb324fe3667731c?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.


```diff
@@            Coverage Diff             @@
##           master    #2575      +/-   ##
==========================================
+ Coverage   72.32%   72.32%   +<.01%     
==========================================
  Files         363      363              
  Lines       19835    19838       +3     
  Branches     2923     2924       +1     
==========================================
+ Hits        14345    14348       +3     
  Misses       4582     4582              
  Partials      908      908
```


| [Impacted Files](https://codecov.io/gh/Azure/azure-cli/pull/2575?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/azure-cli-core/azure/cli/core/\_profile.py](https://codecov.io/gh/Azure/azure-cli/compare/3b6324d00353405e547723056bb324fe3667731c...3eb700a6096e68ea881880a9e976c43d060eeed2?src=pr&el=tree#diff-c3JjL2F6dXJlLWNsaS1jb3JlL2F6dXJlL2NsaS9jb3JlL19wcm9maWxlLnB5) | `84.19% <100%> (+0.13%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/Azure/azure-cli/pull/2575?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/Azure/azure-cli/pull/2575?src=pr&el=footer). Last update [3b6324d...3eb700a](https://codecov.io/gh/Azure/azure-cli/compare/3b6324d00353405e547723056bb324fe3667731c...3eb700a6096e68ea881880a9e976c43d060eeed2?el=footer&src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Patch:
diff --git a/src/azure-cli-core/azure/cli/core/_profile.py b/src/azure-cli-core/azure/cli/core/_profile.py
index 2312aa240..6c68bddcb 100644
--- a/src/azure-cli-core/azure/cli/core/_profile.py
+++ b/src/azure-cli-core/azure/cli/core/_profile.py
@@ -182,16 +182,22 @@ class Profile(object):
                 s[_IS_DEFAULT_SUBSCRIPTION] = False
 
             if not new_active_one:
-                new_active_one = new_subscriptions[0]
-            new_active_one[_IS_DEFAULT_SUBSCRIPTION] = True
-            default_sub_id = new_active_one[_SUBSCRIPTION_ID]
+                new_active_one = Profile._pick_working_subscription(new_subscriptions)
         else:
-            new_subscriptions[0][_IS_DEFAULT_SUBSCRIPTION] = True
-            default_sub_id = new_subscriptions[0][_SUBSCRIPTION_ID]
+            new_active_one = Profile._pick_working_subscription(new_subscriptions)
+
+        new_active_one[_IS_DEFAULT_SUBSCRIPTION] = True
+        default_sub_id = new_active_one[_SUBSCRIPTION_ID]
 
         set_cloud_subscription(active_cloud.name, default_sub_id)
         self._storage[_SUBSCRIPTIONS] = subscriptions
 
+    @staticmethod
+    def _pick_working_subscription(subscriptions):
+        from azure.mgmt.resource.subscriptions.models import SubscriptionState
+        s = next((x for x in subscriptions if x['state'] == SubscriptionState.enabled.value), None)
+        return s or subscriptions[0]
+
     def set_active_subscription(self, subscription):  # take id or name
         subscriptions = self.load_cached_subscriptions(all_clouds=True)
         active_cloud = get_active_cloud()


Test patch:
diff --git a/src/azure-cli-core/tests/test_profile.py b/src/azure-cli-core/tests/test_profile.py
index 08ab8288f..2ce6494ed 100644
--- a/src/azure-cli-core/tests/test_profile.py
+++ b/src/azure-cli-core/tests/test_profile.py
@@ -165,6 +165,19 @@ class Test_Profile(unittest.TestCase):  # pylint: disable=too-many-public-method
         self.assertFalse(storage_mock['subscriptions'][1]['isDefault'])
         self.assertTrue(storage_mock['subscriptions'][0]['isDefault'])
 
+    def test_default_active_subscription_to_non_disabled_one(self):
+        storage_mock = {'subscriptions': None}
+        profile = Profile(storage_mock)
+
+        subscriptions = profile._normalize_properties(
+            self.user2, [self.subscription2, self.subscription1], False)
+
+        profile._set_subscriptions(subscriptions)
+
+        # verify we skip the overdued subscription and default to the 2nd one in the list
+        self.assertEqual(storage_mock['subscriptions'][1]['name'], self.subscription1.display_name)
+        self.assertTrue(storage_mock['subscriptions'][1]['isDefault'])
+
     def test_get_subscription(self):
         storage_mock = {'subscriptions': None}
         profile = Profile(storage_mock)