Skip to content
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

recipe-server: Optionally accept remote authentication #569

Merged
merged 2 commits into from Mar 6, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
56 changes: 48 additions & 8 deletions docs/ops/config.rst
Expand Up @@ -192,19 +192,59 @@ in other Django projects.

.. envvar:: DJANGO_CDN_URL

:default: ``None``
:default: ``None``

The URL of a CDN that is backed by Normandy, if one is in use. This is used to
enforce that immutable content is routed through the CDN. Must end with a
slash (``/``).
The URL of a CDN that is backed by Normandy, if one is in use. This is used to
enforce that immutable content is routed through the CDN. Must end with a
slash (``/``).

.. envvar:: DJANGO_APP_SERVER_URL

:default: ``None``
:default: ``None``

The URL that allows direct access to Normandy, bypassing any CDNs. This
is used for content that cannot be cached. If not specified, Normandy will
assume direct access. Must end with a slash (``/``).

.. envvar:: DJANGO_USE_OIDC

:default: ``False``

If enabled, Normandy will authenticate users by reading a header in requests.
The expectation is that a proxy server, such as Nginx, will perform
authentication using Open ID Connect, and then pass the unique ID of the user
in a header.

.. seealso::

:envvar:`DJANGO_OIDC_REMOTE_AUTH_HEADER` for which header Normandy
reads this value from.

.. warning::

If this feature is enabled, the proxy server providing authentication
*must* sanitize the headers passed along to Normandy. Specifically, the
header defined in :envvar:`DJANGO_OIDC_REMOTE_AUTH_HEADER` must not be
passed on from the user.

Failing to do this will result in any client being able to authenticate
as any user, with no checks.

.. envvar:: DJANGO_OIDC_REMOTE_AUTH_HEADER

:default: ``HTTP_REMOTE_USER``

If :envvar:`DJANGO_USE_OIDC` is ``True``, this is the source of the user to
authenticate. This must match Django header normalization, i.e. it must be
capitalized, dashes replaced with underscores, and be prefixed with ``HTTP_``.

For example, the header ``OIDC-Claim-User-Profile-Email`` becomes
``HTTP_OIDC_CLAIM_USER_PROFILE_EMAIL``.

.. envvar:: DJANGO_OIDC_LOGOUT_URL

The URL that allows direct access to Normandy, bypassing any CDNs. This
is used for content that cannot be cached. If not specified, Normandy will
assume direct access. Must end with a slash (``/``).
If :envvar:`DJANGO_USE_OIDC` is set to ``True``, this settings must be set to
the URL that a user can visit to logout. It may be a relative URL.

Gunicorn settings
-----------------
Expand Down
32 changes: 26 additions & 6 deletions recipe-server/normandy/base/auth_backends.py
@@ -1,6 +1,6 @@
import logging

from django.contrib.auth.backends import ModelBackend
from django.contrib.auth.backends import ModelBackend, RemoteUserBackend


INFO_LOGIN_SUCCESS = 'normandy.auth.I001'
Expand All @@ -10,13 +10,17 @@
logger = logging.getLogger(__name__)


class LoggingModelBackend(ModelBackend):
class LoggingAuthBackendMixin(object):
"""
Model-backed authentication backend that logs the results of login
attempts.
Authentication backend mixin that logs the results of login attempts.
"""
def authenticate(self, username=None, **kwargs):
result = super().authenticate(username=username, **kwargs)
def get_username(self, **kwargs):
raise NotImplemented()

def authenticate(self, **kwargs):
result = super().authenticate(**kwargs)
username = self.get_username(**kwargs)

if result is None:
if username is not None:
logger.warning(
Expand All @@ -34,3 +38,19 @@ def authenticate(self, username=None, **kwargs):
extra={'code': INFO_LOGIN_SUCCESS}
)
return result


class LoggingModelBackend(LoggingAuthBackendMixin, ModelBackend):
"""
Model-backed authentication backend that logs the results of login attempts.
"""
def get_username(self, username=None, **kwargs):
return username


class LoggingRemoteUserBackend(LoggingAuthBackendMixin, RemoteUserBackend):
"""
Remote-user backend that logs the results of login attempts.
"""
def get_username(self, remote_user=None, **kwargs):
return remote_user
36 changes: 31 additions & 5 deletions recipe-server/normandy/base/checks.py
@@ -1,11 +1,14 @@
from django.conf import settings
from django.core.checks import Warning, register as register_check
from django.core.checks import Error, Warning, register as register_check


WARNING_MISCONFIGURED_OIDC_REMOTE_AUTH_HEADER_PREFIX = 'normandy.base.W001'

ERROR_MISCONFIGURED_CDN_URL_SLASH = 'normandy.base.E001'
ERROR_MISCONFIGURED_CDN_URL_HTTPS = 'normandy.base.E002'
ERROR_MISCONFIGURED_APP_SERVER_URL_SLASH = 'normandy.base.E003'
ERROR_MISCONFIGURED_APP_SERVER_URL_HTTPS = 'normandy.base.E004'
ERROR_MISCONFIGURED_OIDC_LOGOUT_URL = 'normandy.base.E005'


def setting_cdn_url(app_configs, **kwargs):
Expand All @@ -14,11 +17,11 @@ def setting_cdn_url(app_configs, **kwargs):
if settings.CDN_URL is not None:
if settings.CDN_URL[-1] != '/':
msg = 'The setting CDN_URL must end in a slash'
errors.append(Warning(msg, id=ERROR_MISCONFIGURED_CDN_URL_SLASH))
errors.append(Error(msg, id=ERROR_MISCONFIGURED_CDN_URL_SLASH))

if not settings.CDN_URL.startswith('https://'):
msg = 'The setting CDN_URL must be an https URL'
errors.append(Warning(msg, id=ERROR_MISCONFIGURED_CDN_URL_HTTPS))
errors.append(Error(msg, id=ERROR_MISCONFIGURED_CDN_URL_HTTPS))

return errors

Expand All @@ -29,14 +32,37 @@ def setting_app_server_url(app_configs, **kwargs):
if settings.APP_SERVER_URL is not None:
if settings.APP_SERVER_URL[-1] != '/':
msg = 'The setting APP_SERVER_URL must end in a slash'
errors.append(Warning(msg, id=ERROR_MISCONFIGURED_APP_SERVER_URL_SLASH))
errors.append(Error(msg, id=ERROR_MISCONFIGURED_APP_SERVER_URL_SLASH))

if not settings.APP_SERVER_URL.startswith('https://'):
msg = 'The setting APP_SERVER_URL must be an https URL'
errors.append(Warning(msg, id=ERROR_MISCONFIGURED_APP_SERVER_URL_HTTPS))
errors.append(Error(msg, id=ERROR_MISCONFIGURED_APP_SERVER_URL_HTTPS))

return errors


def setting_oidc_remote_auth_header(app_configs, **kwargs):
errors = []

if not settings.OIDC_REMOTE_AUTH_HEADER.startswith('HTTP_'):
msg = 'The setting OIDC_REMOTE_AUTH_HEADER should start with HTTP_'
errors.append(Warning(msg, id=WARNING_MISCONFIGURED_OIDC_REMOTE_AUTH_HEADER_PREFIX))

return errors


def setting_oidc_logout_url(app_configs, **kwargs):
errors = []

if settings.USE_OIDC and settings.OIDC_LOGOUT_URL is None:
msg = 'The setting OIDC_LOGOUT_URL must be set when USE_OIDC=True'
errors.append(Error(msg, id=ERROR_MISCONFIGURED_OIDC_LOGOUT_URL))

return errors


def register():
register_check(setting_cdn_url)
register_check(setting_app_server_url)
register_check(setting_oidc_remote_auth_header)
register_check(setting_oidc_logout_url)
19 changes: 19 additions & 0 deletions recipe-server/normandy/base/middleware.py
@@ -1,5 +1,7 @@
from django.conf import settings
from django.utils import timezone
from django.utils.deprecation import MiddlewareMixin
from django.contrib.auth.middleware import RemoteUserMiddleware

from mozilla_cloud_services_logger.django.middleware import (
RequestSummaryLogger as OriginalRequestSummaryLogger
Expand All @@ -24,3 +26,20 @@ class RequestSummaryLogger(MiddlewareMixin, OriginalRequestSummaryLogger):
Adapt mozilla_cloud_services_logger's request logger to Django 1.10 new-style middleware.
"""
pass


class ConfigurableRemoteUserMiddleware(RemoteUserMiddleware):
"""
Makes RemoteUserMiddleware customizable via settings.
"""

@property
def header(self):
"""
Name of request header to grab username from.

This will be the key as used in the request.META dictionary. To
reference HTTP headers, this value should be all upper case and
prefixed with "HTTP_".
"""
return settings.OIDC_REMOTE_AUTH_HEADER
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we have the setting be the real header name, and just apply the transformation here instead of making the person configuring the site do it?

Copy link
Contributor

@relud relud Mar 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would rather specify the header as HTTP_OIDC_CLAIM_USER_PROFILE_EMAIL than Oidc-Claim-User-Profile-Email, because that's how I specify it in openresty lua. It sounds like that would work either way though, and this would allow for more flexible input.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd prefer to stick with the HTTP_OIDC_CLAIM_USER_PROFILE_EMAIL style. I'll clean up the docs, but I think enough tools use this format that it is a fine thing to do.

@@ -1,5 +1,6 @@
{% load i18n %}
{% load render_bundle from normandy_webpack_loader %}
{% load logout_button from normandy_logout_button %}
<!DOCTYPE html>
{% get_current_language as LANGUAGE_CODE %}

Expand All @@ -20,7 +21,7 @@ <h1><a href="{% url 'control:index' %}">SHIELD Control Panel</a></h1>
{% if user.is_authenticated %}
<span>
{% firstof user.get_short_name user.get_username %} //
<a href="{% url 'control:logout' %}">Log Out <i class="fa fa-sign-out post"></i></a>
{% logout_button %}
</span>
{% endif %}
</div>
Expand Down
Empty file.
@@ -0,0 +1,17 @@
from django import template
from django.conf import settings
from django.core.urlresolvers import reverse
from django.utils.safestring import mark_safe


register = template.Library()


@register.simple_tag
def logout_button():
if settings.USE_OIDC:
logout_url = settings.OIDC_LOGOUT_URL
else:
logout_url = reverse('control:logout')

return mark_safe(f'<a href="{logout_url}">Log Out <i class="fa fa-sign-out post"></i></a>')
18 changes: 18 additions & 0 deletions recipe-server/normandy/control/tests/test_templatetags.py
@@ -0,0 +1,18 @@
from django.core.urlresolvers import reverse

from normandy.control.templatetags.normandy_logout_button import logout_button


class TestLogoutButton(object):
def test_without_oidc(self, settings):
settings.USE_OIDC = False
html = logout_button()
assert reverse('control:logout') in html
assert 'None' not in html

def test_with_oidc(self, settings):
settings.USE_OIDC = True
settings.OIDC_LOGOUT_URL = 'https://example.com/auth/logout'
html = logout_button()
assert settings.OIDC_LOGOUT_URL in html
assert reverse('control:logout') not in html
35 changes: 21 additions & 14 deletions recipe-server/normandy/settings.py
Expand Up @@ -63,11 +63,6 @@ class Core(Configuration):

WSGI_APPLICATION = 'normandy.wsgi.application'

# Authentication
AUTHENTICATION_BACKENDS = [
'normandy.base.auth_backends.LoggingModelBackend',
]

# Internationalization
LANGUAGE_CODE = 'en-us'
TIME_ZONE = 'UTC'
Expand All @@ -94,10 +89,7 @@ class Core(Configuration):
}

REST_FRAMEWORK = {
'DEFAULT_AUTHENTICATION_CLASSES': (
'rest_framework.authentication.TokenAuthentication',
'rest_framework.authentication.SessionAuthentication'
),
'DEFAULT_AUTHENTICATION_CLASSES': ['rest_framework.authentication.SessionAuthentication'],
'DEFAULT_FILTER_BACKENDS': ['django_filters.rest_framework.DjangoFilterBackend'],
'TEST_REQUEST_DEFAULT_FORMAT': 'json',
'DEFAULT_RENDERER_CLASSES': (
Expand Down Expand Up @@ -145,13 +137,27 @@ def CSP_DEFAULT_SRC(self):

class Base(Core):
"""Settings that may change per-environment, some with defaults."""

# Flags that affect other settings, via setting methods below
LOGGING_USE_JSON = values.BooleanValue(False)
USE_OIDC = values.BooleanValue(False)

# General settings
DEBUG = values.BooleanValue(False)
ADMINS = values.SingleNestedListValue([])
SILENCED_SYSTEM_CHECKS = values.ListValue([])

# Middleware that _most_ environments will need. Subclasses can
# override this list.
# Authentication
def AUTHENTICATION_BACKENDS(self):
if self.USE_OIDC:
return ['normandy.base.auth_backends.LoggingRemoteUserBackend']
else:
return ['normandy.base.auth_backends.LoggingModelBackend']

OIDC_REMOTE_AUTH_HEADER = values.Value('HTTP_REMOTE_USER')
OIDC_LOGOUT_URL = values.Value(None)

# Middleware that _most_ environments will need. Subclasses can override this list.
EXTRA_MIDDLEWARE = [
'django.contrib.sessions.middleware.SessionMiddleware',
'django.middleware.csrf.CsrfViewMiddleware',
Expand All @@ -165,9 +171,10 @@ def MIDDLEWARE(self):
Determine middleware by combining the core set and
per-environment set.
"""
return Core.MIDDLEWARE + self.EXTRA_MIDDLEWARE

LOGGING_USE_JSON = values.BooleanValue(False)
middleware = Core.MIDDLEWARE + self.EXTRA_MIDDLEWARE
if self.USE_OIDC:
middleware.append('normandy.base.middleware.ConfigurableRemoteUserMiddleware')
return middleware

def LOGGING(self):
return {
Expand Down