sso: Use privileged decorator for actions

Tests:

- Functional tests succeed
- Initial setup run during first setup successfully
  - A key pair is created in /etc/apache2/auth-pubtkt-keys
- User is able successfully login to web UI.
- A non-admin user who has permission to access an app via group membership is
  able to access the app's web interface.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

plinth/modules/sso/__init__.py

@@ -1,17 +1,17 @@
 # SPDX-License-Identifier: AGPL-3.0-or-later
-"""
+"""FreedomBox app to configure Single Sign On services."""
-FreedomBox app to configure Single Sign On services.
-"""
 
 from django.utils.translation import gettext_lazy as _
 
-from plinth import actions
 from plinth import app as app_module
 from plinth.package import Packages
 
+from . import privileged
+
 
 class SSOApp(app_module.App):
     """FreedomBox app for single sign on."""
+
     app_id = 'sso'
 
     _version = 1
@@ -34,4 +34,4 @@ class SSOApp(app_module.App):
     def setup(self, old_version):
         """Install and configure the app."""
         super().setup(old_version)
-        actions.superuser_run('auth-pubtkt', ['create-key-pair'])
+        privileged.create_key_pair()

plinth/modules/sso/privileged.py

@@ -1,44 +1,23 @@
-#!/usr/bin/python3
 # SPDX-License-Identifier: AGPL-3.0-or-later
-"""
+"""Generate a auth_pubtkt ticket.
-Module with utilities to generate a auth_pubtkt ticket and
+
-sign it with the FreedomBox server's private key.
+Sign tickets with the FreedomBox server's private key.
 """
 
-import argparse
 import base64
 import datetime
 import os
 
 from OpenSSL import crypto
 
+from plinth.actions import privileged
+
 KEYS_DIRECTORY = '/etc/apache2/auth-pubtkt-keys'
 
 
-def parse_arguments():
+@privileged
-    """ Return parsed command line arguments as dictionary. """
+def create_key_pair():
-    parser = argparse.ArgumentParser()
+    """Create public/private key pair for signing the tickets."""
-    subparsers = parser.add_subparsers(dest='subcommand', help='Sub command')
-
-    subparsers.add_parser(
-        'create-key-pair', help='create a key pair for the apache server '
-        'to sign auth_pubtkt tickets')
-    gen_tkt = subparsers.add_parser('generate-ticket',
-                                    help='generate auth_pubtkt ticket')
-    gen_tkt.add_argument('--uid', help='username of the user')
-    gen_tkt.add_argument('--private-key-file',
-                         help='path of the private key file of the server')
-    gen_tkt.add_argument('--tokens',
-                         help='tokens, usually containing the user groups')
-
-    subparsers.required = True
-    return parser.parse_args()
-
-
-def subcommand_create_key_pair(_):
-    """Create public/private key pair for signing the auth_pubtkt
-    tickets.
-    """
     private_key_file = os.path.join(KEYS_DIRECTORY, 'privkey.pem')
     public_key_file = os.path.join(KEYS_DIRECTORY, 'pubkey.pem')
 
@@ -64,9 +43,10 @@ def subcommand_create_key_pair(_):
             os.chmod(fil, 0o440)
 
 
-def create_ticket(pkey, uid, validuntil, ip=None, tokens=None, udata=None,
+def _create_ticket(pkey, uid, validuntil, ip=None, tokens=None, udata=None,
-                  graceperiod=None, extra_fields=None):
+                   graceperiod=None, extra_fields=None):
     """Create and return a signed mod_auth_pubtkt ticket."""
+    tokens = ','.join(tokens)
     fields = [
         f'uid={uid}',
         f'validuntil={int(validuntil)}',
@@ -78,49 +58,33 @@ def create_ticket(pkey, uid, validuntil, ip=None, tokens=None, udata=None,
         and ';'.join(['{}={}'.format(k, v) for k, v in extra_fields]),
     ]
     data = ';'.join(filter(None, fields))
-    signature = 'sig={}'.format(sign(pkey, data))
+    signature = 'sig={}'.format(_sign(pkey, data))
     return ';'.join([data, signature])
 
 
-def sign(pkey, data):
+def _sign(pkey, data):
-    """Calculates and returns ticket's signature."""
+    """Calculate and return ticket's signature."""
     sig = crypto.sign(pkey, data.encode(), 'sha512')
     return base64.b64encode(sig).decode()
 
 
-def subcommand_generate_ticket(arguments):
+@privileged
+def generate_ticket(uid: str, private_key_file: str, tokens: list[str]) -> str:
     """Generate a mod_auth_pubtkt ticket using login credentials."""
-    uid = arguments.uid
-    private_key_file = arguments.private_key_file
-    tokens = arguments.tokens
     with open(private_key_file, 'r', encoding='utf-8') as fil:
         pkey = crypto.load_privatekey(crypto.FILETYPE_PEM, fil.read().encode())
-    valid_until = minutes_from_now(12 * 60)
+    valid_until = _minutes_from_now(12 * 60)
-    grace_period = minutes_from_now(11 * 60)
+    grace_period = _minutes_from_now(11 * 60)
-    print(
+    return _create_ticket(pkey, uid, valid_until, tokens=tokens,
-        create_ticket(pkey, uid, valid_until, tokens=tokens,
+                          graceperiod=grace_period)
-                      graceperiod=grace_period))
 
 
-def minutes_from_now(minutes):
+def _minutes_from_now(minutes):
     """Return a timestamp at the given number of minutes from now."""
-    return seconds_from_now(minutes * 60)
+    return _seconds_from_now(minutes * 60)
 
 
-def seconds_from_now(seconds):
+def _seconds_from_now(seconds):
     """Return a timestamp at the given number of seconds from now."""
     return (datetime.datetime.now() +
             datetime.timedelta(0, seconds)).timestamp()
-
-
-def main():
-    """Parse arguments and perform all duties."""
-    arguments = parse_arguments()
-
-    subcommand = arguments.subcommand.replace('-', '_')
-    subcommand_method = globals()['subcommand_' + subcommand]
-    subcommand_method(arguments)
-
-
-if __name__ == '__main__':
-    main()

plinth/modules/sso/tests/test_privileged.py

@@ -7,34 +7,33 @@ import os
 
 import pytest
 
+from plinth.modules.sso import privileged
 from plinth.modules.sso.views import PRIVATE_KEY_FILE_NAME
 
-actions_name = 'auth-pubtkt'
+pytestmark = pytest.mark.usefixtures('mock_privileged')
+privileged_modules_to_mock = ['plinth.modules.sso.privileged']
 
 
 @pytest.fixture(autouse=True)
-def fixture_keys_directory(actions_module, tmpdir):
+def fixture_keys_directory(tmpdir):
     """Set keys directory in the actions module."""
-    actions_module.KEYS_DIRECTORY = str(tmpdir)
+    privileged.KEYS_DIRECTORY = str(tmpdir)
 
 
 @pytest.fixture(name='existing_key_pair')
-def fixture_existing_key_pair(call_action):
+def fixture_existing_key_pair():
     """A fixture to create key pair if needed."""
-    call_action(['create-key-pair'])
+    privileged.create_key_pair()
 
 
-def test_generate_ticket(call_action, existing_key_pair, actions_module):
+def test_generate_ticket(existing_key_pair):
     """Test generating a ticket."""
     username = 'tester'
-    groups = 'freedombox-share,syncthing,web-search'
+    groups = ['freedombox-share', 'syncthing', 'web-search']
 
-    private_key_file = os.path.join(actions_module.KEYS_DIRECTORY,
+    private_key_file = os.path.join(privileged.KEYS_DIRECTORY,
                                     PRIVATE_KEY_FILE_NAME)
-    ticket = call_action([
+    ticket = privileged.generate_ticket(username, private_key_file, groups)
-        'generate-ticket', '--uid', username, '--private-key-file',
-        private_key_file, '--tokens', groups
-    ])
 
     fields = {}
     for item in ticket.split(';'):
@@ -47,5 +46,5 @@ def test_generate_ticket(call_action, existing_key_pair, actions_module):
 
     assert fields['uid'] == username
     assert int(fields['validuntil']) > 0
-    assert fields['tokens'] == groups
+    assert fields['tokens'] == ','.join(groups)
     assert int(fields['graceperiod']) > 0

plinth/modules/sso/views.py

@@ -1,7 +1,5 @@
 # SPDX-License-Identifier: AGPL-3.0-or-later
-"""
+"""Views for the Single Sign On app of FreedomBox."""
-Views for the Single Sign On app of FreedomBox.
-"""
 
 import logging
 import os
@@ -17,8 +15,9 @@ from django.contrib.auth.views import LoginView
 from django.http import HttpResponseRedirect
 from django.utils.translation import gettext as _
 
-from plinth import actions, translation, utils, web_framework
+from plinth import translation, utils, web_framework
 
+from . import privileged
 from .forms import AuthenticationForm, CaptchaAuthenticationForm
 
 PRIVATE_KEY_FILE_NAME = 'privkey.pem'
@@ -29,15 +28,11 @@ logger = logging.getLogger(__name__)
 
 
 def set_ticket_cookie(user, response):
-    """Generate and set a mod_auth_pubtkt as a cookie in the provided
+    """Generate and set a mod_auth_pubtkt as a cookie in the response."""
-    response.
-    """
     tokens = list(map(lambda g: g.name, user.groups.all()))
     private_key_file = os.path.join(KEYS_DIRECTORY, PRIVATE_KEY_FILE_NAME)
-    ticket = actions.superuser_run('auth-pubtkt', [
+    ticket = privileged.generate_ticket(user.username, private_key_file,
-        'generate-ticket', '--uid', user.username, '--private-key-file',
+                                        tokens)
-        private_key_file, '--tokens', ','.join(tokens)
-    ])
     response.set_cookie(SSO_COOKIE_NAME, urllib.parse.quote(ticket))
     return response
 
@@ -46,13 +41,14 @@ class SSOLoginView(LoginView):
     """View to login to FreedomBox and set a auth_pubtkt cookie.
 
     Cookie will be used to provide Single Sign On for some other applications.
-
     """
+
     redirect_authenticated_user = True
     template_name = 'login.html'
     form_class = AuthenticationForm
 
     def dispatch(self, request, *args, **kwargs):
+        """Handle a request and return a HTTP response."""
         response = super().dispatch(request, *args, **kwargs)
         if request.user.is_authenticated:
             translation.set_language(request, response,
@@ -65,15 +61,19 @@ class SSOLoginView(LoginView):
     # axes_form_invalid when axes >= 5.0.0 becomes available in Debian stable.
     @axes_form_invalid
     def form_invalid(self, *args, **kwargs):
+        """Trigger django-axes logic to deal with too many attempts."""
         return super().form_invalid(*args, **kwargs)
 
 
 class CaptchaLoginView(LoginView):
+    """A login view with mandatory CAPTCHA image."""
+
     redirect_authenticated_user = True
     template_name = 'login.html'
     form_class = CaptchaAuthenticationForm
 
     def dispatch(self, request, *args, **kwargs):
+        """Handle a request and return a HTTP response."""
         response = super().dispatch(request, *args, **kwargs)
         if not request.POST:
             return response
@@ -102,7 +102,7 @@ def logout(request):
 
 
 def refresh(request):
-    """Simulate cookie refresh - redirect logged in user with a new cookie"""
+    """Simulate cookie refresh - redirect logged in user with a new cookie."""
     redirect_url = request.GET.get(REDIRECT_FIELD_NAME, '')
     response = HttpResponseRedirect(redirect_url)
     response.delete_cookie(SSO_COOKIE_NAME)