321 Group per-node permissions system #327

Closed treasuretron closed 4 years ago

In GitLab by @thomasmurphy on Feb 20, 2019, 21:28

Establish the approach and estimate the hours for having per-node view-by-members permissions for project content.

In GitLab by @thomasmurphy on Feb 20, 2019, 21:30

changed title from Scope Group per-node permissions system to {+321 +}Scope Group per-node permissions system

In GitLab by @thomasmurphy on Feb 20, 2019, 21:30

changed the description

In GitLab by @jayelless on Feb 21, 2019, 22:57

I spent some considerable time thinking about this, and investigating how it might be implemented. I eventually came to the conclusion that this was not the correct approach, as the maintainer of the group module is very much against anything that alters the standard permission settings specified by the group UI.

I then considered alternatives, and came to the conclusion that if both organization and project groups were re-configured to be more restrictive in which user roles were allowed to view content, then it is a relatively straight-forward process to use a standard node access module like "permissions_by_term" to grant access to users who would otherwise be denied.

I have therefore installed the "permissions_by_term" contrib module and created a new taxonomy called "permissions_tags" which should contain two terms "public" and "private". The group_forum and resources node types have a reference field for one of these tags. If the tag is "public" then anonymous and authenticated users are allowed to view the content item, otherwise only validated group members will be allowed access (enforced by the group module).

I will generate a merge request so thsat this approach can be reviewed.

In GitLab by @jayelless on Feb 21, 2019, 22:58

mentioned in merge request !271

In GitLab by @jayelless on Feb 21, 2019, 23:23

Feature branch: https://pr-271-kpdpztq-ensz4qdpjmziw.us.platform.sh

In GitLab by @jayelless on Feb 25, 2019, 21:04

Release notes.

When this release is merged into dev or master, then there is a need to add the following two taxonomy terms into the new vocabulary "Permissions tags":

Public "This term will allow a tagged node to be viewed by all users."
Private "This term will restrict access to nodes to only members of the group."

Once these have been set up, it may be necessary to re-run the database update that processes all existing content types of group_forum_topic and resources so that these are set to the default of public visibility. This can be done via the following steps:

Enable the devel module
Open up an "Execute php form from the devel menu
Paste and run the following php script: "\Drupal::keyValue('system.schema')->set('spacebase_core', (int) 8024);"
Uninstall the devel module
Run update.php from the browser (do not use drush to run dbupdate!).

In GitLab by @jayelless on Feb 25, 2019, 21:52

I have enhanced the update function to check whether the required "Public" term exists before running, so it should not be necessary to set the module schema level back to 8024 using steps 1 to 4 above. Just try running the update by visiting update.php and confirm that spacebase_core update 8025 is still pending.

In GitLab by @thomasmurphy on Mar 12, 2019, 22:23

@jayelless Rich and I tested this here https://pr-271-kpdpztq-ensz4qdpjmziw.us.platform.sh/node/151

The content was marked as private but was visible to a test user that wasn't a group member.

In GitLab by @thomasmurphy on Mar 12, 2019, 22:29

After an editor creates a discussion thread and sets it to private, other users can comment and afterwards the original poster can set the thread as public again, defeating the privacy setting.

In GitLab by @jayelless on Mar 13, 2019, 17:01

Hi Thomas. A quick check showed that the user permission settings for groups and globally are different in the PR-271 environment from what I expected, as the behaviour described above was not what I had observed when testing in my local development environment. I will compare the two envornments, and update the PR-271 config appropriately. When that is complete, you can repeat your tests. James

In GitLab by @epaatdahlstrom on Mar 14, 2019, 14:53

I"m late looking at this thread and just wanted to know if I understood the feature correctly. In our discussions in Welly, the intention for the members of the project is to be able to post privately to all members of the group - and not just the owner or admin. This is important as members will definitely need to start their own private thread to the proj members to discuss project. Am I correct that this is part of the implementation plan? Thanks.

In GitLab by @thomasmurphy on Mar 14, 2019, 17:25

changed title from 321 {-Scope -}Group per-node permissions system to 321 Group per-node permissions system

In GitLab by @jayelless on Mar 14, 2019, 17:39

To summarize the discussion this morning:

All discussion threads created in project groups will default to "private" which means that only verified team members will be able to view the discussion.
Users with a "group admin" role (eg the project "owner") will be able to specify that a discussion thread is to be "public" at the time it is created. This means that the discussion will be visible to all users (whether logged into Spacebase or not).
A discussion thread cannot be changed to "public" (or from "public" to "private") once it has been created.

It also needs to be noted that only verified project team members will be able to create or participate in project group discussions.

In GitLab by @epaatdahlstrom on Mar 14, 2019, 18:02

Looks good.

Just one more question on the public thread. Will project members be able to comment on the public threads initiated by the admin, or not? I am asking because will they be able to distinguish what is a public or private post from admin? If they are able to comment, they should not make a mistake of commenting that is just for members than public view.

Emeline

In GitLab by @jayelless on Mar 14, 2019, 18:56

I expect that any accepted member should be able to comment on a public discussion topic as well as the private ones. We can look to use themeing to provide a visual indication in the form containing the comment text field, that this is a public discussion.

In GitLab by @epaatdahlstrom on Mar 14, 2019, 18:59

Great, thanks!

In GitLab by @jayelless on Mar 14, 2019, 18:59

All recent discussion on this issue has been looking at project discussion topics, but there is also a requirement for some "resources" to be able to be restricted so they are seen only by project team members.

Is there a similar requirement that a "private" resource should never become "public" and vice-versa? I note that only project admins (and global admins) should be able to create resources.

In GitLab by @epaatdahlstrom on Mar 14, 2019, 20:03

Thanks James. Yes, good point. Admins will be the only people who can create resources and they should be able to toggle from private to public. And yes, anything private should never become public.

-Emeline

In GitLab by @jayelless on Mar 21, 2019, 17:46

Investigation has suggested that the best way to implement a reliable and flexible solution is to integrate the permissions_by_term module to work with group roles as well as global roles. This is being done in the drupal.org issue https://www.drupal.org/project/permissions_by_term/issues/3041315

In GitLab by @thomasmurphy on Mar 24, 2019, 20:39

Deferred to after aerospace challenge launch at sprint meeting on 22/3/19. Group content will default to public instead.

In GitLab by @epaatdahlstrom on Mar 24, 2019, 23:02

Hi Thomas, can you reconfirm what is being deferred and what is "after challenge launch"? Just to be clear with terms Challenge Launch was 8 March. Actual Incubator launch is 1 May. We have set deadlines in April so as to give a buffer for delays and problems. I am still working under the assumption that all the features critical to the Challenge will be ready by May 1. Thanks.

In GitLab by @thomasmurphy on Mar 25, 2019, 00:55

Hi, we have a bunch of issues lined up in this ticket #339 for a 1/4/19 launch, so I was just noting that this issue isn't included in this release any more.

In GitLab by @thomasmurphy on Mar 26, 2019, 21:39

mentioned in merge request !283

In GitLab by @thomasmurphy on Mar 27, 2019, 19:10

We estimate around 20 hours for this when it gets prioritised. However it currently doesn't have a priority as per the list made on 15/3/19 by email, so there is no action occurring on this task currently.

In GitLab by @epaatdahlstrom on Mar 27, 2019, 21:44

Just agreeing that it should be prioritized for the Challenge.

In GitLab by @thomasmurphy on Mar 27, 2019, 21:57

@epaatdahlstrom we received an email from @richbodo on 22/3/19 based on a discussion between you both on slack

"From Em, regarding private resources and discussions:

Keep what we are sharing now visible to the public.
Implement private discussions and resources when we have the ability to support both public and private resources."

That is why I have removed this from the 1/4/19 deadline, which we'll be deploying to dev today and to production on Friday.

In GitLab by @epaatdahlstrom on Mar 28, 2019, 04:18

Yes, just confirming that when I mean Challenge - it is for the actual external deadline of May 1, not the internal deadline of 1 April. Thx.

In GitLab by @thomasmurphy on Mar 28, 2019, 17:20

So this now has a priority of 1.5 / 20 in the main SpaceBase prioritisation list, so adding high back on. (as per sprint meeting on 29/3/19)

In GitLab by @jayelless on Apr 11, 2019, 08:29

mentioned in merge request !289

In GitLab by @jayelless on Apr 11, 2019, 08:57

A patch to the permissions_by_term module has been prepared that integrates group roles with the module to provide the functionality that is required for spacebase.

This is now in the feature branch ready for testing, and eventual merging.

Currently there are two manual edits required to make this work.

The taxonomy vocab "permissions tags" needs to have at a minimum a term labelled "Public" so that the prepared DB update can run to make all existing discussion forums and project resources "public". Ideally, there should be other terms such as "private" in addition. Set permissions for each term appropriately.
The content entities "forum_discussion" and "resource" need to have the default value of the permissions tag set to an appropriate value (it is "private" on the feature branch environment).

In GitLab by @jayelless on Apr 12, 2019, 02:05

The updates necessary to prevent a private topic or resource from being changed to public (or vice-versa) except by global administrators and editors has been generated and pushed to the feature branch.

The permissions set on the two terms are:

Public: global roles of anonymous and authenticated.
Private: project roles of verified, admin, outside administrator, outside editor.

In GitLab by @thomasmurphy on Apr 18, 2019, 02:10

@jayelless I just tested this at https://pr-289-3mr3pha-ensz4qdpjmziw.us.platform.sh/node/226 With a test authenticated user. For the private thread I didn't have access when I wasn't a group member. I then applied for and was approved group membership by my admin account, cleared all caches and rebuilt the permissions but still get a 403 when attempting to view the group content, even though it is listed on the view on the project page here https://pr-289-3mr3pha-ensz4qdpjmziw.us.platform.sh/group/268

In GitLab by @epaatdahlstrom on Apr 20, 2019, 22:42

Hi, I tested this by creating a post while logged in. It looks like the "private" indicator shows up on the actual post but on the list, it does not for my post (the other posts do have it...) Just wanted to flag the inconsistent list.

Screen_Shot_2019-04-21_at_2.40.11_PM

In GitLab by @epaatdahlstrom on Apr 20, 2019, 22:47

For the Resources section, if I click on the "edit" icon while logged in, it opens up to the resource edit page and I can edit. However, if I click on the actual post link (in this case "test3"), it gives me this error. Screen_Shot_2019-04-21_at_2.44.58_PM

Screen_Shot_2019-04-21_at_2.45.18_PM

In GitLab by @jayelless on Apr 23, 2019, 00:24

The issue reported by Tom above is caused by the fact that the test user is not being granted the role of "verified member" at the time their membership application is approved. This means that they do not get access to private group content. I will investigate and fix this problem.

In GitLab by @jayelless on Apr 23, 2019, 00:27

This issue of the status of messages being marked as public or private is that on the full message, the status value is displayed, but it is not shown on the lists. The indication referred to by Emeline above is because the status has been manually appended to the title to allow for easy testing.

If the status needs to be shown in the list, then it can be added, or perhaps CSS used to display private messages with a different background.

In GitLab by @thomasmurphy on Apr 23, 2019, 00:31

Hi James, for simplicity let's just display the taxonomy term at the top of the content.

In GitLab by @jayelless on Apr 23, 2019, 00:35

The issue reported above by Emeline regarding the "test3" link in the resources section was because the link entered into the resource entity form was rubbish and so could not be reached (it just happened to be a link that looked like group content). I have edited the liunk so that it now goes back to the project home page, and it works as expected.

In GitLab by @jayelless on Apr 23, 2019, 18:46

I have pushed a fix to the issue reported by Tom above. This appears to resolve the problem, so the feature branch should be ready to merge to dev.

In GitLab by @thomasmurphy on Apr 23, 2019, 19:51

closed via merge request !289

In GitLab by @thomasmurphy on Apr 23, 2019, 19:51

mentioned in commit a31ec56a41c23e0083630e3ca18b513413a4ddd7

In GitLab by @thomasmurphy on Apr 29, 2019, 01:36

mentioned in merge request !291

In GitLab by @thomasmurphy on May 2, 2019, 21:39

@epaatdahlstrom I see now why you wanted resources to be labelled a private or not in the listing - because they are external resources, there is no full view of the node to display the permission tag on, once they click on the listing it takes you to the external URL.

This listing is made by the same view which is listing external resources for organisations, because you asked us to repurpose as much of the org code as possible. This means that if I change the resource listing for projects, this will affect the resource listing for organisations. On checking, all the logic for project content access permissions is also currently applying to organisations, so it makes sense to display it there, too. I'll have a go tweaking the view to display the status of the resources.

In GitLab by @epaatdahlstrom on May 2, 2019, 22:05

Thanks Thomas.

nz-aerospace-challenge

In GitLab by @thomasmurphy on May 2, 2019, 22:41

I have tweaked the organization_resources view on live, and committed this view change to the direct messaging branch (280) so it ends up merging into master soon. The Public/Private status displays in Parenthesis after the title.

In GitLab by @thomasmurphy on May 2, 2019, 22:46

I've done a similar fix for discussion forum listing eg https://spacebase.co/group/267/forum This is the kind of layout I can without a detailed front end fix, but it's no too bad.

In GitLab by @epaatdahlstrom on May 2, 2019, 22:52

Nice! Yes, certainly works.

nz-aerospace-challenge

spacebase / spacebasenz

321 Group per-node permissions system #327