User with WIKI_CREATE can't create a wiki page unless they also have WIKI_MODIFY

[Ryan J Ollos <ryan.j.ollos@…>]

If a user has WIKI_CREATE but not WIKI_MODIFY, any non-existent page will contain text suggesting that a page can be created, but the create page button is not present, so the text is in fact misleading.

User           Action         
------------------------------
anonymous      BROWSER_VIEW   
anonymous      CHANGESET_VIEW 
anonymous      FILE_VIEW      
anonymous      LOG_VIEW       
anonymous      MILESTONE_VIEW 
anonymous      REPORT_SQL_VIEW
anonymous      REPORT_VIEW    
anonymous      ROADMAP_VIEW   
anonymous      SEARCH_VIEW    
anonymous      TICKET_VIEW    
anonymous      TIMELINE_VIEW  
anonymous      WIKI_CREATE    
anonymous      WIKI_VIEW      
authenticated  TICKET_CREATE  
authenticated  TICKET_MODIFY  
authenticated  WIKI_CREATE    
authenticated  WIKI_MODIFY

Second, it is confusing that WIKI_CREATE without WIKI_MODIFY does not actually allow the user to do anything. The user cannot create a page unless they also have WIKI_MODIFY. This could be dealt with in at least two possible ways:

• WIKI_CREATE granting WIKI_MODIFY.
• WIKI_CREATE actually allowing pages to be created but not edited after creation.

Is there really a use case for which a user should be able to create pages but not modify them? I tend to think that WIKI_CREATE should grant WIKI_MODIFY.

Patch is forthcoming.

[Ryan J Ollos <ryan.j.ollos@…>]

After spending some time on this, I realized that the only issue that needs to be solved here is allowing a user with WIKI_CREATE but not WIKI_MODIFY to actually be able to create a page. With that, the misleading text is no longer misleading, and although I think WIKI_CREATE granting WIKI_MODIFY might be a good idea, such a major change doesn't need to be the subject of this ticket. After applying the patch in ​6f05aace, the behavior of WIKI_CREATE mirrors that of TICKET_CREATE, even if having the ability to create but not modify wiki pages doesn't make quite as much sense in a realistic use-case.

I'll follow-up here with some changesets and comments about the functional tests that I've tried to implement for this ticket.

[Peter Suter]

Does this change possibly leak information about which (​private) pages exist?

[Ryan J Ollos <ryan.j.ollos@…>]

Replying to psuter:

Does this change possibly leak information about which (​private) pages exist?

I tried to do some testing with ​th:PrivateWikiPlugin, but the plugin seems to be completely broken (​th:#10634), so I have some work to do there in order to test further with that plugin. What is leading you to think that information might be leaked? The patch shouldn't change the existing behavior for anyone that doesn't have WIKI_CREATE. Fine-grained permission checks are performed before enabling the submit button for users that have WIKI_CREATE.

I have a suspicion of a way in which information could be leaked about the existence of pages, but the issue precedes this patch. I'll do some testing with fine-grained page permissions in place to see if this pans out.

[Peter Suter]

Right, I misinterpreted your changes. Sorry for the confusion.

[Ryan J Ollos <ryan.j.ollos@…>]

Replying to psuter:

Right, I misinterpreted your changes. Sorry for the confusion.

Thanks for taking a look though, I appreciate the review and want to do anything I can to improve the patch so that we can get it integrated.

[Ryan J Ollos <ryan.j.ollos@…>]

Replying to Ryan J Ollos <ryan.j.ollos@…>:

I tried to do some testing with ​th:PrivateWikiPlugin, but the plugin seems to be completely broken (​th:#10634), …

This was my mistake. It mostly works, but has some strange behavior that I hope to improve on in ​th:#10635.

I've been looking at setting up functional tests for this patch, for which we'd need a user with only TICKET_CREATE and another with only TICKET_MODIFY. The environment that is shared by all of the functional tests doesn't seem to lend itself well to these types of tests. I can see two possibilities for adding the tests:

• Grant and then remove permissions for the users that have been created when the environment is setup (admin, user, anonymous). Example: logout as user admin (at the start of every test case admin is logged in), grant TICKET_CREATE to anonymous, execute the tests and then cleanup - revoke TICKET_CREATE from anonymous and log back in as admin.
• Remove the authenticated group when the environment is created, and then create users in the test case, granting them the permissions needed for that test case and give them names that are very unique (e.g. user10957 = "user" + regression ticket #) so that we don't even need to worry about cleaning up these users at the end of the test.

Or maybe I'm overlooking something and there is a much better way?

[Christian Boos]

Replying to Ryan J Ollos <ryan.j.ollos@…>:

Or maybe I'm overlooking something and there is a much better way?

Well, the way you did it in #10984 seems OK.

[Ryan J Ollos <ryan.j.ollos@…>]

Thanks for the feedback. I'll plan to add a functional test to the patch in this ticket within the next week or so (hopefully sooner). #8976 is another that I think just needs a functional test to accompany the minor code change.

[Ryan J Ollos <ryan.j.ollos@…>]

Functional tests have been added to the Git branch.

• FunctionalTestEnvironment class methods from ticket:11028:t11028-r11682-1.patch​ were added in ​4d83b31a. If ticket:11028:t11028-r11682-1.patch​ is committed before the changes in this ticket then this changeset can be ignored, with two possible exceptions. I found it necessary to restart the web server even in grant_perm, which will cause a conflict as described in comment:6:ticket:11028 (see traceback and associated comment). Also, documentation for the methods was added.
• Functional test case added in ​994c3117.
• The following changes were made to the FunctionalTester class:
  • create_wiki_page will now create a random unique CamelCase name for the page if none is specified, and will return the name of the page.
  • Refactored create_wiki_page to make use of the go_to_wiki method.
  • Extracted part of create_wiki_page to a new edit_wiki_page method.
  • create_wiki_page returns to the created wiki page rather than remaining on the timeline page when the method returns. This seems like more correct behavior, though we might have the concern that a plugin could be depending on the existing behavior of remaining on the timeline page.

The changes in the Git branch need to be rebased against the current Trac trunk. I think that what I need to do is rebase the trunk of my Git fork against the current Trac trunk, and then rebase my branch against the trunk. I haven't figured out how to do that yet, but it is on my todo list.

[Christian Boos]

Fix and test from ​t10957.2 committed in r11770.

Other refactorings from that branch committed separately in r11772 and r11774 (well, the latter should probably have been part of r11770, but I think I grew tired of shuffling things around ;-) ).

[Christian Boos]

And thanks again!