#155 ✓resolved
James

Privacy malfunction possible when a earlier public version is invalid

Reported by James | January 13th, 2009 @ 11:13 AM | in 1.2

Summary

If new validation requirements are added to an item type (i.e. a new required extended field is added to a topic type), it makes older versions of the topic invalid.

A side effect of this is that because the item privacy back-end works by automatically restoring (and validating) the last non-rejected public version to the current version (hiding the latest private version), the item privacy back-end can malfunction when the last public version is invalid.

This leads to the last version (public or private) being left in the record attributes, and published to the public zebra instance and public search results.

In plain terms, if this were to occur, it could result in the exposure of the last version (private or not) to visitors without needing to be logged in.

This has never occurred before now because until now extended field requirements have not been enforced, and so everything proceeded as normal, and as expected.

Core Problem

The core of the issue is that with the new extended field enforced validation and requirements, a previous valid version of a topic (or other item type) can be rendered invalid by adding a new extended field requirement at any time.

In the past the extended field requirements and validations were not enforced, so there was no issue.

This could also pose a problem for places where acts_as_versioned's revert_to method is used (such as flagging and moderation) because the record is saved after reversion and a validation failure will occur.

Comments and changes to this ticket

  • Walter McGinnis

    Walter McGinnis January 13th, 2009 @ 11:37 AM

    I was thinking that we require a default for required extended fields and when reverts, etc happen the default is used. However, upon further reflection, i think that is deceptive. It implies that something was actually inputted by a user.

    Two things:

    • If there is user interaction we should push back to the user to fill out the required fields, such as when a moderator reverts to previous version via the preview page. If the new version based on the old version fails moderation, the controller should redirect to edit page and show errors so the can rectify them.

    • Where automatic reversion happen, such as full moderation or flagging, we should implement a special placeholder value for required fields that fulfills the same role as our special title for pending or private items. It should also be a constant based on a system setting. Actually, simply using those constants again in their matching contexts are fine.

      • One little wrinkle is that you will need to add logic to the ftype based validation for extended fields to pass these values as acceptable.
      • We should probably update the display of extended fields to ignore them, too. In other words, if an extended field has that value, don't even display the label for it (it's like it was blank).
      • When doing edits, these value should be cleared from forms, as well.

    Sound alright? Other thoughts?

  • James

    James January 13th, 2009 @ 11:46 AM

    Another option might be to check whether a blank XML tag is present in the item.extended_content for any required extended fields, and skip validations for any where the blank tag isn't there.

    This would work because blank tags are generated when the fields (with data present or not) are posted from a form.

    This would have the effect of validating the fields when they're posted from a form (i.e. a topic edit form), and skipping them when saving the attributes from a version where the extended field wasn't mapped.

    James

  • Walter McGinnis

    Walter McGinnis January 13th, 2009 @ 11:55 AM

    • State changed from “new” to “open”

    Sounds elegant, but also seems like it could leave future developers scratching their heads.

    Reading "Pending Moderation" or "No Private Version" seems more consistent with our other processes and gives developers a definite clue right there in the data of what is going on.

    That said, your solution probably has less moving parts to manage... If you can convince me that you can do your implementation in a way that is clear and clean and not black magic and prone to giving developers unexpected results, than I'm cool with that.

    Cheers, Walter

  • James

    James January 13th, 2009 @ 12:13 PM

    I am in favour of my suggestion because:

    1. I think it would be unintuitive to require administrators or moderators to provide additional data for topics they are performing moderation actions on (e.g. rejecting), to satisfy extended content requirements.

    2. The net effect of both suggestions is the same in terms of how validation is handled on extended fields behind the scenes, accept that code to handle the situation needs to be adding possibly in multiple places in your suggestion (i.e. in flagging/moderation code, and in item privacy code), whereas my suggestion keeps the code specific to this situation in one place.

    On the whole I think the advantages of being able to cleanly isolate the code to handle this in one location would make it possibly more accessible and easier to understand for developers.

    Although it does work differently to a construct (placeholders) used in a couple of other places.

    James

  • Walter McGinnis

    Walter McGinnis January 13th, 2009 @ 04:04 PM

    Your description of the issue with point 1 seems problematic to me. Moderators that restore an item to a previous version are creating a copy of old version correct, but it seems to me like if at some point in between a field is added that is REQUIRED, that should be honored for all versions going forward and subverting the requirement after the point in time it is established seems equally unintuitive to me.

    We establish in the history that a restored version has its content based on the original version, but is indeed a version in its own right. I think it is logical to say "A required field has been added since the original version was created, please add relevant data" or the like. If the moderator doesn't have the data, they may contact the original's author, etc. and retrigger the restore action again.

    So I suggest a compromise:

    • A restore counts as user interaction and therefore triggers an edit if required fields aren't filled out.

    • Flagging/moderation can just leave the empty xml element out as you suggest rather than go with placeholders.

    One piece of feed back I had on the extended field refactoring was that we need to have explicit documentation of the datastructure and its variants included in item.extended_content in rdoc fashion above the Module definition (admittedly, I'm the original culprit of this omission). This is where the special case of no element for required field should be spelled out.

    Cheers, Walter

  • James

    James January 20th, 2009 @ 04:17 PM

    • State changed from “open” to “to-review”

    This is now ready to review, including unit and integration tests.

    See branch bugfix_155_privacy_issue_related_to_extended_field_validation.

    James

  • Walter McGinnis

    Walter McGinnis January 20th, 2009 @ 04:47 PM

    • State changed from “to-review” to “open”

    I've reviewed the various pieces of code. Go ahead and merge to master and resolve the ticket.

    Cheers, Walter

  • James

    James January 23rd, 2009 @ 11:51 AM

    • State changed from “open” to “resolved”

    This is now merged into master. Resolving.

    James

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป

Kete was developed by Horowhenua Library Trust and Katipo Communications Ltd. to build a digital library of Horowhenua material.

People watching this ticket

Pages