Oauth2 addition

[squirrelo]

This pull request adds Oauth2-style authentication to the API. Every plugin now requires a client_id and client_secret to ping the API with to get an access token. This access token allows connection for one hour, and then the token will need to be re-created and the new one used. Users can also get access by giving a client ID and their username and password. This allows us to have a secure login system for the API that can also be secured on a per-user basis as well as a per-client (plugin) basis.
Two of the four access types have been implemented: Client Credentials Grant for the plugins and Resource Owner Password Credentials Grant for qiita-pet.

[josenavas]

Do not merge this changes until I can test this locally with a working system.

[antgonza]

@squirrelo could you pull from artifact? Thanks!

[squirrelo]

pulled from artifact. Also added daily request limiting.

[squirrelo]

The auth check is now a decorator, so this should be in a final state and ready for review.

[mortonjt]

Hmm. Looks like there are some test files that can't be found ...

[squirrelo]

I can't reproduce the test error on my system. Anyone have ideas?

[josenavas]

@squirrelo will investigate shortly - I'm planning to pull this down, and test locally, so I can also update the plugins accordingly

[squirrelo]

Changes made, save ones we need to discuss as a group.

[josenavas]

?! Define the function here - you can then call the function in the decorator, as you are calling other methods/attributes of the class.

[squirrelo]

As stated previously, the decorator function is for anything extending tornado's RequestHandler, not specifically the oauth handler, and the changes just pushed reflect that. Calling the function in the decorator breaks this functionality.

[josenavas]

I'm getting really confused here. As you stated here, the authenticate_oauth is written so it works with any RequestHandler. On the other hand, you created the base class OauthBaseHandler that all the handlers that require oauth2 need to subclass. The question then is, is the OauthBaseHandler class needed or the authenticate_oauth really need to be as general for any of the Handlers type?

[squirrelo]

The OauthBaseHandler is for anything that needs to actually set and manipulate the authorization keys in the system, or authenticate user/client information, so not every oauth2 using page needs to subclass it. I did it for convenience, as there are things like 404 and error handling that are better used in base classes, but it's not strictly needed. Leaving the decorator open like this allows extensibility in the future with no code change, as all you need to do is import the authenticate_oauth decorator and extend RequestHandler again, exactly the same as the tornado library already works.

[josenavas]

Is there any specific necessity of calling oauth_error as self.oauth_error? If not, remove this line and and just call the function _oauth_error as _oauth_error(self, params...). This is just making maintaining the code harder.

[antgonza]

Can we close this one? I think it has been superseded by #1584 ..

[squirrelo]

It would be much easier to review in two parts, as #1584 becomes monolithic with this PR merged in as well.

[antgonza]

OK, reviewing now.

[antgonza]

Should this have a doc string?

[squirrelo]

It's the inner wrapper of the decorator, so no. The docstring of the containingauthenticate_oauth function is what's going to be seen.

[squirrelo]

Comments addressed

[mortonjt]

Maybe Returns is a misnomer here, since it doesn't actually return anything.

[squirrelo]

That's what I was saying.

[josenavas]

@mortonjt in a previous discussion over email we agreed on documenting the RESTapi using the numpy returns.

We are happy to change to a different standard to document the RESTapi in the same code, but please provide alternatives - it is better to document like this than leave undocumented.

[squirrelo]

Using the examples section would be easier, especially since these all respond differently depending on errors, warnings, etc and that could be documented better.

[josenavas]

👍 @antgonza @mortonjt available for reviewing this?