Fix UAA admin check code

cf_auth_proxy/app.py

@@ -175,7 +175,7 @@ def redirect_to_auth():
         # allowed path
         if session.get("user_id"):
             headers["x-proxy-user"] = session["email"]
-            roles = [("admin" if session.get("is_cf_admin") else "user")]
+            roles = [("admin" if session.get("is_cf_admin") == True else "user")]
             roles += session.get("user_orgs", [])
             headers["x-proxy-roles"] = ",".join(roles)
 

cf_auth_proxy/uaa.py

@@ -19,11 +19,7 @@ def get_client_credentials_token():
         timeout=config.REQUEST_TIMEOUT,
     )
 
-    try:
-        response.raise_for_status()
-    except:
-        return "Unexpected error", 500
-
+    response.raise_for_status()
     return response.json()["access_token"]
 
 
@@ -32,10 +28,7 @@ def is_user_cf_admin(user_id, token):
         s.headers["Authorization"] = f"Bearer {token}"
         url = urljoin(config.UAA_BASE_URL, f"Users/{user_id}")
         response = s.get(url)
-        try:
-            response.raise_for_status()
-        except:
-            return "Unexpected error", 500
+        response.raise_for_status()
         data = response.json()
         user_groups = [group["display"] for group in data.get("groups", [])]
         return config.CF_ADMIN_GROUP_NAME in user_groups

tests/unit/test_app.py

@@ -144,3 +144,25 @@ def test_does_not_modify_existing_xff_lowercase(client):
             s["email"] = "me"
         client.get("/home", headers={"x-forwarded-for": "y.y.y.y"})
         assert m.last_request._request.headers["X-Forwarded-For"] == "y.y.y.y"
+
+
+def test_does_not_accept_truthy_is_cf_admin(client):
+    with requests_mock.Mocker() as m:
+        m.get("http://mock.dashboard/home")
+        with client.session_transaction() as s:
+            s["user_id"] = "me2"
+            s["is_cf_admin"] = "truthy"
+            s["email"] = "me"
+        client.get("/home")
+        assert "admin" not in m.last_request._request.headers["x-proxy-roles"]
+
+
+def test_callback_returns_error_on_uaa_token_failure(client):
+    with requests_mock.Mocker() as m:
+        m.post("http://mock.uaa/token", status_code=401)
+        with client.session_transaction() as s:
+            s["user_id"] = "me2"
+            s["email"] = "me"
+            s["state"] = "foo"
+        response = client.get("/cb?code=fakecode&state=foo")
+        assert response.status_code == 500

tests/unit/test_uaa.py

@@ -1,3 +1,4 @@
+import pytest
 import requests_mock
 from cf_auth_proxy import uaa
 
@@ -33,3 +34,24 @@ def test_user_has_no_groups(uaa_user_has_no_groups_response):
         isAdmin = uaa.is_user_cf_admin("a-user-id", "a-token")
         assert not isAdmin
         assert m.last_request._request.headers["Authorization"] == "Bearer a-token"
+
+
+def test_uaa_has_unexpected_error():
+    with requests_mock.Mocker() as m:
+        m.get("http://mock.uaa/Users/a-user-id", status_code=500)
+        with pytest.raises(Exception):
+            uaa.is_user_cf_admin("a-user-id", "a-token")
+
+
+def test_uaa_has_unauthorized_error():
+    with requests_mock.Mocker() as m:
+        m.get("http://mock.uaa/Users/a-user-id", status_code=401)
+        with pytest.raises(Exception):
+            uaa.is_user_cf_admin("a-user-id", "a-token")
+
+
+def test_uaa_get_client_credentials_token_raises_unexpected_error():
+    with requests_mock.Mocker() as m:
+        m.post("http://mock.uaa/token", status_code=500)
+        with pytest.raises(Exception):
+            uaa.get_client_credentials_token()