Code review comment for lp:~leonardr/launchpad/publish-tokens-2

Revision history for this message
Guilherme Salgado (salgado) wrote :

And here's another follow up review: http://pastebin.ubuntu.com/486932/

+class OAuthTokenBase(OAuthBase):
+ """Base implementation of code to check an OAuth-signed request."""

I'd change that docstring to say just that this is a base class for OAuthToken classes, as we may want to add code not related to signature checking here in the future.

+
+ def checkSignature(self, request):
+ return check_oauth_signature(request, self.consumer, self.secret)

And here you should add a docstring pointing to the interface where the method is defined.

-Now consider a principal authorized to create OAuth tokens. Whenever
-it's not creating OAuth tokens, it has a level of permission
-equivalent to READ_PUBLIC.
+A principal with the GRANT_PERMISSIONS authorization level has a of
+permission equivalent to WRITE_PRIVATE.

Why is the permission changing in this incremental diff? Also, s/of//?

- >>> access_token = token.createAccessToken()
+ >>> access_token = removeSecurityProxy(token.createAccessToken())

Why do you need to remove the security proxy now?

review: Needs Information (code)

« Back to merge proposal