launchpad-reviewers team mailing list archive

[Colin Watson <mp+415966@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> diff --git a/lib/lp/security.py b/lib/lp/security.py
> index 22bc140..98e6f26 100644
> --- a/lib/lp/security.py
> +++ b/lib/lp/security.py
> @@ -3781,3 +3782,12 @@ class EditCIBuild(AdminByBuilddAdmin):
>          if auth_repository.checkAuthenticated(user):
>              return True
>          return super().checkAuthenticated(user)
> +
> +
> +class EditVulnerability(AuthorizationBase):
> +    permission = 'launchpad.Edit'
> +    usedfor = IVulnerability
> +
> +    def checkAuthenticated(self, user):
> +        return (

Don't use CVE as an example here.  It's different because it's maintained automatically by syncing from an external source, so editing it doesn't really make sense.

We definitely need to open this to more than admins; security team members don't have those roles.  I think the best thing here would be something with similar semantics to `lp.bugs.security._has_any_bug_role`, though simpler because a `Vulnerability` row only has a single target (the distribution).  How about something like this?

    return (
        user.in_commercial_admin or user.in_admin or
        user.isOwner(obj.distribution) or
        user.isDriver(obj.distribution) or
        user.isBugSupervisor(obj.distribution))

(This could very likely be optimized further, but since there's only a single distribution to check it's probably not worth bothering.)

> +            user.in_commercial_admin or user.in_admin)


-- 
https://code.launchpad.net/~ilasc/launchpad/+git/launchpad/+merge/415966
Your team Launchpad code reviewers is requested to review the proposed merge of ~ilasc/launchpad:add-vulnerability-orm into launchpad:master.