Modify

Opened 10 years ago

Closed 9 years ago

Last modified 9 years ago

#11367 closed enhancement (fixed)

Restrict projects to a subset of trac users

Reported by: endquote Owned by: falkb
Priority: normal Component: SimpleMultiProjectPlugin
Severity: normal Keywords:
Cc: Trac Release: 1.0

Description

Is there any way to hide certain projects from certain users?

At my company we have many projects and staff members have access to all of them. However we might have a consultant help out with one project and they should only see the one they are working on.

Attachments (0)

Change History (46)

comment:1 Changed 10 years ago by falkb

Nice idea. Maybe another config field on <tracinstance>/admin/projects/simplemultiproject/<id> listing a set of users or groups, respectively, to those the project will be available.

comment:2 in reply to:  1 Changed 10 years ago by falkb

Replying to falkb:

Nice idea. Maybe another config field on <tracinstance>/admin/projects/simplemultiproject/<id> listing a set of users or groups, respectively, to those the project will be available.

Josh, please, describe what "available" would mean to you.

comment:3 Changed 10 years ago by endquote

The ideal configuration setup for my use case would be:

  • All projects are available to all users by default
  • To restrict a user to a subset of projects, provide the username and list of projects somewhere

I've noticed a few plugins for managing users/groups already, perhaps it could be integrated with one of those.

For me, "available" means that the user only sees the projects they have access to in the project dropdown in the new ticket form, only their projects are returned in searches and queries, only their projects appear on the timeline/roadmap, etc.

This would be a big win for my setup, so if any progress is made and you need a tester, let me know.

comment:4 Changed 10 years ago by falkb

Somehow it sounds to me, it's just testing the user, who is currently logged in, against the list of restricted users for a project at certain source code lines. I suppose it's not even necessary to use other plugins for that.

comment:5 Changed 10 years ago by falkb

Keywords: planned added
Status: newassigned

comment:6 Changed 10 years ago by endquote

Agreed, I think the two things that need doing are:

  • Define a list of users and the projects they have access to -- if this were a config-file or a set of trac-admin commands, that would be fine. Maybe a UI could come later, but that's less important.
  • Whenever a project list is displayed, check that list against the currently logged in user to filter the list.

Is there any thought as to when this might make it on the roadmap? Thanks for your consideration!

(This is the same person as before, I just made an account.)

comment:7 Changed 10 years ago by falkb

Josh, I also have "closing of projects to get them out of the lists" on my roadmap. I put that off for some months now (because of missing time (as always ;) ), but the threshold for "must happen now" is almost reached. This ticket gives another trigger to start, it's somehow related to "closing of projects" because it also aims to hide stuff from the lists.

comment:8 Changed 10 years ago by falkb

see also (duplicate) #10863

comment:9 Changed 10 years ago by endquote

Wanted to check in on this... any chance of an update before the end of the year? Is there anything I can do to assist?

comment:10 in reply to:  9 Changed 10 years ago by falkb

Replying to endquote:

Wanted to check in on this... any chance of an update before the end of the year? Is there anything I can do to assist?

I've started on it recently, and extended the db and GUI a bit already, and then I stalled again due a lack of time. I think I can solve it during December. If the db extension is thought to be stable enough, I could commit it as a first draft.

comment:11 Changed 10 years ago by endquote

Good news, thank you :)

comment:12 Changed 10 years ago by falkb

In 13474:

started 2 new features:

  • close of projects (see #11377)
  • user restriction for projects (see #11367)

implemented

  • admin GUI
  • db extension
  • upgrade from plugin version 4 to 5 (two new fields 'restrict' (integer) and 'closed' (text) to table 'smp_project')

comment:13 Changed 10 years ago by falkb

I'm going to filter out all projects which are not allowed for the current user similar to the filtering-out of closed projecs (see comment:ticket:11377:3). The current username is in req.authname and the list of allowed users is in column 5 of the smp_project table.

The idea is to extend get_all_projects_but_closed() in model.py somehow this way:

======= model.py =======
- def get_all_projects_but_closed(self):
+ def get_all_projects_but_closed(self, req):
      all_projects = self.get_all_projects() # get them all unfiltered

      # now filter out
      if all_projects:
          for project in list(all_projects):
              project_name = project[1]
              project_info = self.get_project_info(project_name)
              if project_info:
                  if project_info[4] > 0:
                      # column 4 of table smp_project tells if project is closed
                      all_projects.remove(project)
+                 else:
+                     # column 5 of table smp_project returns the allowed users
+                     restricted_users = project_info[5]
+                     if restricted_users:
+                         user_list = [users.strip() for users in restricted_users.split()]
+                         if (req.authname not in user_list): # current browser user not allowed?
+                             all_projects.remove(project)
      # return what remains after filtering
      return all_projects

This would just mean to remove such projects from all displayed lists and selection-comboboxes, which means to hide them. Though nevertheless manually typing the right URL in the browser address input-line, the user would still be able to access the hidden projects. At present, I'm not sure if this approach is a good idea, and how far "restrict" should actually go. Josh, any idea?

comment:14 Changed 10 years ago by endquote

The approach of extending the closed-project functionality to also include the restricted-project logic makes sense to me, and does most of the work of keeping folks out of projects that they shouldn't be in.

However it would be good if typing/guessing URLs wasn't a way around it. For example someone might have access to a project, so they know about it, and then that access might get revoked, and their old bookmarks should no longer work.

Maybe there is a plugin hook that is invoked before the page is rendered, where the current user can be checked against the current project, and an error shown if access is denied?

comment:15 Changed 10 years ago by falkb

In 13478:

  • more work for user restriction of projects, timeline may work already (see #11367)
  • renamed get_all_projects_but_closed() to get_all_projects_filtered_by_conditions() since it also checks the user-restrictions now

comment:16 Changed 10 years ago by falkb

In 13479:

see #11367: on roadmap page: hide project content from users who are not listed in the 'restricted to users' list (if such list is not empty (empty means 'no restriction')

comment:17 Changed 10 years ago by falkb

Keywords: testing added; planned removed

Josh, I have a feeling I have done all necessary work for this feature. Could you try it and report back?

comment:18 Changed 10 years ago by endquote

I was able to get this working. If I restrict a user from a project, they do not see that project in the dropdowns in the ticket editor.

However, that user can still see a ticket in a restricted project if they go to the ticket URL directly. These tickets also appear in any of the reports under "view tickets" or when searched for in a custom query. Ideally there would be some low-level hook -- whenever tickets are queried from any page, the results would be filtered so that only tickets from projects that the current user has access to would be returned.

Active tickets for closed projects show up in searches as well -- not sure if that's the correct behavior.

Last edited 10 years ago by endquote (previous) (diff)

comment:19 Changed 10 years ago by endquote

Also, thanks for working on this!

comment:20 Changed 10 years ago by anonymous

Keywords: planned added; testing removed

yes, you're right, I missed that case ... coming soon...

comment:21 Changed 10 years ago by endquote

Another thought... maybe block email notifications from going to users that aren't on a project? Say a ticket is owned or cc'd by someone, and then they are removed from the project. It would be nice if they didn't get email notifications.

comment:22 Changed 10 years ago by falkb

In 13484:

see #11367: check permissions of user for the appropriate project on access to versions, milestones, roadmap and tickets

comment:23 in reply to:  18 ; Changed 10 years ago by falkb

Replying to endquote:

However, that user can still see a ticket in a restricted project if they go to the ticket URL directly...

works now.

Another thought... maybe block email notifications...

Uhm... no idea at present how to prevent that... *thinking*

comment:24 Changed 10 years ago by falkb

In 13485:

  • some minor restructuring for check of restricted users list (see #11367)
  • added trial to get user groups from the permission system (disabled by default) (see #10863)

comment:25 Changed 10 years ago by falkb

In 13486:

implemented group support for user-restriction of projects via trac-admin permission add or /admin/general/perm (see #10863, see #11367)

comment:26 Changed 10 years ago by falkb

In 13487:

inversion feature added for "Restrict to users" (see #10863, see #11367)

  • if the list in the user-restriction text field starts with '!' (separated by comma!), the meaning is inverted, thus means "Forbidden to users" then.
  • e.g. "john,paul,george,ringo" restricts to these 4 users for a certain project, while "!,bob,harry" excludes them from a certain project

comment:27 in reply to:  23 ; Changed 10 years ago by falkb

Replying to falkb:

Replying to endquote:

Another thought... maybe block email notifications (for certain users excluded from a project)...

Uhm... no idea at present how to prevent that... *thinking*

Hi Josh, do you use the Announcer plugin? If yes, maybe there's a chance for easy blocking of emails. Have a look here at some source code of QuietPlugin. You can see, that plugin suppresses all email notifications for a recipient list depending on whether Quiet mode is on or off (a click on a special page link). If I was able to find out if the "distribute"-event comes from a ticket change, and if yes, which ticket number it is, in that case I could check if I must suppress the email distribution, instead of looking at the quiet mode. Then I could reuse and adapt the QuietEmailDistributor source code. hasienda, can you tell me how it's possible to check in distribute(self, transport, recipients, event) what the ticket number is?

comment:28 Changed 10 years ago by endquote

I'm not using AnnouncerPlugin, but I'll take a look at it. I'd say that suppressing restricted tickets from search results and keeping direct URLs to them from working is far more important. If I need to remove a user from a project, I could also batch modify their tickets to remove them and suppress notifications that way. Thanks again!

comment:29 in reply to:  28 ; Changed 10 years ago by falkb

Replying to endquote:

I'd say that suppressing restricted tickets from search results

That's the next thing I need to find out.

and keeping direct URLs to them from working

This should work already now. Let me know if not.

If I need to remove a user from a project, I could also batch modify their tickets to remove them and suppress notifications that way.

Yeah, this is also a good idea.

Thanks again!

You're welcome. And thank you too for having an eye on it from a user's point of view. I think this rectriction feature is a topic which comes up every now and again.

Last edited 10 years ago by falkb (previous) (diff)

comment:30 in reply to:  29 Changed 10 years ago by endquote

Replying to falkb:

Replying to endquote:

and keeping direct URLs to them from working

This should work already now. Let me know if not.

Ah, yes, this does work. Cool!

comment:31 in reply to:  29 Changed 10 years ago by falkb

Keywords: testing added; planned removed

Replying to falkb:

Replying to endquote:

I'd say that suppressing restricted tickets from search results

That's the next thing I need to find out.

Please update to [13492], it's implemented now together with ticket access restriction in all queries, reports and search results. I've introduced a new plugin class ProjectTicketsPolicy to get it done, which you need to activate now.

To get it activated

  1. go to Admin-->General-->Plugin-->SimpleMultiProjectPlugin-->ProjectTicketsPolicy and set its checkbox on
  2. edit your trac.ini config file and add ProjectTicketsPolicy like this
    [trac]
    permission_policies = ProjectTicketsPolicy, ... all the other ones ...
    

Now it seems everything (but the email thingy) has been done. I'm still waiting for the feedback of hasienda.

That's why I have a good feeling about this ticket #11367. :-) So I set it back to state "testing", and if nothing bad happens, I'll be off for holiday now. Please, test and report back how it works for you or if you feel something is still missing. A Merry Christmas and a Happy New Year to you all!

comment:32 Changed 10 years ago by endquote

This is great! A restricted user is no longer seeing tickets they shouldn't in search results, and direct links to them return an error.

I think that's all I needed, though blocking notifications would be a bonus.

Happy holidays!

comment:33 in reply to:  27 ; Changed 10 years ago by Steffen Hoffmann

Replying to falkb:

hasienda, can you tell me how it's possible to check in distribute(self, transport, recipients, event) what the ticket number is?

It boils down to the definition of event (announcer.api.AnnouncementEvent) objects in AnnouncerPlugin. In 'announcer/producers.py' you'll see, how such an event is constructed i.e. by TicketChangeProducer. You'll find, that event.target is a Ticket object, if realm == 'ticket'.

comment:34 Changed 10 years ago by falkb

Please update to today's latest version:

  • the new table column 'restrict' introduced a new bug #11461 because that is a special SQL keyword in some databases. I fixed that in [13549].
  • model.py needed a bigger rework to fix intermittent "Cannot operate on a closed cursor" errors (also [13549])
  • Another little issue with data==None in roadmap.py has been fixed with [13551].
Last edited 10 years ago by falkb (previous) (diff)

comment:35 in reply to:  33 Changed 10 years ago by falkb

Replying to hasienda:

Replying to falkb:

hasienda, can you tell me how it's possible to check in distribute(self, transport, recipients, event) what the ticket number is?

It boils down to the definition of event (announcer.api.AnnouncementEvent) objects in AnnouncerPlugin. In 'announcer/producers.py' you'll see, how such an event is constructed i.e. by TicketChangeProducer. You'll find, that event.target is a Ticket object, if realm == 'ticket'.

This little patch could be the one which filters out email recipients excluded from the current ticket's project. hasienda, could you make a review of it, please?:

  • simplemultiprojectplugin/trunk/simplemultiproject/ticket.py

     
    1515from operator import itemgetter
    1616
    1717try:
     18    from announcer.distributors.mail import EmailDistributor
     19except ImportError: #define empty class
     20    class EmailDistributor(Component):
     21        pass
     22           
     23try:
    1824    from trac.web.chrome import add_script_data
    1925except ImportError:
    2026    # Backported from 0.12
     
    172178
    173179    def get_permission_actions(self):
    174180        return
     181
     182class ProjectQuietEmailDistributor(EmailDistributor):
     183    """Specializes Announcer's email distributor to honor quiet mode."""
     184    def distribute(self, transport, recipients, event):
     185        if event and event.realm == "ticket" and event.target:
     186            # some email recipients may not be in the restricted users list of the ticket's project
     187            # they should not get email
     188            recipients = self._filter_email_recipients(event.target, recipients)
     189        EmailDistributor.distribute(self, transport, recipients, event)
     190
     191    def _filter_email_recipients(self, ticket, recipients):
     192        project_name = self.__SmpModel.get_ticket_project(ticket.id)
     193        if project_name and project_name[0]:
     194            project_info = self.__SmpModel.get_project_info(project_name[0])
     195            if project_info:
     196                for reci in list(recipients):
     197                    if self.__SmpModel.is_not_in_restricted_users(reci, project_info):
     198                        recipients.remove(reci)
     199        return recipients

This code is just a draft and I want to know if the API to AnnouncerPlugin is right. And is there a trac.ini setting where I must mention ProjectQuietEmailDistributor?

Last edited 10 years ago by falkb (previous) (diff)

comment:36 Changed 10 years ago by endquote

If I do a query for a project that I don't have access to, I still see its tickets.

http://trac/query?project=QCC2 will show all tickets for a project to anyone who asks. However they cannot click through to view the details of a ticket -- a permission error appears.

comment:37 Changed 10 years ago by falkb

Hi Josh! Currently, a message "no permission" is shown if a user is not in the list of user-restriction. This bloke has suggested a patch to me for full hiding of such projects without any message. What do you think is better?

comment:38 Changed 10 years ago by endquote

I would also prefer if users without access to a project never saw anything about the project -- they shouldn't even know it exists.

But if they do go to a project specific URL, they should get an error, or perhaps get redirected back to the home page. If an error, the message shouldn't be "you don't have permission", it should just be more of a 404 type of thing. A good example is Github -- if you navigate to a project you don't have permission to view (likely because you're not logged in), you just get a 404 error and no clue that the project exists.

comment:39 Changed 9 years ago by aikidoguy@…

I would also like to see some kind of feature in this direction. I was imagining modifying the Genshi template for the "Available Projects" list (which currently really means "Existing Projects")... I don't know how to accomplish this. However, I imagine something like the current authenticated user is checked in the Trac db to see if they have WIKI_VIEW and if not, then the "Available Projects" list would not have that project link.

comment:40 Changed 9 years ago by aikidoguy@…

Or better yet, instead of WIKI_VIEW, have a new permission called "PROJECT_VIEW"...

comment:41 in reply to:  39 Changed 9 years ago by falkb

Replying to aikidoguy@…:

I would also like to see some kind of feature in this direction. I was imagining modifying the Genshi template for the "Available Projects" list (which currently really means "Existing Projects")...

I'm not sure if you mean you want to rename from "Existing Projects" to "Available Projects", am I right? If yes, why do you want to rename it?

I don't know how to accomplish this. However, I imagine something like the current authenticated user is checked in the Trac db to see if they have WIKI_VIEW and if not, then the "Available Projects" list would not have that project link.

Currently, one can only generally switch off access to the project list by PROJECT_SETTINGS_VIEW permission. I'm gonna check if it's possible to show a list for only the projects which are set visible to the user; actually that shouldn't be a problem. Patch welcome during my current lack of time.

comment:42 Changed 9 years ago by aikidoguy@…

Sorry for my noise and misunderstandings. If I understand correctly, "SimpleMultiProjectPlugin" is a way to manage multiple projects within 1 Trac environment. My scenario is when I have multiple Trac environments that I need to manage. I will pursue this further on the trac-users mailing list. Thank you kindly!

comment:43 Changed 9 years ago by falkb

endquote, is this ticket actually sufficiently resolved for you?

comment:44 Changed 9 years ago by endquote

Yes, this seems like it could be closed to me.

comment:45 Changed 9 years ago by falkb

Resolution: fixed
Status: assignedclosed

comment:46 Changed 9 years ago by falkb

Keywords: testing removed

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain falkb.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.