Second argument to can_access_list is accidentally polymorphic in syntax

[sbp]

In server/plugins/messages.py the second argument to plugins.aaa.can_access_list, listname, is a List-Id:

    private_lists_accessible = []
    for listname in private_lists_found:
        if plugins.aaa.can_access_list(session, listname):
            private_lists_accessible.append(listname)

So it would e.g. receive <example.lists.example.org>.

But in server/endpoints/preferences.py the second argument, ml, is an email address:

            can_access = True
            if entry.get("private", True):
                can_access = plugins.aaa.can_access_list(session, ml)

So it would e.g. receive example@lists.example.com.

[Humbedooh]

Yeah, I noticed that a while back, and this does need to conform to one or the other, and only that. A quick hot-fix in the meantime for custom AAA is to do something like:

listname = "<" + listname.strip("<>").replace("@", ".") + ">"

Or the other way around, depending on what's best.

[sbp]

I fix it locally by doing this in custom AAA:

    if "@" not in second_arg:
        second_arg = second_arg.strip("<>").replace(".", "@", 1)

Which is compatible with both kinds of value, and means I don't have to patch upstream.

[sebbASF]

Is there a way to detect such issues at compile time? e.g. define specific classes for different types of string object; these classes could contain the code to convert from raw strings rather than having such code duplicated

[sebbASF]

The openapi.yaml definition currently uses list: in 3 different ways: dev

dev@lists.example.org The StatsResponse definition even has two different usages, dev@lists.example.org and dev in different parts of the response.

[sbp]

Is there a way to detect such issues at compile time?

I'm a big fan of compile time checking, and I have attempted to do string syntax differentiation in other Python projects, but my experiences brought me to the conclusion that in most cases it's not worth the effort. This is of course a highly qualitative opinion and may not apply to foal, but it's very difficult to keep the boilerplate involved to a manageable level, and the effort involved is usually enormous because strings are so ubiquitous. Runtime checks, testing, and documentation are probably still the way forward for now.

[Humbedooh]

I think a good first step is to agree on what we should send to AAA, and then ensure we always do that in the code. We can add tests as we go along later, but addressing the root of the problem should come first.