Feature Proposal: Add a static context

Motivation

Some plugins should not render content when in read-only environments, such as when PDF or HTML for publishing purposes is being generated. Currently there is a mixture of solutions used including:

• Assumption that unauthenticated access == readonly
• Checking for URL parameters like contenttype=application/pdf
• Maintaining a list of incompatible plugins and disabling them

Description and Documentation

This proposal adds a "static" context. Publishing Extensions like GenPDFAddOn, PublishPlugin, and others would set "static" to true. Other plugins such as EditChapterPlugin would test for static before adding editing markup. static would be a "soft" setting - controlling rendering but not actually blocking edits. <strike>This could be extended to also permit core to set / clear and honor a readonly context.</strike>

Initial commitment will be to implement the changes to the documentation, GenPDFAddOn and EditChapterPlugin.

It was decided that any changes to core to implement a true readonly context would require a different proposal. Consensus also indicated that readonly was too confusing with topic permissions, so the context is changed to static.

Examples

• Authentication methods could set the readonly context if an authenticated user is not allowed to edit.
• bin/configure could set a readonly context if site maintenance should block rendering edit links and markup
  • context = "maintenance" and "readonly" = hard enforcement?
• skins could render or remove edit buttons depending upon the readonly context.
• Plugins that add markup via macros would render the macro as null removing the markup. Examples include EditTablePlugin and EditTablerowPlugin

Impact

Initial implementation will only impact the GenPDFAddOn and EditChapterPlugin. These were identified as having a conflict by Tasks.Item8575

Core should probably not add readonly support until it is more widely honored, or possibly enforced by the Store engine.

WhatDoesItAffect:	%WHATDOESITAFFECT%	edit
GenPDFAddOn	Set context=readonly in Switchboard entry.	edit
EditChapterPlugin	Test and exit if context=readonly	edit
Documentation	Define the readonly context. (As available but not widely implemented)	edit

Implementation

-- Contributors: GeorgeClark - 22 Feb 2010

Discussion

From #Foswiki

(09:34:54 AM) gac410: Wondering if I could change EditChapterPlugin to also test a pdf "context" to disable the edit icons
(09:35:26 AM) gac410: The urlparam for contenttype doesn't work with GenPDFAddOn which uses a view script replacement.
(09:35:49 AM) MichaelDaum: actually I'd prefert to minimize the impact on the html that editchapter has got ... using more intelligent jquery
(09:36:32 AM) MichaelDaum: oh it doesnt? why?
(09:36:39 AM) gac410: GenPDFAddOn uses htmldoc which doesn't understand js or css.  Very simple HTML - EditChapter inserts the little pencils into the generated html.
(09:37:07 AM) MichaelDaum: ya right and you'd like to add an if url param contenttype is pdf then no pencils?
(09:37:55 AM) gac410: You already test if urlparam=contenttype=application/pdf - but genpdf gets invoked without that param.
(09:38:08 AM) MichaelDaum: ah
(09:38:43 AM) MichaelDaum: what about adding the pencil behavior dynamically using jquery instead polluting the markup
(09:38:49 AM) MichaelDaum: that would help GoToTopPlugin as well
(09:39:02 AM) MichaelDaum: similarly all %TOC% related css would be much simpler
(09:39:36 AM) gac410: Okay with me - but not simple fix for me -  don't know javascript.
(09:40:06 AM) gac410: EditChapterPlugin already has "return unless context = view"  - just want to tweak that to return if context = pdf.
(09:40:33 AM) MichaelDaum: so which context does genpdf use?
(09:40:40 AM) MichaelDaum: it seems it uses view as well?
(09:40:42 AM) gac410: genpdf provides both context=view and context=pdf.   
(09:40:52 AM) MichaelDaum: why?
(09:40:54 AM) gac410: Because other plugins die if NOT in view context.    Catch22.
(09:41:02 AM) MTempest: May I suggest a "readonly" context?
(09:41:50 AM) MichaelDaum: gac410, I think I get what you want. go ahead. makes sense.
(09:42:09 AM) gac410: Ah - readonly goes along with context = authenticated - which editchapter checks.     And is more general. 
(09:43:08 AM) gac410: Okay - unless objections, I'll add a "readonly" context to genpdf instead of pdf.  
(09:43:43 AM) MTempest: Hmmm. readonly is not necessarily equivalent to "not authenticated"
(09:44:27 AM) gac410: Agree - just commenting that EditChapter does check authenticated - IIRC   and also disables the edit icons if not authenticated.
(09:44:53 AM) MTempest: :)
(09:45:06 AM) gac410: Which I suppose in some environments would be incorrect.  
(09:45:29 AM) ***MTempest nods
(09:46:03 AM) MTempest: But there should never be pencils when rendering for a readonly environment, such as a PDF or a set of static HTML pages
(09:46:47 AM) gac410: Okay - I'll add and document a readonly context.   I'll change EditChapter to honor readonly.    Would probably also apply to EditTable, EditTableRow,  and a few others.
(09:48:17 AM) gac410: And the other "pdf-generator" plugins could set readonly in an early handler of some type as well - but that's for others to add.
(09:48:59 AM) gac410: That would be more general than testing contenttype=application/pdf   and would work for other alternative outputs - if we have any.
(09:49:10 AM) gac410: Thanks MichaelDaum, MTempest.
(09:49:37 AM) MichaelDaum: M-power
(09:49:45 AM) gac410: :-)
(09:50:50 AM) MTempest: hehe - thanks for implementing readonly :)
(09:51:17 AM) gac410: I assume hopefully a small enough change to not need a proposal.
(09:52:44 AM) gac410: Hm...   Maybe worth doing just to get the change more widely documented.  And a place to list other impacted extensions.
(09:53:04 AM) gac410: (Wish I understood how %WHATDOESITAFFECT% is supposed to work - )
(09:56:42 AM) gac410: MTempest:  Does this readonly context impact your proposal LetPluginsDisableOtherPlugins
(09:57:23 AM) ***MTempest looks
(09:57:52 AM) gac410: I think you were trying to accomplish similar things -  PublishPlugin should disable the EditBlah plugins.
(09:58:36 AM) gac410: Readonly would accomplish some of that requirement as well, but from a different angle.
(09:58:44 AM) MTempest: Yes, and I wanted to use contexts to do it.
(09:59:33 AM) MTempest: "Readonly" context makes sense independently of LetPluginsDisableOtherPlugins, and is consistent with it.
(10:00:36 AM) gac410: Okay - I'll add a proposal.  More I think of it, it might make sense at a broader application - we've had requests like "how do I make a wiki readonly during maintenance"
(10:01:06 AM) gac410: Adding context to GenPDF, EditChapter,  etc.  can be done on the edges, but core could pick up the concept.
(10:01:17 AM) MTempest: Agreed
...
(11:22:08 AM) wbniv_: [09:00] <gac410> Okay - I'll add a proposal.  More I think of it, it might make sense at a broader application - we've had requests like "how do I make a wiki readonly during maintenance"
(11:22:26 AM) wbniv_: can anyone write a faq for that?  i've also wonder what the "proper" procedure is
(11:22:32 AM) wbniv_: wondered*
(11:45:11 AM) gac410: I don't think we have a good way to answer that.   we've done things like rename edit script - but as more rest handlers get added, that doesn't help much.
(11:47:43 AM) gac410: Another question is do we need a "really - readonly"  that blocks admins.   IMO I'd say "readonly" should be absolute if set at the core level.  Set as a context it is "soft" and used for plugins, etc.  If set by bin/configure it would be a hard readonly.    I'll add to proposal.
(11:47:58 AM) ktwilight_: redirect all cgi-bin/edit/ to cgi-bin/view/MaintenanceTopic?
(11:48:54 AM) gac410: More complex because more edit functions are moved to REST handlers - but that's  a reasonable solution for 90% of the time.
(11:50:46 AM) ktwilight_: hmmm
(11:51:04 AM) ktwilight_: would be nice if maintenance mode is part of the core
(11:51:11 AM) ktwilight_: then again, there are lots of things that should be in =configure=
(11:52:09 AM) gac410: Yes - I'm trying to work a readonly maintenance mode into the proposal.  
(11:53:02 AM) wbniv_: is there not accepted practice for what we have now?
(11:53:15 AM) gac410: I've never found a good answer
(11:53:26 AM) gac410: (That I can remember anyway)
(11:54:03 AM) wbniv_: yeah, ok :/
(11:54:08 AM) wbniv_: i can believe that
(11:57:46 AM) gac410: Searching T.O - suggestions were "redirect" in web server config to a maintenance page -or-
(11:58:09 AM) gac410: Set ALLOWWEBCHANGE = admin group, and make it a FINAL preference
(11:58:57 AM) gac410: Recognizing that ALLOWTOPICCHANGE can override ALLOWWEBCHANGE.

-- GeorgeClark (remember the signature in thread mode topics)

I recognize this need.

I also rename save and edit and rest when I do updates. It is rare but it happens. You do not want people to save anything when you transition from one server to the next.

I normally make a broadcast message that is 8 lines tall combined by renaming scripts (appending an _)

But a configure setting to turn off editing would be cool. Not high priority but if anyone wants to do it - and it seems George wants to I will support this. People need to know before they hit edit. And it is not enough to protect edit. You can make apps that save directly from forms and as you notice also rest can save. Actually even view can save! EditTablePlugin is a good example of a plugin that saves with view.

So make sure to block the saving the right place. False security is worse than no security.

-- KennethLavrsen - 22 Feb 2010

I'm in favour in principle, however I'm not entirely sure I know what "readonly" means in this context. Does it mean that:

1. Interactive controls in the default skin are all disabled?
2. A user cannot edit, but can still attach? Can still COMMENT?
3. It's purely advisory i.e. a plugin can choose whether to ignore readonly or not?
4. Nothing a user does will affect the database?
5. Something else?

Also, I'm not clear under what circumstances the flag gets set. You mention configure - but does that setting override all other potential setters? Can an extensions set readonly? If it does, what does that mean for other extensions?

Finally I don't think this is a core proposal unless you also address:

1. The default (pattern & plain) skins
2. All the default plugins

Anything else is less than half a job, and runs the risk of masking potential problems.

I think we need a lot more thinking through the detail, answers to these and other questions, so I'm raising a concern.

-- CrawfordCurrie - 23 Feb 2010

Some explanatory short words: the context hash is a way to communicate between different areas of the core code as well as between independent plugins. The purpose of this proposal is to attach a bit of semantics to a flag called "readonly".

This proposal is not about an obligatory mechanism. It is more of a recommendation how to get things work out nicely.

It means: the page being rendered

• is not supposed to contain widgets or forms for user interaction
• it is supposed to be read only

Most of the time this flag is used to render some content type meant to be printed out, but that's not necessarily the case.

Another possible wordings for "readonly" could be "static-content".

Candidate extensions that might set this context:

• all GenPDFxxx plugins
• all Publishxxx plugins
• RtfContrib
• all Exportxxx plugins
• ...

Candidate extensions that might read this context:

• CommentPlugin
• EditTablePlugin
• EditRowPlugin
• JQueryPlugin
• EditChapterPlugin
• EditSectionPlugin
• GoToTopPlugin
• ....

(please flesh out and add more as appropriate).

As far as I see this feature proposal is about to write down the above as part of the DevelopersBible.

-- MichaelDaum - 23 Feb 2010

Readonly does not mean non-authenticated or write-not-allowed, but a plugin could be written to set the readonly context if the current user does not have write permission. If the skins and plugins honour the readonly context, then editing controls would only appear if the user has write permission. This would typically mean that you would have to log in before being able to edit.

This would provide a way to hide edit controls from the random public for those that want it that way for aesthetic reasons. It would not function as a security feature at all.

-- MichaelTempest - 23 Feb 2010

I struck the comments on "maintenance mode" as that confuses the proposal. My original purpose was to control rendering of edit decorations for page export purposes. It could potentially help to discourage edits during maintenance and improve aesthetics for users without update authority. But changes necessary to implement a strictly enforced block on updates would be considerably more complex.

To respond to Crawford's concerns:

1. Interactive controls in the default skin are all disabled?
   • Yes - Either disable or remove from view.
2. A user cannot edit, but can still attach? Can still COMMENT?
   • N/A - The user will not see the edit/attach/comment controls, but if they crafted a URL, or the control is rendered through other than the default mechanism, it would be out of scope of this proposal. Initially it seemed that combining the concepts of a maintenance lock and a rendering control made sense, but the purpose and implementation would be different and complicates things.
3. It's purely advisory i.e. a plugin can choose whether to ignore readonly or not?
   • Yes - although it probably would be considered a bug if edit markup was being rendered into PDF or other external export.
4. Nothing a user does will affect the database?
   • No - not a maintenance mode. However this would assist by hiding the obvious, so if a "Maintenance" mode were added, it would also set readonly.

Also, I'm not clear under what circumstances the flag gets set. You mention configure - but does that setting override all other potential setters? Can an extensions set readonly? If it does, what does that mean for other extensions?

• I don't think we should have a mechanism to block the context from being cleared. Context is not persistent, so I'm not sure I understand why an extension would want to clear the context. So if set in configure, I don't see it being "overriding" in that it could be cleared. Is is sufficient to add to the documentation that readonly should be set, but not cleared.
• Yes an extension could set the readonly context, however it would have to be in an early handler or would become dependent upon the extension execution order. I don't believe it would make much sense to set for most extensions. It would be more for use by Contribs that render output for export.
• Configure was an afterthought. Primary purpose is for an extension to ask that edit markup be omitted from the rendered version. Disabling a plugin is not the right approach as it will often leave behind the %MACRO% markup in the topic. So this flag tells the plugins that they are active, but should omit any buttons, pencils, or other markup that would not be useful in an exported document.
• Use by a Authentication contrib might make sense in some cases - I don't think it should be precluded. Our current model is to show edit controls to un-authenticated users and to prompt for authentication when edit is requested. I am not suggesting that the model be changed. However this would make it easier to implement and alternative model where "change" operations are hidden until the user is authenticated.

Finally I don't think this is a core proposal unless you also address:

1. The default (pattern & plain) skins
   • Agree
2. All the default plugins
   • Agree

Plugins that do external rendering probably use the print skin which omits the skin provided controls. But I see that adding the readonly processing to the other skins makes sense to do.

I see implementation by plugins something that can be done immediately for "in-topic" rendering of markup. Changes to the skins would be done for 1.1, and not planned for 1.0.x stream.

-- GeorgeClark - 23 Feb 2010

With that clarification of spec, I remove my concern. Thanks, George!

-- CrawfordCurrie - 23 Feb 2010

Regarding plugin rendering, CommentPlugin for example, disables but renders the Comment form and button when it detects a readonly type environment, such as "preview". Since the purpose of preview is to show the topic as it would appear, I believe that readonly context is different from preview, which is also readonly, but should render the markup.

(What is proper way to document these changes? Edit them into the WhatDoesItEffect box, describing the needed changes to the plugin?)

-- GeorgeClark - 23 Feb 2010

is readonly is the evaluation of the TOPICCHANGE ACL? if so, can we call it ALLOWCHANGE ? and add the equivalent ACL context's too?

this is slightly related to my older AddAContextVAR proposal - in that I want to define the conditional rendering of edit actions using css statements - reducing the need for server evaluated =IF='s.

note that that proposal is superceeded by the much more general CleanerSyntaxForMetaDataAccess.

to re-iterate my point - given a sufficiently detailed <body class="%!EVAL{context}%" most of this doesn't need intopic IF's, it can be done using static css.

-- SvenDowideit - 24 Feb 2010

Initially I had not planned to tie this into the ACLs to change a topic. It is a rendering context (like view, diff, etc) The purpose is to render the topic as if edits were not allowed. Primary use is to notify plugins that any %MACROS% should not insert edit markup (Edit button beneath the table, etc). Informing the plugins that the output of the rendering process is destined for a use where editing actions would not make sense. (PDF, HTML, RTF, etc.)

Setting an ACL based context sounds like a good idea, and I'm willing to give it a try. Could someone who understands the ACL evaluation internals comment. Are all ACLs evaluated on every access such that adding a set of ACL based context's would not add significant overhead? (ie. Is ALLOWTOPICCHANGE evaluated for a vew action). I wouldn't want to add significant overhead to set context variables.

For added contexts, ALLOWCHANGE, ALLOWRENAME - are there any others? the WEB* permissions cascade to the TOPIC level, so for the readonly use case, I don't think they make sense.

Also I think if we tie it into ACL's it needs to be optional for the skin. If all the edit links disappear for the WikiGuest, then the user can't click edit to initiate the login, so it's a significant change to the interaction. (Although I think I'd prefer to replace the Template Login page to a small Login box that appears in the left/right bar similar to Google. (And the POST should optionally use HTTPS by config setting). But that's getting WAY off topic.

Looking through view.pm,. change access is not evaluated. I'm concerned about the performance impact of checking additional permissions for every view. Rather than separate calls to Meta::haveAccess to test Change and Rename access, it might be better to add a function to return the permissions in a single call. Add the following method to Meta:

ObjectMethod getAccess($cUID) -> (view, change, rename)

• $cUID - Canonical user id (defaults to current user)

Retrieve the modes of access this user has for this topic. This call may result in the topic being read. Returns a list of ALLOW/DENY VIEW, CHANGE and RENAME. A user who cannot change a topic would return (ALLOWVIEW, DENYCHANGE, DENYRENAME)

Next question - would it make sense to add DENY... and ALLOW... contexts, so that either can be tested for true.

So I propose: View.pm would then set ALLOWVIEW = 1, DENYCHANGE = 1 and DENYRENAME = 1 as contexts. ALLOWCHANGE and ALLOWRENAME would not be set. A plugin would then either test DENYCHANGE or ALLOWCHANGE instead of readonly context. And GenPDF would set DENYCHANGE instead of readonly.

-- GeorgeClark - 25 Feb 2010

Sven is right, in that readonly is a bad name for the concept, it's too easy to confuse with ACLs (from which it must be independent). noninteractive (or static) would be better. There may even be a real requirement for a whole family; noninteractive, uneditable, nocss.

I think this is another conditional rendering mechanism. If you were going to do it today, you might use a different skin, one that does not show interactive elements. What we are talking about here is a set of modifiers that skins can use to classify the current rendering operation.

-- CrawfordCurrie - 25 Feb 2010

I agree with Crawford and Sven that readonly is a bad name. This should not be confused with ACLs at all. Please limit the scope of this proposal to exclude any ACL-related functionality.

However, this is not just about skins. This is also about the plugins we have now. Those plugins can test for contexts now - the infrastructure is already there. Please don't require plugins that create interactive elements to also integrate with the plugin system.

-- MichaelTempest - 25 Feb 2010

I like either of static or noninteractive - leaning toward static.

I don't believe that there is anything in this proposal that would require a plugin to create interactive elements. Unless objections are raised I'll rename readonly to static saying that rendering is being done for a non-interactive environment, such as PDF, external HTML, RTF or other published output.

As far as the ACL contexts, I believe that there is value, as Sven commented, in providing plugins and skins also with an easy-to-test context reflecting the ability for the user to change the topic. It appears to be a relatively easy change. The one complicating factor is the Cache work. From what I can tell, it appears that Cache doesn't cache the ACL's, and will return the cached topic without an ACL check. I don't undersand the implications of this yet.

-- GeorgeClark - 25 Feb 2010

Sorry, George. I meant: Some plugins produce interactive elements. Those plugins should not have to interact with the skin system.

-- MichaelTempest - 26 Feb 2010

Note that Tasks.Item8656 was recently opened requesting that the %EDITTABLE not render the button if the user is not allowed to edit.

-- GeorgeClark - 03 Mar 2010

There has been a good discussion around many thing this feature could be used for.

What has been proposed is the simple feature of adding a new context readonly.

The proposal does not involve changing behaviour of any default plugins or core code. That would be separate proposals.

Accepted by 14-day rule and by consensus after good discussion.

-- KennethLavrsen - 09 Mar 2010

I think the last few comments moved toward using "static" or "noninteractive" - as the term readonly is too confusing as to whether it implies access rights vs. rendering. Unless there are further concerns, I'm going to use static as the context - to indicate that the page is being rendered into a static environment and should not show edit controls.

-- GeorgeClark - 09 Mar 2010

Sorry, I was not exact in my conclusion

Naturally it is the feature with the name called "static" that got accepted.

You can avoid the confusion in future by always updating the proposal at the top.

-- KennethLavrsen - 09 Mar 2010