mirror of
https://github.com/zulip/zulip.git
synced 2025-10-30 19:43:47 +00:00
api: Stop using API keys for Django->Tornado authentication.
As part of our effort to change the data model away from each user having a single API key, we're eliminating the couple requests that were made from Django to Tornado (as part of a /register or home request) where we used the user's API key grabbed from the database for authentication. Instead, we use the (already existing) internal_notify_view authentication mechanism, which uses the SHARED_SECRET setting for security, for these requests, and just fetch the user object using get_user_profile_by_id directly. Tweaked by Yago to include the new /api/v1/events/internal endpoint in the exempt_patterns list in test_helpers, since it's an endpoint we call through Tornado. Also added a couple missing return type annotations.
This commit is contained in:
@@ -73,8 +73,8 @@ can be extremely valuable for investigating performance problems.
|
|||||||
2016-05-20 14:50:22.272 INFO [zr] 127.0.0.1 GET 200 124ms (db: 3ms/2q) /login/ (unauth via ?)
|
2016-05-20 14:50:22.272 INFO [zr] 127.0.0.1 GET 200 124ms (db: 3ms/2q) /login/ (unauth via ?)
|
||||||
2016-05-20 14:50:26.333 INFO [zr] 127.0.0.1 POST 302 37ms (db: 6ms/7q) /accounts/login/local/ (unauth via ?)
|
2016-05-20 14:50:26.333 INFO [zr] 127.0.0.1 POST 302 37ms (db: 6ms/7q) /accounts/login/local/ (unauth via ?)
|
||||||
[20/May/2016 14:50:26]"POST /accounts/login/local/ HTTP/1.0" 302 0
|
[20/May/2016 14:50:26]"POST /accounts/login/local/ HTTP/1.0" 302 0
|
||||||
2016-05-20 14:50:26.538 INFO [zr] 127.0.0.1 GET 200 12ms (db: 1ms/2q) (+start: 53ms) /api/v1/events [1463769771:0/0] (cordelia@zulip.com via internal)
|
2016-05-20 14:50:26.538 INFO [zr] 127.0.0.1 POST 200 12ms (db: 1ms/2q) (+start: 53ms) /api/v1/events/internal [1463769771:0/0] (cordelia@zulip.com via internal)
|
||||||
2016-05-20 14:50:26.657 INFO [zr] 127.0.0.1 GET 200 10ms (+start: 8ms) /api/v1/events [1463769771:0/0] (cordelia@zulip.com via internal)
|
2016-05-20 14:50:26.657 INFO [zr] 127.0.0.1 POST 200 10ms (+start: 8ms) /api/v1/events/internal [1463769771:0/0] (cordelia@zulip.com via internal)
|
||||||
2016-05-20 14:50:26.959 INFO [zr] 127.0.0.1 GET 200 588ms (db: 26ms/21q) / [1463769771:0] (cordelia@zulip.com via website)
|
2016-05-20 14:50:26.959 INFO [zr] 127.0.0.1 GET 200 588ms (db: 26ms/21q) / [1463769771:0] (cordelia@zulip.com via website)
|
||||||
```
|
```
|
||||||
|
|
||||||
|
|||||||
@@ -35,6 +35,7 @@ server {
|
|||||||
include /etc/nginx/zulip-include/location-sockjs;
|
include /etc/nginx/zulip-include/location-sockjs;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
# We don't need /api/v1/events/internal, because that doesn't go through the loadbalancer.
|
||||||
location ~ /json/events|/api/v1/events {
|
location ~ /json/events|/api/v1/events {
|
||||||
proxy_pass https://staging;
|
proxy_pass https://staging;
|
||||||
include /etc/nginx/zulip-include/proxy_longpolling;
|
include /etc/nginx/zulip-include/proxy_longpolling;
|
||||||
|
|||||||
@@ -378,6 +378,7 @@ def write_instrumentation_reports(full_suite: bool) -> None:
|
|||||||
exempt_patterns = set([
|
exempt_patterns = set([
|
||||||
# We exempt some patterns that are called via Tornado.
|
# We exempt some patterns that are called via Tornado.
|
||||||
'api/v1/events',
|
'api/v1/events',
|
||||||
|
'api/v1/events/internal',
|
||||||
'api/v1/register',
|
'api/v1/register',
|
||||||
# We also exempt some development environment debugging
|
# We also exempt some development environment debugging
|
||||||
# static content URLs, since the content they point to may
|
# static content URLs, since the content they point to may
|
||||||
|
|||||||
@@ -18,9 +18,11 @@ def setup_tornado_rabbitmq() -> None: # nocoverage
|
|||||||
autoreload.add_reload_hook(lambda: queue_client.close())
|
autoreload.add_reload_hook(lambda: queue_client.close())
|
||||||
|
|
||||||
def create_tornado_application() -> tornado.web.Application:
|
def create_tornado_application() -> tornado.web.Application:
|
||||||
urls = (r"/notify_tornado",
|
urls = (
|
||||||
|
r"/notify_tornado",
|
||||||
r"/json/events",
|
r"/json/events",
|
||||||
r"/api/v1/events",
|
r"/api/v1/events",
|
||||||
|
r"/api/v1/events/internal",
|
||||||
)
|
)
|
||||||
|
|
||||||
# Application is an instance of Django's standard wsgi handler.
|
# Application is an instance of Django's standard wsgi handler.
|
||||||
|
|||||||
@@ -560,17 +560,18 @@ def request_event_queue(user_profile: UserProfile, user_client: Client, apply_ma
|
|||||||
'client_gravatar': ujson.dumps(client_gravatar),
|
'client_gravatar': ujson.dumps(client_gravatar),
|
||||||
'all_public_streams': ujson.dumps(all_public_streams),
|
'all_public_streams': ujson.dumps(all_public_streams),
|
||||||
'client': 'internal',
|
'client': 'internal',
|
||||||
|
'user_profile_id': user_profile.id,
|
||||||
'user_client': user_client.name,
|
'user_client': user_client.name,
|
||||||
'narrow': ujson.dumps(narrow),
|
'narrow': ujson.dumps(narrow),
|
||||||
|
'secret': settings.SHARED_SECRET,
|
||||||
'lifespan_secs': queue_lifespan_secs}
|
'lifespan_secs': queue_lifespan_secs}
|
||||||
if event_types is not None:
|
if event_types is not None:
|
||||||
req['event_types'] = ujson.dumps(event_types)
|
req['event_types'] = ujson.dumps(event_types)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
resp = requests_client.get(settings.TORNADO_SERVER + '/api/v1/events',
|
resp = requests_client.post(settings.TORNADO_SERVER +
|
||||||
auth=requests.auth.HTTPBasicAuth(
|
'/api/v1/events/internal',
|
||||||
user_profile.email, user_profile.api_key),
|
data=req)
|
||||||
params=req)
|
|
||||||
except requests.adapters.ConnectionError:
|
except requests.adapters.ConnectionError:
|
||||||
logging.error('Tornado server does not seem to be running, check %s '
|
logging.error('Tornado server does not seem to be running, check %s '
|
||||||
'and %s for more information.' %
|
'and %s for more information.' %
|
||||||
@@ -587,14 +588,16 @@ def request_event_queue(user_profile: UserProfile, user_client: Client, apply_ma
|
|||||||
|
|
||||||
def get_user_events(user_profile: UserProfile, queue_id: str, last_event_id: int) -> List[Dict[Any, Any]]:
|
def get_user_events(user_profile: UserProfile, queue_id: str, last_event_id: int) -> List[Dict[Any, Any]]:
|
||||||
if settings.TORNADO_SERVER:
|
if settings.TORNADO_SERVER:
|
||||||
resp = requests_client.get(settings.TORNADO_SERVER + '/api/v1/events',
|
post_data = {
|
||||||
auth=requests.auth.HTTPBasicAuth(
|
'queue_id': queue_id,
|
||||||
user_profile.email, user_profile.api_key),
|
|
||||||
params={'queue_id': queue_id,
|
|
||||||
'last_event_id': last_event_id,
|
'last_event_id': last_event_id,
|
||||||
'dont_block': 'true',
|
'dont_block': 'true',
|
||||||
'client': 'internal'})
|
'user_profile_id': user_profile.id,
|
||||||
|
'secret': settings.SHARED_SECRET,
|
||||||
|
'client': 'internal'
|
||||||
|
} # type: Dict[str, Any]
|
||||||
|
resp = requests_client.post(settings.TORNADO_SERVER + '/api/v1/events/internal',
|
||||||
|
data=post_data)
|
||||||
resp.raise_for_status()
|
resp.raise_for_status()
|
||||||
|
|
||||||
return extract_json_response(resp)['events']
|
return extract_json_response(resp)['events']
|
||||||
|
|||||||
@@ -9,10 +9,10 @@ from django.utils.translation import ugettext as _
|
|||||||
|
|
||||||
from zerver.decorator import REQ, RespondAsynchronously, \
|
from zerver.decorator import REQ, RespondAsynchronously, \
|
||||||
_RespondAsynchronously, asynchronous, \
|
_RespondAsynchronously, asynchronous, \
|
||||||
has_request_variables, internal_notify_view
|
has_request_variables, internal_notify_view, process_client
|
||||||
from zerver.lib.response import json_error, json_success
|
from zerver.lib.response import json_error, json_success
|
||||||
from zerver.lib.validator import check_bool, check_list, check_string
|
from zerver.lib.validator import check_bool, check_list, check_string
|
||||||
from zerver.models import Client, UserProfile, get_client
|
from zerver.models import Client, UserProfile, get_client, get_user_profile_by_id
|
||||||
from zerver.tornado.event_queue import fetch_events, \
|
from zerver.tornado.event_queue import fetch_events, \
|
||||||
get_client_descriptor, process_notification
|
get_client_descriptor, process_notification
|
||||||
from zerver.tornado.exceptions import BadEventQueueIdError
|
from zerver.tornado.exceptions import BadEventQueueIdError
|
||||||
@@ -35,7 +35,18 @@ def cleanup_event_queue(request: HttpRequest, user_profile: UserProfile,
|
|||||||
return json_success()
|
return json_success()
|
||||||
|
|
||||||
@asynchronous
|
@asynchronous
|
||||||
def get_events(request: HttpRequest, user_profile: UserProfile, handler: BaseHandler):
|
@internal_notify_view(True)
|
||||||
|
@has_request_variables
|
||||||
|
def get_events_internal(request: HttpRequest, handler: BaseHandler,
|
||||||
|
user_profile_id: int=REQ()) -> Union[HttpResponse, _RespondAsynchronously]:
|
||||||
|
user_profile = get_user_profile_by_id(user_profile_id)
|
||||||
|
request._email = user_profile.email
|
||||||
|
process_client(request, user_profile, client_name="internal")
|
||||||
|
return get_events_backend(request, user_profile, handler)
|
||||||
|
|
||||||
|
@asynchronous
|
||||||
|
def get_events(request: HttpRequest, user_profile: UserProfile,
|
||||||
|
handler: BaseHandler) -> Union[HttpResponse, _RespondAsynchronously]:
|
||||||
return get_events_backend(request, user_profile, handler)
|
return get_events_backend(request, user_profile, handler)
|
||||||
|
|
||||||
@has_request_variables
|
@has_request_variables
|
||||||
|
|||||||
@@ -613,6 +613,7 @@ for app_name in settings.EXTRA_INSTALLED_APPS:
|
|||||||
urls += [
|
urls += [
|
||||||
# Used internally for communication between Django and Tornado processes
|
# Used internally for communication between Django and Tornado processes
|
||||||
url(r'^notify_tornado$', zerver.tornado.views.notify, name='zerver.tornado.views.notify'),
|
url(r'^notify_tornado$', zerver.tornado.views.notify, name='zerver.tornado.views.notify'),
|
||||||
|
url(r'^api/v1/events/internal$', zerver.tornado.views.get_events_internal),
|
||||||
]
|
]
|
||||||
|
|
||||||
# Python Social Auth
|
# Python Social Auth
|
||||||
|
|||||||
Reference in New Issue
Block a user