Diff comments:
>
> === modified file 'lib/lp/code/xmlrpc/git.py'
> --- lib/lp/code/xmlrpc/git.py 2019-05-09 15:44:59 +0000
> +++ lib/lp/code/xmlrpc/git.py 2019-05-10 16:51:16 +0000
> @@ -390,17 +397,18 @@
> getUtility(IGitRefScanJobSource).create(
> removeSecurityProxy(repository))
>
> + @return_fault
> def authenticateWithPassword(self, username, password):
> """See `IGitAPI`."""
> - # XXX cjwatson 2016-10-06: We only support free-floating macaroons
> - # at the moment, not ones bound to a user.
> - if not username:
> - verified = self._verifyMacaroon(password)
> - if verified:
> - auth_params = {"macaroon": password}
> - if _is_issuer_internal(verified):
> - auth_params["user"] = LAUNCHPAD_SERVICES
> - return auth_params
> + user = getUtility(IPersonSet).getByName(username) if username else
> None
> + verified = self._verifyMacaroon(password, user=user)
> + if verified:
> + auth_params = {"macaroon": password}
> + if user is not None:
> + auth_params["uid"] = user.id
Fair point on checking the user. I've added the user to
MacaroonVerificationResult, explicitly distinguishing "issuer forgot to do any
checks" from "issuer positively verified the lack of a user", and arranged for
both GitAPI and the authserver to double-check this against what they expect.
Please re-review.
I don't really understand your comment about an extra auth parameter. This
sort of thing might be a problem if there were some way to inject extra auth
parameters after authentication, but there isn't. Can you explain what attack
vector you see here? (Also, on a practical note, the only other way to
authenticate at the moment is via an SSH key, and we can't change the auth
parameters for that until we get the git redeployment done so that we can
deploy a newer version of turnip.)
> + elif _is_issuer_internal(verified):
> + auth_params["user"] = LAUNCHPAD_SERVICES
> + return auth_params
> # Only macaroons are supported for password authentication.
> return faults.Unauthorized()
>
--
https://code.launchpad.net/~cjwatson/launchpad/git-honour-access-tokens/+merge/366053
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
