XWIKI-23827: Store password properties in dedicated table #4956
Conversation
* Define a PasswordProperty and the associated table in xwiki.hbm.xml * Rework editobject.vm deprecated properties to handle obfuscation UC * Provide a new integration test to cover it
| <joined-subclass name="com.xpn.xwiki.objects.PasswordProperty" table="xwikipasswords"> | ||
| <key> | ||
| <column name="XWP_ID" /> | ||
| <column name="XWP_NAME" /> |
There was a problem hiding this comment.
I would index it, just in case someone wants to manipulate it in a query (no reason to forbid this use case, and the size of that property allows it).
| org.xwiki.internal.document.DocumentRequiredRightsReader | ||
| org.xwiki.internal.document.RequiredRightClassMandatoryDocumentInitializer | ||
| org.xwiki.internal.document.DefaultSimpleDocumentCache | ||
| org.xwiki.model.objects.ObjectsScriptService |
There was a problem hiding this comment.
I don't see that class in the commit.
There was a problem hiding this comment.
Indeed, I removed it. I first created this script service for having a method checking if a property was a PasswordProperty, but in the end I used: $objecttool.instanceOf($prop.property, 'com.xpn.xwiki.objects.PasswordProperty'). I guess it's enough, wdyt?
There was a problem hiding this comment.
Doesn't $prop.property require programming right?
There was a problem hiding this comment.
yes it does, but here it's a filesystem template so it's executed with PR AFAIK, shouldn't be an issue no?
There was a problem hiding this comment.
It's not an issue here, but it will be an issue when the next developer tries to use the same pattern in a wiki page.
There was a problem hiding this comment.
Ah sure, so you're saying we may need the script service?
There was a problem hiding this comment.
I think it would definitely be good to have an easy-to-use script service (and a non-script component) to check if a property is safe to be displayed/filtered/sorted so we stop repeating the checks for password class or email class and obfuscation, we already have way too many of them. I'm not saying it needs to be in this PR, but I guess we should also modify every place where we currently check for PasswordClass to check for the new password property type? I mean places like
There was a problem hiding this comment.
Yes, we really need to introduce a more generic API to know if something is supposed to be obfuscated (and we keep saying it in each pull request which adds a new check for the password or email type...).
There was a problem hiding this comment.
Ok thanks for the feedbacks, so I guess I'll introduce it, probably in the SecurityScriptService or something like that, and not really an ObjectScriptService then.
There was a problem hiding this comment.
Not sure. Might be simpler to expose that directly in BaseProperty and api.Property.
Jira URL
Changes
Description
Clarifications
Screenshots & Video
Executed Tests
Expected merging strategy
*