This change has been marked as completed.

Describe the completed change (optional):

Pushed to release-5.0.x (4d0e851)

Summary

Add settings for client web-based logon flow.

Review Request #13045 — Created May 19, 2023 and submitted 1 year, 12 months ago — Latest diff uploaded 2 years ago

Information

Owner

maubin*

Repository

Review Board

Branch

release-5.0.x

Bugs

Depends On

~~12982~~, ~~13051~~

Reviewers

Groups

reviewboard

People

Description*

This adds some settings to the Authentication settings page for the
upcoming client web-based logon flow. There is a new section for
general API Authentication settings, with a field for setting the
automatic expiration of client API tokens.

Testing Done


Ran unit tests.
Manually tested the settings page putting different expirations,

  saving the form, playing with other settings.

Files

API Authentication Settings

Diff Revision 1

This is not the most recent revision of the diff. The latest diff is revision 3. See what's changed.

orig

Commits

First	Last	Summary	ID	Author
		Support web-based login for clients.	0540962a83a1b9e1f652c60a13a4cdc22ea5e46c	Michelle Aubin
		Add settings for client web-based logon.	283fd7d68173d9a5d14e13e117aee8e3d094b180	Michelle Aubin

Files

	reviewboard/admin/forms/auth_settings.py
	reviewboard/admin/tests/test_web_client_logon_settings_form.py

-    Revision c60e413642a49097370017f1ecf8f849b60a754b
+    New Change
     """Administration form for authentication settings."""
     import logging
     from typing import Iterator, Tuple
     from django import forms
     from django.utils.translation import gettext_lazy as _
     from djblets.forms.widgets import AmountSelectorWidget
     from djblets.siteconfig.forms import SiteSettingsForm
     from reviewboard.accounts.forms.auth import LegacyAuthModuleSettingsForm
     from reviewboard.admin.siteconfig import load_site_config
             required=True,
             widget=forms.Select(attrs={
                 'data-subform-group': 'auth-backend',
             }))
         client_web_login_enabled = forms.BooleanField(
             label=_('Enable web-based logon for clients.'),
             help_text=_('Allow users to authenticate clients (e.g. RBTools) to '
                         'Review Board by logging in via a browser.'),
             required=False)
         def __init__(self, siteconfig, *args, **kwargs):
             """Initialize the settings form.
             This will load the list of available authentication backends and
             their settings forms, allowing the browser to show the appropriate
                     form = backend.settings_form(siteconfig, *args, **kwargs)
                     form.load()
                     self.sso_backend_forms[backend_id] = form
             # Settings for web-based client logon.
             client_web_login_form = ClientWebBasedLoginSettingsForm(siteconfig,
                                                                     *args,
                                                                     **kwargs)
             client_web_login_form.load()
             self.client_web_login_forms = {
                 'client_web_login': client_web_login_form,
+            }
             self.load()
         def load(self):
             """Load settings from the form.
             This will populate initial fields based on the site configuration.
             """
             super(AuthenticationSettingsForm, self).load()
             self.fields['auth_anonymous_access'].initial = \
                 not self.siteconfig.get('auth_require_sitewide_login')
             self.fields['client_web_login_enabled'].initial = \
                 self.siteconfig.get('client_web_login')
         def save(self):
             """Save the form.
             This will write the new configuration to the database. It will then
             force a site configuration reload.
             """
             self.siteconfig.set('auth_require_sitewide_login',
                                 not self.cleaned_data['auth_anonymous_access'])
             self.siteconfig.set('client_web_login',
                                 self.cleaned_data['client_web_login_enabled'])
             auth_backend = self.cleaned_data['auth_backend']
             if auth_backend in self.auth_backend_forms:
                 self.auth_backend_forms[auth_backend].save()
-

 2
+            for form, enable_field_id in self._iter_sso_backend_forms():
                 if self[enable_field_id].data:
                     form.save()
             for form, enable_field_id in self._iter_client_web_login_forms():
                 if self[enable_field_id].data:
                     form.save()
             super(AuthenticationSettingsForm, self).save()
             # Reload any important changes into the Django settings.
             load_site_config()
             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
             for form, enable_field_id in self._iter_client_web_login_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.
                 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()
                 for form, enable_field_id in self._iter_client_web_login_forms():
                     if (self[enable_field_id].data):
                         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()
                 for form in self.client_web_login_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
         def _iter_client_web_login_forms(self) -> Iterator[Tuple[forms.Form, str]]:
             """Yield the client web-based login settings forms.
             This only yields one form.
             Yields:
                 tuple:
                 A 2-tuple of:
                 Tuple:
 (django.forms.Form):
                         The client web-based login settings form.
 (str):
                         The name of the form field to enable the client web-based
                         login flow.
             """
             for field_id, form in self.client_web_login_forms.items():
                 enable_field_id = '%s_enabled' % field_id
                 yield form, enable_field_id
         class Meta:
             title = _('Authentication Settings')
             save_blacklist = ('auth_anonymous_access',)
             subforms = (
+                {
                     'subforms_attr': 'sso_backend_forms',
                     'controller_field': None,
                     'enable_checkbox': True,
                 },
+                {
                     'subforms_attr': 'client_web_login_forms',
                     'controller_field': None,
                     'enable_checkbox': True,
                 },
+            )
             fieldsets = (
+                {
                     'classes': ('wide',),
-                    'fields': ['auth_anonymous_access', 'auth_backend'],
+                    'fields': ['auth_anonymous_access',
                                'auth_backend',
                                'client_web_login_enabled'],
                 },
+            )
     class ClientWebBasedLoginSettingsForm(SiteSettingsForm):
         """A form for configuring the settings for client web-based login.
         Version Added:
 .0.5
         """
         api_token_expiration = forms.IntegerField(
             label=_('Client API token expiration'),
             help_text=_('API tokens are automatically created to authenticate '
                         'clients during web-based login. This sets their '
                         'expiration.'),
             required=False,
             widget=AmountSelectorWidget(unit_choices=[
                 (1, _('days')),
                 (7, _('weeks')),
                 (30, _('months')),
                 (365, _('years')),
                 (None, _('Never')),
             ], number_attrs={
                 'min': 0,
             }))
         def load(self) -> None:
             """Load settings from the form.
             This will populate initial fields based on the site configuration.
             """
             super().load()
             self.fields['api_token_expiration'].initial = \
                 self.siteconfig.get('client_web_login_token_expiration')
         def save(
             self,
             *args,
             **kwargs
         ) -> None:
             """Save the form.
             This will write the new configuration to the database. It will then
             force a site configuration reload.
             Args:
                 *args (tuple):
                     Positional arguments to pass to the parent method.
                 **kwargs (dict):
                     Keyword arguments to pass to the parent method.
             """
             self.siteconfig.set('client_web_login_token_expiration',
                                 self.cleaned_data['api_token_expiration'])
             super().save(*args, **kwargs)
             # Reload any important changes into the Django settings.
             load_site_config()
         class Meta:
             title = _('Client Web-based Logon Settings')

reviewboard/admin/tests/test_web_client_logon_settings_form.py

chipx86:

Might make sense at this point to do:

for forms in (self._iter_sso_backend_forms(),
              self._iter_client_web_login_forms()):
    for form, enable_field_id in forms:
        ...

Same with the other form loops below.

In fact, maybe we want to have a method that iterates all these subforms so all these loops dealing with subforms can remain simple, automatically handling new ones going forward? It could do:

yield from self._iter_sso_backend_forms()
yield from self._iter_client_web_login_forms()

maubin:

I'm going to drop this since I'm no longer using the subform pattern for the API authentication settings.

Formatting should be more like:

widget=AmountSelectorWidget(
    unit_choices=[
        ...
    ],
    number_attrs={
        ...
    }
)


That way there's no confusion about which parts of this belong to the IntegerField vs. the widget.