at the moment, every document in XWiki has three authors:
the content author, responsible for the rights on content and title
the effective metadata author, responsible for the rights on everything else
the original metadata author, that we display
I propose that we effectively get rid of the effective metadata author and instead use the content author for all rights. The original metadata author would be kept as it has important use cases in showing who triggered an action that shouldn’t change the rights of the page/need to use a different user for the rights.
First, why am I proposing this:
the concepts are confusing, for users and even for committers who’re not working daily with them
we’ve already had several security vulnerabilities because of using the wrong author
it slows XWiki down because in several places, we need to set the content author to the effective metadata author to ensure that right checks (which are always using the content author) use the correct author and for this we need to clone the document which can be slow
Now how can we do this without breaking any APIs?
We keep the methods and fields but deprecate the methods for the effective metadata author
With a migration, we set the content author to the effective metadata author on all documents. As the effective metadata author is updated every time the document is saved, this should be the proper latest author.
During saving, we (always) set the content author to be the effective metadata author and not just when the content is dirty.
Setting the content author also sets the effective metadata author, so code that wants to set authors only need to update the content author.
I haven’t looked at that part yet, but we probably should also unify the dirty flags into a single dirty flag.
That way, from this moment on, we can stop using the effective metadata author for right checks and solely rely on the content author. However, code that still uses it also won’t be wrong as we still update it.
Could this still break anything? I fear yes. At the moment, users without script right can customize the global dashboard and despite seeing a warning that this might break the content, it doesn’t break the code that is in the content of the dashboard because the content author isn’t updated. I suggest we simply fix that by moving the code to another page and referencing it either with the include macro or as a sheet (haven’t checked yet what would be simpler).
I’m opening this as a proposal to get some feedback if you have feedback, in particular any concerns that would need to be addressed or important objections against this proposal. For the actual decision, I assume we should have a proper vote due to the possibly breaking nature of this change.
To be complete on the topic and make sure we don’t forget anything, I’d be curious to remember why we introduced the effective metadata author (ie a different rights than for the content). Maybe we had some use cases in mind where the same right wouldn’t be needed to be the same for content and for xobjects.
This was before my time, and we would probably need @ludovic to confirm, but I suspect it was to be able to add “harmless” xobjects, like comments or tags, without breaking the document content. Back then the original author did not exist, and the effective author was never really used for right checking (yes, it means textareas were not executed with a very accurate author…). Then when we started wanting to fix all those xobject properties related vulnerabilities we naturally used the effective author.
These days, we don’t touch the content or effective authors anymore when adding a comment (to not break anything related to rights), and we only update on the original author.
I agree the risk of breakage is probably low, and I suspect it will generally go in the direction having less rights than expected and not more (like breaking the content because the effective author have less right). I’m sure there are use cases that rely on the content and the xobjects having different authors, but it’s possible those are old and some would probably benefit from relying on the original author instead.
And yes, it will be much simpler to understand and reduce the risk of mistakes. It’s what XWikiDocument#setAuthor(UserReference userReference) was here for too, covering most common use cases in practice, but it will be more global.
But I’m not a fan of using APIs explicitly referring to the “content” to actually manipulate the right of the whole document, and I think it’s also important to make more clear that the logic changed. So it would probably be cleaner to deprecate both the “content author” and “effective metadata author” API in favor of a more generic one (“right author” ? “effective author” ?). We might also want to deprecate the “original metadata author” in favor of a “metadata”-less version (like “original author”).
I totally agree it’s cleaner, I just didn’t want to cause even more deprecated methods, in particular given that most of these author APIs were only introduced a few years ago. So my idea was to basically go back to what we had initially, which is a content author for right checking and a (original) metadata author for display. And “content” would be a broad definition that compromises the whole document and not just the content field - we already use the content author for the title right now.
Unfortunately, I don’t have any good suggestions for new names.