UpdateOnlyTextField is not compatible with DependConstraint

[kumy]

Important notices

Before you add a new report, we ask you kindly to acknowledge the following:

• [x] I have read the contributing guide lines at https://github.com/opnsense/core/blob/master/CONTRIBUTING.md
• [x] I am convinced that my issue is new after having checked both open and closed issues at https://github.com/opnsense/core/issues?q=is%3Aissue

Describe the bug

Using DependConstraint with UpdateOnlyTextField fields always trigger the validation message

To Reproduce In a custom plugin…

Given such xml (simplified) model, the aws_secret_access_key field validation always trigger an error.

<model>
    <mount>//foo/bar</mount>
    <description>Foobar</description>
    <items>
        <enabled type="BooleanField">
            <default>0</default>
            <Required>Y</Required>
            <Constraints>
                <check003>
                    <reference>aws_secret_access_key.check001</reference>
                </check003>
            </Constraints>
        </enabled>
        <aws_secret_access_key type="UpdateOnlyTextField">
            <Required>N</Required>
            <Constraints>
                <check001>
                    <ValidationMessage>An AWS_SECRET_ACCESS_KEY is required.</ValidationMessage>
                    <type>DependConstraint</type>
                    <addFields>
                        <field1>enabled</field1>
                    </addFields>
                </check001>
            </Constraints>
        </aws_secret_access_key>
    </items>
</model>

Expected behavior

Should not trigger an error when the user enter a value, and should not trigger error on further saves

Describe alternatives you considered

Switching the field type to TextField don't trigger intepestive error.

Screenshots We can see that a value is really posted.

Relevant log files

N/A

Additional context

I think I get the problem… The validate() from DependConstraint class check the value by calling isEmpty() (link). The isEmpty() from BaseConstraint checks if the value is empty using empty((string)$node) (link). Calling this on the $node will call the __toString() method from the Fieldtype which normally return the field internalValue (link)

But the UpdateOnlyTextField overrides __toString() for good reason ;) (link)…

Initial idea was to replace empty((string)$node) check by empty((string)$node->internalValue), but $internalValue is protected value from BaseField thus cannot be used.

I didn't found any other getter for it, should one be created?

Environment

Fresh install in VirtualBox

OPNsense 24.7.4_1-amd64 FreeBSD 14.1-RELEASE-p4 OpenSSL 3.0.15

[AdSchellevis]

@kumy the UpdateOnlyTextField isn't used very often, but the easiest fix to make it work would be to add a public function to get the value in BaseField and refactor the constraints to use that method instead of the current one. We only have to come up with a sensible name and change the few callers where this is relevant. Maybe something like getCurrentValue would be work?

[kumy]

Ok, looks like we have the same conclusion. Would you like a PR or you are on it?

getCurrentValue looks fine.

[AdSchellevis]

@kumy if you're up for a PR, I'll just wait for it :)

[Monviech]

FWIW: Since it was said the fieldtype is rare, I wanted to add an example where I use it:

https://github.com/opnsense/plugins/blob/b8699cde8cf210e864e4bfab7329dcaaf40ec995/www/caddy/src/opnsense/mvc/app/models/OPNsense/Caddy/Caddy.xml#L414

https://github.com/opnsense/plugins/blob/b8699cde8cf210e864e4bfab7329dcaaf40ec995/www/caddy/src/opnsense/mvc/app/controllers/OPNsense/Caddy/Api/ReverseProxyController.php#L298

I dont have anything more to add. Just thought it might be interesting.