← Back to team overview

launchpad-reviewers team mailing list archive

Re: [Merge] lp:~leonardr/launchpad/true-anonymous-access into lp:launchpad

 

Review: Approve
That looks good to me, though I'm not very expert on Launchpad's security internals.  

>  If you screw up your OAuth implementation, or you try to authenticate with Basic Auth and your Launchpad username/password, you will no longer get an exception. You'll get anonymous read-only access. But, I don't think it's worth making a big deal of this, trying to get failure modes back by checking for incomplete OAuth implementations, etc. (At most, I might check for an Authorization header that's not a valid OAuth Authorization header.)

I think it would be reasonable to handle that as a separate bug if and when anyone actually files it.  If the OAuth header is corrupt as opposed to just absent I think we should still give an error.

Did you interactively test that against your dev instance?

Thanks very much, this is a nice step forward for API usability.
-- 
https://code.launchpad.net/~leonardr/launchpad/true-anonymous-access/+merge/30088
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~leonardr/launchpad/true-anonymous-access into lp:launchpad.



References