IPA-EPN: First version. #4676

fcami · 2020-05-08T13:54:40Z

EPN stands for Expiring Password Notification. It is a standalone
tool designed to build a list of users whose password would expire
in the near future, and either display the list in a machine-readable
format, or send email notifications to these users.

EPN provides command-line options to display the list of affected users.
This provides data introspection and helps understand how many emails
would be sent for a given day, or a given date range.
The command-line options can also be used by a monitoring system to alert
whenever a number of emails over the SMTP quota would be sent.

EPN is meant to be launched once a day from an IPA client (preferred)
or replica from a systemd timer.

EPN does not keep state. The list of affected users is built at runtime
but never kept.

TLS/STARTTLS SMTP code is untested and unlikely to work as-is.

Parts of code contributed by Rob Crittenden.
Ideas and feedback contributed by Christian Heimes and Michal Polovka.

Fixes: https://pagure.io/freeipa/issue/3687

rcritten · 2020-05-08T18:47:56Z

Note to self: something to be investigated:
[08/May/2020:14:46:09.221690605 -0400] conn=63 op=1 SRCH base="cn=users,cn=accounts,dc=example,dc=test" scope=2 filter="(&(!(nsAccountLock=TRUE))(krbPasswordExpiration<=20200515000000Z)(krbPasswordExpiration>=20200514000000Z))" attrs="krbPasswordExpiration mail cn uid"
[08/May/2020:14:46:09.222218146 -0400] conn=63 op=1 RESULT err=0 tag=101 nentries=1 etime=0.000655780 notes=U details="Partially Unindexed Filter

fcami · 2020-05-08T21:06:41Z

/AzurePipelines run

azure-pipelines · 2020-05-08T21:06:53Z

Azure Pipelines successfully started running 1 pipeline(s).

rcritten · 2020-05-12T17:21:20Z

While not 100% feature complete this patch represents the majority of the minimal viable product of sending expiring e-mail.

I'll squash the patches together to clean up the history but doing it now would mess up PR-CI's understanding of the test patch.

Left to do for MVP:

log rotation for epn logs (and I may rename it from ipaepn.log to ipa-epn.log for consistency
add a configurable delay between mails sent to not spam the queue
add a max number of emails to send but this will require saving state and since epn only runs once a day by default could fall behind. This may not make MVP.
enable and document TLS and STARTTLS support

flo-renaud · 2020-05-14T14:16:04Z

The azure pipeline failure should be fixed with a rebase including b6fbee53

rcritten · 2020-05-14T15:33:28Z

/AzurePipelines run

azure-pipelines · 2020-05-14T15:33:43Z

Azure Pipelines successfully started running 1 pipeline(s).

ipaclient/install/ipa_epn.py

tiran · 2020-05-15T09:47:08Z

install/share/expire_msg.template

@@ -0,0 +1,5 @@
+Hi {{ fullname }},


How about your turn the template into a RFC 822 message and parse the output of JINJA with email.message_from_string?

This would allow an admin to specify email headers including subject or custom headers in the template. I did a quick test. The parser automatically converts headers with non-ASCII char.

>>> text = """\ ... Subject: Blä ... ... Test äöü ... """ >>> print(email.message_from_string(text).as_string()) Subject: =?utf-8?q?Bl=C3=A4?= Test äöü

In the future EPN could also look at the language and locale attribute of the user and choose a language specific template (e.g. expire_msg.fr.template). With my proposal you have the subject and body in one file.

I agree that it'd be nice to have a configurable subject. Francois took the approach of having the body of the message be an attachment so the encoding is automatic. Arguably this will make it difficult for text-based readers. He did want to allow html-based e-mails which is why there is the content subtype (defaults to plain). We could add subject as an option in the config file, and may allso allow X- headers to be defined.

I'm not sure about separate templates. We don't have information on the target other than what is in LDAP and we don't currently maintain language.

tiran · 2020-05-15T10:39:51Z

ipaclient/install/ipa_epn.py

+        self._msg["From"] = formataddr(("IPA-EPN", mail_from))
+        self._msg["To"] = ", ".join(self._subscribers)
+        self._msg["Date"] = formatdate(localtime=True)
+        self._msg["Subject"] = Header(self._subject, self._charset)


I recommend to include a Message-ID. MTAs tend to dislike and block messages without a proper id. You can use email.utils.make_msgid to generate one.

postfix is generating a message-id for us:

Message-Id: 20200515145329.668C8389A20E@ipa.example.test

We are trying to be agnostic about the MTA so I suppose adding it would be fine.

rcritten · 2020-05-15T15:41:55Z

Added patch to include message-id, configurable subject and better template error handling.

rcritten · 2020-05-15T17:27:52Z

So I don't lose this, Christian suggested moving some of the options into new sections. The current ipa config only supports a single section, global. He provided a candidate patch to look in all sections.

diff --git a/ipalib/config.py b/ipalib/config.py
index 4c7ad4920..c3afbed31 100644
--- a/ipalib/config.py
+++ b/ipalib/config.py
@@ -378,13 +378,22 @@ class Env:
         if not parser.has_section(CONFIG_SECTION):
             parser.add_section(CONFIG_SECTION)
         items = parser.items(CONFIG_SECTION)
-        if len(items) == 0:
-            return 0, 0
         i = 0
         for (key, value) in items:
             if key not in self:
                 self[key] = value
                 i += 1
+
+        # also load other section and map the entries to section_key = value
+        for section in parser.sections():
+            if section == CONFIG_SECTION:
+                pass
+            for key, value in parser.items(section):
+                key = f"{section}_{key}"
+                if key not in self:
+                    self[key] = value
+                    i += 1
+
         if 'config_loaded' not in self: # we loaded at least 1 file
             self['config_loaded'] = True
         return i, len(items)

I'm not sure what if any confusion this would cause. It could be difficult to troubleshoot since all the sections are treated equally and with the ipa config reader the first one "wins".

rcritten · 2020-05-15T17:33:00Z

Things I need help with. At this point I've worked on it enough that I'm a bit biased.

The name of the log file. /var/log/ipaepn.log doesn't fit the model of ipa-*.log (see next)
Should we configure log rotation? I think probably so, but it's going to require crond and more effort from users when configuring. We could create /var/log/epn/something.log
Is the current logging reasonable enough for auditing, debugging, etc?
I fixed starttls/ssl modes and have unit tests for them but the tests can't pass because of the package installation bug. Worth pushing in the client-side changes without the tests or wait? (I'm still working on the ssl part, it's quite a lot of config to do)

A sample of an e-mail sent out via postfix is:

From root@localhost  Fri May 15 11:49:43 2020
Return-Path: <root@localhost>
X-Original-To: tuser1@example.test
Delivered-To: tuser1@example.test
Received: from ipa.example.test (localhost [IPv6:::1])
        by ipa.example.test (Postfix) with ESMTP id 644B2389A21D
        for <tuser1@example.test>; Fri, 15 May 2020 11:49:43 -0400 (EDT)
Content-Type: multipart/mixed; -charset="utf8"; boundary="===============1923913668708037366=="
MIME-Version: 1.0
From: IPA-EPN <noreply@example.test>
To: tuser1@example.test
Date: Fri, 15 May 2020 11:49:43 -0400
Subject: =?utf8?q?Your_password_will_expire_soon=2E?=
Message-Id: <158955778339.27665.7541123698243046586@ipa.example.test>
X-Mailer: IPA-EPN

Multipart message
--===============1923913668708037366==
Content-Type: text/plain; charset="utf8"
MIME-Version: 1.0
Content-Transfer-Encoding: base64

U3ViamVjdDogWW91ciBwYXNzd29yZCB3aWxsIGV4cGlyZSBzb29uClgtTWFpbGVyOiBmb28KCkhp
ICBVc2VyLApZb3VyIGxvZ2luIGVudHJ5IHR1c2VyMSBpcyBnb2luZyB0byBleHBpcmUgb24KMjAy
MC0wNS0yMiAxNDozMzo1My4gUGxlYXNlIGNoYW5nZSBpdCBzb29uLgoKWW91ciBmcmllbmRseSBu
ZWlnaGJvcmhvb2QgYWRtaW5zLgoK

--===============1923913668708037366==--

rcritten · 2020-05-20T18:07:21Z

@tiran suggested moving ipa_epn.py into its own subdirectory inside ipaclient. That results in:

ipaclient/epn/ipa_epn.py:42: [W9901(ipa-forbidden-import), ] Forbidden import ipaclient.install.client.is_ipa_client_installed (can't import from ipaclient.install in ipaclient))
ipaclient/epn/ipa_epn.py:45: [W9901(ipa-forbidden-import), ] Forbidden import ipalib.install.sysrestore (can't import from ipalib.install in ipaclient))

rcritten · 2020-05-20T19:11:05Z

I squashed from 21 to 12 patches and moved the temp commit to the top of the queue to verify that I got the rebases correct.

rcritten · 2020-05-20T20:28:56Z

Tests pass, removing temp commit per @tiran

tiran · 2020-05-22T13:13:58Z

I just realized that the PR introduces a new top level directory tools. I find it confusing to have a new tools directory for client tools while server-side tools are in install/tools.

rcritten · 2020-05-22T13:23:57Z

If desired I can move ipa-epn into client and rename to ipa-epn.in so it gets processed like the other client tools and the man pages into client/man.

ipaclient/install/ipa_epn.py

fcami · 2020-06-04T20:57:31Z

hi @tiran I think I have addressed all your concerns, and squashed commits into manageable chunks. Could you please review a final time?

rcritten · 2020-06-04T22:05:52Z

ipatests/test_integration/test_epn.py

+        except Exception:
+            # postfix isn't yet used so this shouldn't cause existing
+            # tests to fail.
+            pass


You might want to do away with this try/except. This was to workaround the PR-CI issue where packages weren't being installed properly.

Actually PR-CI is not fixed yet.
Failed run without the try/except:
http://freeipa-org-pr-ci.s3-website.eu-central-1.amazonaws.com/jobs/983787d0-a703-11ea-bfc3-fa163e401a00/report.html
There is a proposed fix from @wladich in:
freeipa/freeipa-pr-ci#363

@fcami My PR freeipa/freeipa-pr-ci#373 is not about fixing dnf issue, it is about fixing other issue (resolv.conf overwritten by NetworkManager) without breaking the dnf on master.
The dnf issue was expected to be resolved by freeipa/freeipa-pr-ci#367 which reverted my previous attempt to solve aforementioned problem with resolv.conf.

@wladich argh, I mixed things up. Do you know if the current runners are up to date?
Because that's what I get (on a client):

[ipatests.pytest_ipa.integration.host.Host.master.cmd48] Error: Error downloading packages: [ipatests.pytest_ipa.integration.host.Host.master.cmd48] Curl error (6): Couldn't resolve host name for https://mirrors.fedoraproject.org/metalink?repo=updates-released-f32&arch=x86_64 [Could not resolve host: mirrors.fedoraproject.org] [ipatests.pytest_ipa.integration.host.Host.master.cmd48] Exit code: 1 ipa: ERROR: stderr: Error: Error downloading packages: Curl error (6): Couldn't resolve host name for https://mirrors.fedoraproject.org/metalink?repo=updates-released-f32&arch=x86_64 [Could not resolve host: mirrors.fedoraproject.org]

What's strange is that ipa-healthcheck similarly installs itself during its testing and that is passing. I wonder if the timing of the package install is different between the tests (e.g. before/after ipa-server-install).

Good catch... Let me try changing that

wladich · 2020-06-05T10:54:20Z

@fcami
Unfortunately I do not know, I think we should ask @netoarmando

fcami · 2020-06-05T14:51:13Z

For reference, installing packages before installing IPA provides a good run.

client/man/epn.conf.5

client/man/ipa-epn.1

freeipa.spec.in

init/systemd/ipa-epn.service.in

init/systemd/ipa-epn.timer.in

ipaclient/install/ipa_epn.py

miskopo

Apart from aforementioned comments, nice! 😸

miskopo

Please, see inline comments

ipaclient/install/ipa_epn.py

EPN stands for Expiring Password Notification. It is a standalone tool designed to build a list of users whose password would expire in the near future, and either display the list in a machine-readable format, or send email notifications to these users. EPN provides command-line options to display the list of affected users. This provides data introspection and helps understand how many emails would be sent for a given day, or a given date range. The command-line options can also be used by a monitoring system to alert whenever a number of emails over the SMTP quota would be sent. EPN is meant to be launched once a day from an IPA client (preferred) or replica from a systemd timer. EPN does not keep state. The list of affected users is built at runtime but never kept. TLS/STARTTLS SMTP code is untested and unlikely to work as-is. Parts of code contributed by Rob Crittenden. Ideas and feedback contributed by Christian Heimes and Michal Polovka. Fixes: https://pagure.io/freeipa/issue/3687 Signed-off-by: François Cami <fcami@redhat.com> Signed-off-by: Rob Crittenden <rcritten@redhat.com>

Initial test suite for EPN. Fixes: https://pagure.io/freeipa/issue/3687 Signed-off-by: François Cami <fcami@redhat.com> Signed-off-by: Rob Crittenden <rcritten@redhat.com>

Add options for character set (default utf8) and message subtype (default plain). This will allow for more control for users to do either HTML mail or use ascii for the character set so the attachment is not base64-encoded to make it easier for all mail clients. Collect first and last name as well for each user in order to provide more options for the template engine. Make the From address configurable, defaulting to noreply@ipa_domain Make Subject configurable too. Don't rely on the MTA to set Message-Id: set it using the email module. Fixes: https://pagure.io/freeipa/issue/3687 Signed-off-by: Rob Crittenden <rcritten@redhat.com>

Expiring Password Notifications search for expiring passwords between dates. Add an equality index for this attribute. https://pagure.io/freeipa/issue/3687 Signed-off-by: Rob Crittenden <rcritten@redhat.com>

fcami · 2020-06-08T21:04:08Z

hi @miskopo I've addressed all your concerns I think.
Please let me know if I can remove the temp commit to get a complete gating run.

miskopo · 2020-06-08T21:06:07Z

@fcami it looks good, please remove the temp commit, I'll ACK in the morning.

fcami · 2020-06-08T21:09:58Z

Thanks @miskopo temp commit removed. Running gating now.

miskopo · 2020-06-09T05:58:52Z

Hi @fcami, you have a GO for take-off... I mean, pushing.

fcami · 2020-06-09T06:45:11Z

master:

b8886c3 IPA-EPN: First version.
3805eff IPA-EPN: Test suite.
03caa7f Add a jinja2 e-mail template for EPN
451cbae Add index for krbPasswordExpiration for EPN

fcami added WIP Work in progress - not ready yet for review ipa-4-8 Mark for backport to ipa 4.8 labels May 8, 2020

rcritten self-assigned this May 8, 2020

fcami mentioned this pull request May 9, 2020

Failure in dnf install during PRCI tests freeipa/freeipa-pr-ci#363

Closed

rcritten added needs review Pull Request is waiting for a review and removed WIP Work in progress - not ready yet for review labels May 12, 2020

tiran reviewed May 15, 2020

View reviewed changes

rcritten added the re-run Trigger a new run of PR-CI label May 21, 2020

freeipa-pr-ci removed the re-run Trigger a new run of PR-CI label May 21, 2020

tiran reviewed May 28, 2020

View reviewed changes

ipaclient/install/ipa_epn.py Outdated Show resolved Hide resolved

rcritten reviewed Jun 4, 2020

View reviewed changes

miskopo requested changes Jun 8, 2020

View reviewed changes

miskopo approved these changes Jun 8, 2020

View reviewed changes

miskopo self-requested a review June 8, 2020 16:10

miskopo requested changes Jun 8, 2020

View reviewed changes

fcami commented Jun 8, 2020

View reviewed changes

ipaclient/install/ipa_epn.py Show resolved Hide resolved

fcami and others added 4 commits June 8, 2020 22:59

IPA-EPN: Test suite.

0187487

Initial test suite for EPN. Fixes: https://pagure.io/freeipa/issue/3687 Signed-off-by: François Cami <fcami@redhat.com> Signed-off-by: Rob Crittenden <rcritten@redhat.com>

Add index for krbPasswordExpiration for EPN

46ee6ea

Expiring Password Notifications search for expiring passwords between dates. Add an equality index for this attribute. https://pagure.io/freeipa/issue/3687 Signed-off-by: Rob Crittenden <rcritten@redhat.com>

netoarmando mentioned this pull request Jun 9, 2020

prevent NetworkManager from updating /etc/resolv.conf on ipa client freeipa/freeipa-pr-ci#373

Closed

miskopo approved these changes Jun 9, 2020

View reviewed changes

miskopo added ack Pull Request approved, can be merged and removed needs review Pull Request is waiting for a review labels Jun 9, 2020

fcami added the pushed Pull Request has already been pushed label Jun 9, 2020

fcami closed this Jun 9, 2020

fcami mentioned this pull request Jun 9, 2020

[Backport][ipa-4-8] IPA-EPN: First version. #4793

Closed

IPA-EPN: First version. #4676

IPA-EPN: First version. #4676

Conversation

fcami commented May 8, 2020

rcritten commented May 8, 2020

fcami commented May 8, 2020

azure-pipelines bot commented May 8, 2020

rcritten commented May 12, 2020

flo-renaud commented May 14, 2020

rcritten commented May 14, 2020

azure-pipelines bot commented May 14, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rcritten commented May 15, 2020

rcritten commented May 15, 2020

rcritten commented May 15, 2020

rcritten commented May 20, 2020

rcritten commented May 20, 2020

rcritten commented May 20, 2020

tiran commented May 22, 2020

rcritten commented May 22, 2020

fcami commented Jun 4, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wladich Jun 5, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wladich commented Jun 5, 2020

fcami commented Jun 5, 2020

miskopo left a comment • edited

Choose a reason for hiding this comment

miskopo left a comment

Choose a reason for hiding this comment

fcami commented Jun 8, 2020

miskopo commented Jun 8, 2020

fcami commented Jun 8, 2020

miskopo commented Jun 9, 2020

fcami commented Jun 9, 2020

wladich Jun 5, 2020 •

edited

miskopo left a comment •

edited