- 
                Notifications
    You must be signed in to change notification settings 
- Fork 79
Oauth2 addition #1562
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Oauth2 addition #1562
Changes from 25 commits
3ec03d1
              7547ef8
              1df6292
              0107b15
              0f22b12
              2d279e8
              599cdb0
              ec40863
              2357500
              8692357
              964873f
              f721773
              96a5be6
              476ecab
              13b0896
              61df629
              edbbebb
              7c58a6b
              19a227a
              f49fa50
              ab3cbed
              8fe3c2c
              11c680a
              dcd78a1
              e3ef9d9
              4b79d66
              4d1d100
              df7a469
              0c7b152
              4e2d728
              f0c25b3
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,306 @@ | ||
| # ----------------------------------------------------------------------------- | ||
| # Copyright (c) 2014--, The Qiita Development Team. | ||
| # | ||
| # Distributed under the terms of the BSD 3-clause License. | ||
| # | ||
| # The full license is in the file LICENSE, distributed with this software. | ||
| # ----------------------------------------------------------------------------- | ||
|  | ||
| from base64 import urlsafe_b64decode | ||
| from string import ascii_letters, digits | ||
| import datetime | ||
| from random import SystemRandom | ||
| import functools | ||
| from traceback import format_exception | ||
|  | ||
| from tornado.web import RequestHandler | ||
|  | ||
| from moi import r_client | ||
| from qiita_core.exceptions import (IncorrectPasswordError, IncorrectEmailError, | ||
| UnverifiedEmailError) | ||
| import qiita_db as qdb | ||
|  | ||
|  | ||
| def login_client(client_id, client_secret=None): | ||
| """Login client_id, optionally with client_secret | ||
|  | ||
| Parameters | ||
| ---------- | ||
| client_id : str | ||
| The client ID to validate | ||
| client_secret : str, optional | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this is actually correct, neither if we want to support it. According to the RFC6749: According to this description, the only time that the parameter can be omitted is that if the  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 
 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had to read the entire code to understand why this is correct. My take on this is that this is really dangerous and it is really easy to introduce bugs in the code. Now, from the code structure correctness, this function is used in 2 places, the function  Can you remove this function and just write the call in the given functions? | ||
| The client secret code. If not given, will only validate if client_id | ||
| exists | ||
|  | ||
| Returns | ||
| ------- | ||
| bool | ||
| Whether client_id exists and, if client_secret given, the secret | ||
| matches the id | ||
| """ | ||
| with qdb.sql_connection.TRN: | ||
| sql = """SELECT EXISTS( | ||
| SELECT * | ||
| FROM qiita.oauth_identifiers | ||
| WHERE client_id = %s {0})""" | ||
| sql_info = [client_id] | ||
| if client_secret is not None: | ||
| sql = sql.format("AND client_secret = %s") | ||
| sql_info.append(client_secret) | ||
| else: | ||
| sql = sql.format("AND client_secret IS NULL") | ||
| qdb.sql_connection.TRN.add(sql, sql_info) | ||
| return qdb.sql_connection.TRN.execute_fetchlast() | ||
|  | ||
|  | ||
| def _oauth_error(self, error_msg, error): | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you change  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, thinking a bit more about this, shouldn't this just simply raise an HTTPError, so we don't need our own function? Based on the docs it looks like it will do everything that we need. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking a bit more around - if this is authentication, should this behave similarly as the authenticate from tornado, reference code here the error code should be 403 right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is built with ides from the tornado decorator, but pursuant to the Oauth2 framework linked: So this needs to return 400 error code and the controlled vocab error json in the body. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @squirrelo ping about the first comment in here - change  | ||
| self.set_status(400) | ||
| self.write({'error': error, | ||
| 'error_description': error_msg}) | ||
| self.finish() | ||
|  | ||
|  | ||
| def authenticate_oauth(f): | ||
| """Decorate methods to require valid Oauth2 Authorization header[1] | ||
|  | ||
| If a valid header is given, the handoff is done and the page is rendered. | ||
| If an invalid header is given, a 400 error code is returned and the json | ||
| error message is automatically sent. | ||
|  | ||
| References | ||
| --------- | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing 1  | ||
| [1] The OAuth 2.0 Authorization Framework. | ||
| http://tools.ietf.org/html/rfc6749 | ||
| """ | ||
| @functools.wraps(f) | ||
| def wrapper(self, *args, **kwargs): | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this have a doc string? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's the inner wrapper of the decorator, so no. The docstring of the containingauthenticate_oauth function is what's going to be seen. | ||
| header = self.request.headers.get('Authorization', None) | ||
| if header is None: | ||
| _oauth_error(self, 'Oauth2 error: invalid access token', | ||
| 'invalid_request') | ||
| return | ||
| token_info = header.split() | ||
| if len(token_info) != 2 or token_info[0] != 'Bearer': | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are you looking for 'Bearer', could you clarify via documentation? Perhaps this is something obvious but someone without previous experience, me, it's not. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Required by the OAuth2 authorization framework, which is explicitly linked in the docstring. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess these 2 comments show how is not fully clear how this works and expecting that everybody reads a full RFC seems unrealistic, what about adding the main assumptions/cases you are using here so is easier to fix/improve in the future? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd argue you shouldn't be modifying code that must conform to a very specific set of rules laid out in the RFC unless you have read and understand the RFC. This is a web-wide standard we did not write, and it is very explicit on what it requires, which allows for things like the libraries used to do client verification in #1584. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like having the code well documented doesn't hurt any one ... do you feel otherwise? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good documentation is one thing. Overly documenting is another, and frowned upon by pep8. What do you gain from having the documentation lines like  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree that your example doesn't help - not even sure why will you use it - but I was expecting something more like: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree with @antgonza on adding a bit more points of documentation. @squirrelo we are not asking to duplicate the entire RFC, but you can't expect that every single developer will read the RFC in order to understand the code that is written here. As an example of documentation that I would expect having here can be seen here: https://github.com/idan/oauthlib/blob/master/oauthlib/oauth2/rfc6749/request_validator.py Also, looking at that - the question that raises is why did you re-implement the RFC if the oauthlib already includes an implementation-independent framework for OAuth2? We shouldn't be reinventing the wheel, specially on topics that we are not experts. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with @antgonza and @josenavas. Documentation is essential and it doesn't make sense to require every reader of the code to be an expert on topics they use infrequently. @squirrelo, please work with the rest of the group on this productively in this and similar instances -- part of the reason why we're so productive is precisely that the good documentation means people don't have to waste a lot of additional time looking up arcane topics. | ||
| _oauth_error(self, 'Oauth2 error: invalid access token', | ||
| 'invalid_grant') | ||
| return | ||
|  | ||
| token = token_info[1] | ||
| db_token = r_client.hgetall(token) | ||
| if not db_token: | ||
| # token has timed out or never existed | ||
| _oauth_error(self, 'Oauth2 error: token has timed out', | ||
| 'invalid_grant') | ||
| return | ||
| # Check daily rate limit for key if password style key | ||
| if db_token['grant_type'] == 'password': | ||
| limit_key = '%s_%s_daily_limit' % (db_token['client_id'], | ||
| db_token['user']) | ||
| limiter = r_client.get(limit_key) | ||
| if limiter is None: | ||
| # Set limit to 5,000 requests per day | ||
| r_client.setex(limit_key, 5000, 86400) | ||
| else: | ||
| r_client.decr(limit_key) | ||
| if int(r_client.get(limit_key)) <= 0: | ||
| _oauth_error( | ||
| self, 'Oauth2 error: daily request limit reached', | ||
| 'invalid_grant') | ||
| return | ||
|  | ||
| return f(self, *args, **kwargs) | ||
| return wrapper | ||
|  | ||
|  | ||
| class OauthBaseHandler(RequestHandler): | ||
| def write_error(self, status_code, **kwargs): | ||
| """Overriding the default write error in tornado RequestHandler | ||
|  | ||
| Instead of writing all errors to stderr, this writes them to the logger | ||
| tables. | ||
|  | ||
| Parameters | ||
| ---------- | ||
| status_code : int | ||
| HTML status code of the error | ||
| **kwargs : dict | ||
| Other parameters describing the error | ||
|  | ||
| Notes | ||
| ----- | ||
| This function is automatically called by the tornado package on errors, | ||
| and should never be called directly. | ||
| """ | ||
| if status_code in {403, 404, 405}: | ||
| # We don't need to log these failues in the logging table | ||
| return | ||
| # log the error | ||
| exc_info = kwargs['exc_info'] | ||
| trace_info = ''.join(['%s\n' % line for line in | ||
| format_exception(*exc_info)]) | ||
| req_dict = self.request.__dict__ | ||
| # must trim body to 1024 chars to prevent huge error messages | ||
| req_dict['body'] = req_dict.get('body', '')[:1024] | ||
| request_info = ''.join(['<strong>%s</strong>: %s\n' % | ||
| (k, req_dict[k]) for k in | ||
| req_dict.keys() if k != 'files']) | ||
| error = exc_info[1] | ||
| qdb.logger.LogEntry.create( | ||
| 'Runtime', | ||
| 'ERROR:\n%s\nTRACE:\n%s\nHTTP INFO:\n%s\n' % | ||
| (error, trace_info, request_info)) | ||
|  | ||
| # Allow call to oauth_error function as part of the class | ||
| oauth_error = _oauth_error | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ?! Define the function here - you can then call the function in the decorator, as you are calling other methods/attributes of the class. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As stated previously, the decorator function is for anything extending tornado's  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm getting really confused here. As you stated here, the  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there any specific necessity of calling  | ||
|  | ||
| def head(self): | ||
| """Adds proper response for head requests""" | ||
| self.finish() | ||
|  | ||
|  | ||
| class TokenAuthHandler(OauthBaseHandler): | ||
| def generate_access_token(self): | ||
| """Creates the random alphanumeric token | ||
|  | ||
| Returns | ||
| ------- | ||
| str | ||
| 55 character random alphanumeric string | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where does the 55 come from? It is required by oauth2, please link the docs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, a cryptographically secure limit that needs to be hard coded so we can set the same varchar length for the postgres column storing the key. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, magic numbers are not useful w/o some context for other devs. You may know what they're for but not other devs, which makes this code hard to maintain. If that is something that you decided, at least add a comment, specially because 55 looks random... | ||
|  | ||
| Notes | ||
| ----- | ||
| 55 was chosen as a cryptographically secure limit that needs to be | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I kind of understand this explanation. However I still don't understand why 55. Why we don't use 100? wouldn't it be more secure? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 62^55 = 3.815425e+98 which is probably secure enough, given that we also have request and rate limits. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you did not understand the point that I was trying to make. It looks like 55 is secure enough but 54 isn't? That is the point that I want to make, if you decided 55 based on some documentation that you read online, yo should link it, if it is something that you decided, you should have some other reason that 'looks like secure enough'. 55 is not a multiple of 2 and looks like strange number - this is too arbitrary and the same questions that I have are the same questions that another dev in 2 years can have, and we may or may not be around to answer those questions. So please, include the needed documentation so a dev that looks at the system for the first time understands what is going on. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can update the database column to store 255 characters (max you can safely put in an HTTP variable), but as stated there is no documentation to cite for length. The number is not specified as part of the Oauth2 spec, so it's up to the implementer to decide. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still don't understand the decision of the 55 chars. The only answer that I received here is "I have the feeling that is secure enough". What are other systems using? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, I just noticed that the description here is totally incorrect. As far as I know, the  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The explanation is still there is no set size for the key, and it must arbitrarily be decided by the implementer because there is no standard. Twitter seems to be using 21 characters although this is an example so it may be a different length, oauthlib seems to be using 30 characters by default, sometimes more, etc. It is completely arbitrary, and most systems don't even say how long the code is because it may change length at any time. The only fixture is that it can not be more than 255 characters for HTTP transfer reasons. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, can you actually add the comment that the exact length of the token can change, but it should never be longer than 255? That is a technology limit and it will be good to have it exposed here in the documentation. I don't see any check in here but, what happens if the token that is generated already exists in the system? The RFC does not say anything specific, but I think we should double check that each new access_token is unique, or am I missing something? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done | ||
| hard coded so we can set the same varchar length for the postgres | ||
| column storing the key. | ||
| """ | ||
| pool = ascii_letters + digits | ||
| return ''.join((SystemRandom().choice(pool) for _ in range(55))) | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 
 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, thanks! | ||
|  | ||
| def set_token(self, client_id, grant_type, user=None, timeout=3600): | ||
| """Create access token for the client on redis and send json response | ||
|  | ||
| Parameters | ||
| ---------- | ||
| client_id : str | ||
| Client that requested the token | ||
| grant_type : str | ||
| Type of key being requested | ||
| user : str, optional | ||
| If password grant type requested, the user requesting the key. | ||
| timeout : int, optional | ||
| The timeout, in seconds, for the token. Default 3600 | ||
| """ | ||
| token = self.generate_access_token() | ||
|  | ||
| r_client.hset(token, 'timestamp', datetime.datetime.now()) | ||
| r_client.hset(token, 'client_id', client_id) | ||
| r_client.hset(token, 'grant_type', grant_type) | ||
| r_client.expire(token, timeout) | ||
| if user: | ||
| r_client.hset(token, 'user', user) | ||
| if grant_type == 'password': | ||
| # Check if client has access limit key, and if not, create it | ||
| limit_key = '%s_%s_daily_limit' % (client_id, user) | ||
| limiter = r_client.get(limit_key) | ||
| if limiter is None: | ||
| # Set limit to 5,000 requests per day | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems redundant? Isn't it already set up above? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Technically this is the first set up here. The one above is in case the access token's validity spans when the rate limit expires, and we need to reset the rate limit key during the lookup of token validity instead of the initial creation of the token. | ||
| r_client.setex(limit_key, 5000, 86400) | ||
|  | ||
| self.write({'access_token': token, | ||
| 'token_type': 'Bearer', | ||
| 'expires_in': '3600'}) | ||
| self.finish() | ||
|  | ||
| def validate_client(self, client_id, client_secret): | ||
| """Make sure client exists, then set the token and send it | ||
|  | ||
| Parameters | ||
| ---------- | ||
| client_id : str | ||
| The client making the request | ||
| client_secret : str | ||
| The secret key for the client | ||
| """ | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Although this doesn't have a  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm unsure what this means. Could you elaborate? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Check the documentation of the other handlers. In the returns section we are including the explanation of the JSON that you're returning. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That does not fit with the standard numpy documentation methods. If a change like this happened, it should have been discussed at the meetings. | ||
| if login_client(client_id, client_secret): | ||
| self.set_token(client_id, 'client') | ||
| else: | ||
| self.oauth_error('Oauth2 error: invalid client information', | ||
| 'invalid_client') | ||
|  | ||
| def validate_resource_owner(self, username, password, client_id): | ||
| """Make sure user and client exist, then set the token and send it | ||
|  | ||
| Parameters | ||
| ---------- | ||
| username : str | ||
| The username to validate | ||
| password : str | ||
| The password for the username | ||
| client_id : str | ||
| The client making the request | ||
| """ | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here re documenting returns | ||
| try: | ||
| qdb.user.User.login(username, password) | ||
| except (IncorrectEmailError, IncorrectPasswordError, | ||
| UnverifiedEmailError): | ||
| self.oauth_error('Oauth2 error: invalid user information', | ||
| 'invalid_client') | ||
| return | ||
|  | ||
| if login_client(client_id): | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code and the one in  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done | ||
| self.set_token(client_id, 'password', user=username) | ||
| else: | ||
| self.oauth_error('Oauth2 error: invalid client information', | ||
| 'invalid_client') | ||
|  | ||
| def post(self): | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just to double check on how oauth2 works. You get a new token by posting? It looks counter-intuitive. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that is how it works. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add numpy documentation in the post. Also, as it was agreed over email on documenting the RESTapi handlers (@ElDeveloper, @antgonza and I agreed, and no body complain), add the  | ||
| # first check for header version of sending auth, meaning client ID | ||
| header = self.request.headers.get('Authorization', None) | ||
| if header is not None: | ||
| header_info = header.split() | ||
| if header_info[0] != 'Basic': | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's Basic and why is invalid ? | ||
| # Invalid Authorization header type for this page | ||
| self.oauth_error('Oauth2 error: invalid token type', | ||
| 'invalid_request') | ||
| return | ||
|  | ||
| # Get client information from the header and validate it | ||
| grant_type = self.get_argument('grant_type', None) | ||
| if grant_type != 'client': | ||
| self.oauth_error('Oauth2 error: invalid grant_type', | ||
| 'invalid_request') | ||
| return | ||
| try: | ||
| client_id, client_secret = urlsafe_b64decode( | ||
| header_info[1]).split(':') | ||
| except ValueError: | ||
| # Split didn't work, so invalid information sent | ||
| self.oauth_error('Oauth2 error: invalid base64 encoded info', | ||
| 'invalid_request') | ||
| return | ||
| self.validate_client(client_id, client_secret) | ||
| return | ||
|  | ||
| # Otherwise, do either password or client based authentication | ||
| client_id = self.get_argument('client_id', None) | ||
| grant_type = self.get_argument('grant_type', None) | ||
| if grant_type == 'password': | ||
| username = self.get_argument('username', None) | ||
| password = self.get_argument('password', None) | ||
| if not all([username, password, client_id]): | ||
| self.oauth_error('Oauth2 error: missing user information', | ||
| 'invalid_request') | ||
| else: | ||
| self.validate_resource_owner(username, password, client_id) | ||
|  | ||
| elif grant_type == 'client': | ||
| client_secret = self.get_argument('client_secret', None) | ||
| if not all([client_id, client_secret]): | ||
| self.oauth_error('Oauth2 error: missing client information', | ||
| 'invalid_request') | ||
| return | ||
| self.validate_client(client_id, client_secret) | ||
| else: | ||
| self.oauth_error('Oauth2 error: invalid grant_type', | ||
| 'unsupported_grant_type') | ||
| return | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Group the import from moi with tornado rather than with qiita (3rd party lib group)