Some refactoring

cas_server/admin.py

@@ -1,42 +1,57 @@
+"""module for the admin interface of the app"""
 from django.contrib import admin
-from models import *
-from forms import *
+from .models import ServiceTicket, ProxyTicket, ProxyGrantingTicket, User, ServicePattern
+from .models import Username, ReplaceAttributName, ReplaceAttributValue, FilterAttributValue
+from .forms import TicketForm
 # Register your models here.
 
 class ServiceTicketInline(admin.TabularInline):
+    """`ServiceTicket` in admin interface"""
     model = ServiceTicket
     extra = 0
     form = TicketForm
 class ProxyTicketInline(admin.TabularInline):
+    """`ProxyTicket` in admin interface"""
     model = ProxyTicket
     extra = 0
     form = TicketForm
 class ProxyGrantingInline(admin.TabularInline):
+    """`ProxyGrantingTicket` in admin interface"""
     model = ProxyGrantingTicket
     extra = 0
     form = TicketForm
 
 class UserAdmin(admin.ModelAdmin):
+    """`User` in admin interface"""
     inlines = (ServiceTicketInline, ProxyTicketInline, ProxyGrantingInline)
 
 class UsernamesInline(admin.TabularInline):
-    model = Usernames
+    """`Username` in admin interface"""
+    model = Username
     extra = 0
 class ReplaceAttributNameInline(admin.TabularInline):
+    """`ReplaceAttributName` in admin interface"""
     model = ReplaceAttributName
     extra = 0
 class ReplaceAttributValueInline(admin.TabularInline):
+    """`ReplaceAttributValue` in admin interface"""
     model = ReplaceAttributValue
     extra = 0
 class FilterAttributValueInline(admin.TabularInline):
+    """`FilterAttributValue` in admin interface"""
     model = FilterAttributValue
     extra = 0
 
 class ServicePatternAdmin(admin.ModelAdmin):
-    inlines = (UsernamesInline, ReplaceAttributNameInline, ReplaceAttributValueInline, FilterAttributValueInline)
+    """`ServicePattern` in admin interface"""
+    inlines = (
+        UsernamesInline,
+        ReplaceAttributNameInline,
+        ReplaceAttributValueInline,
+        FilterAttributValueInline
+    )
     list_display = ('pos', 'name', 'pattern', 'proxy')
 
 
 admin.site.register(User, UserAdmin)
 admin.site.register(ServicePattern, ServicePatternAdmin)
-#admin.site.register(ProxyGrantingTicketIOU, admin.ModelAdmin)

cas_server/auth.py

@@ -1,4 +1,5 @@
 # ⁻*- coding: utf-8 -*-
+"""Some authentication classes for the CAS"""
 from django.conf import settings
 from django.contrib.auth.models import User
 try:
@@ -7,29 +8,38 @@ try:
     import crypt
 except ImportError:
     MySQLdb = None
+
 class DummyAuthUser(object):
+    """A Dummy authentication class"""
     def __init__(self, username):
         self.username = username
 
     def test_password(self, password):
+        """test `password` agains the user"""
         return False
 
     def attributs(self):
+        """return a dict of user attributes"""
         return {}
 
 
 class TestAuthUser(DummyAuthUser):
+    """A test authentication class with one user test having
+    alose test as password and some attributes"""
     def __init__(self, username):
-        self.username = username
+        super(TestAuthUser, self).__init__(username)
 
     def test_password(self, password):
+        """test `password` agains the user"""
         return self.username == "test" and password == "test"
 
     def attributs(self):
+        """return a dict of user attributes"""
         return {'nom':'Nymous', 'prenom':'Ano', 'email':'anonymous@example.net'}
 
 
 class MysqlAuthUser(DummyAuthUser):
+    """A mysql auth class: authentication user agains a mysql database"""
     user = None
     def __init__(self, username):
         mysql_config = {
@@ -49,6 +59,7 @@ class MysqlAuthUser(DummyAuthUser):
         super(MysqlAuthUser, self).__init__(username)
 
     def test_password(self, password):
+        """test `password` agains the user"""
         if not self.user:
             return False
         else:
@@ -62,13 +73,14 @@ class MysqlAuthUser(DummyAuthUser):
                     return crypt.crypt(password, self.user["password"][:2]) == self.user["password"]
 
     def attributs(self):
+        """return a dict of user attributes"""
         if not self.user:
             return {}
         else:
             return self.user
 
-        
 class DjangoAuthUser(DummyAuthUser):
+    """A django auth class: authenticate user agains django internal users"""
     user = None
     def __init__(self, username):
         try:
@@ -79,16 +91,18 @@ class DjangoAuthUser(DummyAuthUser):
 
 
     def test_password(self, password):
+        """test `password` agains the user"""
         if not self.user:
             return False
         else:
             return self.user.check_password(password)
 
     def attributs(self):
+        """return a dict of user attributes"""
         if not self.user:
             return {}
         else:
             attr = {}
             for field in self.user._meta.fields:
-                attr[field.attname]=getattr(self.user, field.attname)
+                attr[field.attname] = getattr(self.user, field.attname)
             return attr

cas_server/default_settings.py

@@ -1,7 +1,10 @@
+"""Default values for the app's settings"""
 from django.conf import settings
-import auth
+from . import auth
+
 
 def setting_default(name, default_value):
+    """if the config `name` is not set, set it the `default_value`"""
     value = getattr(settings, name, default_value)
     setattr(settings, name, value)
 
@@ -18,6 +21,7 @@ setting_default('CAS_SQL_USERNAME', '')
 setting_default('CAS_SQL_PASSWORD', '')
 setting_default('CAS_SQL_DBNAME', '')
 setting_default('CAS_SQL_DBCHARSET', 'utf8')
-setting_default('CAS_SQL_USER_QUERY', 'SELECT user AS usersame, pass AS password, users.* FROM users WHERE user = %s')
+setting_default('CAS_SQL_USER_QUERY', 'SELECT user AS usersame, pass AS ' \
+    'password, users.* FROM users WHERE user = %s')
 setting_default('CAS_SQL_PASSWORD_CHECK', 'crypt') # crypt or plain
 

cas_server/forms.py

@@ -1,12 +1,14 @@
-import default_settings
+"""forms for the app"""
+import cas_server.default_settings
 
 from django import forms
 from django.conf import settings
 from django.utils.translation import ugettext_lazy as _
 
-import models
+from . import models
 
 class UserCredential(forms.Form):
+    """Form used on the login page to retrive user credentials"""
     username = forms.CharField(label=_('login'))
     service = forms.CharField(widget=forms.HiddenInput(), required=False)
     password = forms.CharField(label=_('password'), widget=forms.PasswordInput)
@@ -22,17 +24,20 @@ class UserCredential(forms.Form):
         if auth.test_password(cleaned_data.get("password")):
             try:
                 user = models.User.objects.get(username=auth.username)
-                user.attributs=auth.attributs()
+                user.attributs = auth.attributs()
                 user.save()
             except models.User.DoesNotExist:
-                user = models.User.objects.create(username=auth.username, attributs=auth.attributs())
+                user = models.User.objects.create(
+                    username=auth.username,
+                    attributs=auth.attributs()
+                )
                 user.save()
-            self.user = user
         else:
             raise forms.ValidationError(_(u"Bad user"))
 
 
 class TicketForm(forms.ModelForm):
+    """Form for Tickets in the admin interface"""
     class Meta:
         model = models.Ticket
         exclude = []

cas_server/migrations/0012_auto_20150527_1956.py

@@ -0,0 +1,18 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import models, migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('cas_server', '0011_auto_20150523_1731'),
+    ]
+
+    operations = [
+        migrations.RenameModel(
+            old_name='Usernames',
+            new_name='Username',
+        ),
+    ]

cas_server/models.py

@@ -1,5 +1,6 @@
 # ⁻*- coding: utf-8 -*-
-import default_settings
+"""models for the app"""
+import cas_server.default_settings
 
 from django.conf import settings
 from django.db import models
@@ -16,21 +17,37 @@ import string
 from concurrent.futures import ThreadPoolExecutor
 from requests_futures.sessions import FuturesSession
 
-import utils
+from . import utils
+
 def _gen_ticket(prefix):
-    return '%s-%s' % (prefix, ''.join(random.choice(string.ascii_letters + string.digits) for _ in range(settings.CAS_ST_LEN)))
+    """Generate a ticket with prefix `prefix`"""
+    return '%s-%s' % (
+        prefix,
+        ''.join(
+            random.choice(
+                string.ascii_letters + string.digits
+            ) for _ in range(settings.CAS_ST_LEN)
+        )
+    )
 
 def _gen_st():
+    """Generate a Service Ticket"""
     return _gen_ticket('ST')
 
 def _gen_pt():
+    """Generate a Proxy Ticket"""
     return _gen_ticket('PT')
 
 def _gen_pgt():
+    """Generate a Proxy Granting Ticket"""
     return _gen_ticket('PGT')
 
+def gen_pgtiou():
+    """Generate a Proxy Granting Ticket IOU"""
+    return _gen_ticket('PGTIOU')
 
 class User(models.Model):
+    """A user logged into the CAS"""
     username = models.CharField(max_length=30, unique=True)
     attributs = PickledObjectField()
     date = models.DateTimeField(auto_now_add=True, auto_now=True)
@@ -39,6 +56,7 @@ class User(models.Model):
         return self.username
 
     def logout(self, request):
+        """Sending SSO request to all services the user logged in"""
         async_list = []
         session = FuturesSession(executor=ThreadPoolExecutor(max_workers=10))
         for ticket in ServiceTicket.objects.filter(user=self, validate=True):
@@ -53,67 +71,112 @@ class User(models.Model):
         for future in async_list:
             try:
                 future.result()
-            except Exception as e:
-                messages.add_message(request, messages.WARNING, _(u'Error during service logout %s') % e)
+            except Exception as error:
+                messages.add_message(
+                    request,
+                    messages.WARNING,
+                    _(u'Error during service logout %r') % error
+                )
 
-    def delete(self):
-        super(User, self).delete()
-
-
-    def get_ticket(self, TicketClass, service, service_pattern, renew): 
-        attributs = dict((a.name, a.replace if a.replace else a.name) for a in service_pattern.attributs.all())
-        replacements = dict((a.name, (a.pattern, a.replace)) for a in service_pattern.replacements.all())
+    def get_ticket(self, ticket_class, service, service_pattern, renew):
+        """
+           Generate a ticket using `ticket_class` for the service
+           `service` matching `service_pattern` and asking or not for
+           authentication renewal with `renew
+        """
+        attributs = dict(
+            (a.name, a.replace if a.replace else a.name) for a in service_pattern.attributs.all()
+        )
+        replacements = dict(
+            (a.name, (a.pattern, a.replace)) for a in service_pattern.replacements.all()
+        )
         service_attributs = {}
-        for (k,v) in self.attributs.items():
-            if k in attributs:
-                if k in replacements:
-                    v = re.sub(replacements[k][0], replacements[k][1], v)
-                service_attributs[attributs[k]] = v
-        ticket = TicketClass.objects.create(user=self, attributs = service_attributs, service=service, renew=renew, service_pattern=service_pattern)
+        for (key, value) in self.attributs.items():
+            if key in attributs:
+                if key in replacements:
+                    value = re.sub(replacements[key][0], replacements[key][1], value)
+                service_attributs[attributs[key]] = value
+        ticket = ticket_class.objects.create(
+            user=self,
+            attributs=service_attributs,
+            service=service,
+            renew=renew,
+            service_pattern=service_pattern
+        )
         ticket.save()
         return ticket
 
     def get_service_url(self, service, service_pattern, renew):
+        """Return the url to which the user must be redirected to
+        after a Service Ticket has been generated"""
         ticket = self.get_ticket(ServiceTicket, service, service_pattern, renew)
         url = utils.update_url(service, {'ticket':ticket.value})
         return url
 
 class BadUsername(Exception):
+    """Exception raised then an non allowed username
+    try to get a ticket for a service"""
     pass
 class BadFilter(Exception):
+    """"Exception raised then a user try
+    to get a ticket for a service and do not reach a condition"""
     pass
+
 class UserFieldNotDefined(Exception):
+    """Exception raised then a user try to get a ticket for a service
+    using as username an attribut not present on this user"""
     pass
 class ServicePattern(models.Model):
+    """Allowed services pattern agains services are tested to"""
     class Meta:
         ordering = ("pos", )
 
     pos = models.IntegerField(default=100)
-    name = models.CharField(max_length=255, unique=True, blank=True, null=True, help_text="Un nom pour le service")
+    name = models.CharField(
+        max_length=255,
+        unique=True,
+        blank=True,
+        null=True,
+        help_text="Un nom pour le service"
+    )
     pattern = models.CharField(max_length=255, unique=True)
-    user_field = models.CharField(max_length=255, default="", blank=True, help_text="Nom de l'attribut transmit comme username, vide = login")
-    #usernames = models.CharField(max_length=255, default="", blank=True, help_text="Liste d'utilisateurs acceptés séparé par des virgules, vide = tous les utilisateur")
-    #attributs = models.CharField(max_length=255, default="", blank=True, help_text="Liste des nom d'attributs à transmettre au service, séparé par une virgule. vide = aucun")
-    restrict_users = models.BooleanField(default=False, help_text="Limiter les utilisateur autorisé a se connecté a ce service à celle ci-dessous")
-    proxy = models.BooleanField(default=False,  help_text="Un ProxyGrantingTicket peut être délivré au service pour s'authentifier en temps que l'utilisateur sur d'autres services")
-    #filter = models.CharField(max_length=255, default="", blank=True, help_text="Une lambda fonction pour filtrer sur les utilisateur où leurs attribut, arg1: username, arg2:attrs_dict. vide = pas de filtre")
+    user_field = models.CharField(
+        max_length=255,
+        default="",
+        blank=True,
+        help_text="Nom de l'attribut transmit comme username, vide = login"
+    )
+    restrict_users = models.BooleanField(
+        default=False,
+        help_text="Limiter les utilisateur autorisé a se connecté a ce service à celle ci-dessous"
+    )
+    proxy = models.BooleanField(
+        default=False,
+        help_text="Un ProxyGrantingTicket peut être délivré au service pour " \
+        "s'authentifier en temps que l'utilisateur sur d'autres services"
+    )
 
     def __unicode__(self):
         return u"%s: %s" % (self.pos, self.pattern)
 
     def check_user(self, user):
+        """Check if `user` if allowed to use theses services"""
         if self.restrict_users and not self.usernames.filter(value=user.username):
             raise BadUsername()
-        for f in self.filters.all():
-            if isinstance(user.attributs[f.attribut], list):
-                l = user.attributs[f.attribut]
+        for filtre in self.filters.all():
+            if isinstance(user.attributs[filtre.attribut], list):
+                attrs = user.attributs[filtre.attribut]
             else:
-                l = [user.attributs[f.attribut]]
-            for v in l:
-                if re.match(f.pattern, str(v)):
+                attrs = [user.attributs[filtre.attribut]]
+            for value in attrs:
+                if re.match(filtre.pattern, str(value)):
                     break
             else:
-                raise BadFilter('%s do not match %s %s' % (f.pattern, f.attribut, user.attributs[f.attribut]) )
+                raise BadFilter('%s do not match %s %s' % (
+                    filtre.pattern,
+                    filtre.attribut,
+                    user.attributs[filtre.attribut]
+                ))
         if self.user_field and not user.attributs.get(self.user_field):
             raise UserFieldNotDefined()
         return True
@@ -121,20 +184,35 @@ class ServicePattern(models.Model):
 
     @classmethod
     def validate(cls, service):
-        for s in cls.objects.all().order_by('pos'):
-            if re.match(s.pattern, service):
-                return s
+        """Check if a Service Patern match `service` and
+        return it, else raise `ServicePattern.DoesNotExist`"""
+        for service_pattern in cls.objects.all().order_by('pos'):
+            if re.match(service_pattern.pattern, service):
+                return service_pattern
         raise cls.DoesNotExist()
 
-class Usernames(models.Model):
+class Username(models.Model):
+    """A list of allowed usernames on a service pattern"""
     value = models.CharField(max_length=255)
     service_pattern = models.ForeignKey(ServicePattern, related_name="usernames")
 
+    def __unicode__(self):
+        return self.value
+
 class ReplaceAttributName(models.Model):
+    """A list of replacement of attributs name for a service pattern"""
     class Meta:
         unique_together = ('name', 'replace', 'service_pattern')
-    name = models.CharField(max_length=255, help_text=u"nom d'un attributs à transmettre au service")
-    replace = models.CharField(max_length=255, blank=True, help_text=u"nom sous lequel l'attribut sera présenté au service. vide = inchangé")
+    name = models.CharField(
+        max_length=255,
+        help_text=u"nom d'un attributs à transmettre au service"
+    )
+    replace = models.CharField(
+        max_length=255,
+        blank=True,
+        help_text=u"nom sous lequel l'attribut sera présenté " \
+        u"au service. vide = inchangé"
+    )
     service_pattern = models.ForeignKey(ServicePattern, related_name="attributs")
 
     def __unicode__(self):
@@ -144,17 +222,35 @@ class ReplaceAttributName(models.Model):
             return u"%s → %s" % (self.name, self.replace)
 
 class FilterAttributValue(models.Model):
-    attribut = models.CharField(max_length=255, help_text=u"Nom de l'attribut devant vérifier pattern")
-    pattern = models.CharField(max_length=255, help_text=u"Une expression régulière")
+    """A list of filter on attributs for a service pattern"""
+    attribut = models.CharField(
+        max_length=255,
+        help_text=u"Nom de l'attribut devant vérifier pattern"
+    )
+    pattern = models.CharField(
+        max_length=255,
+        help_text=u"Une expression régulière"
+    )
     service_pattern = models.ForeignKey(ServicePattern, related_name="filters")
 
     def __unicode__(self):
         return u"%s %s" % (self.attribut, self.pattern)
 
 class ReplaceAttributValue(models.Model):
-    attribut = models.CharField(max_length=255, help_text=u"Nom de l'attribut dont la valeur doit être modifié")
-    pattern = models.CharField(max_length=255, help_text=u"Une expression régulière de ce qui doit être modifié")
-    replace = models.CharField(max_length=255, blank=True, help_text=u"Par quoi le remplacer, les groupes sont capturé par \\1, \\2 …")
+    """Replacement to apply on attributs values for a service pattern"""
+    attribut = models.CharField(
+        max_length=255,
+        help_text=u"Nom de l'attribut dont la valeur doit être modifié"
+    )
+    pattern = models.CharField(
+        max_length=255,
+        help_text=u"Une expression régulière de ce qui doit être modifié"
+    )
+    replace = models.CharField(
+        max_length=255,
+        blank=True,
+        help_text=u"Par quoi le remplacer, les groupes sont capturé par \\1, \\2 …"
+    )
     service_pattern = models.ForeignKey(ServicePattern, related_name="replacements")
 
     def __unicode__(self):
@@ -162,6 +258,7 @@ class ReplaceAttributValue(models.Model):
 
 
 class Ticket(models.Model):
+    """Generic class for a Ticket"""
     class Meta:
         abstract = True
     user = models.ForeignKey(User, related_name="%(class)s")
@@ -173,33 +270,59 @@ class Ticket(models.Model):
     renew = models.BooleanField(default=False)
 
     def __unicode__(self):
-        return u"%s: %s %s" % (self.user, self.value, self.service)
+        return u"Ticket(%s, %s)" % (self.user, self.service)
 
     def logout(self, request, session):
-        #if self.validate:
+        """Send a SSO request to the ticket service"""
+        if self.validate:
             xml = """<samlp:LogoutRequest xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol"
      ID="%(id)s" Version="2.0" IssueInstant="%(datetime)s">
     <saml:NameID xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"></saml:NameID>
     <samlp:SessionIndex>%(ticket)s</samlp:SessionIndex>
-  </samlp:LogoutRequest>""" % {'id' : os.urandom(20).encode("hex"), 'datetime' : int(time.time()), 'ticket': self.value}
+  </samlp:LogoutRequest>""" % \
+            {
+                'id' : os.urandom(20).encode("hex"),
+                'datetime' : int(time.time()),
+                'ticket': self.value
+            }
             headers = {'Content-Type': 'text/xml'}
             try:
-                return session.post(self.service.encode('utf-8'), data=xml.encode('utf-8'), headers=headers)
-            except Exception as e:
-                messages.add_message(request, messages.WARNING, _(u'Error during service logout %(service)s:\n%(error)s') % {'service': self.service, 'error':e})
+                return session.post(
+                    self.service.encode('utf-8'),
+                    data=xml.encode('utf-8'),
+                    headers=headers
+                )
+            except Exception as error:
+                messages.add_message(
+                    request,
+                    messages.WARNING,
+                    _(u'Error during service logout %(service)s:\n%(error)s') %
+                    {'service': self.service, 'error':error}
+                )
 
 class ServiceTicket(Ticket):
+    """A Service Ticket"""
     value = models.CharField(max_length=255, default=_gen_st, unique=True)
+    def __unicode__(self):
+        return u"ServiceTicket(%s, %s, %s)" % (self.user, self.value, self.service)
 class ProxyTicket(Ticket):
+    """A Proxy Ticket"""
     value = models.CharField(max_length=255, default=_gen_pt, unique=True)
+    def __unicode__(self):
+        return u"ProxyTicket(%s, %s, %s)" % (self.user, self.value, self.service)
 class ProxyGrantingTicket(Ticket):
+    """A Proxy Granting Ticket"""
     value = models.CharField(max_length=255, default=_gen_pgt, unique=True)
-#class ProxyGrantingTicketIOU(Ticket):
-#    value = models.CharField(max_length=255, default=lambda:_gen_ticket('PGTIOU'), unique=True)
+    def __unicode__(self):
+        return u"ProxyGrantingTicket(%s, %s, %s)" % (self.user, self.value, self.service)
 
 class Proxy(models.Model):
+    """A list of proxies on `ProxyTicket`"""
     class Meta:
         ordering = ("-pk", )
     url = models.CharField(max_length=255)
     proxy_ticket = models.ForeignKey(ProxyTicket, related_name="proxies")
 
+    def __unicode__(self):
+        return self.url
+

cas_server/urls.py

@@ -1,19 +1,21 @@
 # ⁻*- coding: utf-8 -*-
+"""urls for the app"""
 from django.conf.urls import patterns, url
 from django.views.generic import RedirectView
 
-import views
+from . import views
 
-urlpatterns = patterns('',
+urlpatterns = patterns(
+    '',
     url(r'^$', RedirectView.as_view(pattern_name="login")),
     url('^login$', views.login, name='login'),
     url('^logout$', views.logout, name='logout'),
     url('^validate$', views.validate, name='validate'),
-    url('^serviceValidate$', views.serviceValidate, name='serviceValidate'),
-    url('^proxyValidate$', views.proxyValidate, name='proxyValidate'),
+    url('^serviceValidate$', views.service_validate, name='serviceValidate'),
+    url('^proxyValidate$', views.proxy_validate, name='proxyValidate'),
     url('^proxy$', views.proxy, name='proxy'),
-    url('^p3/serviceValidate$', views.p3_serviceValidate, name='p3_serviceValidate'),
-    url('^p3/proxyValidate$', views.p3_proxyValidate, name='p3_proxyValidate'),
-    url('^samlValidate$', views.samlValidate, name='samlValidate'),
+    url('^p3/serviceValidate$', views.p3_service_validate, name='p3_serviceValidate'),
+    url('^p3/proxyValidate$', views.p3_proxy_validate, name='p3_proxyValidate'),
+    url('^samlValidate$', views.saml_validate, name='samlValidate'),
 )
 

cas_server/utils.py

@@ -1,7 +1,9 @@
+"""Some util function for the app"""
 import urlparse
 import urllib
 
 def update_url(url, params):
+    """update params in the `url` query string"""
     url_parts = list(urlparse.urlparse(url))
     query = dict(urlparse.parse_qsl(url_parts[4]))
     query.update(params)

cas_server/views.py

@@ -1,38 +1,50 @@
 # ⁻*- coding: utf-8 -*-
-import default_settings
+"""views for the app"""
+import cas_server.default_settings
 
 from django.shortcuts import render, redirect
-from django.http import HttpResponse, StreamingHttpResponse
+from django.http import HttpResponse, HttpResponseRedirect
 from django.conf import settings
 from django.contrib import messages
 from django.views.decorators.csrf import csrf_exempt
 from django.utils.translation import ugettext as _
 from django.core.urlresolvers import reverse
+from django.utils import timezone
 
 import requests
 import urllib
-from datetime import datetime, timedelta
 from lxml import etree
+from datetime import timedelta
 
-import utils
-import forms
-import models
+from . import utils
+from . import forms
+from . import models
 
 def _logout(request):
-    try: del request.session["authenticated"]
-    except KeyError: pass
-    try: del request.session["username"]
-    except KeyError: pass
-    try: del request.session["warn"]
-    except KeyError: pass
+    """Clean sessions variables"""
+    try:
+        del request.session["authenticated"]
+    except KeyError:
+        pass
+    try:
+        del request.session["username"]
+    except KeyError:
+        pass
+    try:
+        del request.session["warn"]
+    except KeyError:
+        pass
 
 
-def redirect_params(url_name, params={}):
-    url = reverse(url_name, args = args)
-    params = urllib.urlencode(params)
+def redirect_params(url_name, params=None):
+    """Redirect to `url_name` with `params` as querystring"""
+    url = reverse(url_name)
+    params = urllib.urlencode(params if params else {})
     return HttpResponseRedirect(url + "?%s" % params)
 
+
 def login(request):
+    """credential requestor / acceptor"""
     user = None
     form = None
     service_pattern = None
@@ -45,7 +57,10 @@ def login(request):
         method = request.POST.get('method')
 
         if not request.session.get("authenticated") or renew:
-            form = forms.UserCredential(request.POST, initial={'service':service,'method':method,'warn':request.session.get("warn")})
+            form = forms.UserCredential(
+                request.POST,
+                initial={'service':service, 'method':method, 'warn':request.session.get("warn")}
+            )
             if form.is_valid():
                 user = models.User.objects.get(username=form.cleaned_data['username'])
                 request.session.set_expiry(0)
@@ -63,10 +78,13 @@ def login(request):
         method = request.GET.get('method')
 
         if not request.session.get("authenticated") or renew:
-            form = forms.UserCredential(initial={'service':service,'method':method,'warn':request.session.get("warn")})
+            form = forms.UserCredential(
+                initial={'service':service, 'method':method, 'warn':request.session.get("warn")}
+            )
 
     # if authenticated and successfully renewed authentication if needed
-    if request.session.get("authenticated") and request.session.get("username") and (not renew or renewed):
+    if request.session.get("authenticated") and \
+    request.session.get("username") and (not renew or renewed):
         try:
             user = models.User.objects.get(username=request.session["username"])
         except models.User.DoesNotExist:
@@ -80,20 +98,51 @@ def login(request):
                 service_pattern = models.ServicePattern.validate(service)
                 # is the current user allowed on this service
                 service_pattern.check_user(user)
-                # if the user has asked to be warned before any login to a service (no transparent SSO)
+                # if the user has asked to be warned before any login to a service
                 if request.session.get("warn", True) and not warned:
-                    messages.add_message(request, messages.WARNING, _(u"Authentication has been required by service %(name)s (%(url)s)") % {'name':service_pattern.name, 'url':service})
-                    return render(request, settings.CAS_WARN_TEMPLATE, {'service_ticket_url':user.get_service_url(service, service_pattern, renew=renew)})
+                    messages.add_message(
+                        request,
+                        messages.WARNING,
+                        _(u"Authentication has been required by service %(name)s (%(url)s)") % \
+                        {'name':service_pattern.name, 'url':service}
+                    )
+                    return render(
+                        request,
+                        settings.CAS_WARN_TEMPLATE,
+                        {'service_ticket_url':user.get_service_url(
+                            service,
+                            service_pattern,
+                            renew=renew
+                        )}
+                    )
                 else:
-                    return redirect(user.get_service_url(service, service_pattern, renew=renew)) # redirect, using method ?
+                    # redirect, using method ?
+                    return redirect(user.get_service_url(service, service_pattern, renew=renew))
             except models.ServicePattern.DoesNotExist:
-                messages.add_message(request, messages.ERROR, _(u'Service %(url)s non allowed.') % {'url' : service})
+                messages.add_message(
+                    request,
+                    messages.ERROR,
+                    _(u'Service %(url)s non allowed.') % {'url' : service}
+                )
             except models.BadUsername:
-                messages.add_message(request, messages.ERROR, _(u"Username non allowed"))
+                messages.add_message(
+                    request,
+                    messages.ERROR,
+                    _(u"Username non allowed")
+                )
             except models.BadFilter:
-                messages.add_message(request, messages.ERROR, _(u"User charateristics non allowed"))
+                messages.add_message(
+                    request,
+                    messages.ERROR,
+                    _(u"User charateristics non allowed")
+                )
             except models.UserFieldNotDefined:
-                messages.add_message(request, messages.ERROR, _(u"The attribut %(field)s is needed to use that service") % {'field':service_pattern.user_field})
+                messages.add_message(
+                    request,
+                    messages.ERROR,
+                    _(u"The attribut %(field)s is needed to use" \
+                       " that service") % {'field':service_pattern.user_field}
+                )
 
             # if gateway is set and auth failed redirect to the service without authentication
             if gateway:
@@ -106,17 +155,32 @@ def login(request):
             try:
                 service_pattern = models.ServicePattern.validate(service)
                 if gateway:
-                    list(messages.get_messages(request)) # clean messages before leaving the django app
+                    list(messages.get_messages(request)) # clean messages before leaving django
                     return redirect(service)
                 if request.session.get("authenticated") and renew:
-                    messages.add_message(request, messages.WARNING, _(u"Authentication renewal required by service %(name)s (%(url)s).") % {'name':service_pattern.name, 'url':service})
+                    messages.add_message(
+                        request,
+                        messages.WARNING,
+                        _(u"Authentication renewal required by service" \
+                           " %(name)s (%(url)s).") % {'name':service_pattern.name, 'url':service}
+                    )
                 else:
-                    messages.add_message(request, messages.WARNING, _(u"Authentication required by service %(name)s (%(url)s).") % {'name':service_pattern.name, 'url':service})
+                    messages.add_message(
+                        request,
+                        messages.WARNING,
+                        _(u"Authentication required by service" \
+                           " %(name)s (%(url)s).") % {'name':service_pattern.name, 'url':service}
+                    )
             except models.ServicePattern.DoesNotExist:
-                messages.add_message(request, messages.ERROR, _(u'Service %s non allowed') % service)
+                messages.add_message(
+                    request,
+                    messages.ERROR,
+                    _(u'Service %s non allowed') % service
+                )
         return render(request, settings.CAS_LOGIN_TEMPLATE, {'form':form})
 
 def logout(request):
+    """destroy CAS session (logout)"""
     service = request.GET.get('service')
     if request.session.get("authenticated"):
         user = models.User.objects.get(username=request.session["username"])
@@ -133,12 +197,19 @@ def logout(request):
         return redirect("login")
 
 def validate(request):
+    """service ticket validation"""
     service = request.GET.get('service')
     ticket = request.GET.get('ticket')
     renew = True if request.GET.get('renew') else False
     if service and ticket:
         try:
-            ticket = models.ServiceTicket.objects.get(value=ticket, service=service, validate=False, renew=renew, creation__gt=(datetime.now() - timedelta(seconds=settings.CAS_TICKET_VALIDITY)))
+            ticket = models.ServiceTicket.objects.get(
+                value=ticket,
+                service=service,
+                validate=False,
+                renew=renew,
+                creation__gt=(timezone.now() - timedelta(seconds=settings.CAS_TICKET_VALIDITY))
+            )
             ticket.validate = True
             ticket.save()
             return HttpResponse("yes\n", content_type="text/plain")
@@ -148,118 +219,269 @@ def validate(request):
         return HttpResponse("no\n", content_type="text/plain")
 
 
-def psValidate(request, typ=['ST']):
+def ps_validate(request, ticket_type=None):
+    """factorization for serviceValidate and proxyValidate"""
+    if ticket_type is None:
+        ticket_type = ['ST']
     service = request.GET.get('service')
     ticket = request.GET.get('ticket')
-    pgtUrl = request.GET.get('pgtUrl')
+    pgt_url = request.GET.get('pgtUrl')
     renew = True if request.GET.get('renew') else False
     if service and ticket:
-        for t in typ:
-            if ticket.startswith(t):
+        for typ in ticket_type:
+            if ticket.startswith(typ):
                 break
         else:
-            return render(request, "cas_server/serviceValidateError.xml", {'code':'INVALID_TICKET'}, content_type="text/xml; charset=utf-8")
+            return render(
+                request,
+                "cas_server/serviceValidateError.xml",
+                {'code':'INVALID_TICKET'},
+                content_type="text/xml; charset=utf-8"
+            )
         try:
             proxies = []
             if ticket.startswith("ST"):
-                ticket = models.ServiceTicket.objects.get(value=ticket, service=service, validate=False, renew=renew, creation__gt=(datetime.now() - timedelta(seconds=settings.CAS_TICKET_VALIDITY)))
+                ticket = models.ServiceTicket.objects.get(
+                    value=ticket,
+                    service=service,
+                    validate=False,
+                    renew=renew,
+                    creation__gt=(timezone.now() - timedelta(seconds=settings.CAS_TICKET_VALIDITY))
+                )
             elif ticket.startswith("PT"):
-                ticket = models.ProxyTicket.objects.get(value=ticket, service=service, validate=False, renew=renew, creation__gt=(datetime.now() - timedelta(seconds=settings.CAS_TICKET_VALIDITY)))
-                for p in ticket.proxies.all():
-                    proxies.append(p.url)
+                ticket = models.ProxyTicket.objects.get(
+                    value=ticket,
+                    service=service,
+                    validate=False,
+                    renew=renew,
+                    creation__gt=(timezone.now() - timedelta(seconds=settings.CAS_TICKET_VALIDITY))
+                )
+                for prox in ticket.proxies.all():
+                    proxies.append(prox.url)
             ticket.validate = True
             ticket.save()
             attributes = []
             for key, value in ticket.attributs.items():
                 if isinstance(value, list):
-                    for v in value:
-                        attributes.append((key, v))
+                    for elt in value:
+                        attributes.append((key, elt))
                 else:
                     attributes.append((key, value))
             params = {'username':ticket.user.username, 'attributes':attributes, 'proxies':proxies}
-            if ticket.service_pattern.user_field and ticket.user.attributs.get(ticket.service_pattern.user_field):
+            if ticket.service_pattern.user_field and \
+            ticket.user.attributs.get(ticket.service_pattern.user_field):
                 params['username'] = ticket.user.attributs.get(ticket.service_pattern.user_field)
-            if pgtUrl and pgtUrl.startswith("https://"):
-                pattern = models.ServicePattern.validate(pgtUrl)
+            if pgt_url and pgt_url.startswith("https://"):
+                pattern = models.ServicePattern.validate(pgt_url)
                 if pattern.proxy:
-                    proxyid = models._gen_ticket('PGTIOU')
-                    pticket = models.ProxyGrantingTicket.objects.create(user=ticket.user, service=pgtUrl, service_pattern=pattern)
-                    url = utils.update_url(pgtUrl, {'pgtIou':proxyid, 'pgtId':pticket.value})
+                    proxyid = models.gen_pgtiou()
+                    pticket = models.ProxyGrantingTicket.objects.create(
+                        user=ticket.user,
+                        service=pgt_url,
+                        service_pattern=pattern
+                    )
+                    url = utils.update_url(pgt_url, {'pgtIou':proxyid, 'pgtId':pticket.value})
                     try:
-                        r = requests.get(url, verify=settings.CAS_PROXY_CA_CERTIFICATE_PATH)
-                        if r.status_code == 200:
+                        ret = requests.get(url, verify=settings.CAS_PROXY_CA_CERTIFICATE_PATH)
+                        if ret.status_code == 200:
                             params['proxyGrantingTicket'] = proxyid
                         else:
                             pticket.delete()
-                        return render(request, "cas_server/serviceValidate.xml", params, content_type="text/xml; charset=utf-8")
+                        return render(
+                            request,
+                            "cas_server/serviceValidate.xml",
+                            params,
+                            content_type="text/xml; charset=utf-8"
+                        )
                     except requests.exceptions.SSLError:
-                        return render(request, "cas_server/serviceValidateError.xml", {'code':'INVALID_PROXY_CALLBACK'}, content_type="text/xml; charset=utf-8")
+                        return render(
+                            request,
+                            "cas_server/serviceValidateError.xml",
+                            {'code':'INVALID_PROXY_CALLBACK'},
+                            content_type="text/xml; charset=utf-8"
+                        )
                 else:
-                    return render(request, "cas_server/serviceValidateError.xml", {'code':'INVALID_PROXY_CALLBACK'}, content_type="text/xml; charset=utf-8")
+                    return render(
+                        request,
+                        "cas_server/serviceValidateError.xml",
+                        {'code':'INVALID_PROXY_CALLBACK'},
+                        content_type="text/xml; charset=utf-8"
+                    )
             else:
-                return render(request, "cas_server/serviceValidate.xml", params, content_type="text/xml; charset=utf-8")
-        except (models.ServiceTicket.DoesNotExist, models.ProxyTicket.DoesNotExist, models.ServicePattern.DoesNotExist):
-            return render(request, "cas_server/serviceValidateError.xml", {'code':'INVALID_TICKET'}, content_type="text/xml; charset=utf-8")
+                return render(
+                    request,
+                    "cas_server/serviceValidate.xml",
+                    params,
+                    content_type="text/xml; charset=utf-8"
+                )
+        except (models.ServiceTicket.DoesNotExist, models.ProxyTicket.DoesNotExist):
+            return render(
+                request,
+                "cas_server/serviceValidateError.xml",
+                {'code':'INVALID_TICKET'},
+                content_type="text/xml; charset=utf-8"
+            )
+        except models.ServicePattern.DoesNotExist:
+            return render(
+                request,
+                "cas_server/serviceValidateError.xml",
+                {'code':'INVALID_TICKET'},
+                content_type="text/xml; charset=utf-8"
+            )
     else:
-        return render(request, "cas_server/serviceValidateError.xml", {'code':'INVALID_REQUEST'}, content_type="text/xml; charset=utf-8")
+        return render(
+            request,
+            "cas_server/serviceValidateError.xml",
+            {'code':'INVALID_REQUEST'},
+            content_type="text/xml; charset=utf-8"
+        )
 
-def serviceValidate(request):
-    return psValidate(request)
-def proxyValidate(request):
-    return psValidate(request, ["ST", "PT"])
+def service_validate(request):
+    """service ticket validation CAS 2.0 (also work for CAS 3.0)"""
+    return ps_validate(request)
+def proxy_validate(request):
+    """service/proxy ticket validation CAS 2.0 (also work for CAS 3.0)"""
+    return ps_validate(request, ["ST", "PT"])
 
 def proxy(request):
+    """proxy ticket service"""
     pgt = request.GET.get('pgt')
-    targetService = request.GET.get('targetService')
-    if pgt and targetService:
+    target_service = request.GET.get('targetService')
+    if pgt and target_service:
         try:
-            pattern = models.ServicePattern.validate(targetService)
-            ticket = models.ProxyGrantingTicket.objects.get(value=pgt, creation__gt=(datetime.now() - timedelta(seconds=settings.CAS_TICKET_VALIDITY)))
+            # is the target service allowed
+            pattern = models.ServicePattern.validate(target_service)
+            # is the proxy granting ticket valid
+            ticket = models.ProxyGrantingTicket.objects.get(
+                value=pgt,
+                creation__gt=(timezone.now() - timedelta(seconds=settings.CAS_TICKET_VALIDITY))
+            )
+            # is the pgt user allowed on the target service
             pattern.check_user(ticket.user)
-            pticket = ticket.user.get_ticket(models.ProxyTicket, targetService, pattern, False)
+            pticket = ticket.user.get_ticket(models.ProxyTicket, target_service, pattern, False)
             pticket.proxies.create(url=ticket.service)
-            return render(request, "cas_server/proxy.xml", {'ticket':pticket.value}, content_type="text/xml; charset=utf-8")
-        except (models.ProxyGrantingTicket.DoesNotExist, models.ServicePattern.DoesNotExist, models.BadUsername, models.BadFilter):
-            return render(request, "cas_server/serviceValidateError.xml", {'code':'INVALID_TICKET'}, content_type="text/xml; charset=utf-8")
+            return render(
+                request,
+                "cas_server/proxy.xml",
+                {'ticket':pticket.value},
+                content_type="text/xml; charset=utf-8"
+            )
+        except models.ProxyGrantingTicket.DoesNotExist:
+            return render(
+                request,
+                "cas_server/serviceValidateError.xml",
+                {'code':'INVALID_TICKET'},
+                content_type="text/xml; charset=utf-8"
+            )
+        except models.ServicePattern.DoesNotExist:
+            return render(
+                request,
+                "cas_server/serviceValidateError.xml",
+                {'code':'INVALID_TICKET'},
+                content_type="text/xml; charset=utf-8"
+            )
+        except models.BadUsername:
+            return render(
+                request,
+                "cas_server/serviceValidateError.xml",
+                {'code':'INVALID_TICKET'},
+                content_type="text/xml; charset=utf-8"
+            )
+        except models.BadFilter:
+            return render(
+                request,
+                "cas_server/serviceValidateError.xml",
+                {'code':'INVALID_TICKET'},
+                content_type="text/xml; charset=utf-8"
+            )
+        except models.UserFieldNotDefined:
+            return render(
+                request,
+                "cas_server/serviceValidateError.xml",
+                {'code':'INVALID_TICKET'},
+                content_type="text/xml; charset=utf-8"
+            )
     else:
-        return render(request, "cas_server/serviceValidateError.xml", {'code':'INVALID_REQUEST'}, content_type="text/xml; charset=utf-8")
+        return render(
+            request,
+            "cas_server/serviceValidateError.xml",
+            {'code':'INVALID_REQUEST'},
+            content_type="text/xml; charset=utf-8"
+        )
 
-def p3_serviceValidate(request):
-    return serviceValidate(request)
+def p3_service_validate(request):
+    """service ticket validation CAS 3.0"""
+    return service_validate(request)
 
-def p3_proxyValidate(request):
-    return proxyValidate(request)
+def p3_proxy_validate(request):
+    """service/proxy ticket validation CAS 3.0"""
+    return proxy_validate(request)
 
 @csrf_exempt
-def samlValidate(request):
+def saml_validate(request):
+    """checks the validity of a Service Ticket by a SAML 1.1 request"""
     if request.method == 'POST':
         target = request.GET.get('TARGET')
         root = etree.fromstring(request.body)
         try:
             auth_req = root.getchildren()[1].getchildren()[0]
-            IssueInstant = auth_req.attrib['IssueInstant']
-            RequestID = auth_req.attrib['RequestID']
+            issue_instant = auth_req.attrib['IssueInstant']
+            request_id = auth_req.attrib['RequestID']
             ticket = auth_req.getchildren()[0].text
-            ticket = models.ServiceTicket.objects.get(value=ticket, service=target, validate=False, creation__gt=(datetime.now() - timedelta(seconds=settings.CAS_TICKET_VALIDITY)))
+            ticket = models.ServiceTicket.objects.get(
+                value=ticket,
+                service=target,
+                validate=False,
+                creation__gt=(timezone.now() - timedelta(seconds=settings.CAS_TICKET_VALIDITY))
+            )
             ticket.validate = True
             ticket.save()
-            expireInstant = (ticket.creation + timedelta(seconds=settings.CAS_TICKET_VALIDITY)).isoformat()
+            expire_instant = (ticket.creation + \
+            timedelta(seconds=settings.CAS_TICKET_VALIDITY)).isoformat()
             attributes = []
             for key, value in ticket.attributs.items():
                 if isinstance(value, list):
-                    for v in value:
-                        attributes.append((key, v))
+                    for elt in value:
+                        attributes.append((key, elt))
                 else:
                     attributes.append((key, value))
-            params = {'IssueInstant':IssueInstant, 'expireInstant':expireInstant,'Recipient':target, 'ResponseID':RequestID, 'username':ticket.user.username, 'attributes':attributes}
-            if ticket.service_pattern.user_field and ticket.user.attributs.get(ticket.service_pattern.user_field):
+            params = {
+                'IssueInstant':issue_instant,
+                'expireInstant':expire_instant,
+                'Recipient':target,
+                'ResponseID':request_id,
+                'username':ticket.user.username,
+                'attributes':attributes
+            }
+            if ticket.service_pattern.user_field and \
+            ticket.user.attributs.get(ticket.service_pattern.user_field):
                 params['username'] = ticket.user.attributs.get(ticket.service_pattern.user_field)
-            return render(request, "cas_server/samlValidate.xml", params, content_type="text/xml; charset=utf-8")
+            return render(
+                request,
+                "cas_server/samlValidate.xml",
+                params,
+                content_type="text/xml; charset=utf-8"
+            )
         except IndexError:
-            return render(request, "cas_server/samlValidateError.xml", {'code':'VersionMismatch'}, content_type="text/xml; charset=utf-8")
+            return render(
+                request,
+                "cas_server/samlValidateError.xml",
+                {'code':'VersionMismatch'},
+                content_type="text/xml; charset=utf-8"
+            )
         except KeyError:
-            return render(request, "cas_server/samlValidateError.xml", {'code':'VersionMismatch'}, content_type="text/xml; charset=utf-8")
+            return render(
+                request,
+                "cas_server/samlValidateError.xml",
+                {'code':'VersionMismatch'},
+                content_type="text/xml; charset=utf-8"
+            )
         except models.ServiceTicket.DoesNotExist:
-            return render(request, "cas_server/samlValidateError.xml", {'code':'AuthnFailed'}, content_type="text/xml; charset=utf-8")
+            return render(
+                request,
+                "cas_server/samlValidateError.xml",
+                {'code':'AuthnFailed'},
+                content_type="text/xml; charset=utf-8"
+            )
     else:
         return redirect("login")