[Patchew-devel] [PATCH] test: more testcases around authorization

[Fam Zheng]

On Thu, 05/17 19:30, Paolo Bonzini wrote:
> On 17/05/2018 19:25, Shubham Jain wrote:
> >     > +        if request.user.is_superuser or 'importers' in
> >     user_group_names:
> >     > +            projects = [p for p in Project.objects.all() if
> >     p.recognizes(m)]
> >     > +        else:
> >     > +            projects =  [p for p in Project.objects.all() if
> >     p.maintained_by(self.request.user)]
> > 
> >     I think the else branch should still union the p.recognizes() filtering.
> > 
> > As you have mentioned in the test case below
> > "test_maintainer_of_project_x_import_a_patch_to_project_y()", does this
> > mean even if user is maintainer of one project, but result in message is
> > simply imported to all the recognised project. If yes, why are we just
> > not using only recognised condition? 

Here is how maintainership and recoginsed condition work together.

Usually, all patches in all projects are imported by "importers", it's a "bot".

Maintainers are humans. He could "override" importers in the scope where he
maintains a project, e.g. by importing additional patches or removing unwanted
patches. But a maintainer of project X shouldn't be allowed to make such changes
to project Y.

On the other hand, user A could be a maintainer for both projects A & B, but not
for C. When he submits a new Message M, it could end up in project A or project
B, or both, depending on the p.recognizes() test, but M cannot be added to
project C because he doesn't maintain it, even if he attempts to, or project C
does recognize the message.

On the other hand, AND'ing p.maintained_by() and p.recognizes() makes it easy
for the user since he only needs to submit for once, leaving the project list to
the server. In this case the message shouldn't be added to _all_ maintained
projects.

> 
> The idea is that if a user wants to import to a particular project he
> maintains, he uses /projects/.../messages.  If a user wants to recognize
> the projects based on the recipients, he uses /messages.
> 
> (In practice the former happens rarely, but it was already complicated
> enough and it makes sense to have it as part of the REST API!)

Yes, the latter is more important.

Fam