rusackas commented on code in PR #33924:
URL: https://github.com/apache/superset/pull/33924#discussion_r3599758137


##########
tests/integration_tests/security/api_tests.py:
##########
@@ -195,6 +195,174 @@ def test_post_guest_token_authorized(self):
         assert user == decoded_token["user"]
         assert resource == decoded_token["resources"][0]
 
+    @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+    def test_post_guest_token_with_attributes(self):

Review Comment:
   This is a test module, where Superset relaxes typing for `tests.*` in the 
mypy config (`disallow_untyped_defs = false`), and the existing methods in this 
file do not carry return annotations either. This matches the file convention, 
so resolving as not applicable.



##########
tests/integration_tests/security_tests.py:
##########
@@ -2436,6 +2436,170 @@ def test_get_guest_user(self):
         assert guest_user is not None
         assert "test_guest" == guest_user.username
 
+    def create_guest_token_with_attributes(self):

Review Comment:
   This is a test module, where Superset relaxes typing for `tests.*` in the 
mypy config (`disallow_untyped_defs = false`), and the existing methods in this 
file do not carry return annotations either. This matches the file convention, 
so resolving as not applicable.



##########
tests/integration_tests/security_tests.py:
##########
@@ -2436,6 +2436,170 @@ def test_get_guest_user(self):
         assert guest_user is not None
         assert "test_guest" == guest_user.username
 
+    def create_guest_token_with_attributes(self):
+        user = {
+            "username": "test_guest_with_attrs",
+            "first_name": "Test",
+            "last_name": "Guest",
+            "attributes": {
+                "department": "Engineering",
+                "region": "US",
+                "role": "developer",
+                "team": "data-platform",
+            },
+        }
+        resources = [{"some": "resource"}]
+        rls = [{"dataset": 1, "clause": "access = 1"}]
+        return security_manager.create_guest_access_token(user, resources, rls)
+
+    def test_create_guest_access_token_with_attributes(self):

Review Comment:
   This is a test module, where Superset relaxes typing for `tests.*` in the 
mypy config (`disallow_untyped_defs = false`), and the existing methods in this 
file do not carry return annotations either. This matches the file convention, 
so resolving as not applicable.



##########
tests/integration_tests/security_tests.py:
##########
@@ -2436,6 +2436,170 @@ def test_get_guest_user(self):
         assert guest_user is not None
         assert "test_guest" == guest_user.username
 
+    def create_guest_token_with_attributes(self):
+        user = {
+            "username": "test_guest_with_attrs",
+            "first_name": "Test",
+            "last_name": "Guest",
+            "attributes": {
+                "department": "Engineering",
+                "region": "US",
+                "role": "developer",
+                "team": "data-platform",
+            },
+        }
+        resources = [{"some": "resource"}]
+        rls = [{"dataset": 1, "clause": "access = 1"}]
+        return security_manager.create_guest_access_token(user, resources, rls)
+
+    def test_create_guest_access_token_with_attributes(self):
+        """Test creating guest access token with user attributes."""
+        user_with_attributes = {
+            "username": "test_guest_attrs",
+            "first_name": "Test",
+            "last_name": "Guest",
+            "attributes": {
+                "department": "Engineering",
+                "region": "US",
+                "clearance_level": "standard",
+                "projects": ["analytics", "ml-platform"],
+                "team_lead": True,
+            },
+        }
+        resources = [{"type": "dashboard", "id": "test-dashboard"}]
+        rls = [{"dataset": 1, "clause": "id = 1"}]
+
+        token = security_manager.create_guest_access_token(
+            user_with_attributes, resources, rls
+        )
+
+        # Decode and verify the token contains attributes
+        aud = get_url_host()
+        decoded_token = jwt.decode(
+            token,
+            self.app.config["GUEST_TOKEN_JWT_SECRET"],
+            algorithms=[self.app.config["GUEST_TOKEN_JWT_ALGO"]],
+            audience=aud,
+        )
+
+        assert "user" in decoded_token
+        user = decoded_token["user"]
+        assert "attributes" in user
+        assert user["attributes"]["department"] == "Engineering"
+        assert user["attributes"]["region"] == "US"
+        assert user["attributes"]["clearance_level"] == "standard"
+        assert user["attributes"]["projects"] == ["analytics", "ml-platform"]
+        assert user["attributes"]["team_lead"] is True
+
+    def test_get_guest_user_with_attributes(self):

Review Comment:
   This is a test module, where Superset relaxes typing for `tests.*` in the 
mypy config (`disallow_untyped_defs = false`), and the existing methods in this 
file do not carry return annotations either. This matches the file convention, 
so resolving as not applicable.



##########
tests/integration_tests/security_tests.py:
##########
@@ -2436,6 +2436,170 @@ def test_get_guest_user(self):
         assert guest_user is not None
         assert "test_guest" == guest_user.username
 
+    def create_guest_token_with_attributes(self):
+        user = {
+            "username": "test_guest_with_attrs",
+            "first_name": "Test",
+            "last_name": "Guest",
+            "attributes": {
+                "department": "Engineering",
+                "region": "US",
+                "role": "developer",
+                "team": "data-platform",
+            },
+        }
+        resources = [{"some": "resource"}]
+        rls = [{"dataset": 1, "clause": "access = 1"}]
+        return security_manager.create_guest_access_token(user, resources, rls)
+
+    def test_create_guest_access_token_with_attributes(self):
+        """Test creating guest access token with user attributes."""
+        user_with_attributes = {
+            "username": "test_guest_attrs",
+            "first_name": "Test",
+            "last_name": "Guest",
+            "attributes": {
+                "department": "Engineering",
+                "region": "US",
+                "clearance_level": "standard",
+                "projects": ["analytics", "ml-platform"],
+                "team_lead": True,
+            },
+        }
+        resources = [{"type": "dashboard", "id": "test-dashboard"}]
+        rls = [{"dataset": 1, "clause": "id = 1"}]
+
+        token = security_manager.create_guest_access_token(
+            user_with_attributes, resources, rls
+        )
+
+        # Decode and verify the token contains attributes
+        aud = get_url_host()
+        decoded_token = jwt.decode(
+            token,
+            self.app.config["GUEST_TOKEN_JWT_SECRET"],
+            algorithms=[self.app.config["GUEST_TOKEN_JWT_ALGO"]],
+            audience=aud,
+        )
+
+        assert "user" in decoded_token
+        user = decoded_token["user"]
+        assert "attributes" in user
+        assert user["attributes"]["department"] == "Engineering"
+        assert user["attributes"]["region"] == "US"
+        assert user["attributes"]["clearance_level"] == "standard"
+        assert user["attributes"]["projects"] == ["analytics", "ml-platform"]
+        assert user["attributes"]["team_lead"] is True
+
+    def test_get_guest_user_with_attributes(self):
+        """Test that guest user properly retains attributes from token."""
+        token = self.create_guest_token_with_attributes()
+        fake_request = FakeRequest()
+        fake_request.headers[current_app.config["GUEST_TOKEN_HEADER_NAME"]] = 
token
+
+        guest_user = security_manager.get_guest_user_from_request(fake_request)
+
+        assert guest_user is not None
+        assert "test_guest_with_attrs" == guest_user.username
+
+        # Verify attributes are accessible through guest_token
+        assert hasattr(guest_user, "guest_token")
+        token_user = guest_user.guest_token["user"]
+        assert "attributes" in token_user
+        assert token_user["attributes"]["department"] == "Engineering"
+        assert token_user["attributes"]["region"] == "US"
+        assert token_user["attributes"]["role"] == "developer"
+        assert token_user["attributes"]["team"] == "data-platform"
+
+    def test_create_guest_access_token_without_attributes(self):

Review Comment:
   This is a test module, where Superset relaxes typing for `tests.*` in the 
mypy config (`disallow_untyped_defs = false`), and the existing methods in this 
file do not carry return annotations either. This matches the file convention, 
so resolving as not applicable.



##########
tests/integration_tests/security_tests.py:
##########
@@ -2436,6 +2436,170 @@ def test_get_guest_user(self):
         assert guest_user is not None
         assert "test_guest" == guest_user.username
 
+    def create_guest_token_with_attributes(self):
+        user = {
+            "username": "test_guest_with_attrs",
+            "first_name": "Test",
+            "last_name": "Guest",
+            "attributes": {
+                "department": "Engineering",
+                "region": "US",
+                "role": "developer",
+                "team": "data-platform",
+            },
+        }
+        resources = [{"some": "resource"}]
+        rls = [{"dataset": 1, "clause": "access = 1"}]
+        return security_manager.create_guest_access_token(user, resources, rls)
+
+    def test_create_guest_access_token_with_attributes(self):
+        """Test creating guest access token with user attributes."""
+        user_with_attributes = {
+            "username": "test_guest_attrs",
+            "first_name": "Test",
+            "last_name": "Guest",
+            "attributes": {
+                "department": "Engineering",
+                "region": "US",
+                "clearance_level": "standard",
+                "projects": ["analytics", "ml-platform"],
+                "team_lead": True,
+            },
+        }
+        resources = [{"type": "dashboard", "id": "test-dashboard"}]
+        rls = [{"dataset": 1, "clause": "id = 1"}]
+
+        token = security_manager.create_guest_access_token(
+            user_with_attributes, resources, rls
+        )
+
+        # Decode and verify the token contains attributes
+        aud = get_url_host()
+        decoded_token = jwt.decode(
+            token,
+            self.app.config["GUEST_TOKEN_JWT_SECRET"],
+            algorithms=[self.app.config["GUEST_TOKEN_JWT_ALGO"]],
+            audience=aud,
+        )
+
+        assert "user" in decoded_token
+        user = decoded_token["user"]
+        assert "attributes" in user
+        assert user["attributes"]["department"] == "Engineering"
+        assert user["attributes"]["region"] == "US"
+        assert user["attributes"]["clearance_level"] == "standard"
+        assert user["attributes"]["projects"] == ["analytics", "ml-platform"]
+        assert user["attributes"]["team_lead"] is True
+
+    def test_get_guest_user_with_attributes(self):
+        """Test that guest user properly retains attributes from token."""
+        token = self.create_guest_token_with_attributes()
+        fake_request = FakeRequest()
+        fake_request.headers[current_app.config["GUEST_TOKEN_HEADER_NAME"]] = 
token
+
+        guest_user = security_manager.get_guest_user_from_request(fake_request)
+
+        assert guest_user is not None
+        assert "test_guest_with_attrs" == guest_user.username
+
+        # Verify attributes are accessible through guest_token
+        assert hasattr(guest_user, "guest_token")
+        token_user = guest_user.guest_token["user"]
+        assert "attributes" in token_user
+        assert token_user["attributes"]["department"] == "Engineering"
+        assert token_user["attributes"]["region"] == "US"
+        assert token_user["attributes"]["role"] == "developer"
+        assert token_user["attributes"]["team"] == "data-platform"
+
+    def test_create_guest_access_token_without_attributes(self):
+        """Test creating guest access token without user attributes.
+
+        This test ensures backward compatibility.
+        """
+        user_without_attributes = {
+            "username": "test_guest_no_attrs",
+            "first_name": "Test",
+            "last_name": "Guest",
+        }
+        resources = [{"type": "dashboard", "id": "test-dashboard"}]
+        rls = [{"dataset": 1, "clause": "id = 1"}]
+
+        token = security_manager.create_guest_access_token(
+            user_without_attributes, resources, rls
+        )
+
+        # Decode and verify the token works without attributes
+        aud = get_url_host()
+        decoded_token = jwt.decode(
+            token,
+            self.app.config["GUEST_TOKEN_JWT_SECRET"],
+            algorithms=[self.app.config["GUEST_TOKEN_JWT_ALGO"]],
+            audience=aud,
+        )
+
+        assert "user" in decoded_token
+        user = decoded_token["user"]
+        assert "attributes" not in user
+        assert user["username"] == "test_guest_no_attrs"
+
+    def test_create_guest_access_token_with_empty_attributes(self):

Review Comment:
   This is a test module, where Superset relaxes typing for `tests.*` in the 
mypy config (`disallow_untyped_defs = false`), and the existing methods in this 
file do not carry return annotations either. This matches the file convention, 
so resolving as not applicable.



##########
tests/integration_tests/security/api_tests.py:
##########
@@ -195,6 +195,174 @@ def test_post_guest_token_authorized(self):
         assert user == decoded_token["user"]
         assert resource == decoded_token["resources"][0]
 
+    @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+    def test_post_guest_token_with_attributes(self):
+        """
+        Security API: Create a guest token with user attributes
+        """
+        self.dash = 
db.session.query(Dashboard).filter_by(slug="births").first()
+        self.embedded = EmbeddedDashboardDAO.upsert(self.dash, [])
+        self.login(ADMIN_USERNAME)
+
+        user = {
+            "username": "bob_with_attrs",
+            "first_name": "Bob",
+            "last_name": "Also Bob",
+            "attributes": {
+                "department": "Engineering",
+                "region": "US",
+                "role": "developer",
+                "team": "data-platform",
+                "clearance_level": "standard",
+                "projects": ["analytics", "ml-platform"],
+                "team_lead": True,
+            },
+        }
+        resource = {"type": "dashboard", "id": str(self.embedded.uuid)}
+        rls_rule = {"dataset": 1, "clause": "1=1"}
+        params = {"user": user, "resources": [resource], "rls": [rls_rule]}
+
+        response = self.client.post(
+            self.uri, data=json.dumps(params), content_type="application/json"
+        )
+
+        self.assert200(response)
+        token = json.loads(response.data)["token"]
+        decoded_token = jwt.decode(
+            token,
+            self.app.config["GUEST_TOKEN_JWT_SECRET"],
+            audience=get_url_host(),
+            algorithms=["HS256"],
+        )
+
+        # Verify user attributes are preserved in the token
+        assert user == decoded_token["user"]
+        assert "attributes" in decoded_token["user"]
+        assert decoded_token["user"]["attributes"]["department"] == 
"Engineering"
+        assert decoded_token["user"]["attributes"]["region"] == "US"
+        assert decoded_token["user"]["attributes"]["role"] == "developer"
+        assert decoded_token["user"]["attributes"]["team"] == "data-platform"
+        assert decoded_token["user"]["attributes"]["clearance_level"] == 
"standard"
+        assert decoded_token["user"]["attributes"]["projects"] == [
+            "analytics",
+            "ml-platform",
+        ]
+        assert decoded_token["user"]["attributes"]["team_lead"] is True
+        assert resource == decoded_token["resources"][0]
+
+    @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+    def test_post_guest_token_with_empty_attributes(self):
+        """
+        Security API: Create a guest token with empty user attributes
+        """
+        self.dash = 
db.session.query(Dashboard).filter_by(slug="births").first()
+        self.embedded = EmbeddedDashboardDAO.upsert(self.dash, [])
+        self.login(ADMIN_USERNAME)
+
+        user = {
+            "username": "bob_empty_attrs",
+            "first_name": "Bob",
+            "last_name": "Also Bob",
+            "attributes": {},
+        }
+        resource = {"type": "dashboard", "id": str(self.embedded.uuid)}
+        rls_rule = {"dataset": 1, "clause": "1=1"}
+        params = {"user": user, "resources": [resource], "rls": [rls_rule]}
+
+        response = self.client.post(
+            self.uri, data=json.dumps(params), content_type="application/json"
+        )
+
+        self.assert200(response)
+        token = json.loads(response.data)["token"]
+        decoded_token = jwt.decode(
+            token,
+            self.app.config["GUEST_TOKEN_JWT_SECRET"],
+            audience=get_url_host(),
+            algorithms=["HS256"],
+        )
+
+        # Verify empty attributes are preserved in the token
+        assert user == decoded_token["user"]
+        assert "attributes" in decoded_token["user"]
+        assert decoded_token["user"]["attributes"] == {}
+        assert resource == decoded_token["resources"][0]
+
+    @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+    def test_post_guest_token_with_null_attributes(self):

Review Comment:
   This is a test module, where Superset relaxes typing for `tests.*` in the 
mypy config (`disallow_untyped_defs = false`), and the existing methods in this 
file do not carry return annotations either. This matches the file convention, 
so resolving as not applicable.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to