Skip to content
GitLab
Unverified Commit b84b9263 authored by Enrico Zini's avatar Enrico Zini
Browse files

Use Inconsistency tracking in CheckLDAPConsistency. Fixes:

parents bd61f3ca 769cc8c9
Hide whitespace changes
Inline Side-by-side
.gitlab-ci.yml
View file @ b84b9263
......@@ -15,7 +15,6 @@
gnupg
debian-keyring
python3-git
python3-testfixtures
libjs-bootstrap4
fonts-fork-awesome
libjs-jquery-datatables
......
dsa/housekeeping.py
View file @ b84b9263
......@@ -6,6 +6,7 @@ from django.utils.timezone import now
from backend.housekeeping import MakeLink, Housekeeper
from . import udldap
from backend import const
from sitechecks.models import Inconsistency
import backend.models as bmodels
import process.models as pmodels
import backend.ops as bops
......@@ -140,8 +141,10 @@ class CheckLDAPConsistency(hk.Task):
fpr = entry.single("keyFingerPrint")
if fpr:
log.warn("%s: %s has fingerprint %s and gid %s in LDAP, but is not in our db",
self.IDENTIFIER, entry.uid, fpr, entry.single("gidNumber"))
Inconsistency.objects.found(
self.IDENTIFIER,
f"{entry.uid} has fingerprint {fpr} and gid {entry.single('gidNumber')} in LDAP,"
" but is not in our db")
else:
ldap_fields_args = {
"cn": entry.single("cn"),
......@@ -199,9 +202,12 @@ class CheckLDAPConsistency(hk.Task):
elif not entry.is_dd:
# mom040267 has a locked account (see the FD comments on the site)
if person.ldap_fields.uid != "mom040267":
log.warn("%s: %s has accountStatus '%s' (comment: %s) but in our db the state is %s",
self.IDENTIFIER, self.hk.link(person), entry.single("accountStatus"),
entry.single("accountComment"), const.ALL_STATUS_DESCS[person.status])
Inconsistency.objects.found(
self.IDENTIFIER,
f"person has accountStatus '{entry.single('accountStatus')}'"
f" (comment: {entry.single('accountComment')})"
f" but in our db the state is {const.ALL_STATUS_DESCS[person.status]}",
person=person)
if entry.is_dd:
# if person.status not in (const.STATUS_REMOVED_DD, const.STATUS_EMERITUS_DD):
......@@ -210,26 +216,36 @@ class CheckLDAPConsistency(hk.Task):
if person.status_changed > self.email_forwarding_cutoff:
if dsa_status == "retiring" and person.status != const.STATUS_EMERITUS_DD:
log.warn("%s: %s has accountStatus '%s' (comment: %s) but in our db the state is %s [retiring]",
self.IDENTIFIER, self.hk.link(person), entry.single("accountStatus"),
entry.single("accountComment"), const.ALL_STATUS_DESCS[person.status])
Inconsistency.objects.found(
self.IDENTIFIER,
f"person has accountStatus '{entry.single('accountStatus')}'"
f" (comment: {entry.single('accountComment')})"
f" but in our db the state is {const.ALL_STATUS_DESCS[person.status]} [retiring]",
person=person)
if dsa_status == "inactive" and person.status != const.STATUS_REMOVED_DD:
log.warn("%s: %s has accountStatus '%s' (comment: %s) but in our db the state is %s [inactive]",
self.IDENTIFIER, self.hk.link(person), entry.single("accountStatus"),
entry.single("accountComment"), const.ALL_STATUS_DESCS[person.status])
Inconsistency.objects.found(
self.IDENTIFIER,
f"person has accountStatus '{entry.single('accountStatus')}'"
f" (comment: {entry.single('accountComment')})"
f" but in our db the state is {const.ALL_STATUS_DESCS[person.status]} [inactive]",
person=person)
if dsa_status == "locked":
parsed = entry.single("accountStatus").split()
if len(parsed) == 1:
log.warn("%s: %s has accountStatus '%s' (comment: %s) locked with no date",
self.IDENTIFIER, self.hk.link(person), entry.single("accountStatus"),
entry.single("accountComment"))
Inconsistency.objects.found(
self.IDENTIFIER,
f"person has accountStatus '{entry.single('accountStatus')}'"
f"(comment: {entry.single('accountComment')}) locked with no date",
person=person)
else:
if dsa_status_date is not None and dsa_status_date < self.email_forwarding_cutoff.date():
log.warn("%s: %s has accountStatus '%s' (comment: %s) locked for longer than 6 months",
self.IDENTIFIER, self.hk.link(person), entry.single("accountStatus"),
entry.single("accountComment"))
Inconsistency.objects.found(
self.IDENTIFIER,
f"person has accountStatus '{entry.single('accountStatus')}'"
f" (comment: {entry.single('accountComment')}) locked for longer than 6 months",
person=person)
def run_main(self, stage):
# Prefetch people and index them by uid
......@@ -253,15 +269,19 @@ class CheckLDAPConsistency(hk.Task):
try:
dsa_status_date = datetime.datetime.strptime(parsed[1], "%Y-%m-%d").date()
except ValueError:
log.warn("%s: %s has accountStatus '%s' (comment: %s) with unparsable date",
self.IDENTIFIER, self.hk.link(person), entry.single("accountStatus"),
entry.single("accountComment"))
Inconsistency.objects.found(
self.IDENTIFIER,
"person has accountStatus '{entry.single('accountStatus')}'"
" (comment: {entry.single('accountComment')}) with unparsable date",
person=person)
dsa_status_date = None
else:
dsa_status_date = None
if dsa_status not in ("retiring", "inactive", "memorial", "locked", "renamed"):
log.warn("%s: %s has unknown accountStatus %s",
self.IDENTIFIER, self.hk.link(person), entry.single("accountStatus"))
Inconsistency.objects.found(
self.IDENTIFIER,
"person has unknown accountStatus {entry.single('accountStatus')}",
person=person)
else:
self.check_account_status(person, entry, dsa_status, dsa_status_date)
else:
......@@ -269,14 +289,18 @@ class CheckLDAPConsistency(hk.Task):
# DDs we don't expect
if entry.is_dd:
if person.status not in (const.STATUS_DD_U, const.STATUS_DD_NU):
log.warn("%s: %s has supplementaryGid 'Debian', but in our db the state is %s",
self.IDENTIFIER, self.hk.link(person),
const.ALL_STATUS_DESCS[person.status])
Inconsistency.objects.found(
self.IDENTIFIER,
"person has supplementaryGid 'Debian',"
f" but in our db the state is {const.ALL_STATUS_DESCS[person.status]}",
person=person)
else:
if person.status in (const.STATUS_DD_U, const.STATUS_DD_NU):
log.warn("%s: %s has no supplementaryGid 'Debian', but in our db the state is %s",
self.IDENTIFIER, self.hk.link(person),
const.ALL_STATUS_DESCS[person.status])
Inconsistency.objects.found(
self.IDENTIFIER,
"person has no supplementaryGid 'Debian',"
f" but in our db the state is {const.ALL_STATUS_DESCS[person.status]}",
person=person)
email = entry.single("emailForward")
if email != person.ldap_fields.email:
......
dsa/tests/test_housekeeping.py
View file @ b84b9263
......@@ -5,8 +5,8 @@ from dsa.housekeeping import CheckLDAPConsistency
from backend.models import Person
import backend.const as const
from process.models import Process
from sitechecks.models import Inconsistency
from unittest import mock
from testfixtures import LogCapture
class MockHousekeeper:
......@@ -16,6 +16,12 @@ class MockHousekeeper:
def link(self, person):
return str(person.ldap_fields.uid or person.pk)
def run(self, cls, stage: str = "main"):
task = cls(self)
task.IDENTIFIER = f"test.{cls.__name__}"
getattr(task, f"run_{stage}")(None)
return task
class MockEntry:
def __init__(self, uid, **attrs):
......@@ -34,16 +40,27 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase):
super().setUp()
self.hk = MockHousekeeper(self.persons.oldam)
def assertInconsistenciesEqual(self, items):
res = list(Inconsistency.objects.all())
self.assertEqual(len(res), len(items))
for inc, item in zip(res, items):
if "person" in item:
self.assertEqual(inc.person, item["person"])
if "text" in item:
self.assertEqual(inc.text, item["text"])
def test_new_removed(self):
with mock.patch("dsa.udldap.list_people") as m:
m.return_value = [
MockEntry("newdd", supplementaryGid=["Debian"], cn="tcn",
mn="tmn", sn="tsn", emailForward="test@example.org", accountStatus=None)
]
with LogCapture() as lc:
task = CheckLDAPConsistency(self.hk)
task.run_main(None)
lc.check(("dsa.housekeeping", "WARNING", "None: newdd: created to mirror a removed DD account from LDAP"))
with self.assertLogs() as log:
self.hk.run(CheckLDAPConsistency)
self.assertEqual(log.output, [
"WARNING:dsa.housekeeping:test.CheckLDAPConsistency:"
" newdd: created to mirror a removed DD account from LDAP",
])
p = Person.objects.get(ldap_fields__uid="newdd")
audit = ["{}:{}".format(l.author.lookup_key, l.notes) for l in p.audit_log.all()]
......@@ -64,11 +81,11 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase):
MockEntry("newguest", supplementaryGid=[], cn="tcn", mn="tmn",
sn="tsn", emailForward="test@example.org", accountStatus=None)
]
with LogCapture() as lc:
task = CheckLDAPConsistency(self.hk)
task.run_main(None)
lc.check(("dsa.housekeeping", "WARNING",
"None: newguest: created to mirror a removed guest account from LDAP"))
with self.assertLogs() as log:
self.hk.run(CheckLDAPConsistency)
self.assertEqual(log.output, [
"WARNING:dsa.housekeeping:test.CheckLDAPConsistency:"
" newguest: created to mirror a removed guest account from LDAP"])
p = Person.objects.get(ldap_fields__uid="newguest")
audit = ["{}:{}".format(l.author.lookup_key, l.notes) for l in p.audit_log.all()]
......@@ -90,12 +107,12 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase):
emailForward="test@example.org", keyFingerPrint="66B4DFB68CB24EBBD8650BC4F4B4B0CC797EBFAB",
accountStatus=None)
]
with LogCapture() as lc:
task = CheckLDAPConsistency(self.hk)
task.run_main(None)
lc.check(("dsa.housekeeping", "WARNING",
"None: newdd has fingerprint 66B4DFB68CB24EBBD8650BC4F4B4B0CC797EBFAB and gid 800 in LDAP,"
" but is not in our db"))
self.hk.run(CheckLDAPConsistency)
self.assertInconsistenciesEqual([
{"person": None,
"text": "newdd has fingerprint 66B4DFB68CB24EBBD8650BC4F4B4B0CC797EBFAB and gid 800 in LDAP,"
" but is not in our db"},
])
with self.assertRaises(Person.DoesNotExist):
Person.objects.get(ldap_fields__uid="newdd")
......@@ -106,16 +123,15 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase):
MockEntry("dd_u", supplementaryGid=["Debian"], cn="tcn",
mn="tmn", sn="tsn", emailForward="test@example.org", accountStatus=None)
]
with LogCapture() as lc:
task = CheckLDAPConsistency(self.hk)
task.run_main(None)
lc.check(
("dsa.housekeeping", "INFO", "None: dd_u changing email_ldap from dd_u@example.org to test@example.org"
" (source: LDAP)"),
('dsa.housekeeping', 'INFO', 'None: dd_u changing cn from Dd_u to tcn (source: LDAP)'),
('dsa.housekeeping', 'INFO', 'None: dd_u changing mn from to tmn (source: LDAP)'),
('dsa.housekeeping', 'INFO', 'None: dd_u changing sn from to tsn (source: LDAP)'),
)
with self.assertLogs() as log:
self.hk.run(CheckLDAPConsistency)
self.assertEqual(log.output, [
"INFO:dsa.housekeeping:test.CheckLDAPConsistency: dd_u changing email_ldap from dd_u@example.org"
" to test@example.org (source: LDAP)",
'INFO:dsa.housekeeping:test.CheckLDAPConsistency: dd_u changing cn from Dd_u to tcn (source: LDAP)',
'INFO:dsa.housekeeping:test.CheckLDAPConsistency: dd_u changing mn from to tmn (source: LDAP)',
'INFO:dsa.housekeeping:test.CheckLDAPConsistency: dd_u changing sn from to tsn (source: LDAP)',
])
p = self.persons.dd_u
p.refresh_from_db()
......@@ -137,12 +153,10 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase):
MockEntry("dm", supplementaryGid=["Debian"], cn="Dm", mn="", sn="", emailForward="dm@example.org",
keyFingerPrint="66B4DFB68CB24EBBD8650BC4F4B4B0CC797EBFAB", accountStatus=None)
]
with self.assertLogs() as log:
task = CheckLDAPConsistency(self.hk)
task.run_main(None)
self.assertEqual(log.output, [
"WARNING:dsa.housekeeping:"
"None: dm has supplementaryGid 'Debian', but in our db the state is Debian Maintainer",
self.hk.run(CheckLDAPConsistency)
self.assertInconsistenciesEqual([
{"person": self.persons.dm,
"text": "person has supplementaryGid 'Debian', but in our db the state is Debian Maintainer"},
])
p = self.persons.dm
......@@ -156,13 +170,13 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase):
emailForward="dd_u@example.org", keyFingerPrint="66B4DFB68CB24EBBD8650BC4F4B4B0CC797EBFAB",
accountStatus="inactive 2018-03-20"),
]
with self.assertLogs() as log:
task = CheckLDAPConsistency(self.hk)
task.run_main(None)
self.assertEqual(log.output, [
"WARNING:dsa.housekeeping:"
"None: dd_u has accountStatus 'inactive 2018-03-20' (comment: None)"
" but in our db the state is Debian Developer, uploading [inactive]",
self.hk.run(CheckLDAPConsistency)
self.assertInconsistenciesEqual([
{"person": self.persons.dd_u,
"text":
"person has accountStatus 'inactive 2018-03-20' (comment: None)"
" but in our db the state is Debian Developer, uploading [inactive]"}
])
p = self.persons.dd_u
......@@ -177,12 +191,14 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase):
keyFingerPrint="66B4DFB68CB24EBBD8650BC4F4B4B0CC797EBFAB",
accountStatus="inactive 2018-03-20", accountComment="RT#1234"),
]
with self.assertLogs() as log:
task = CheckLDAPConsistency(self.hk)
task.run_main(None)
self.assertEqual(log.output, [
"WARNING:dsa.housekeeping:None: dd_u has accountStatus 'inactive 2018-03-20' "
'(comment: RT#1234) but in our db the state is Debian Developer, uploading [inactive]'])
self.hk.run(CheckLDAPConsistency)
self.assertInconsistenciesEqual([
{"person": self.persons.dd_u,
"text":
"person has accountStatus 'inactive 2018-03-20' "
'(comment: RT#1234) but in our db the state is Debian Developer, uploading [inactive]'}
])
p = self.persons.dd_u
p.refresh_from_db()
......@@ -196,23 +212,21 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase):
MockEntry("dd_u", supplementaryGid=["Debian"], cn="Dd_u", mn="", sn="",
emailForward="dd_u@example.org", accountStatus="retiring 2018-03-20"),
]
with self.assertLogs() as log:
task = CheckLDAPConsistency(self.hk)
task.run_main(None)
self.assertEqual(log.output, [
"WARNING:dsa.housekeeping:None: dd_u has accountStatus 'retiring 2018-03-20' "
'(comment: None) but in our db the state is Debian Developer, uploading '
'[retiring]',
self.hk.run(CheckLDAPConsistency)
self.assertInconsistenciesEqual([
{"person": self.persons.dd_u,
"text": "person has accountStatus 'retiring 2018-03-20' "
'(comment: None) but in our db the state is Debian Developer, uploading '
'[retiring]'},
])
process.approved_by = self.persons.dam
process.approved_time = now()
process.save()
with self.assertLogs() as log:
task = CheckLDAPConsistency(self.hk)
task.run_main(None)
self.hk.run(CheckLDAPConsistency)
self.assertEqual(log.output, [
"INFO:dsa.housekeeping:None: dd_u closed from dsa: retiring 2018-03-20",
"INFO:dsa.housekeeping:test.CheckLDAPConsistency: dd_u closed from dsa: retiring 2018-03-20",
])
p = self.persons.dd_u
......@@ -234,24 +248,25 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase):
emailForward="test@example.org", accountStatus="retiring 2018-03-20"),
]
with self.assertLogs() as log:
task = CheckLDAPConsistency(self.hk)
task.run_main(None)
self.hk.run(CheckLDAPConsistency)
self.assertEqual(log.output, [
"WARNING:dsa.housekeeping:None: dd_u has accountStatus 'retiring 2018-03-20' "
'(comment: None) but in our db the state is Debian Developer, uploading '
'[retiring]',
"INFO:dsa.housekeeping:None: dd_u changing email_ldap from dd_u@example.org to test@example.org"
" (source: LDAP)",
"INFO:dsa.housekeeping:test.CheckLDAPConsistency: dd_u changing email_ldap"
" from dd_u@example.org to test@example.org (source: LDAP)",
])
self.assertInconsistenciesEqual([
{"person": self.persons.dd_u,
"text": "person has accountStatus 'retiring 2018-03-20' "
'(comment: None) but in our db the state is Debian Developer, uploading '
'[retiring]'},
])
process.approved_by = self.persons.dam
process.approved_time = now()
process.save()
with self.assertLogs() as log:
task = CheckLDAPConsistency(self.hk)
task.run_main(None)
self.hk.run(CheckLDAPConsistency)
self.assertEqual(log.output, [
"INFO:dsa.housekeeping:None: dd_u closed from dsa: retiring 2018-03-20",
"INFO:dsa.housekeeping:test.CheckLDAPConsistency: dd_u closed from dsa: retiring 2018-03-20",
])
p = self.persons.dd_u
......
sitechecks/housekeeping.py
View file @ b84b9263
from __future__ import annotations
from collections import defaultdict
import logging
import django_housekeeping as hk
......
sitechecks/models.py
View file @ b84b9263
from __future__ import annotations
from django.db import models
from backend.models import Person
from process.models import Process
......
test-instance/deploy.yml
View file @ b84b9263
......@@ -48,7 +48,6 @@
- gnupg
- debian-keyring
- python3-git
- python3-testfixtures
- libjs-bootstrap4
- fonts-fork-awesome
- libjs-jquery-datatables
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment