Differential D9035 Diff 29334 profiles/views/webhooks.py

Changeset View

Standalone View

profiles/views/webhooks.py

This file was added.

				import hashlib
				import hmac
				import json
				import logging

				from django.conf import settings
				from django.contrib.auth.models import Group
				from django.http import HttpResponse, HttpResponseBadRequest
				from django.http.request import HttpRequest
				from django.views.decorators.csrf import csrf_exempt
				from django.views.decorators.http import require_POST
				from requests_oauthlib import OAuth2Session

				from blender_id_oauth_client.views import blender_id_oauth_settings
				import profiles.queries


				logger = logging.getLogger(__name__)
				WEBHOOK_MAX_BODY_SIZE = 1024 * 10 # 10 kB is large enough for


				@csrf_exempt
				@require_POST
				def user_modified_webhook(request: HttpRequest):
				hmac_secret = settings.BLENDER_ID['WEBHOOK_USER_MODIFIED_SECRET']
				sybrenUnsubmitted Done Inline Actions Return type declaration & docstring are missing. sybren: Return type declaration & docstring are missing.
				sybrenUnsubmitted Done Inline Actions This function is doing too many things at once, better to split it up into a few smaller functions. sybren: This function is doing too many things at once, better to split it up into a few smaller…

				# Check the content type
				if request.content_type != 'application/json':
				logger.info(f'unexpected content type {request.content_type}')
				return HttpResponseBadRequest('Unsupported Content-Type')

				# Check the length of the body
				content_length = int(request.headers['CONTENT_LENGTH'])
				if content_length > WEBHOOK_MAX_BODY_SIZE:
				return HttpResponse('Payload Too Large', status=413)
				body = request.body
				if len(body) != content_length:
				sybrenUnsubmitted Not Done Inline Actions You're careful here with the length check, which is nice. An extra layer of carefulness could be added by using `body = request.body[:content_length+1]`. That way you read at most only a byte too many, avoiding DoS attacks. sybren: You're careful here with the length check, which is nice. An extra layer of carefulness could…
				raillaAuthorUnsubmitted Done Inline Actions This is a copypaste from Blender Cloud's code. I think this shouldn't be here at all, and should instead be handled by the webserver (e.g. nginx), not the application. railla: This is a copypaste from Blender Cloud's code. I think this shouldn't be here at all, and…
				return HttpResponseBadRequest("Content-Length header doesn't match content")

				# Validate the request
				mac = hmac.new(hmac_secret.encode(), body, hashlib.sha256)
				req_hmac = request.headers.get('X-Webhook-HMAC', '')
				sybrenUnsubmitted Done Inline Actions If the secret is going to be encoded anyway, why not store it in the config file as bytes? sybren: If the secret is going to be encoded anyway, why not store it in the config file as bytes?
				our_hmac = mac.hexdigest()
				if not hmac.compare_digest(req_hmac, our_hmac):
				logger.info(f'Invalid HMAC {req_hmac}, expected {our_hmac}')
				return HttpResponseBadRequest('Invalid HMAC')

				try:
				payload = json.loads(body)
				# TODO(anna) validate the payload
				logger.info('payload: %s', payload)
				except json.JSONDecodeError:
				logger.exception('Malformed JSON received')
				return HttpResponseBadRequest('Malformed JSON')

				# TODO(anna) move to a background task
				oauth_user_id = str(payload['id'])
				oauth_user_info = profiles.queries.get_oauth_user_info(oauth_user_id)
				user = oauth_user_info.user
				profile = user.profile

				# FIXME(anna) payload doesn't have username/nickname in it
				token = profiles.queries.get_oauth_token(oauth_user_id)
				bid_settings = blender_id_oauth_settings()
				bid = OAuth2Session(bid_settings.client, token={
				'access_token': token.access_token,
				'refresh_token': token.refresh_token,
				})
				user_info_resp = bid.get(bid_settings.url_userinfo)
				if user_info_resp.status_code == 200:
				user_info = user_info_resp.json()
				sybrenUnsubmitted Done Inline Actions What if the response is something else? sybren: What if the response is something else?
				if user_info['nickname'] != user.username:
				user.username = user_info['nickname']
				user.save()
				if payload['email'] != user.email:
				sybrenUnsubmitted Not Done Inline Actions What if there is already a user with that username? sybren: What if there is already a user with that username?
				user.email = payload['email']
				user.save()
				sybrenUnsubmitted Not Done Inline Actions What if there is already a user with that email address? sybren: What if there is already a user with that email address?

				if payload['full_name'] != profile.full_name:
				profile.full_name = payload['full_name']
				profile.save()
				if payload.get('avatar_changed') or not profile.avatar.name:
				profile.set_avatar()

				# Sync roles to groups
				payload_roles = payload.get('roles')
				# Remove user from the groups that are no longer in the user info payload
				groups_to_remove_from = [group for group in user.groups.all() if group.name not in payload_roles]
				if groups_to_remove_from:
				logger.warning(f'Removing user #{user.pk} from the following groups: {groups_to_remove_from}')
				user.groups.remove(*groups_to_remove_from)
				# Add them to groups that are in the info payload
				for group_name in payload_roles:
				group, _ = Group.objects.get_or_create(name=group_name)
				currently_in_group = user.groups.filter(name=group_name).exists()
				if not currently_in_group:
				group.user_set.add(user)

				sybrenUnsubmitted Not Done Inline Actions This is doing quite a few DB queries. It'll probably be simpler to get all the groups in the same query that fetches the user (with `select_related()`), then use sets to compute the differences. sybren: This is doing quite a few DB queries. It'll probably be simpler to get all the groups in the…
				raillaAuthorUnsubmitted Done Inline Actions Updated the code to use `select_related('user')` and a bulk query for adding to groups, same a with removing from groups. railla: Updated the code to use `select_related('user')` and a bulk query for adding to groups, same a…
				return HttpResponse(status=204)