fix: return-empty-trait-list-if-persistence-not-allowed-from-sdk (#5399)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Rodrigo López Dato <rodrigo.lopezdato@flagsmith.com>

Author: 3 people

api/environments/sdk/serializers.py

@@ -193,7 +193,5 @@ def save(self, **kwargs):  # type: ignore[no-untyped-def]
     def validate_traits(self, traits: typing.List[dict] = None):  # type: ignore[no-untyped-def,type-arg,assignment]
         request = self.context["request"]
         if traits and not request.environment.trait_persistence_allowed(request):
-            raise serializers.ValidationError(
-                "Setting traits not allowed with client key."
-            )
+            return []
         return traits

api/tests/unit/environments/identities/test_unit_identities_views.py

@@ -832,11 +832,10 @@ def test_get_identities_calls_forward_identity_request_with_correct_arguments(
     assert kwargs["kwargs"]["query_params"] == {"identifier": identity.identifier}
 
 
-def test_post_identities_with_traits_fails_if_client_cannot_set_traits(
+def test_post_identities_returns_empty_traits_if_client_cannot_set_traits(
     identity: Identity,
     environment: Environment,
     api_client: APIClient,
-    feature: Feature,
 ) -> None:
     # Given
     url = reverse("api-v1:sdk-identities")
@@ -856,7 +855,9 @@ def test_post_identities_with_traits_fails_if_client_cannot_set_traits(
 
     # Then
     assert Trait.objects.count() == 0
-    assert response.status_code == status.HTTP_400_BAD_REQUEST
+    assert response.status_code == status.HTTP_200_OK
+    assert response.json()["traits"] == []
+    assert response.json()["identifier"] == identity.identifier
 
 
 def test_post_identities_with_traits_success_if_client_cannot_set_traits_server_key(

api/tests/unit/environments/sdk/test_unit_sdk_serializers.py

@@ -115,3 +115,82 @@ def test_identify_with_traits_serializer__transient__identity_and_traits_not_per
     # Then
     assert not Identity.objects.filter(identifier=identity_identifier).exists()
     assert not Trait.objects.filter(identity__identifier=identity_identifier).exists()
+
+
+def test_identify_with_traits_serializer_validate_traits_returns_empty_list_when_persistence_not_allowed(
+    mocker: MockerFixture,
+    environment: Environment,
+) -> None:
+    # Given
+    data = {
+        "identifier": "test_user",
+        "traits": [
+            {"trait_key": "key1", "trait_value": "value1"},
+            {"trait_key": "key2", "trait_value": "value2"},
+        ],
+    }
+
+    environment.allow_client_traits = False
+    environment.save()
+
+    mock_request = mocker.MagicMock()
+    mock_request.environment = environment
+
+    serializer = IdentifyWithTraitsSerializer(
+        data=data, context={"environment": environment, "request": mock_request}
+    )
+
+    # When
+    assert serializer.is_valid()
+    validated_traits = serializer.validated_data.get("traits")
+
+    serializer.save()  # type: ignore[no-untyped-call]
+    # Then
+    assert validated_traits == []
+
+    assert Identity.objects.filter(identifier="test_user").exists()
+    assert not Trait.objects.filter(identity__identifier="test_user").exists()
+
+
+def test_identify_with_traits_serializer_does_not_erase_existing_traits_when_persistence_not_allowed(
+    mocker: MockerFixture,
+    environment: Environment,
+) -> None:
+    # Given
+    identity = Identity.objects.create(environment=environment, identifier="new_user")
+    Trait.objects.create(
+        identity=identity,
+        trait_key="existing_key",
+        string_value="existing_value",
+        value_type="string",
+    )
+
+    environment.allow_client_traits = False
+    environment.save()
+
+    data = {
+        "identifier": "new_user",
+        "traits": [
+            {"trait_key": "new_key", "trait_value": "new_value"},
+            {"trait_key": "new_second_key", "trait_value": "new_second_value"},
+        ],
+    }
+
+    mock_request = mocker.MagicMock()
+    mock_request.environment = environment
+
+    serializer = IdentifyWithTraitsSerializer(
+        data=data, context={"environment": environment, "request": mock_request}
+    )
+
+    # When
+    assert serializer.is_valid()
+    serializer.save()  # type: ignore[no-untyped-call]
+
+    # Then
+    identity.refresh_from_db()
+    traits = Trait.objects.filter(identity=identity)
+
+    assert traits.count() == 1
+    assert traits[0].trait_key == "existing_key"
+    assert traits[0].string_value == "existing_value"

docs/docs/basic-features/segments.md

@@ -37,7 +37,7 @@ The Flagsmith API to set user traits, e.g. the `setTraits` method from the JavaS
 authentication or credentials. This means that users can change their own traits, which could be a security problem if
 you are using segments for authorisation or access control. If you must use segments for access control, make sure to
 disable the
-["Allow client SDKs to set user traits" option](system-administration/security.md#preventing-client-sdks-from-setting-traits)
+["Persist traits when using client-side SDK keys" option](system-administration/security.md#preventing-client-sdks-from-setting-traits)
 on every environment that needs it, and use server-side SDKs to set traits instead. You can still use client-side SDKs
 to read traits and flags derived from segments in this case.
 

docs/docs/system-administration/security.md

@@ -8,7 +8,7 @@ There may be use-cases where you want to prevent client-side SDKs from setting t
 setting `plan=silver` as a trait, and then enabling/disabling features based on that plan, a malicious user could, with
 a client-side SDK, update their trait to `plan=gold` and unlock features they have not paid for.
 
-You can prevent this by disabling the "Allow client SDKs to set user traits" option. This option defaults to "On".
+You can prevent this by disabling the "Persist traits when using client-side SDK keys" option. This option defaults to "On".
 Turning it "Off" will not allow client-side SDKs to write Traits to Flagsmith. In order to write traits, you will need
 to use a [server-side SDK and server-side Key](/clients).
 

frontend/web/components/pages/EnvironmentSettingsPage.tsx

@@ -274,7 +274,7 @@ const EnvironmentSettingsPage: React.FC<EnvironmentSettingsPageProps> = ({
       return
     }
     const editedEnv = { ...currentEnv, ...newEnv }
-    
+
     AppActions.editEnv(
       Object.assign({}, currentEnv, {
         allow_client_traits: !!editedEnv?.allow_client_traits,
@@ -794,8 +794,8 @@ const EnvironmentSettingsPage: React.FC<EnvironmentSettingsPageProps> = ({
                           </div>
                           <div className='mt-4'>
                             <Setting
-                              title='Allow client SDKs to set user traits'
-                              description={`Disabling this option will prevent client SDKs from using the client key from setting traits.`}
+                              title='Persist traits when using client-side SDK keys'
+                              description={'If enabled, Flagsmith will persist any non-transient traits sent by SDKs using client-side keys when remotely evaluating flags.'}
                               checked={currentEnv?.allow_client_traits}
                               onChange={(value) => {
                                 updateCurrentEnv(