Merge ~adam-collard/maas:rbac-error-1847244 into maas:master

[Adam Collard]

Failure to get RBAC user details shouldn't traceback

[MAAS Lander (maas-lander)]

UNIT TESTS
-b rbac-error-1847244 lp:~adam-collard/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 6429e3a29c4a05c0a45cbac3db43bb8638b5abfd

review: Approve

[Alberto Donato (ack)]

+1

review: Approve

diff --git a/src/maasserver/macaroon_auth.py b/src/maasserver/macaroon_auth.py
index 44affa9..90e8c64 100644
--- a/src/maasserver/macaroon_auth.py
+++ b/src/maasserver/macaroon_auth.py
@@ -417,14 +417,11 @@ def _validate_user_rbac(auth_info, username, client=None):
                 "admin-machines",
             ).values()
         )
+        user_details = client.get_user_details(username)
     except APIError:
         raise UserValidationFailed()
 
-    return (
-        is_admin or access_to_pools,
-        is_admin,
-        client.get_user_details(username),
-    )
+    return (is_admin or access_to_pools, is_admin, user_details)
 
 
 class _IDClient(bakery.IdentityClient):
diff --git a/src/maasserver/tests/test_macaroon_auth.py b/src/maasserver/tests/test_macaroon_auth.py
index 1aed07a..d53682b 100644
--- a/src/maasserver/tests/test_macaroon_auth.py
+++ b/src/maasserver/tests/test_macaroon_auth.py
@@ -409,6 +409,30 @@ class TestValidateUserExternalAuthWithRBAC(MAASServerTestCase):
         # user is still enabled
         self.assertTrue(self.user.is_active)
 
+    def test_failed_user_details_check(self):
+        self.client.allowed_for_user.side_effect = [
+            {"admin": []},
+            {
+                "view": ["pool1", "pool2"],
+                "view-all": [],
+                "deploy-machines": [],
+                "admin-machines": [],
+            },
+        ]
+        self.client.get_user_details.side_effect = APIError(500, "fail!")
+        valid = validate_user_external_auth(
+            self.user,
+            self.auth_info,
+            now=lambda: self.now,
+            rbac_client=self.client,
+        )
+        self.assertFalse(valid)
+        # user is checked again, last check time is updated
+        self.client.get_user_details.assert_called()
+        self.assertEqual(self.user.userprofile.auth_last_check, self.now)
+        # user is still enabled
+        self.assertTrue(self.user.is_active)
+
 
 class MacaroonBakeryMockMixin:
     """Mixin providing mock helpers for tests involving macaroonbakery."""