AngellusMortis / django_microsoft_auth

Simple app to enable Microsoft Account, Office 365 and Xbox Live authentcation as a Django authentcation backend.
MIT License
137 stars 84 forks source link

Unnecessary set of CSRF cookies and major performance issue with the msauth context processor #466

Open bartTC opened 2 years ago

bartTC commented 2 years ago

The usage of a context processor to load variables into the login.html template is not the right place and has major disadvantages.

https://github.com/AngellusMortis/django_microsoft_auth/blob/master/microsoft_auth/context_processors.py#L51-L58

The context processor is evaluated on every request, and due the get_token() call it will reset the CSRF token over and over again. That disables pretty much all upstream caches such as CloudFlare or Cloudfront.

Plus the unnecessary cost of generating the authorisation URL. Let aside the needless HTTPS warnings when developing on a localhost.

Since these variables are only used on the login.html template, loading them only there using a template tag is the better choice. I've changed the setup this way:

1) Create a new template tag get_microsoft_auth_variables.

from django import template
from microsoft_auth.context_processors import microsoft

register = template.Library()

@register.simple_tag(takes_context=True)
def get_microsoft_auth_variables(context):
    """Adds template variables for microsoft_auth"""
    context.update(microsoft(context["request"]))
    return ''

2) Remove microsoft_auth.context_processors.microsoft from the context_processors list.

3) Add a new Login template by copying the one from msauth and adjust it:

{% load i18n static microsoft_auth_tags %}

{% block content %}
  {% get_microsoft_auth_variables %}
  ...

{% block footer %}
  {% get_microsoft_auth_variables %}
  ...

Curious if I miss something here. Otherwise I'm happy to provide a PR with the necessary changes.