launchpad-reviewers team mailing list archive

[Colin Watson <mp+433538@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> diff --git a/lib/lp/code/xmlrpc/git.py b/lib/lp/code/xmlrpc/git.py
> index 36a19eb..cbec524 100644
> --- a/lib/lp/code/xmlrpc/git.py
> +++ b/lib/lp/code/xmlrpc/git.py
> @@ -207,13 +289,21 @@ class GitAPI(LaunchpadXMLRPCView):
>              # macaroon that specifically grants access to this repository.
>              # This is only permitted for macaroons not bound to a user.
>              writable = _can_internal_issuer_write(verified)
> +        elif (
> +            isinstance(verified, AccessTokenVerificationResult)
> +            and not verified.can_push
> +        ):
> +            # The user authenticated with an access token without the

You can only have user credentials by the time you get here if you've authenticated one way or another.  That means either providing an SSH key that turnip checks against the authserver, or providing HTTPS authentication that satisfies `authenticateWithPassword` (either a macaroon or, as of this branch, an access token).  If you provide neither, then you'll be "logged in" as the anonymous user; Zope security will prevent reading any private data in that case, and `check_permission("launchpad.Edit", repository)` will return False.

> +            # "repository:push" scope, so pushing isn't allowed no matter
> +            # what permissions they might ordinarily have.
> +            writable = False
> +        elif repository.repository_type != GitRepositoryType.HOSTED:
> +            # Normal users can never push to non-hosted repositories.
> +            writable = False
>          else:
>              # This isn't an authorised internal service, so perform normal
>              # user authorisation.
> -            writable = (
> -                repository.repository_type == GitRepositoryType.HOSTED
> -                and check_permission("launchpad.Edit", repository)
> -            )
> +            writable = check_permission("launchpad.Edit", repository)
>              if not writable:
>                  grants = naked_repository.findRuleGrantsByGrantee(requester)
>                  if not grants.is_empty():


-- 
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/433538
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:personal-access-token-git into launchpad:master.