diff --git a/dev-requirements.txt b/dev-requirements.txt index f16f233eab29b39e6edabb3bd2a59036ecae34e9..1c0d6a76ee80383e0dd6cdae94f7f90b5d626625 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -30,3 +30,4 @@ wheel ReviewBoard[ldap] ReviewBoard[s3] ReviewBoard[swift] +ReviewBoard[saml] diff --git a/docs/manual/admin/configuration/authentication-settings.rst b/docs/manual/admin/configuration/authentication-settings.rst index 06459b72277677cb0a5f2c44fc5b1dd76c73b5be..b393680e86abba2048907e14d59b6168571dc037 100644 --- a/docs/manual/admin/configuration/authentication-settings.rst +++ b/docs/manual/admin/configuration/authentication-settings.rst @@ -11,6 +11,7 @@ Authentication Settings * :ref:`nis-authentication-settings` * :ref:`legacy-authentication-settings` * :ref:`fixing-a-broken-authentication-setup` +* :ref:`saml-settings` .. _auth-general-settings: @@ -222,3 +223,96 @@ Board server to fix it. In this case, you can reset the authentication backend back to the builtin database method with the :command:`rb-site` command:: $ rb-site manage /path/to/site set-siteconfig -- --key=auth_backend --value=builtin + + +.. _saml-settings: + +SAML 2.0 Authentication +======================= + +Review Board supports SAML 2.0 for Single Sign-On (SSO). This requires +installing additional dependencies: + +.. code-block:: console + + $ pip install -U 'ReviewBoard[saml]' + +To enable SAML 2.0, you'll need to configure both the settings in Review Board +(the Service Provider) and your Identity Provider. + + +Review Board Configuration +-------------------------- + +For the Review Board configuration, you'll need to start by checking the +:guilabel:`Enable SAML 2.0 authentication` box. You'll then see a new section +to configure Review Board to know about your Identity Provider. + +Your Identity Provider should provide the following for you to put into the +Review Board configuration: + +1. URLs for the Issuer and SAML/SLO endpoints, as well as the binding type for + each. +2. A copy of the X.509 certificate. +3. Possibly, specific digest and signature algorithm types. + +The :guilabel:`Require login to link` setting allows you to control the +behavior when first authenticating a user via SSO who already has an account on +Review Board. If you have a trusted internal environment where you're confident +that the Identity Provider is sending the correct usernames, you can leave this +field unchecked. If you enable this, existing users will be asked to enter +their Review Board password a single time before linking the SAML identity. + + +Identity Provider Configuration +------------------------------- + +On the Identity Provider side, you'll need to configure it with the following +URLs. Replace the server name with your configured server name. + +Audience/Metadata + Example: ``https://example.com/account/sso/saml/metadata/`` + +ACS/Recipient + Example: ``https://example.com/account/sso/saml/acs/`` + +Single Logout + Example: ``https://example.com/account/sso/saml/sls/`` + +You'll also need to configure your assertion parameters. The desired username +should be sent in the SAML ``NameID`` field. The other parameters that should +be sent in the assertion are ``User.email``, ``User.FirstName``, and +``User.LastName``. + + +User Authentication +------------------- + +Depending on how authentication is configured with Review Board, users may or +may not have a working password. For example, a server that is using both +Active Directory and SAML will allow users to log in either with the SSO +provider or with the standard AD credentials. A server that is configured with +standard authentication and has registration turned off will force all users to +go through SSO. + +In the case where users do not have a password, they will need to use API +tokens for any external tools, including the RBTools command-line. API tokens +can be created through the user's :ref:`account-settings`. + +After creating an API token, users can use it to authenticate. + +To configure RBTools to authenticate by adding the token to +:file:`.reviewboardrc`, include the following:: + + API_TOKEN = "" + +Alternatively, if you don't want to store the token, pass it to :command:`rbt +login`. This will create a session cookie that will be used for subsequent +RBTools commands. This may require periodic re-authentication as the sessions +expire. + +.. code-block:: console + + $ rbt login --api-token + +See :ref:`api-tokens` for more information on creating API tokens. diff --git a/reviewboard/accounts/__init__.py b/reviewboard/accounts/__init__.py index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..bf3716a0f9e8be17f8381517e773d345dc9b0726 100644 --- a/reviewboard/accounts/__init__.py +++ b/reviewboard/accounts/__init__.py @@ -0,0 +1 @@ +"""App for handling login and account management.""" diff --git a/reviewboard/accounts/admin.py b/reviewboard/accounts/admin.py index 957e4262e44f6e43aefefd35ac6ae352b3e7d410..effcef525809ba7af43de47d63ff02f55f61a45c 100644 --- a/reviewboard/accounts/admin.py +++ b/reviewboard/accounts/admin.py @@ -5,8 +5,10 @@ from django.contrib.auth.forms import UserChangeForm, UserCreationForm from django.contrib.auth.models import User from django.utils.translation import gettext_lazy as _ -from reviewboard.accounts.models import (ReviewRequestVisit, Profile, - LocalSiteProfile) +from reviewboard.accounts.models import (LinkedAccount, + LocalSiteProfile, + Profile, + ReviewRequestVisit) from reviewboard.admin import ModelAdmin, admin_site from reviewboard.reviews.models import Group @@ -121,6 +123,17 @@ class RBUserAdmin(UserAdmin): ] +class LinkedAccountAdmin(ModelAdmin): + """Admin definitions for the LinkedAccount model. + + Version Added: + 5.0 + """ + + list_display = ('user', 'service_id', 'service_user_id') + raw_id_fields = ('user',) + + class ReviewRequestVisitAdmin(ModelAdmin): """Admin definitions for the ReviewRequestVisit model.""" @@ -157,6 +170,7 @@ def fix_review_counts(): admin_site.unregister(User) admin_site.register(User, RBUserAdmin) -admin_site.register(ReviewRequestVisit, ReviewRequestVisitAdmin) -admin_site.register(Profile, ProfileAdmin) +admin_site.register(LinkedAccount, LinkedAccountAdmin) admin_site.register(LocalSiteProfile, LocalSiteProfileAdmin) +admin_site.register(Profile, ProfileAdmin) +admin_site.register(ReviewRequestVisit, ReviewRequestVisitAdmin) diff --git a/reviewboard/accounts/apps.py b/reviewboard/accounts/apps.py new file mode 100644 index 0000000000000000000000000000000000000000..2c36d5436576232f3e905db90e8c7d6a1963d5fe --- /dev/null +++ b/reviewboard/accounts/apps.py @@ -0,0 +1,25 @@ +"""App definition for reviewboard.accounts. + +Version Added: + 5.0 +""" + +from django.apps import AppConfig + + +class AccountsAppConfig(AppConfig): + """App configuration for reviewboard.accounts. + + Version Added + 5.0 + """ + + name = 'reviewboard.accounts' + + def ready(self): + """Configure the app once it's ready. + + This will populate the SSO backends registry. + """ + from reviewboard.accounts.sso.backends import sso_backends + sso_backends.populate() diff --git a/reviewboard/accounts/evolutions/__init__.py b/reviewboard/accounts/evolutions/__init__.py index b3e64d36479d28cf636d7c608e35ea8a1a8478ab..d375b616f84f697268e4b643026e46222905c38a 100644 --- a/reviewboard/accounts/evolutions/__init__.py +++ b/reviewboard/accounts/evolutions/__init__.py @@ -13,4 +13,5 @@ SEQUENCE = [ 'reviewrequestvisit_visibility', 'profile_settings', 'profile_default_use_rich_text_boolean_field', + 'linkedaccount_unique_together', ] diff --git a/reviewboard/accounts/evolutions/linkedaccount_unique_together.py b/reviewboard/accounts/evolutions/linkedaccount_unique_together.py new file mode 100644 index 0000000000000000000000000000000000000000..1480b6dab98d41891ee9da19a43e234c6aa2e289 --- /dev/null +++ b/reviewboard/accounts/evolutions/linkedaccount_unique_together.py @@ -0,0 +1,13 @@ +"""Set unique_together for LinkedAccount. + +Version Added: + 5.0 +""" + +from django_evolution.mutations import ChangeMeta + + +MUTATIONS = [ + ChangeMeta('LinkedAccount', 'unique_together', + [('service_user_id', 'service_id')]), +] diff --git a/reviewboard/accounts/models.py b/reviewboard/accounts/models.py index 70d996a72d88bed6ac7db53cff084cf4a4109857..f3c48ba26772865dbca58b4bb1b78d4b6845e5e7 100644 --- a/reviewboard/accounts/models.py +++ b/reviewboard/accounts/models.py @@ -77,6 +77,35 @@ class ReviewRequestVisit(models.Model): verbose_name_plural = _('Review Request Visits') +class LinkedAccount(models.Model): + """A linked account on an external service. + + This can be used to associate user accounts on Review Board with accounts + on external services. These services might be third parties that Review + Board interacts with (such as hosting services), or alternative methods of + authentication like OpenID or SAML. + + Version Added: + 5.0 + """ + + user = models.ForeignKey(User, + on_delete=models.CASCADE, + related_name='linked_accounts') + service_id = models.CharField(max_length=64) + service_user_id = models.CharField(max_length=254) + service_data = JSONField( + help_text=_('Used to store private data such as session keys.')) + + extra_data = JSONField() + + class Meta: + """Metadata for the LinkedAccount model.""" + + db_table = 'accounts_linkedaccount' + unique_together = ('service_user_id', 'service_id') + + class Profile(models.Model): """User profile which contains some basic configurable settings.""" diff --git a/reviewboard/accounts/sso/__init__.py b/reviewboard/accounts/sso/__init__.py new file mode 100644 index 0000000000000000000000000000000000000000..3099a0bb26741c9c4e994ee3bea286f317f628bf --- /dev/null +++ b/reviewboard/accounts/sso/__init__.py @@ -0,0 +1,5 @@ +"""Single Sign-On for Review Board. + +Version Added: + 5.0 +""" diff --git a/reviewboard/accounts/sso/backends/__init__.py b/reviewboard/accounts/sso/backends/__init__.py new file mode 100644 index 0000000000000000000000000000000000000000..4f81355b11f26eb0dae6194d615ba60709bcb17f --- /dev/null +++ b/reviewboard/accounts/sso/backends/__init__.py @@ -0,0 +1,13 @@ +"""Backends for Single Sign-On. + +Version Added: + 5.0 +""" + +from djblets.registries.importer import lazy_import_registry + + +#: The SSO backends registry +sso_backends = lazy_import_registry( + 'reviewboard.accounts.sso.backends.registry', + 'SSOBackendRegistry') diff --git a/reviewboard/accounts/sso/backends/base.py b/reviewboard/accounts/sso/backends/base.py new file mode 100644 index 0000000000000000000000000000000000000000..aa057156556b4f997fe0913803c5fefdc4394e52 --- /dev/null +++ b/reviewboard/accounts/sso/backends/base.py @@ -0,0 +1,110 @@ +"""Base classes for SSO backends. + +Version Added: + 5.0 +""" + +from django.contrib import auth +from djblets.siteconfig.models import SiteConfiguration + +from reviewboard.accounts.backends import StandardAuthBackend + + +class BaseSSOBackend(object): + """Base class for SSO backends. + + Version Added: + 5.0 + """ + + #: The ID of this SSO backend. + #: + #: Type: + #: str + backend_id = None + + #: The name of this SSO backend. + #: + #: Type: + #: str + name = None + + #: The form used for settings. + #: + #: This must be a subclass of + #: :py:class:`~djblets.siteconfig.forms.SiteSettingsForm`. + #: + #: Type: + #: type + settings_form = None + + #: The defaults for siteconfig entries. + #: + #: Type: + #: dict + siteconfig_defaults = {} + + #: The text to show in the label for the login button. + #: + #: Type: + #: str + login_label = None + + #: The URL to the login flow. + #: + #: Type: + #: str + login_url = None + + #: The class type for the view handling the initial login request. + #: + #: Type: + #: type + login_view_cls = None + + #: A list of URLs to register. + #: + #: Type: + #: list + urls = [] + + def __init__(self): + """Initialize the SSO backend.""" + + def is_available(self): + """Return whether this backend is available. + + Returns: + tuple: + A two-tuple. The items in the tuple are: + + 1. A bool indicating whether the backend is available. + 2. If the first element is ``False``, this will be a user-visible + string indicating the reason why the backend is not available. + If the backend is available, this element will be ``None``. + """ + raise NotImplementedError + + def is_enabled(self): + """Return whether this backend is enabled. + + Returns: + bool: + ``True`` if this SSO backend is enabled. + """ + siteconfig = SiteConfiguration.objects.get_current() + return siteconfig.get('%s_enabled' % self.backend_id, False) + + def login_user(self, request, user): + """Log in the given user. + + Args: + request (django.http.HttpRequest): + The HTTP request. + + user (django.contrib.auth.models.User): + The user to log in. + """ + user.backend = '%s.%s' % (StandardAuthBackend.__module__, + StandardAuthBackend.__name__) + auth.login(request, user) diff --git a/reviewboard/accounts/sso/backends/registry.py b/reviewboard/accounts/sso/backends/registry.py new file mode 100644 index 0000000000000000000000000000000000000000..0f3d7dfd3060971768df3a60128ac25992b2846d --- /dev/null +++ b/reviewboard/accounts/sso/backends/registry.py @@ -0,0 +1,128 @@ +"""Registry for SSO backends. + +Version Added: + 5.0 +""" + +import re +from importlib import import_module + +from django.urls import include, path, re_path +from django.utils.translation import gettext_lazy as _ +from djblets.registries.registry import (ALREADY_REGISTERED, + NOT_REGISTERED) + +from reviewboard.registries.registry import Registry + + +class SSOBackendRegistry(Registry): + """A registry for managing SSO backends. + + Version Added: + 5.0 + """ + + entry_point = 'reviewboard.sso_backends' + lookup_attrs = ['backend_id'] + + errors = { + ALREADY_REGISTERED: _( + '"%(item)s" is already a registered SSO backend.' + ), + NOT_REGISTERED: _( + '"%(attr_value)s" is not a registered SSO backend.' + ), + } + + def __init__(self): + """Initialize the registry.""" + super().__init__() + self._url_patterns = {} + + def get_defaults(self): + """Yield the built-in SSO backends. + + This will make sure the standard SSO backends are always present in the + registry. + + Yields: + reviewboard.accounts.sso.backends.base.BaseSSOBackend: + The SSO backend instances. + """ + builtin_backends = ( + ('saml.sso_backend', 'SAMLSSOBackend'), + ) + + for _module, _backend_cls_name in builtin_backends: + mod = import_module('reviewboard.accounts.sso.backends.%s' + % _module) + cls = getattr(mod, _backend_cls_name) + yield cls() + + def get_siteconfig_defaults(self): + """Return defaults for the site configuration. + + Returns: + dict: + The defaults to register for the site configuration. + """ + defaults = {} + + for backend in self: + defaults.update(backend.siteconfig_defaults) + + return defaults + + def register(self, backend): + """Register an SSO backend. + + This also adds the URL patterns defined by the backend. If the backend + has a + :py:attr:`~reviewboard.accounts.sso.backends.base.SSOBackend.urls` + attribute that is non-``None``, they will be automatically added. + + Args: + backend (reviewboard.accounts.sso.backends.base.BaseSSOBackend): + The backend instance. + """ + super().register(backend) + + from reviewboard.accounts.urls import sso_dynamic_urls + + if backend.urls: + backend_id = backend.backend_id + backend_urls = backend.urls + + if backend.login_view_cls: + backend_urls.append(path( + 'login/', + backend.login_view_cls.as_view(sso_backend=backend), + name='login')) + + dynamic_urls = [ + re_path( + r'^(?P%s)/' % re.escape(backend_id), + include((backend_urls, 'accounts'), + namespace=backend_id)), + ] + self._url_patterns[backend_id] = dynamic_urls + sso_dynamic_urls.add_patterns(dynamic_urls) + + def unregister(self, backend): + """Unregister an SSO backend. + + This will remove all registered URLs that the backend has defined. + + Args: + backend (reviewboard.accounts.sso.backends.base.BaseSSOBackend): + The backend instance. + """ + super().unregister(backend) + + from reviewboard.accounts.urls import sso_dynamic_urls + + try: + dynamic_urls = self._url_patterns.pop(backend.backend_id) + sso_dynamic_urls.remove_patterns(dynamic_urls) + except KeyError: + pass diff --git a/reviewboard/accounts/sso/backends/saml/__init__.py b/reviewboard/accounts/sso/backends/saml/__init__.py new file mode 100644 index 0000000000000000000000000000000000000000..6e5ac23037cdb449976b85e1322aa4c232a6ecac --- /dev/null +++ b/reviewboard/accounts/sso/backends/saml/__init__.py @@ -0,0 +1,5 @@ +"""SAML SSO backend. + +Version Added: + 5.0 +""" diff --git a/reviewboard/accounts/sso/backends/saml/forms.py b/reviewboard/accounts/sso/backends/saml/forms.py new file mode 100644 index 0000000000000000000000000000000000000000..71c5c6606488dd242bb379fcf9597bd2fd645f94 --- /dev/null +++ b/reviewboard/accounts/sso/backends/saml/forms.py @@ -0,0 +1,157 @@ +"""Forms for SAML SSO. + +Version Added: + 5.0 +""" + +from cryptography import x509 +from cryptography.hazmat.backends import default_backend +from django import forms +from django.contrib.auth.forms import AuthenticationForm +from django.core.exceptions import ValidationError +from django.core.validators import URLValidator +from django.utils.translation import gettext_lazy as _ +from djblets.siteconfig.forms import SiteSettingsForm + +from reviewboard.accounts.sso.backends.saml.settings import ( + SAMLBinding, + SAMLDigestAlgorithm, + SAMLSignatureAlgorithm) + + +class SAMLLinkUserForm(AuthenticationForm): + """Form for linking existing user accounts after SAML authentication. + + Version Added: + 5.0 + """ + + provision = forms.BooleanField( + widget=forms.HiddenInput(), + required=False) + + def __init__(self, *args, **kwargs): + """Initialize the form. + + Args: + *args (tuple): + Positional arguments to pass through to the parent class. + + **kwargs (dict): + Keyword arguments to pass through to the parent class. + """ + super().__init__(*args, **kwargs) + + # If we're in provision mode, we don't want username and password to be + # required. + if kwargs.get('initial', {}).get('provision', False): + self.fields['username'].required = False + self.fields['password'].required = False + + def clean(self): + """Run validation on the form. + + Returns: + dict: + The cleaned data. + """ + if self.cleaned_data.get('provision'): + # If we're provisioning a new user, we don't actually care about + # authenticating the login/password. + return self.cleaned_data + else: + return super(SAMLLinkUserForm, self).clean() + + +def validate_x509(value): + """Validate that the given value is a correct X.509 certificate. + + Args: + value (str): + The value to validate. + + Raises: + django.core.exceptions.ValidationError: + The given value was not a correct X.509 certificate. + """ + try: + x509.load_pem_x509_certificate(value.encode('ascii'), + default_backend()) + except ValueError: + raise ValidationError('Could not parse X.509 certificate') + + +class SAMLSettingsForm(SiteSettingsForm): + """Form for configuring SAML authentication. + + Version Added: + 5.0 + """ + + saml_login_button_text = forms.CharField( + label=_('Login button label')) + + saml_issuer = forms.CharField( + label=_('IdP issuer URL (or Entity ID)'), + validators=[URLValidator(schemes=['http', 'https'])], + widget=forms.TextInput(attrs={'size': '60'})) + + saml_signature_algorithm = forms.ChoiceField( + label=_('Signature algorithm'), + choices=SAMLSignatureAlgorithm.CHOICES) + + saml_digest_algorithm = forms.ChoiceField( + label=_('Digest algorithm'), + choices=SAMLDigestAlgorithm.CHOICES) + + saml_verification_cert = forms.CharField( + label=_('X.509 verification certificate'), + validators=[validate_x509], + widget=forms.Textarea()) + + saml_sso_url = forms.CharField( + label=_('SAML 2.0 endpoint'), + validators=[URLValidator(schemes=['http', 'https'])], + widget=forms.TextInput(attrs={'size': '60'})) + + saml_sso_binding_type = forms.ChoiceField( + label=_('SAML 2.0 endpoint binding'), + choices=SAMLBinding.CHOICES, + initial=SAMLBinding.HTTP_POST) + + saml_slo_url = forms.CharField( + label=_('SLO endpoint'), + validators=[URLValidator(schemes=['http', 'https'])], + widget=forms.TextInput(attrs={'size': '60'})) + + saml_slo_binding_type = forms.ChoiceField( + label=_('SLO endpoint binding'), + choices=SAMLBinding.CHOICES, + initial=SAMLBinding.HTTP_REDIRECT) + + saml_require_login_to_link = forms.BooleanField( + label=_('Require login to link'), + help_text=_('When a matching user is found, ask them to log in with ' + 'their existing password before linking. If unchecked, ' + 'make sure that your Identity Provider is trusted and is ' + 'sending the correct username for all existing users.'), + required=False) + + class Meta: + """Metadata for the SAMLSettingsForm.""" + + title = _('SAML 2.0 Authentication Settings') + fieldsets = ( + (None, { + 'fields': ('saml_login_button_text', + 'saml_issuer', + 'saml_signature_algorithm', + 'saml_digest_algorithm', + 'saml_verification_cert', + 'saml_sso_url', + 'saml_sso_binding_type', + 'saml_slo_url', + 'saml_slo_binding_type', + 'saml_require_login_to_link'), + }), + ) diff --git a/reviewboard/accounts/sso/backends/saml/settings.py b/reviewboard/accounts/sso/backends/saml/settings.py new file mode 100644 index 0000000000000000000000000000000000000000..6982227519be0e3cbb2fa57c9b9ec35782167f4c --- /dev/null +++ b/reviewboard/accounts/sso/backends/saml/settings.py @@ -0,0 +1,175 @@ +"""Settings for SAML SSO. + +Version Added: + 5.0 +""" + +from django.urls import reverse +from django.utils.translation import gettext_lazy as _ +from djblets.siteconfig.models import SiteConfiguration +try: + from onelogin.saml2.constants import OneLogin_Saml2_Constants as constants +except ImportError: + constants = None + +from reviewboard.admin.server import build_server_url + + +class SAMLSignatureAlgorithm(object): + """Definitions for the signature algorithm. + + Version Added: + 5.0 + """ + + DSA_SHA1 = 'dsa-sha1' + RSA_SHA1 = 'rsa-sha1' + RSA_SHA256 = 'rsa-sha256' + RSA_SHA384 = 'rsa-sha384' + RSA_SHA512 = 'rsa-sha512' + + CHOICES = ( + (DSA_SHA1, 'DSA-SHA1'), + (RSA_SHA1, 'RSA-SHA1'), + (RSA_SHA256, 'RSA-SHA256'), + (RSA_SHA384, 'RSA-SHA384'), + (RSA_SHA512, 'RSA-SHA512'), + ) + + if constants: + TO_SAML2_SETTING_MAP = { + DSA_SHA1: constants.DSA_SHA1, + RSA_SHA1: constants.RSA_SHA1, + RSA_SHA256: constants.RSA_SHA256, + RSA_SHA384: constants.RSA_SHA384, + RSA_SHA512: constants.RSA_SHA512, + } + + FROM_SAML2_SETTING_MAP = { + constants.DSA_SHA1: DSA_SHA1, + constants.RSA_SHA1: RSA_SHA1, + constants.RSA_SHA256: RSA_SHA256, + constants.RSA_SHA384: RSA_SHA384, + constants.RSA_SHA512: RSA_SHA512, + } + else: + TO_SAML2_SETTING_MAP = {} + FROM_SAML2_SETTING_MAP = {} + + +class SAMLDigestAlgorithm(object): + """Definitions for the digest algorithm. + + Version Added: + 5.0 + """ + + SHA1 = 'sha1' + SHA256 = 'sha256' + SHA384 = 'sha384' + SHA512 = 'sha512' + + CHOICES = ( + (SHA1, 'SHA1'), + (SHA256, 'SHA256'), + (SHA384, 'SHA384'), + (SHA512, 'SHA512'), + ) + + if constants: + TO_SAML2_SETTING_MAP = { + SHA1: constants.SHA1, + SHA256: constants.SHA256, + SHA384: constants.SHA384, + SHA512: constants.SHA512, + } + + FROM_SAML2_SETTING_MAP = { + constants.SHA1: SHA1, + constants.SHA256: SHA256, + constants.SHA384: SHA384, + constants.SHA512: SHA512, + } + else: + TO_SAML2_SETTING_MAP = {} + FROM_SAML2_SETTING_MAP = {} + + +class SAMLBinding(object): + """Definitions for the binding type. + + Version Added: + 5.0 + """ + + HTTP_POST = 'http-post' + HTTP_REDIRECT = 'http-redirect' + + CHOICES = ( + (HTTP_POST, _('HTTP POST')), + (HTTP_REDIRECT, _('HTTP Redirect')), + ) + + if constants: + TO_SAML2_SETTING_MAP = { + HTTP_POST: constants.BINDING_HTTP_POST, + HTTP_REDIRECT: constants.BINDING_HTTP_REDIRECT, + } + + FROM_SAML2_SETTING_MAP = { + constants.BINDING_HTTP_POST: HTTP_POST, + constants.BINDING_HTTP_REDIRECT: HTTP_REDIRECT, + } + else: + TO_SAML2_SETTING_MAP = {} + FROM_SAML2_SETTING_MAP = {} + + +def get_saml2_settings(): + """Return the SAML2.0 settings. + + Version Added: + 5.0 + + Returns: + dict: + A dictionary of the settings to use for SAML operations. + """ + siteconfig = SiteConfiguration.objects.get_current() + + assert constants is not None + return { + 'strict': True, + 'debug': True, + 'idp': { + 'entityId': siteconfig.get('saml_issuer'), + 'singleSignOnService': { + 'url': siteconfig.get('saml_sso_url'), + 'binding': SAMLBinding.TO_SAML2_SETTING_MAP[ + siteconfig.get('saml_sso_binding_type')], + }, + 'singleLogoutService': { + 'url': siteconfig.get('saml_slo_url'), + 'binding': SAMLBinding.TO_SAML2_SETTING_MAP[ + siteconfig.get('saml_slo_binding_type')], + }, + 'x509cert': siteconfig.get('saml_verification_cert'), + }, + 'sp': { + 'entityId': build_server_url( + reverse('sso:saml:metadata', kwargs={'backend_id': 'saml'})), + 'assertionConsumerService': { + 'url': build_server_url( + reverse('sso:saml:acs', kwargs={'backend_id': 'saml'})), + 'binding': constants.BINDING_HTTP_POST, + }, + 'singleLogoutService': { + 'url': build_server_url( + reverse('sso:saml:sls', kwargs={'backend_id': 'saml'})), + 'binding': constants.BINDING_HTTP_REDIRECT, + }, + 'NameIDFormat': constants.NAMEID_PERSISTENT, + 'x509cert': '', + 'privateKey': '', + }, + } diff --git a/reviewboard/accounts/sso/backends/saml/sso_backend.py b/reviewboard/accounts/sso/backends/saml/sso_backend.py new file mode 100644 index 0000000000000000000000000000000000000000..cf147b162bcd637aa3c0fa7fd01059a623581a1e --- /dev/null +++ b/reviewboard/accounts/sso/backends/saml/sso_backend.py @@ -0,0 +1,127 @@ +"""SAML SSO backend. + +Version Added: + 5.0 +""" + +from importlib import import_module + +from django.urls import path, reverse +from django.utils.translation import gettext_lazy as _, gettext +from djblets.siteconfig.models import SiteConfiguration +from djblets.util.decorators import cached_property + +from reviewboard.accounts.sso.backends.base import BaseSSOBackend +from reviewboard.accounts.sso.backends.saml.forms import SAMLSettingsForm +from reviewboard.accounts.sso.backends.saml.settings import ( + SAMLBinding, + SAMLDigestAlgorithm, + SAMLSignatureAlgorithm) +from reviewboard.accounts.sso.backends.saml.views import ( + SAMLACSView, + SAMLLinkUserView, + SAMLLoginView, + SAMLMetadataView, + SAMLSLSView) + + +class SAMLSSOBackend(BaseSSOBackend): + """SAML SSO backend. + + Version Added: + 5.0 + """ + + backend_id = 'saml' + name = _('SAML 2.0') + settings_form = SAMLSettingsForm + siteconfig_defaults = { + 'saml_digest_algorithm': SAMLDigestAlgorithm.SHA1, + 'saml_enabled': False, + 'saml_issuer': '', + 'saml_login_button_text': _('Login with SAML SSO'), + 'saml_require_login_to_link': True, + 'saml_signature_algorithm': SAMLSignatureAlgorithm.DSA_SHA1, + 'saml_slo_binding_type': SAMLBinding.HTTP_REDIRECT, + 'saml_slo_url': '', + 'saml_sso_binding_type': SAMLBinding.HTTP_POST, + 'saml_sso_url': '', + 'saml_verfication_cert': '', + } + login_view_cls = SAMLLoginView + + def __init__(self): + """Initialize the SSO backend.""" + super().__init__() + + self._available = None + + @cached_property + def login_label(self): + """The text to show on the login button. + + Type: + str + """ + siteconfig = SiteConfiguration.objects.get_current() + return siteconfig.get('saml_login_button_text') + + @cached_property + def login_url(self): + """The URL to the login page. + + Type: + str + """ + return reverse( + 'sso:%s:login' % self.backend_id, + kwargs={'backend_id': self.backend_id}) + + @cached_property + def urls(self): + """A list of URLs to register for this backend. + + Type: + list + """ + return [ + path('metadata/', + SAMLMetadataView.as_view(sso_backend=self), + name='metadata'), + path('acs/', + SAMLACSView.as_view(sso_backend=self), + name='acs'), + path('sls/', + SAMLSLSView.as_view(sso_backend=self), + name='sls'), + path('link-user/', + SAMLLinkUserView.as_view(sso_backend=self), + name='link-user'), + ] + + def is_available(self): + """Return whether this backend is available. + + Returns: + tuple: + A two-tuple. The items in the tuple are: + + 1. A bool indicating whether the backend is available. + 2. If the first element is ``False``, this will be a user-visible + string indicating the reason why the backend is not available. + If the backend is available, this element will be ``None``. + """ + if self._available is None: + try: + import_module('onelogin.saml2.constants') + self._available = True + except ImportError: + self._available = False + + if self._available: + return True, None + else: + return False, gettext( + 'To enable SAML 2.0 support, install the ' + 'ReviewBoard[saml] Python package and restart ' + 'the web server.') diff --git a/reviewboard/accounts/sso/backends/saml/views.py b/reviewboard/accounts/sso/backends/saml/views.py new file mode 100644 index 0000000000000000000000000000000000000000..84bc454c2b7f6e05c11daca8b7136013f9925337 --- /dev/null +++ b/reviewboard/accounts/sso/backends/saml/views.py @@ -0,0 +1,619 @@ +"""Views for SAML SSO. + +Version Added: + 5.0 +""" + +from enum import Enum +from urllib.parse import urlparse +import logging + +from django.conf import settings +from django.contrib.auth.models import User +from django.contrib.auth.views import LoginView +from django.http import (Http404, + HttpResponse, + HttpResponseBadRequest, + HttpResponseRedirect, + HttpResponseServerError) +from django.utils.decorators import method_decorator +from django.views.decorators.csrf import csrf_protect +from django.views.generic.base import View +from djblets.db.query import get_object_or_none +from djblets.siteconfig.models import SiteConfiguration +from djblets.util.decorators import cached_property +try: + from onelogin.saml2.auth import OneLogin_Saml2_Auth + from onelogin.saml2.errors import OneLogin_Saml2_Error + from onelogin.saml2.settings import OneLogin_Saml2_Settings + from onelogin.saml2.utils import OneLogin_Saml2_Utils +except ImportError: + OneLogin_Saml2_Auth = None + OneLogin_Saml2_Error = None + OneLogin_Saml2_Settings = None + OneLogin_Saml2_Utils = None + +from reviewboard.accounts.models import LinkedAccount +from reviewboard.accounts.sso.backends.saml.forms import SAMLLinkUserForm +from reviewboard.accounts.sso.backends.saml.settings import get_saml2_settings +from reviewboard.accounts.sso.users import (find_suggested_username, + find_user_for_sso_user_id) +from reviewboard.accounts.sso.views import BaseSSOView +from reviewboard.admin.server import get_server_url +from reviewboard.site.urlresolvers import local_site_reverse + + +logger = logging.getLogger(__file__) + + +class SAMLViewMixin(View): + """Mixin to provide common functionality for SAML views. + + Version Added: + 5.0 + """ + + def __init__(self, *args, **kwargs): + """Initialize the view. + + Args: + *args (tuple): + Positional arguments to pass through to the base class. + + **kwargs (dict): + Keyword arguments to pass through to the base class. + """ + super().__init__(*args, **kwargs) + self._saml_auth = None + self._saml_request = None + + def get_saml_request(self, request): + """Return the SAML request. + + Args: + request (django.http.HttpRequest): + The HTTP request from the client. + + Returns: + dict: + Information about the SAML request. + """ + if self._saml_request is None: + server_url = urlparse(get_server_url()) + + if server_url.scheme == 'https': + https = 'on' + else: + https = 'off' + + self._saml_request = { + 'https': https, + 'http_host': server_url.hostname, + 'get_data': request.GET.copy(), + 'post_data': request.POST.copy(), + 'query_string': request.META['QUERY_STRING'], + 'request_uri': request.path, + 'script_name': request.META['PATH_INFO'], + 'server_port': server_url.port, + } + + return self._saml_request + + def get_saml_auth(self, request): + """Return the SAML auth information. + + Args: + request (django.http.HttpRequest): + The HTTP request from the client. + + Returns: + onelogin.saml2.auth.OneLogin_Saml2_Auth: + The SAML Auth object. + """ + if self._saml_auth is None: + assert OneLogin_Saml2_Auth is not None + self._saml_auth = OneLogin_Saml2_Auth( + self.get_saml_request(request), + get_saml2_settings()) + + return self._saml_auth + + def dispatch(self, *args, **kwargs): + """Handle a dispatch for the view. + + Args: + *args (tuple): + Positional arguments to pass through to the parent class. + + **kwargs (dict): + Keyword arguments to pass through to the parent class. + + Returns: + django.http.HttpResponse: + The response to send back to the client. + + Raises: + django.http.Http404: + The SAML backend is not enabled, so treat all SAML views as + 404. + """ + if not self.sso_backend.is_enabled(): + raise Http404 + + return super().dispatch(*args, **kwargs) + + +class SAMLACSView(SAMLViewMixin, BaseSSOView): + """ACS view for SAML SSO. + + Version Added: + 5.0 + """ + + @property + def success_url(self): + """The URL to redirect to after a successful login. + + Type: + str + """ + url = self.request.POST.get('RelayState') + + assert OneLogin_Saml2_Utils is not None + self_url = OneLogin_Saml2_Utils.get_self_url( + self.get_saml_request(self.request)) + + if url is not None and self_url != url: + saml_auth = self.get_saml_auth(self.request) + return saml_auth.redirect_to(url) + else: + return settings.LOGIN_REDIRECT_URL + + @cached_property + def link_user_url(self): + """The URL to the link-user flow. + + Type: + str + """ + assert self.sso_backend is not None + return local_site_reverse( + 'sso:%s:link-user' % self.sso_backend.backend_id, + request=self.request, + kwargs={'backend_id': self.sso_backend.backend_id}) + + def post(self, request, *args, **kwargs): + """Handle a POST request. + + Args: + request (django.http.HttpRequest): + The request from the client. + + *args (tuple): + Additional positional arguments. + + **kwargs (dict): + Additional keyword arguments. + + Returns: + django.http.HttpResponse: + The response to send back to the client. + """ + auth = self.get_saml_auth(request) + session = request.session + + try: + auth.process_response(request_id=session.get('AuthNRequestID')) + except OneLogin_Saml2_Error as e: + logger.exception('SAML: Unable to process SSO request: %s', e, + exc_info=True) + return HttpResponseBadRequest('Bad SSO response: %s' % str(e), + content_type='text/plain') + + # TODO: store/check last request ID, last message ID, last assertion ID + # to prevent replay attacks. + + error = auth.get_last_error_reason() + + if error: + logger.error('SAML: Unable to process SSO request: %s', error) + return HttpResponseBadRequest('Bad SSO response: %s' % error, + content_type='text/plain') + + # Store some state on the session to identify where we are in the SAML + # workflow. + session.pop('AuthNRequestID', None) + + linked_account = get_object_or_none(LinkedAccount, + service_id='sso:saml', + service_user_id=auth.get_nameid()) + + if linked_account: + user = linked_account.user + self.sso_backend.login_user(request, user) + return HttpResponseRedirect(self.success_url) + else: + username = auth.get_nameid() + + try: + email = self._get_user_attr_value(auth, 'User.email') + first_name = self._get_user_attr_value(auth, 'User.FirstName') + last_name = self._get_user_attr_value(auth, 'User.LastName') + except KeyError as e: + logger.error('SAML: Assertion is missing %s attribute', e) + return HttpResponseBadRequest('Bad SSO response: assertion is ' + 'missing %s attribute' + % e, + content_type='text/plain') + + request.session['sso'] = { + 'user_data': { + 'id': username, + 'first_name': first_name, + 'last_name': last_name, + 'email': email, + }, + 'raw_user_attrs': auth.get_attributes(), + 'session_index': auth.get_session_index(), + } + + return HttpResponseRedirect(self.link_user_url) + + def _get_user_attr_value(self, auth, key): + """Return the value of a user attribute. + + Args: + auth (onelogin.saml2.auth.OneLogin_Saml2_Auth): + The SAML authentication object. + + key (str): + The key to look up. + + Returns: + str: + The attribute, if it exists. + + Raises: + KeyError: + The given key was not present in the SAML assertion. + """ + value = auth.get_attribute(key) + + if value and isinstance(value, list): + return value[0] + + raise KeyError(key) + + +@method_decorator(csrf_protect, name='dispatch') +class SAMLLinkUserView(SAMLViewMixin, BaseSSOView, LoginView): + """Link user view for SAML SSO. + + This can have several behaviors depending on what combination of state we + get from the Identity Provider and what we have stored in the database. + + The first major case is where we are given data that matches an existing + user in the database. Ideally this is via the "username" field, but may + also be a matching e-mail address, or parsing a username out of the e-mail + address. + + In this case, there are two paths. The simple path is where the + administrator trusts both the authority and integrity of their Identity + Provider and has turned off the "Require login to link" setting. For this, + we'll just create the LinkedAccount, authenticate the user, and redirect to + the success URL. + + If the require login setting is turned on, the user will have a choice. + They can enter the password for the detected user to complete the link. If + they have an account but the detected one is not correct, they can log in + with their username and password to link the other account. Finally, they + can provision a new user if they do not yet have one. + + The second major case is where we cannot find an existing user. In this + case, we'll offer the user a choice: if they have an existing login that + wasn't found, they can log in with their (non-SSO) username and password. + If they don't have an account, they will be able to provision one. + + Version Added: + 5.0 + """ + + # TODO: This has a lot of logic which will likely be applicable to other + # SSO backend implementations. When we add a new backend, this should be + # refactored to pull out most of the logic into a common base class, and + # just implement SAML-specific data here. + + form_class = SAMLLinkUserForm + + class Mode(Enum): + CONNECT_EXISTING_ACCOUNT = 'connect' + CONNECT_WITH_LOGIN = 'connect-login' + PROVISION = 'provision' + + def dispatch(self, *args, **kwargs): + """Dispatch the view. + + Args: + *args (tuple): + Positional arguments to pass to the parent class. + + **kwargs (dict): + Keyword arguments to pass to the parent class. + """ + self._sso_user_data = \ + self.request.session.get('sso', {}).get('user_data') + self._sso_data_username = self._sso_user_data.get('id') + self._sso_data_email = self._sso_user_data.get('email') + computed_username = find_suggested_username(self._sso_data_email) + self._provision_username = self._sso_data_username or computed_username + self._sso_user = find_user_for_sso_user_id( + self._sso_data_username, + self._sso_data_email, + computed_username) + + requested_mode = self.request.GET.get('mode') + + if requested_mode and requested_mode in self.Mode: + self._mode = requested_mode + elif self._sso_user: + self._mode = self.Mode.CONNECT_EXISTING_ACCOUNT + else: + self._mode = self.Mode.PROVISION + + return super(SAMLLinkUserView, self).dispatch(*args, **kwargs) + + def get_template_names(self): + """Return the template to use when rendering. + + Returns: + list: + A single-item list with the template name to use when rendering. + + Raises: + ValueError: + The current mode is not valid. + """ + if self._mode == self.Mode.CONNECT_EXISTING_ACCOUNT: + return ['accounts/sso/link-user-connect-existing.html'] + elif self._mode == self.Mode.CONNECT_WITH_LOGIN: + return ['accounts/sso/link-user-login.html'] + elif self._mode == self.Mode.PROVISION: + return ['accounts/sso/link-user-provision.html'] + else: + raise ValueError('Unknown link-user mode "%s"' % self._mode) + + def get_initial(self): + """Return the initial data for the form. + + Returns: + dict: + Initial data for the form. + """ + initial = super(SAMLLinkUserView, self).get_initial() + + if self._sso_user is not None: + initial['username'] = self._sso_user.username + else: + initial['username'] = self._provision_username + + initial['provision'] = (self._mode == self.Mode.PROVISION) + + return initial + + def get_context_data(self, **kwargs): + """Return additional context data for rendering the template. + + Args: + **kwargs (dict): + Keyword arguments for the view. + + Returns: + dict: + Additional data to inject into the render context. + """ + context = super(SAMLLinkUserView, self).get_context_data(**kwargs) + context['user'] = self._sso_user + context['mode'] = self._mode + context['username'] = self._provision_username + + return context + + def get(self, request, *args, **kwargs): + """Handle a GET request for the form. + + Args: + request (django.http.HttpRequest): + The HTTP request from the client. + + *args (tuple): + Positional arguments to pass through to the base class. + + **kwargs (dict): + Keyword arguments to pass through to the base class. + + Returns: + django.http.HttpResponse: + The response to send back to the client. + """ + if not self._sso_user_data: + return HttpResponseRedirect( + local_site_reverse('login', request=request)) + + siteconfig = SiteConfiguration.objects.get_current() + + if self._sso_user and not siteconfig.get('saml_require_login_to_link'): + return self.link_user(self._sso_user) + + return super(SAMLLinkUserView, self).get(request, *args, **kwargs) + + def form_valid(self, form): + """Handler for when the form has successfully authenticated. + + Args: + form (reviewboard.accounts.sso.backends.saml.forms. + SAMLLinkUserForm): + The link-user form. + + Returns: + django.http.HttpResponseRedirect: + A redirect to the next page. + """ + if form.cleaned_data['provision']: + # We can't provision if there's an existing matching user. + # TODO: show an error? + assert not self._sso_user + assert self._provision_username + + first_name = self._sso_user_data.get('first_name') + last_name = self._sso_user_data.get('last_name') + + logger.info('SAML: Provisiong user "%s" (%s <%s %s>)', + self._provision_username, self._sso_data_email, + first_name, last_name) + + user = User.objects.create( + username=self._provision_username, + email=self._sso_data_email, + first_name=first_name, + last_name=last_name) + else: + user = form.get_user() + + return self.link_user(user) + + def link_user(self, user): + """Link the given user. + + Args: + user (django.contrib.auth.models.User): + The user to link. + + Returns: + django.http.HttpResponseRedirect: + A redirect to the success URL. + """ + sso_id = self._sso_user_data.get('id') + + logger.info('SAML: Linking SSO user "%s" to Review Board user "%s"', + sso_id, user.username) + + user.linked_accounts.create( + service_id='sso:saml', + service_user_id=sso_id) + self.sso_backend.login_user(self.request, user) + return HttpResponseRedirect(self.get_success_url()) + + +class SAMLLoginView(SAMLViewMixin, BaseSSOView): + """Login view for SAML SSO. + + Version Added: + 5.0 + """ + + def get(self, request, *args, **kwargs): + """Handle a GET request for the login URL. + + Args: + request (django.http.HttpRequest): + The request from the client. + + *args (tuple, unused): + Additional positional arguments. + + **kwargs (dict, unused): + Additional keyword arguments. + + Returns: + django.http.HttpResponseRedirect: + A redirect to start the login flow. + """ + auth = self.get_saml_auth(request) + + return HttpResponseRedirect( + auth.login(settings.LOGIN_REDIRECT_URL)) + + +class SAMLMetadataView(SAMLViewMixin, BaseSSOView): + """Metadata view for SAML SSO. + + Version Added: + 5.0 + """ + + def get(self, request, *args, **kwargs): + """Handle a GET request. + + Args: + request (django.http.HttpRequest): + The HTTP request from the client. + + *args (tuple): + Positional arguments from the URL definition. + + **kwargs (dict): + Keyword arguments from the URL definition. + """ + assert OneLogin_Saml2_Settings is not None + saml_settings = OneLogin_Saml2_Settings( + get_saml2_settings(), + sp_validation_only=True) + + metadata = saml_settings.get_sp_metadata() + errors = saml_settings.validate_metadata(metadata) + + if errors: + logger.error('SAML: Got errors from metadata validation: %s', + ', '.join(errors)) + return HttpResponseServerError(', '.join(errors), + content_type='text/plain') + + return HttpResponse(metadata, content_type='text/xml') + + +class SAMLSLSView(SAMLViewMixin, BaseSSOView): + """SLS view for SAML SSO. + + Version Added: + 5.0 + """ + + def get(self, request, *args, **kwargs): + """Handle a POST request. + + Args: + request (django.http.HttpRequest): + The request from the client. + + *args (tuple): + Additional positional arguments. + + **kwargs (dict): + Additional keyword arguments. + + Returns: + django.http.HttpResponse: + The response to send back to the client. + """ + auth = self.get_saml_auth(request) + request_id = None + + if 'LogoutRequestId' in request.session: + request_id = request.session['LogoutRequestId'] + + redirect_url = auth.process_slo( + request_id=request_id, + delete_session_cb=lambda: request.session.flush()) + + errors = auth.get_errors() + + if errors: + error_text = ', '.join(errors) + logger.error('SAML: Unable to process SLO request: %s', error_text) + return HttpResponseBadRequest('Bad SLO response: %s' % error_text, + content_type='text/plain') + + if redirect_url: + return HttpResponseRedirect(redirect_url) + else: + return HttpResponseRedirect(settings.LOGIN_URL) diff --git a/reviewboard/accounts/sso/errors.py b/reviewboard/accounts/sso/errors.py new file mode 100644 index 0000000000000000000000000000000000000000..6e018767be53c042d51cdf9ab84a1d72c2f35cd1 --- /dev/null +++ b/reviewboard/accounts/sso/errors.py @@ -0,0 +1,21 @@ +"""Error definitions for SSO. + +Version Added: + 5.0 +""" + + +class InvalidUsernameError(ValueError): + """Error for when a username is invalid. + + Version Added: + 5.0 + """ + + +class BadSSOResponseError(ValueError): + """Error for when we get a bad response from the SSO provider. + + Version Added: + 5.0 + """ diff --git a/reviewboard/accounts/sso/users.py b/reviewboard/accounts/sso/users.py new file mode 100644 index 0000000000000000000000000000000000000000..a038060cf1984cfa7356c793b1f27b0c39a2da50 --- /dev/null +++ b/reviewboard/accounts/sso/users.py @@ -0,0 +1,99 @@ +"""Utilities for managing users with SSO. + +Version Added: + 5.0 +""" + +import re + +from django.contrib.auth.models import User +from django.db.models import Q + +from reviewboard.accounts.sso.errors import InvalidUsernameError + + +# This matches what's used in Review Board URLs. +INVALID_USERNAME_CHARS_RE = re.compile(r'[^\w.@+-]+') + + +def find_suggested_username(email): + """Return the suggested username for a given e-mail address. + + Version Added: + 5.0 + + Args: + email (str): + The user's e-mail address. + + Returns: + str: + The suggested username. + + Raises: + reviewboard.accounts.sso.errors.InvalidUsernameError: + The suggested username does not work with Review Board's rules. + """ + # Normalize the username, factoring in everything before the '@' (if any) + # and converting any non-alphanumeric characters to dashes. Then we'll + # sanity-check that it meets our username criteria. + norm_user_id = INVALID_USERNAME_CHARS_RE.sub('-', email.split('@')[0]) + + if (norm_user_id.startswith('-') or + norm_user_id.endswith('-') or + '--' in norm_user_id): + # This is an invalid username. + raise InvalidUsernameError('Invalid SSO username "%s"' % norm_user_id) + + return norm_user_id + + +def find_user_for_sso_user_id(username, email, alternate_username): + """Find a matching user for SSO login. + + Version Added: + 5.0 + + Args: + username (str): + The username, if available. May be ``None``. + + email (str): + The user's e-mail address. + + alternate_username (str): + An alternate username to try, if ``username`` is ``None``. This + probably comes from :py:func:`find_suggested_username`. + + Returns: + django.contrib.auth.models.User: + The user object, if one exists. + """ + q = Q() + + if username: + q |= Q(username=username) + + q |= Q(email=email) | Q(username=alternate_username) + + # Fetch all candidates first to save on DB queries. + candidate_users = list(User.objects.filter(q)) + + # First try by username. This will be present if the username has been + # specified in the SSO provider. + if username: + for user in candidate_users: + if user.username == username: + return user + + # Next see if we have a user with a matching e-mail address. + for user in candidate_users: + if user.email == email: + return user + + # Finally, try a computed username from the e-mail address. + for user in candidate_users: + if user.username == alternate_username: + return user + + return None diff --git a/reviewboard/accounts/sso/views.py b/reviewboard/accounts/sso/views.py new file mode 100644 index 0000000000000000000000000000000000000000..7da02d30f1bbbfc81952544da9aab75858c3082b --- /dev/null +++ b/reviewboard/accounts/sso/views.py @@ -0,0 +1,31 @@ +"""Base classes for SSO views. + +Version Added: + 5.0 +""" + +import logging + +from django.utils.decorators import method_decorator +from django.views.decorators.cache import never_cache +from django.views.decorators.debug import sensitive_post_parameters +from django.views.generic.base import View + + +logger = logging.getLogger(__file__) + + +@method_decorator([sensitive_post_parameters(), never_cache], + name='dispatch') +class BaseSSOView(View): + """Base class for SSO views. + + Version Added: + 5.0 + """ + + #: The SSO backend. + #: + #: Type: + #: reviewboard.accounts.sso.backends.SSOBackend + sso_backend = None diff --git a/reviewboard/accounts/tests/test_saml_forms.py b/reviewboard/accounts/tests/test_saml_forms.py new file mode 100644 index 0000000000000000000000000000000000000000..7c1a1fa8314a806733880830a23ae9c7713c6ca6 --- /dev/null +++ b/reviewboard/accounts/tests/test_saml_forms.py @@ -0,0 +1,129 @@ +"""Unit tests for SAML forms.""" + +from djblets.siteconfig.models import SiteConfiguration + +from reviewboard.accounts.sso.backends import sso_backends +from reviewboard.accounts.sso.backends.saml.forms import (SAMLLinkUserForm, + SAMLSettingsForm) +from reviewboard.accounts.sso.backends.saml.settings import ( + SAMLBinding, + SAMLDigestAlgorithm, + SAMLSignatureAlgorithm) +from reviewboard.testing import TestCase + + +VALID_CERT = """-----BEGIN CERTIFICATE----- +MIICZjCCAc+gAwIBAgIBADANBgkqhkiG9w0BAQ0FADBQMQswCQYDVQQGEwJ1czEL +MAkGA1UECAwCQ0ExFjAUBgNVBAoMDUJlYW5iYWcsIEluYy4xHDAaBgNVBAMME2h0 +dHBzOi8vZXhhbXBsZS5jb20wHhcNMjIwNTA2MTU0NjI1WhcNMjMwNTA2MTU0NjI1 +WjBQMQswCQYDVQQGEwJ1czELMAkGA1UECAwCQ0ExFjAUBgNVBAoMDUJlYW5iYWcs +IEluYy4xHDAaBgNVBAMME2h0dHBzOi8vZXhhbXBsZS5jb20wgZ8wDQYJKoZIhvcN +AQEBBQADgY0AMIGJAoGBANCsbj4mvUiQERBy80R7yqA6hU3FMM4siC2UcUS3ltFF +grkVOAPr+zUnrdadmAiTH35AB94oMzf0Qh8OJCr7wG5JQm686TRkVm2xUxhJUcoq +7LjBTKeEXBcrEzdNlagFXxHUSz5bPSdwDt/zbOfe+9RZKeb4FggFCEYw/mi69+Dx +AgMBAAGjUDBOMB0GA1UdDgQWBBS4cP9Y+IM7ZHZChUDdx68QExTZUDAfBgNVHSME +GDAWgBS4cP9Y+IM7ZHZChUDdx68QExTZUDAMBgNVHRMEBTADAQH/MA0GCSqGSIb3 +DQEBDQUAA4GBALht5/NfJU+GxYfQKiGkZ4Ih/T/48rzXAT7/7f61s7w72UR2S5e2 +WsR7/JPkZ5+u5mCgmABjNcd9NzaBM2RfSrrurwbjXMQ8nb/+REvhXXJ4STsS48y5 +bef2JtIf7mGDw8/KsUrAA2jEIpCedToGyQxyE6GdN5b69ITWvyAemnIM +-----END CERTIFICATE-----""" + + +class SAMLLinkUserFormTests(TestCase): + """Unit tests for SAMLLinkUserForm.""" + + fixtures = ['test_users'] + + def test_valid_login(self): + """Testing SAMLLinkUserForm validation in login mode""" + form = SAMLLinkUserForm(data={ + 'username': 'doc', + 'password': 'doc', + 'provision': False, + }) + + self.assertTrue(form.is_valid()) + + def test_invalid_login(self): + """Testing SAMLLinkUserForm validation with incorrect password in login + mode. + """ + form = SAMLLinkUserForm(data={ + 'username': 'doc', + 'password': 'nope', + 'provision': False, + }) + + self.assertFalse(form.is_valid()) + self.assertEqual(form.errors['__all__'], + ['Please enter a correct username and password. Note ' + 'that both fields may be case-sensitive.']) + + def test_provision(self): + """Testing SAMLLinkUserForm validation in provision mode""" + form = SAMLLinkUserForm(data={ + 'username': 'doc', + 'password': 'nope', + 'provision': True, + }) + + self.assertTrue(form.is_valid()) + + +class SAMLSettingsFormTests(TestCase): + """Unit tests for SAMLSettingsForm.""" + + def setUp(self): + """Set up the test case.""" + self.siteconfig = SiteConfiguration.objects.get_current() + + saml_backend = sso_backends.get('backend_id', 'saml') + + # Ensure everything is set to defaults. + for key, value in saml_backend.siteconfig_defaults.items(): + self.siteconfig.set(key, value) + + self.siteconfig.save() + + def test_save(self): + """Testing SAMLSettingsForm.save""" + siteconfig = self.siteconfig + + form = SAMLSettingsForm( + siteconfig, + data={ + 'saml_login_button_text': 'Login', + 'saml_issuer': 'https://example.com/saml/issuer', + 'saml_signature_algorithm': SAMLSignatureAlgorithm.RSA_SHA1, + 'saml_digest_algorithm': SAMLDigestAlgorithm.SHA512, + 'saml_verification_cert': VALID_CERT, + 'saml_sso_url': 'https://example.com/saml/sso', + 'saml_sso_binding_type': SAMLBinding.HTTP_POST, + 'saml_slo_url': 'https://example.com/saml/slo', + 'saml_slo_binding_type': SAMLBinding.HTTP_REDIRECT, + 'saml_require_login_to_link': False, + }) + + self.assertTrue(form.is_valid()) + form.save() + + siteconfig.refresh_from_db() + self.assertEqual(siteconfig.get('saml_login_button_text'), + 'Login') + self.assertEqual(siteconfig.get('saml_issuer'), + 'https://example.com/saml/issuer') + self.assertEqual(siteconfig.get('saml_signature_algorithm'), + SAMLSignatureAlgorithm.RSA_SHA1) + self.assertEqual(siteconfig.get('saml_digest_algorithm'), + SAMLDigestAlgorithm.SHA512) + self.assertEqual(siteconfig.get('saml_verification_cert'), + VALID_CERT) + self.assertEqual(siteconfig.get('saml_sso_url'), + 'https://example.com/saml/sso') + self.assertEqual(siteconfig.get('saml_sso_binding_type'), + SAMLBinding.HTTP_POST) + self.assertEqual(siteconfig.get('saml_slo_url'), + 'https://example.com/saml/slo') + self.assertEqual(siteconfig.get('saml_slo_binding_type'), + SAMLBinding.HTTP_REDIRECT) + self.assertFalse(siteconfig.get('saml_require_login_to_link')) diff --git a/reviewboard/accounts/tests/test_saml_views.py b/reviewboard/accounts/tests/test_saml_views.py new file mode 100644 index 0000000000000000000000000000000000000000..c093b207f517b4d8de927017374d02f3789dedd9 --- /dev/null +++ b/reviewboard/accounts/tests/test_saml_views.py @@ -0,0 +1,272 @@ +"""Unit tests for SAML views.""" + +from xml.etree import ElementTree + +from django.contrib.auth.models import User +from django.urls import reverse + +from reviewboard.accounts.sso.backends.saml.views import SAMLLinkUserView +from reviewboard.testing import TestCase + + +VALID_CERT = """-----BEGIN CERTIFICATE----- +MIICZjCCAc+gAwIBAgIBADANBgkqhkiG9w0BAQ0FADBQMQswCQYDVQQGEwJ1czEL +MAkGA1UECAwCQ0ExFjAUBgNVBAoMDUJlYW5iYWcsIEluYy4xHDAaBgNVBAMME2h0 +dHBzOi8vZXhhbXBsZS5jb20wHhcNMjIwNTA2MTU0NjI1WhcNMjMwNTA2MTU0NjI1 +WjBQMQswCQYDVQQGEwJ1czELMAkGA1UECAwCQ0ExFjAUBgNVBAoMDUJlYW5iYWcs +IEluYy4xHDAaBgNVBAMME2h0dHBzOi8vZXhhbXBsZS5jb20wgZ8wDQYJKoZIhvcN +AQEBBQADgY0AMIGJAoGBANCsbj4mvUiQERBy80R7yqA6hU3FMM4siC2UcUS3ltFF +grkVOAPr+zUnrdadmAiTH35AB94oMzf0Qh8OJCr7wG5JQm686TRkVm2xUxhJUcoq +7LjBTKeEXBcrEzdNlagFXxHUSz5bPSdwDt/zbOfe+9RZKeb4FggFCEYw/mi69+Dx +AgMBAAGjUDBOMB0GA1UdDgQWBBS4cP9Y+IM7ZHZChUDdx68QExTZUDAfBgNVHSME +GDAWgBS4cP9Y+IM7ZHZChUDdx68QExTZUDAMBgNVHRMEBTADAQH/MA0GCSqGSIb3 +DQEBDQUAA4GBALht5/NfJU+GxYfQKiGkZ4Ih/T/48rzXAT7/7f61s7w72UR2S5e2 +WsR7/JPkZ5+u5mCgmABjNcd9NzaBM2RfSrrurwbjXMQ8nb/+REvhXXJ4STsS48y5 +bef2JtIf7mGDw8/KsUrAA2jEIpCedToGyQxyE6GdN5b69ITWvyAemnIM +-----END CERTIFICATE-----""" + + +class SAMLViewTests(TestCase): + """Unit tests for SAML views.""" + + fixtures = ['test_users'] + + def test_metadata_view(self): + """Testing SAMLMetadataView""" + settings = { + 'saml_enabled': True, + 'saml_verification_cert': VALID_CERT, + } + + with self.siteconfig_settings(settings): + url = reverse('sso:saml:metadata', kwargs={'backend_id': 'saml'}) + rsp = self.client.get(url) + + self.assertEqual(rsp.status_code, 200) + + root = ElementTree.fromstring(rsp.content) + + namespaces = {'md': 'urn:oasis:names:tc:SAML:2.0:metadata'} + + descriptor = root.find('md:SPSSODescriptor', namespaces) + assert descriptor is not None + + sls = descriptor.find('md:SingleLogoutService', namespaces) + assert sls is not None + + self.assertEqual( + sls.get('Binding'), + 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect') + self.assertEqual( + sls.get('Location'), + 'http://example.com/account/sso/saml/sls/') + + acs = descriptor.find('md:AssertionConsumerService', namespaces) + assert acs is not None + + self.assertEqual( + acs.get('Binding'), + 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST') + self.assertEqual( + acs.get('Location'), + 'http://example.com/account/sso/saml/acs/') + + def test_get_link_user_existing_account(self): + """Testing SAMLLinkUserView form render with existing account""" + settings = { + 'saml_enabled': True, + 'saml_require_login_to_link': True, + } + + with self.siteconfig_settings(settings): + session = self.client.session + session['sso'] = { + 'user_data': { + 'id': 'doc', + 'first_name': 'Doc', + 'last_name': 'Dwarf', + 'email': 'doc@example.com', + }, + } + session.save() + + url = reverse('sso:saml:link-user', kwargs={'backend_id': 'saml'}) + rsp = self.client.get(url) + + self.assertEqual(rsp.status_code, 200) + + context = rsp.context + self.assertEqual(context['user'].username, 'doc') + self.assertEqual(context['mode'], + SAMLLinkUserView.Mode.CONNECT_EXISTING_ACCOUNT) + + def test_get_link_user_existing_account_email_match(self): + """Testing SAMLLinkUserView form render with existing account matching + email address + """ + settings = { + 'saml_enabled': True, + 'saml_require_login_to_link': True, + } + + with self.siteconfig_settings(settings): + session = self.client.session + session['sso'] = { + 'user_data': { + 'id': 'doc2', + 'first_name': 'Doc', + 'last_name': 'Dwarf', + 'email': 'doc@example.com', + }, + } + session.save() + + url = reverse('sso:saml:link-user', kwargs={'backend_id': 'saml'}) + rsp = self.client.get(url) + + self.assertEqual(rsp.status_code, 200) + + context = rsp.context + self.assertEqual(context['user'].username, 'doc') + self.assertEqual(context['mode'], + SAMLLinkUserView.Mode.CONNECT_EXISTING_ACCOUNT) + + def test_get_link_user_existing_account_email_username_match(self): + """Testing SAMLLinkUserView form render with existing account matching + username from email address + """ + settings = { + 'saml_enabled': True, + 'saml_require_login_to_link': True, + } + + with self.siteconfig_settings(settings): + session = self.client.session + session['sso'] = { + 'user_data': { + 'id': 'doc2', + 'first_name': 'Doc', + 'last_name': 'Dwarf', + 'email': 'doc@example.org', + }, + } + session.save() + + url = reverse('sso:saml:link-user', kwargs={'backend_id': 'saml'}) + rsp = self.client.get(url) + + self.assertEqual(rsp.status_code, 200) + + context = rsp.context + self.assertEqual(context['user'].username, 'doc') + self.assertEqual(context['mode'], + SAMLLinkUserView.Mode.CONNECT_EXISTING_ACCOUNT) + + def test_get_link_user_no_match(self): + """Testing SAMLLinkUserView form render with no match""" + settings = { + 'saml_enabled': True, + 'saml_require_login_to_link': True, + } + + with self.siteconfig_settings(settings): + session = self.client.session + session['sso'] = { + 'user_data': { + 'id': 'doc2', + 'first_name': 'Doc', + 'last_name': 'Dwarf', + 'email': 'doc2@example.org', + }, + } + session.save() + + url = reverse('sso:saml:link-user', kwargs={'backend_id': 'saml'}) + rsp = self.client.get(url) + + self.assertEqual(rsp.status_code, 200) + + context = rsp.context + self.assertEqual(context['user'], None) + self.assertEqual(context['mode'], + SAMLLinkUserView.Mode.PROVISION) + + def test_post_link_user_login(self): + """Testing SAMLLinkUserView form POST with login""" + settings = { + 'saml_enabled': True, + 'saml_require_login_to_link': True, + } + + with self.siteconfig_settings(settings): + session = self.client.session + session['sso'] = { + 'user_data': { + 'id': 'doc2', + 'first_name': 'Doc', + 'last_name': 'Dwarf', + 'email': 'doc@example.com', + }, + } + session.save() + + user = User.objects.get(username='doc') + self.assertFalse(user.linked_accounts.exists()) + + url = reverse('sso:saml:link-user', kwargs={'backend_id': 'saml'}) + rsp = self.client.post(url, { + 'username': 'doc', + 'password': 'doc', + 'provision': False, + }) + + self.assertEqual(rsp.status_code, 302) + + linked_accounts = list(user.linked_accounts.all()) + + self.assertEqual(len(linked_accounts), 1) + linked_account = linked_accounts[0] + self.assertEqual(linked_account.service_id, 'sso:saml') + self.assertEqual(linked_account.service_user_id, 'doc2') + + def test_post_link_user_provision(self): + """Testing SAMLLinkUserView form POST with provision""" + settings = { + 'saml_enabled': True, + 'saml_require_login_to_link': True, + } + + with self.siteconfig_settings(settings): + session = self.client.session + session['sso'] = { + 'user_data': { + 'id': 'sleepy', + 'first_name': 'Sleepy', + 'last_name': 'Dwarf', + 'email': 'sleepy@example.com', + }, + } + session.save() + + self.assertFalse(User.objects.filter(username='sleepy').exists()) + + url = reverse('sso:saml:link-user', kwargs={'backend_id': 'saml'}) + rsp = self.client.post(url, { + 'username': '', + 'password': '', + 'provision': True, + }) + + self.assertEqual(rsp.status_code, 302) + + user = User.objects.get(username='sleepy') + self.assertEqual(user.first_name, 'Sleepy') + self.assertEqual(user.last_name, 'Dwarf') + self.assertEqual(user.email, 'sleepy@example.com') + + linked_accounts = list(user.linked_accounts.all()) + + self.assertEqual(len(linked_accounts), 1) + linked_account = linked_accounts[0] + self.assertEqual(linked_account.service_id, 'sso:saml') + self.assertEqual(linked_account.service_user_id, 'sleepy') diff --git a/reviewboard/accounts/tests/test_sso_backend_registry.py b/reviewboard/accounts/tests/test_sso_backend_registry.py new file mode 100644 index 0000000000000000000000000000000000000000..6c18dfb3c2393efe5b847fc482fba044d93419f0 --- /dev/null +++ b/reviewboard/accounts/tests/test_sso_backend_registry.py @@ -0,0 +1,91 @@ +"""Unit tests for the SSO backend registry.""" + +from django.http import HttpResponse +from django.urls import NoReverseMatch, path, reverse +from djblets.registries.errors import AlreadyRegisteredError, ItemLookupError + +from reviewboard.accounts.sso.backends import sso_backends +from reviewboard.accounts.sso.backends.base import BaseSSOBackend +from reviewboard.testing import TestCase + + +def backend_test_view(request, backend_id): + return HttpResponse(str(backend_id)) + + +class SSOBackendRegistryTests(TestCase): + """Unit tests for the SSO backend registry.""" + + class DummyBackend(BaseSSOBackend): + backend_id = 'dummy' + name = 'Dummy' + + class DummyBackendWithURLs(BaseSSOBackend): + backend_id = 'dummy-with-urls' + name = 'DummyWithURLs' + + @property + def urls(self): + return [ + path('sso-endpoint/', + backend_test_view, + name='dummy-backend-sso-endpoint') + ] + + def setUp(self): + """Set up the test case.""" + self.dummy_backend = self.DummyBackend() + self.dummy_backend_with_urls = self.DummyBackendWithURLs() + + def tearDown(self): + """Tear down the test case.""" + super(SSOBackendRegistryTests, self).tearDown() + + try: + sso_backends.unregister(self.dummy_backend) + except ItemLookupError: + pass + + try: + sso_backends.unregister(self.dummy_backend_with_urls) + except ItemLookupError: + pass + + def test_register_without_urls(self): + """Testing SSO backend registration""" + sso_backends.register(self.dummy_backend) + + with self.assertRaises(AlreadyRegisteredError): + sso_backends.register(self.dummy_backend) + + def test_unregister_without_urls(self): + """Testing SSO backend unregistration""" + sso_backends.register(self.dummy_backend) + sso_backends.unregister(self.dummy_backend) + + def test_register_with_urls(self): + """Testing SSO backend registration with URLs""" + sso_backends.register(self.dummy_backend_with_urls) + + self.assertEqual( + reverse( + 'sso:dummy-with-urls:dummy-backend-sso-endpoint', + kwargs={ + 'backend_id': 'dummy-with-urls', + }), + '/account/sso/dummy-with-urls/sso-endpoint/') + + with self.assertRaises(AlreadyRegisteredError): + sso_backends.register(self.dummy_backend_with_urls) + + def test_unregister_with_urls(self): + """Testing SSO backend registration with URLs""" + sso_backends.register(self.dummy_backend_with_urls) + sso_backends.unregister(self.dummy_backend_with_urls) + + with self.assertRaises(NoReverseMatch): + reverse( + 'sso:dummy-with-urls:dummy-backend-sso-endpoint', + kwargs={ + 'backend_id': 'dummy-with-urls', + }) diff --git a/reviewboard/accounts/urls.py b/reviewboard/accounts/urls.py index f4aed47ad1fd9843b02bbfebe1d53317e15542b4..9e8333f08d795905605c410e55de588ac29ae1a5 100644 --- a/reviewboard/accounts/urls.py +++ b/reviewboard/accounts/urls.py @@ -1,14 +1,19 @@ from django.conf import settings -from django.urls import path, re_path +from django.urls import include, path, re_path from django.contrib.auth import views as auth_views +from djblets.urls.resolvers import DynamicURLResolver from reviewboard.accounts.forms.auth import AuthenticationForm +from reviewboard.accounts.views import LoginView from reviewboard.accounts import views as accounts_views +sso_dynamic_urls = DynamicURLResolver() + + urlpatterns = [ path('login/', - auth_views.LoginView.as_view( + LoginView.as_view( template_name='accounts/login.html', authentication_form=AuthenticationForm), name='login'), @@ -55,4 +60,5 @@ urlpatterns = [ path('preferences/preview-email/password-changed/', accounts_views.preview_password_changed_email, name='preview-password-change-email'), + path('sso/', include(([sso_dynamic_urls], 'accounts'), namespace='sso')), ] diff --git a/reviewboard/accounts/views.py b/reviewboard/accounts/views.py index 98a23cb2e81c02cbc3381b5f9aa78f196ebd5b82..7462f50532ecc6e334132bbde6e822584b081241 100644 --- a/reviewboard/accounts/views.py +++ b/reviewboard/accounts/views.py @@ -3,6 +3,7 @@ import logging from django.conf import settings from django.contrib.auth.decorators import login_required from django.contrib.auth.models import User +from django.contrib.auth.views import LoginView as DjangoLoginView from django.forms.forms import ErrorDict from django.http import HttpResponseRedirect from django.shortcuts import get_object_or_404, render @@ -26,6 +27,7 @@ from reviewboard.accounts.forms.registration import RegistrationForm from reviewboard.accounts.mixins import CheckLoginRequiredViewMixin from reviewboard.accounts.pages import AccountPage, OAuth2Page, PrivacyPage from reviewboard.accounts.privacy import is_consent_missing +from reviewboard.accounts.sso.backends import sso_backends from reviewboard.admin.decorators import check_read_only from reviewboard.avatars import avatar_services from reviewboard.notifications.email.decorators import preview_email @@ -42,6 +44,37 @@ from reviewboard.site.urlresolvers import local_site_reverse logger = logging.getLogger(__name__) +class LoginView(DjangoLoginView): + """A view for rendering the login page. + + Version Added: + 5.0 + """ + + template_name = 'accounts/login.html' + + def get_context_data(self, **kwargs): + """Return extra data for rendering the template. + + Args: + **kwargs (dict): + Keyword arguments to pass to the parent class. + + Returns: + dict: + Context to use when rendering the template. + """ + context = super().get_context_data(**kwargs) + + context['enabled_sso_backends'] = [ + sso_backend + for sso_backend in sso_backends + if sso_backend.is_enabled() + ] + + return context + + class UserInfoboxView(CheckLoginRequiredViewMixin, CheckLocalSiteAccessViewMixin, ETagViewMixin, diff --git a/reviewboard/admin/forms/auth_settings.py b/reviewboard/admin/forms/auth_settings.py index 3b2bde3a41213ec75192f305bf92e7ff51ce8146..c60e413642a49097370017f1ecf8f849b60a754b 100644 --- a/reviewboard/admin/forms/auth_settings.py +++ b/reviewboard/admin/forms/auth_settings.py @@ -58,11 +58,11 @@ class AuthenticationSettingsForm(SiteSettingsForm): **kwargs (dict): Additional keyword arguments for the parent class. """ - from reviewboard.accounts.backends import auth_backends - super(AuthenticationSettingsForm, self).__init__(siteconfig, *args, **kwargs) + from reviewboard.accounts.backends import auth_backends + self.auth_backend_forms = {} cur_auth_backend = (self['auth_backend'].data or @@ -108,6 +108,39 @@ class AuthenticationSettingsForm(SiteSettingsForm): backend_choices.append(self.CUSTOM_AUTH_CHOICE) self.fields['auth_backend'].choices = backend_choices + from reviewboard.accounts.sso.backends import sso_backends + + self.sso_backend_forms = {} + + form_fields = self.Meta.fieldsets[0]['fields'] + + for backend in sso_backends: + backend_id = backend.backend_id + + if backend.settings_form: + field_id = '%s_enabled' % backend_id + + try: + available, reason = backend.is_available() + except Exception as e: + available = False + reason = str(e) + + self.fields[field_id] = forms.BooleanField( + label=_('Enable %s Authentication') % backend.name, + disabled=not available, + help_text=reason or '', + required=False) + + if field_id not in form_fields: + form_fields.append(field_id) + + form = backend.settings_form(siteconfig, *args, **kwargs) + form.load() + self.sso_backend_forms[backend_id] = form + + self.load() + def load(self): """Load settings from the form. @@ -132,6 +165,10 @@ class AuthenticationSettingsForm(SiteSettingsForm): if auth_backend in self.auth_backend_forms: self.auth_backend_forms[auth_backend].save() + for form, enable_field_id in self._iter_sso_backend_forms(): + if self[enable_field_id].data: + form.save() + super(AuthenticationSettingsForm, self).save() # Reload any important changes into the Django settings. @@ -154,7 +191,15 @@ class AuthenticationSettingsForm(SiteSettingsForm): backend_id = self.cleaned_data['auth_backend'] backend_form = self.auth_backend_forms[backend_id] - return backend_form.is_valid() + if not backend_form.is_valid(): + return False + + for form, enable_field_id in self._iter_sso_backend_forms(): + if (self.cleaned_data[enable_field_id] and + not form.is_valid()): + return False + + return True def full_clean(self): """Clean and validate all form fields. @@ -177,10 +222,31 @@ class AuthenticationSettingsForm(SiteSettingsForm): if auth_backend in self.auth_backend_forms: self.auth_backend_forms[auth_backend].full_clean() + + for form, enable_field_id in self._iter_sso_backend_forms(): + if (self[enable_field_id].data or + self.fields[enable_field_id].initial): + form.full_clean() else: for form in self.auth_backend_forms.values(): form.full_clean() + for form in self.sso_backend_forms.values(): + form.full_clean() + + def _iter_sso_backend_forms(self): + """Yield the SSO backend forms. + + Yields: + tuple: + A 2-tuple of the SSO backend form and the name of the form field + to enable that backend. + """ + for sso_backend_id, form in self.sso_backend_forms.items(): + enable_field_id = '%s_enabled' % sso_backend_id + + yield form, enable_field_id + class Meta: title = _('Authentication Settings') save_blacklist = ('auth_anonymous_access',) @@ -190,11 +256,16 @@ class AuthenticationSettingsForm(SiteSettingsForm): 'subforms_attr': 'auth_backend_forms', 'controller_field': 'auth_backend', }, + { + 'subforms_attr': 'sso_backend_forms', + 'controller_field': None, + 'enable_checkbox': True, + }, ) fieldsets = ( { 'classes': ('wide',), - 'fields': ('auth_anonymous_access', 'auth_backend'), + 'fields': ['auth_anonymous_access', 'auth_backend'], }, ) diff --git a/reviewboard/admin/siteconfig.py b/reviewboard/admin/siteconfig.py index c816dcb0e1297852061b1767a57c9143fd06a0e9..7b98a73b96b8cf6fdf4900c8ed8ae08ec363e966 100644 --- a/reviewboard/admin/siteconfig.py +++ b/reviewboard/admin/siteconfig.py @@ -46,6 +46,7 @@ from haystack import connections as haystack_connections from reviewboard.accounts.backends import auth_backends from reviewboard.accounts.privacy import recompute_privacy_consents +from reviewboard.accounts.sso.backends import sso_backends from reviewboard.avatars import avatar_services from reviewboard.oauth.features import oauth2_service_feature from reviewboard.notifications.email.message import EmailMessage @@ -234,6 +235,10 @@ def load_site_config(full_reload=False): # Populate defaults if they weren't already set. if not siteconfig.get_defaults(): + # We don't actually access the sso_backends registry until we're here, + # because otherwise we might hit circular imports while building up URL + # patterns. + defaults.update(sso_backends.get_siteconfig_defaults()) siteconfig.add_defaults(defaults) # The default value for DEFAULT_EMAIL_FROM (webmaster@localhost) diff --git a/reviewboard/admin/urls.py b/reviewboard/admin/urls.py index f01568cd1e4beb77f6cc82b18909d89188bce3e3..15f9b9a28bc348dcdfb4a1e7cb5af04aeb303816 100644 --- a/reviewboard/admin/urls.py +++ b/reviewboard/admin/urls.py @@ -82,7 +82,6 @@ urlpatterns = [ views.site_settings, kwargs={ 'form_class': EMailSettingsForm, - 'template_name': 'admin/settings.html', }, name='settings-email'), @@ -90,7 +89,6 @@ urlpatterns = [ views.site_settings, kwargs={ 'form_class': DiffSettingsForm, - 'template_name': 'admin/settings.html', }, name='settings-diffs'), @@ -98,7 +96,6 @@ urlpatterns = [ views.site_settings, kwargs={ 'form_class': LoggingSettingsForm, - 'template_name': 'admin/settings.html', }, name='settings-logging'), @@ -125,7 +122,6 @@ urlpatterns = [ views.site_settings, kwargs={ 'form_class': SupportSettingsForm, - 'template_name': 'admin/settings.html', }, name='settings-support'), diff --git a/reviewboard/hostingsvcs/service.py b/reviewboard/hostingsvcs/service.py index fcf7524313610a086e059871e366cbffc2446cbf..64fa418b7fd9f34276df97e5c69cdc56e3bd3767 100644 --- a/reviewboard/hostingsvcs/service.py +++ b/reviewboard/hostingsvcs/service.py @@ -814,18 +814,18 @@ class HostingServiceClient(object): - Return credentials for use in the HTTP request. * :py:meth:`build_http_request` - - Build the :py:class:`HostingServiceHTTPRequest` object. + - Build the :py:class:`HostingServiceHTTPRequest` object. * :py:meth:`open_http_request` - Performs the actual HTTP request. * :py:meth:`process_http_response` - Performs post-processing on a response from the service, or raises - an error. + an error. * :py:meth:`process_http_error` - Processes a raised exception, handling it in some form or - converting it into another error. + converting it into another error. See those methods for more information. diff --git a/reviewboard/reviews/detail.py b/reviewboard/reviews/detail.py index 878871080aa656580bba8326b6a15c1387f02973..b848c8ae6969fe3d0ed9553280013219a9ad7934 100644 --- a/reviewboard/reviews/detail.py +++ b/reviewboard/reviews/detail.py @@ -753,7 +753,7 @@ class BaseReviewRequestPageEntry(object): Version Changed: 4.0.4: - Added ``entry`` and ``**kwargs` arguments. + Added ``entry`` and ``**kwargs`` arguments. Args: data (ReviewRequestPageData): diff --git a/reviewboard/static/rb/css/common.less b/reviewboard/static/rb/css/common.less index d9fab7776a7f3940c66b270b47a65b3c1ddef892..5f0f37e028eb06f8137b7b913caf05b0bcb7a55c 100644 --- a/reviewboard/static/rb/css/common.less +++ b/reviewboard/static/rb/css/common.less @@ -239,11 +239,15 @@ button, input { font-size: 120%; - margin: 0; + margin: 0 0 0.5em; padding: 0.6em; width: 100%; box-sizing: border-box; } + + a { + font-size: inherit; + } } .auth-form-row { @@ -288,7 +292,8 @@ } .auth-header { - margin: 0 0 1em 0; + margin: 0 auto 1em auto; + max-width: 60em; h1 { font-size: 120%; diff --git a/reviewboard/static/rb/js/ui/views/formView.es6.js b/reviewboard/static/rb/js/ui/views/formView.es6.js index df6030617af93cfc6edc89ef021ac6ebe6ac1583..b4b3d64de8046b2a7a1ba6252ffd0f3d805c57e8 100644 --- a/reviewboard/static/rb/js/ui/views/formView.es6.js +++ b/reviewboard/static/rb/js/ui/views/formView.es6.js @@ -173,8 +173,10 @@ RB.FormView = Backbone.View.extend({ const $subform = $(subformEl); const controllerID = $subform.data('subform-controller'); const subformID = $subform.data('subform-id'); + const enablerID = $subform.data('subform-enabler'); let group = $subform.data('subform-group'); let $controller; + let $enabler; if (!subformID) { console.error('Subform %o is missing data-subform-id=', @@ -182,10 +184,10 @@ RB.FormView = Backbone.View.extend({ return; } - if (!group && !controllerID) { + if (!group && !controllerID && !enablerID) { console.error( - 'Subform %o is missing either data-subform-group= ' + - 'or data-subform-controller=', + 'Subform %o is missing data-subform-group=, ' + + 'data-subform-controller=, or data-subform-enable=', subformEl); return; } @@ -217,6 +219,12 @@ RB.FormView = Backbone.View.extend({ subformEl, controllerID); return; } + } else if (enablerID) { + $enabler = this.$(`#${enablerID}`); + window.$form = this.$el; + + console.assert($enabler.length === 1, + `Missing enabler #${enablerID}`); } /* Register the subforms so that they can be looked up later. */ @@ -248,6 +256,26 @@ RB.FormView = Backbone.View.extend({ })); } } + + /* + * If there's an enabler, set the current subform's visibility + * based on the state of that element, and listen for changes. + */ + if ($enabler) { + const enabled = $enabler.is(':checked'); + + $subform + .setVisible(enabled) + .prop('disabled', !enabled); + + $enabler.on('change', () => { + const enabled = $enabler.is(':checked'); + + $subform + .setVisible(enabled) + .prop('disabled', !enabled); + }); + } }); }, diff --git a/reviewboard/templates/accounts/login.html b/reviewboard/templates/accounts/login.html index e3b49660b339ac20d38c0028f2b22eb47d319d1c..a16904382ce8c8a9fd6fe0bcbbd1d41b19d1d55a 100644 --- a/reviewboard/templates/accounts/login.html +++ b/reviewboard/templates/accounts/login.html @@ -37,6 +37,11 @@
+{% for sso_backend in enabled_sso_backends %} + + + +{% endfor %}
diff --git a/reviewboard/templates/accounts/sso/link-user-connect-existing.html b/reviewboard/templates/accounts/sso/link-user-connect-existing.html new file mode 100644 index 0000000000000000000000000000000000000000..32a71fa958c2ffdbcf59232b822f06dee16d1cd9 --- /dev/null +++ b/reviewboard/templates/accounts/sso/link-user-connect-existing.html @@ -0,0 +1,54 @@ +{% extends "accounts/base.html" %} +{% load avatars djblets_deco i18n %} + +{% block title %}{% trans "Link Account" %}{% endblock %} + +{% block auth_content %} +
+

{% trans "Connect account" %}

+

+{% blocktrans %} + It looks like you already have an account on {{PRODUCT_NAME}}. + Log in with your existing password to complete SSO setup. +{% endblocktrans %} +

+ {% avatar user 175 %} +

{{user.username}}

+{% if form.errors %} +{% errorbox %}{{form.non_field_errors}}{% enderrorbox %} +{% endif %} +
+ +
+{% block hidden_fields %} + + + {{form.provision}} + {% csrf_token %} +{% endblock %} + +
+ {{form.password.label_tag}} + {{form.password}} + {{form.errors.password}} +
+ +
+
+ +
+
+
+ +
+

{% trans "Not you?" %}

+

{% trans "I have a different account" %}

+

+{% blocktrans %} + Don't have an account? Please contact your system administrator about + correcting the username in your SSO provider. +{% endblocktrans %} +

+
+{% endblock auth_content %} diff --git a/reviewboard/templates/accounts/sso/link-user-login.html b/reviewboard/templates/accounts/sso/link-user-login.html new file mode 100644 index 0000000000000000000000000000000000000000..969e4602d8a98f1b183aa75bb7f0933f8a08e31f --- /dev/null +++ b/reviewboard/templates/accounts/sso/link-user-login.html @@ -0,0 +1,56 @@ +{% extends "accounts/base.html" %} +{% load avatars djblets_deco i18n %} + +{% block title %}{% trans "Link Account" %}{% endblock %} + +{% block auth_content %} +
+

{% trans "Log in to connect" %}

+

+{% blocktrans %} + Log in with your existing {{PRODUCT_NAME}} username and password to complete + SSO setup. +{% endblocktrans %} +

+{% if form.errors %} +{% errorbox %}{{form.non_field_errors}}{% enderrorbox %} +{% endif %} +
+ +
+{% block hidden_fields %} + + {{form.provision}} + {% csrf_token %} +{% endblock %} + +
+ {{form.username.label_tag}} + {{form.username}} + {{form.errors.username}} +
+ +
+ {{form.password.label_tag}} + {{form.password}} + {{form.errors.password}} +
+ +
+
+ +
+
+
+ +
+

{% trans "Don't have an account?" %}

+

+{% blocktrans %} + Please contact your system administrator about correcting the username in your + SSO provider. +{% endblocktrans %} +

+
+{% endblock auth_content %} diff --git a/reviewboard/templates/accounts/sso/link-user-provision.html b/reviewboard/templates/accounts/sso/link-user-provision.html new file mode 100644 index 0000000000000000000000000000000000000000..b652ab5bb77539d6aa1f611ad3b8713b5e1f8d8c --- /dev/null +++ b/reviewboard/templates/accounts/sso/link-user-provision.html @@ -0,0 +1,42 @@ +{% extends "accounts/base.html" %} +{% load avatars djblets_deco i18n %} + +{% block title %}{% trans "Create Account" %}{% endblock %} + +{% block auth_content %} +
+

{% trans "Create Review Board account" %}

+

+{% blocktrans %} + Create a new account on {{PRODUCT_NAME}}. If you already have a username and + password, log in with the link below. If you have an account but cannot log + in, stop and contact your administrator for assistance. +{% endblocktrans %} +

+{% if form.errors %} +{% errorbox %}{{form.non_field_errors}}{% enderrorbox %} +{% endif %} +
+ +
+{% block hidden_fields %} + + + + {{form.provision}} + {% csrf_token %} +{% endblock %} + +
+
+ +
+
+
+ +
+

{% trans "Already have an account?" %}

+

{% trans "Log in to connect" %}

+
+{% endblock auth_content %} diff --git a/reviewboard/templates/admin/settings.html b/reviewboard/templates/admin/settings.html index 084e086ae17807620b79318e4da9cd1378e62063..bd60607c27a5c8626852681fdb4902fb8188e72e 100644 --- a/reviewboard/templates/admin/settings.html +++ b/reviewboard/templates/admin/settings.html @@ -5,9 +5,15 @@ {% block after_field_sets %} {% if form.Meta.subforms %} {% for subform_info in form.Meta.subforms %} -{% with subforms=form|getattr:subform_info.subforms_attr subform_controller_field=form|getitem:subform_info.controller_field %} -{% if subforms and subform_controller_field %} -{% include "forms/subforms.html" with subform_controller=subform_controller_field.id_for_label %} +{% with subforms=form|getattr:subform_info.subforms_attr %} +{% if subforms %} +{% if subform_info.controller_field %} +{% with subform_controller_field=form|getitem:subform_info.controller_field %} +{% include "forms/subforms.html" with subform_controller=subform_controller_field.id_for_label %} +{% endwith %} +{% else %} +{% include "forms/subforms.html" %} +{% endif %} {% endif %} {% endwith %} {% endfor %} diff --git a/reviewboard/templates/forms/subform.html b/reviewboard/templates/forms/subform.html index 32c3bc3ddc7ba1c8c835c759ea90ccffa6faaf78..7bc72c42be0c9c107f19a9059789a00f2280bdb6 100644 --- a/reviewboard/templates/forms/subform.html +++ b/reviewboard/templates/forms/subform.html @@ -2,6 +2,7 @@ data-subform-id="{{subform_id}}" {% if subform_controller %}data-subform-controller="{{subform_controller}}"{% endif %} {% if subform_group %}data-subform-group="{{subform_group}}"{% endif %} + {% if subform_info.enable_checkbox %}data-subform-enabler="id_{{subform_id}}_enabled"{% endif %} disabled hidden> {% spaceless %} {% if subform.Meta.title %} diff --git a/setup.py b/setup.py index 819bce8323e21d737ffe11fd4aeaeaea4fe8515f..207c2783b2ab3eddf4e65711fc51f511dd02ba26 100755 --- a/setup.py +++ b/setup.py @@ -482,6 +482,7 @@ setup( 'postgres': ['psycopg2-binary<2.9'], 's3': ['django-storages>=1.8,<1.9'], + 'saml': ['python3-saml'], 'subvertpy': ['subvertpy'], 'swift': ['django-storage-swift'], },