first_boot: Fix various issues and refactor

- Fix major regression so that steps can submitted multiple times in
  case of errors.

- Don't serve the welcome page (and other pages) only once.  Show it
  until action is an taken.  This does not apply to the final step.

- Eliminate all coupling of one first boot step on another.

- Move first boot helper methods to __init__.py instead of middleware as
  it is more generic than middleware.

- Implement caching the first boot state to avoid an SQL query on every
  page load.  The down side is that if first boot state is modified in
  the backend DB outside Plinth, Plinth will need to be restarted to
  catch the modified value.

- Mark some methods as private.

- Refactor middleware code for slightly more simplicity.

- Don't show sidebar in pagekite first boot step.  Set width like other
  pages.

plinth/modules/first_boot/__init__.py

@@ -19,6 +19,11 @@
 Plinth module for first boot wizard
 """
 
+from django.urls import reverse
+import operator
+
+from plinth import module_loader
+
 version = 1
 
 is_essential = True
@@ -35,3 +40,89 @@ first_boot_steps = [
         'order': 10
     }
 ]
+
+_all_first_boot_steps = None
+
+_is_completed = None
+
+
+def is_firstboot_url(path):
+    """Return whether a path is a firstboot step URL.
+
+    :param path: path of url to be checked
+    :return: true if its a first boot URL false otherwise
+    """
+    for step in _get_steps():
+        if path.startswith(reverse(step['url'])):
+            return True
+
+    return False
+
+
+def _get_steps():
+    """Return list of all firstboot steps."""
+    global _all_first_boot_steps
+    if _all_first_boot_steps is not None:
+        return _all_first_boot_steps
+
+    steps = []
+    modules = module_loader.loaded_modules
+    for module_object in modules.values():
+        if getattr(module_object, 'first_boot_steps', None):
+            steps.extend(module_object.first_boot_steps)
+
+    _all_first_boot_steps = sorted(steps, key=operator.itemgetter('order'))
+    return _all_first_boot_steps
+
+
+def next_step():
+    """Return the resolved next first boot step URL required to go to.
+
+    If there are no more step remaining, return index page.
+    """
+    return next_step_or_none() or 'index'
+
+
+def next_step_or_none():
+    """Return the next first boot step required to run.
+
+    If there are no more step remaining, return None.
+    """
+    from plinth import kvstore
+
+    for step in _get_steps():
+        done = kvstore.get_default(step['id'], 0)
+        if not done:
+            return step.get('url')
+
+
+def mark_step_done(id):
+    """Marks the status of a first boot step as done.
+
+    :param id: id of the firstboot step
+    """
+    from plinth import kvstore
+
+    kvstore.set(id, 1)
+    if not next_step_or_none():
+        kvstore.set('setup_state', 1)
+
+
+def is_completed():
+    """Return whether first boot process is completed."""
+    from plinth import kvstore
+
+    global _is_completed
+    if _is_completed is None:
+        _is_completed = kvstore.get_default('setup_state', 0)
+
+    return _is_completed
+
+
+def set_completed():
+    """Set the first boot process as completed."""
+    from plinth import kvstore
+
+    global _is_completed
+    _is_completed = True
+    kvstore.set('setup_state', 1)

plinth/modules/first_boot/middleware.py

@@ -24,13 +24,12 @@ from django.http.response import HttpResponseRedirect
 from django.urls import reverse
 from django.conf import settings
 import logging
-from operator import itemgetter
+
-from plinth import kvstore, module_loader
+from plinth import kvstore
+from plinth.modules import first_boot
 
 LOGGER = logging.getLogger(__name__)
 
-firstboot_steps = []
-
 
 class FirstBootMiddleware(object):
     """Forward to firstboot page if firstboot isn't finished yet."""
@@ -38,80 +37,39 @@ class FirstBootMiddleware(object):
     @staticmethod
     def process_request(request):
         """Handle a request as Django middleware request handler."""
-        old_state = kvstore.get_default('firstboot_state', 0)
+        # Don't interfere with login page
-        state = kvstore.get_default('setup_state', 0)
-        if state == 0 and old_state == 10:
-            state = 1
-            kvstore.set('setup_state', 1)
-
-        user_requests_firstboot = is_firstboot(request.path)
         user_requests_login = request.path.startswith(
             reverse(settings.LOGIN_URL))
-        help_index_url = reverse('help:index')
+        if user_requests_login:
-        user_requests_help = request.path.startswith(help_index_url)
+            return
-        if not user_requests_login and not user_requests_help:
-            if state == 1 and user_requests_firstboot:
-                return HttpResponseRedirect(reverse('index'))
-            elif state == 0 and not user_requests_firstboot:
-                url = next_step()
-                return HttpResponseRedirect(reverse(url))
 
+        # Don't interfere with help pages
+        user_requests_help = request.path.startswith(reverse('help:index'))
+        if user_requests_help:
+            return
 
-def is_firstboot(path):
+        state = first_boot.is_completed()
-    """
-    Returns whether the path is a firstboot step url
-    :param path: path of current url
-    :return: true if its a first boot url false otherwise
-    """
-    steps = get_firstboot_steps()
-    for step in steps:
-        if reverse(step.get('url')) == path:
-            return True
 
-    return False
+        # Migrate from old settings variable
+        if state == 0:
+            old_state = kvstore.get_default('firstboot_state', 0)
+            if old_state == 10:
+                state = 1
+                first_boot.set_completed()
 
+        user_requests_firstboot = first_boot.is_firstboot_url(request.path)
 
-def get_firstboot_steps():
+        # Redirect to first boot if requesting normal page and first
-    """Returns all firstboot steps"""
+        # boot is not complete.
-    steps = []
+        if state == 0 and not user_requests_firstboot:
-    modules = module_loader.loaded_modules
+            next_step = first_boot.next_step_or_none()
-    for (module_name, module_object) in modules.items():
+            if next_step:
-        if getattr(module_object, 'first_boot_steps', None):
+                return HttpResponseRedirect(reverse(next_step))
-            for step in module_object.first_boot_steps:
+            else:
-                steps.append(step)
+                # No more steps in first boot
+                first_boot.set_completed()
 
-    steps = sorted(steps, key=itemgetter('order'))
+        # Redirect to index page if request firstboot after it is
-    return steps
+        # finished.
-
+        if state == 1 and user_requests_firstboot:
-
+            return HttpResponseRedirect(reverse('index'))
-def next_step():
-    """ Returns the next first boot step required to run """
-    global firstboot_steps
-    if len(firstboot_steps) == 0:
-        firstboot_steps = get_firstboot_steps()
-
-    for step in firstboot_steps:
-        done = kvstore.get_default(step.get('id'), 0)
-        if done == 0:
-            return step.get('url')
-
-
-def mark_step_done(id):
-    """
-    Marks the status of a first boot step is done
-    :param id: id of the firstboot step
-    """
-    kvstore.set(id, 1)
-    global firstboot_steps
-    if len(firstboot_steps) == 0:
-        firstboot_steps = get_firstboot_steps()
-
-    setup_done = True
-    for step in firstboot_steps:
-        done = kvstore.get_default(step.get('id'), 0)
-        if done == 0:
-            setup_done = False
-            break
-
-    if setup_done:
-        kvstore.set('setup_state', 1)

plinth/modules/first_boot/templates/firstboot_welcome.html

@@ -36,10 +36,14 @@
          alt="{{ box_name }}" width="640"/>
   </p>
 
-  <p class="text-center">
+  <form class="form text-center" method="post">
-    <a href="{% url next_url %}"
+    {% csrf_token %}
-       class="btn btn-primary btn-lg">{% trans "Start Setup" %}</a>
+
-  </p>
+    <p>
+      <input type="submit" class="btn btn-lg btn-primary"
+             value="{% trans "Start Setup" %}"/>
+    </p>
+  </form>
 
   <p class="text-center" style="font-size: larger">
     {% blocktrans trimmed %}

plinth/modules/first_boot/urls.py

@@ -22,7 +22,7 @@ URLs for the First Boot module
 from django.conf.urls import url
 from stronghold.decorators import public
 
-from .views import WelcomeView, complete
+from .views import WelcomeView, CompleteView
 
 
 urlpatterns = [
@@ -30,5 +30,5 @@ urlpatterns = [
     url(r'^firstboot/$', public(WelcomeView.as_view()), name='index'),
     url(r'^firstboot/welcome/$', public(WelcomeView.as_view()),
         name='welcome'),
-    url(r'^firstboot/complete/$', complete, name='complete'),
+    url(r'^firstboot/complete/$', CompleteView.as_view(), name='complete'),
 ]

plinth/modules/first_boot/views.py

@@ -15,12 +15,13 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 
-from django.contrib.auth.models import User
+from django import http
-from django.shortcuts import render
+from django.urls import reverse
 from django.utils.translation import ugettext as _
 from django.views.generic import TemplateView
+
 from plinth import network
-from .middleware import mark_step_done, next_step
+from plinth.modules import first_boot
 
 
 class WelcomeView(TemplateView):
@@ -28,25 +29,29 @@ class WelcomeView(TemplateView):
 
     template_name = 'firstboot_welcome.html'
 
-    def get_context_data(self, **kwargs):
+    def post(self, request, *args, **kwargs):
-        """Returns the context data for the template."""
+        """On POST, mark this step as done and move to next step."""
-        context = super(WelcomeView, self).get_context_data(**kwargs)
+        first_boot.mark_step_done('firstboot_welcome')
-        mark_step_done('firstboot_welcome')
+        return http.HttpResponseRedirect(reverse(first_boot.next_step()))
-        context['next_url'] = next_step()
-        return context
 
 
-def complete(request):
+class CompleteView(TemplateView):
     """Show summary after all firstboot setup is done.
 
     After viewing this page the firstboot module can't be accessed anymore.
     """
-    # Make sure that a user exists before finishing firstboot
-    if User.objects.all():
-        mark_step_done('firstboot_complete')
 
-    connections = network.get_connection_list()
+    template_name = 'firstboot_complete.html'
 
-    return render(request, 'firstboot_complete.html',
+    def get(self, request, *args, **kwargs):
-                  {'title': _('Setup Complete'),
+        """Mark as done as soon as page is served."""
-                   'connections': connections})
+        response = super().get(self, request, *args, **kwargs)
+        first_boot.mark_step_done('firstboot_complete')
+        return response
+
+    def get_context_data(self, **kwargs):
+        """Add network connections to context list."""
+        context = super().get_context_data(**kwargs)
+        context['connections'] = network.get_connection_list()
+        context['title'] = _('Setup Complete')
+        return context

plinth/modules/pagekite/templates/pagekite_firstboot.html

@@ -22,46 +22,44 @@
 {% load i18n %}
 {% load static %}
 
-{% block content %}
+{% block content_row %}
 
-  <h2>{% trans "Setup a freedombox.me subdomain with your voucher" %}</h2>
+  <div class="col-md-6 col-md-offset-3">
+    <h2>{% trans "Setup a freedombox.me subdomain with your voucher" %}</h2>
 
-  <p>
+    <p>
-    {% url 'first_boot:complete' as finish_firstboot_url %}
+      {% url 'pagekite:firstboot-skip' as finish_firstboot_url %}
-    {% blocktrans trimmed %}
+      {% blocktrans trimmed %}
-      <a href="{{ finish_firstboot_url }}">Skip this step</a> if you
+        <a href="{{ finish_firstboot_url }}">Skip this step</a> if you
-      do not have a voucher or want to configure PageKite later with a
+        do not have a voucher or want to configure PageKite later with a
-      different domain or credentials.
+        different domain or credentials.
-    {% endblocktrans %}
+      {% endblocktrans %}
-  </p>
+    </p>
 
-  <p>
+    <p>
-    {% blocktrans trimmed %}
+      {% blocktrans trimmed %}
-      You can use an already redeemed voucher but it will only work
+        You can use an already redeemed voucher but it will only work
-      with the initially registered subdomain.
+        with the initially registered subdomain.
-    {% endblocktrans %}
+      {% endblocktrans %}
-  </p>
+    </p>
 
-  <div class="row">
+    <form class='firstboot form-horizontal' role="form" action=""
-    <div class="col-md-8">
+          method="post">
-      <form class='firstboot form-horizontal' role="form" action=""
+      {% csrf_token %}
-            method="post">
-         {% csrf_token %}
 
-         {{ form|bootstrap_horizontal:'col-lg-3' }}
+      {{ form|bootstrap_horizontal:'col-lg-3' }}
-         <div class="form-group">
+      <div class="form-group">
-           <div class="col-sm-offset-3 col-sm-9">
+        <div class="col-sm-offset-3 col-sm-9">
-             <button type="submit" class="btn btn-primary">
+          <button type="submit" class="btn btn-primary">
-               {% trans "Register" %}
+            {% trans "Register" %}
-             </button>
+          </button>
-             <a href="{% url 'first_boot:complete' %}" class="btn btn-default"
+          <a href="{% url 'pagekite:firstboot-skip' %}" class="btn btn-default"
-                role="button">
+             role="button">
-               {% trans "Skip Registration" %}
+            {% trans "Skip Registration" %}
-             </a>
+          </a>
-           </div>
+        </div>
-         </div>
+      </div>
-      </form>
+    </form>
-    </div>
   </div>
 {% endblock %}
 

plinth/modules/pagekite/urls.py

@@ -22,7 +22,7 @@ URLs for the PageKite module
 from django.conf.urls import url
 
 from .views import StandardServiceView, CustomServiceView, ConfigurationView, \
-    DeleteServiceView, index, FirstBootView
+    DeleteServiceView, index, FirstBootView, first_boot_skip
 
 urlpatterns = [
     url(r'^sys/pagekite/$', index, name='index'),
@@ -36,4 +36,6 @@ urlpatterns = [
         name='delete-custom-service'),
     url(r'^sys/pagekite/firstboot/$', FirstBootView.as_view(),
         name='firstboot'),
+    url(r'^sys/pagekite/firstboot/skip/$', first_boot_skip,
+        name='firstboot-skip'),
 ]

plinth/modules/pagekite/views.py

@@ -16,7 +16,7 @@
 #
 
 from django.contrib import messages
-from django.http.response import HttpResponseRedirect
+from django.http import HttpResponseRedirect
 from django.template.response import TemplateResponse
 from django.urls import reverse, reverse_lazy
 from django.utils.translation import ugettext_lazy as _
@@ -26,9 +26,10 @@ from django.views.generic.edit import FormView
 from . import utils
 from .forms import ConfigurationForm, StandardServiceForm, \
     AddCustomServiceForm, DeleteCustomServiceForm, FirstBootForm
+from plinth import cfg
 from plinth.errors import DomainRegistrationError
+from plinth.modules import first_boot
 from plinth.modules import pagekite
-from plinth.modules.first_boot.middleware import mark_step_done
 
 subsubmenu = [{'url': reverse_lazy('pagekite:index'),
                'text': _('About PageKite')},
@@ -139,10 +140,13 @@ class FirstBootView(FormView):
     template_name = 'pagekite_firstboot.html'
     form_class = FirstBootForm
 
-    def get(self, *args, **kwargs):
+    def get(self, request, *args, **kwargs):
-        """Respond to GET request."""
+        """Skip if this first boot step if it is not relavent."""
-        mark_step_done('pagekite_firstboot')
+        if not cfg.danube_edition:
-        return super(FirstBootView, self).get(*args, **kwargs)
+            first_boot.mark_step_done('pagekite_firstboot')
+            return HttpResponseRedirect(reverse(first_boot.next_step()))
+
+        return super().get(request, *args, **kwargs)
 
     def form_valid(self, form):
         """Act on valid form submission."""
@@ -150,10 +154,17 @@ class FirstBootView(FormView):
             form.register_domain()
         except DomainRegistrationError as error:
             messages.error(self.request, error)
-            return HttpResponseRedirect(reverse_lazy('pagekite:firstboot'))
+            return self.form_invalid(form)
-        else:
+
-            form.setup_pagekite()
+        form.setup_pagekite()
-            message = _('Pagekite setup finished. The HTTP and HTTPS services '
+        first_boot.mark_step_done('pagekite_firstboot')
-                        'are activated now.')
+        message = _('Pagekite setup finished. The HTTP and HTTPS services '
-            messages.success(self.request, message)
+                    'are activated now.')
-            return super(FirstBootView, self).form_valid(form)
+        messages.success(self.request, message)
+        return HttpResponseRedirect(reverse(first_boot.next_step()))
+
+
+def first_boot_skip(request):
+    """Skip the first boot step."""
+    first_boot.mark_step_done('pagekite_firstboot')
+    return HttpResponseRedirect(reverse(first_boot.next_step()))

plinth/modules/users/views.py

@@ -28,9 +28,8 @@ from django.utils.translation import ugettext as _, ugettext_lazy
 from .forms import CreateUserForm, UserChangePasswordForm, UserUpdateForm, \
     FirstBootForm
 from plinth import actions
-from plinth import cfg
 from plinth.errors import ActionError
-from plinth.modules.first_boot.middleware import mark_step_done, next_step
+from plinth.modules import first_boot
 
 subsubmenu = [{'url': reverse_lazy('users:index'),
                'text': ugettext_lazy('Users')},
@@ -171,20 +170,22 @@ class UserChangePassword(ContextMixin, SuccessMessageMixin, FormView):
 class FirstBootView(django.views.generic.CreateView):
     """Create user account and log the user in."""
     template_name = 'users_firstboot.html'
+
     form_class = FirstBootForm
-    success_url = ''
-
-    def __init__(self, *args, **kwargs):
-        """Initialize the view object."""
-        if not cfg.danube_edition:
-            mark_step_done('pagekite_firstboot')
-
-        mark_step_done('users_firstboot')
-        self.success_url = next_step()
-        return super(FirstBootView, self).__init__(*args, **kwargs)
 
     def get_form_kwargs(self):
         """Make request available to the form (to insert messages)"""
         kwargs = super(FirstBootView, self).get_form_kwargs()
         kwargs['request'] = self.request
         return kwargs
+
+    def form_valid(self, form):
+        """Mark this first boot step as completed and save form."""
+        if User.objects.all():
+            first_boot.mark_step_done('users_firstboot')
+
+        return super().form_valid(form)
+
+    def get_success_url(self):
+        """Return the next first boot step after valid form submission."""
+        return reverse(first_boot.next_step())