Skip to content

Commit

Permalink
allow case insensitive login, add login tests
Browse files Browse the repository at this point in the history
The 2024 Tech AARs asked for case insensitive login, as we already have
with Clubhouse. I don't totally love this, because the specification
for SMTP clearly says
"The local-part of a mailbox MUST BE treated as case sensitive"
even though most mail servers treat it as case insensitive in practice.

My preference would be for us to only treat the part after the "@" as
case insensitive, but whatever we do, it should align with Clubhouse.

https://www.ietf.org/rfc/rfc2821.txt
  • Loading branch information
srabraham committed Feb 22, 2025
1 parent c930a0f commit 3511e9d
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 16 deletions.
43 changes: 29 additions & 14 deletions src/ims/directory/clubhouse_db/_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
from ims.directory import IMSDirectory, IMSGroupID, IMSTeamID, IMSUser, userFromRanger
from ims.model import Ranger

from ._dms import DutyManagementSystem
from ._dms import DutyManagementSystem, Position, Team


__all__ = ()
Expand All @@ -48,22 +48,37 @@ async def personnel(self) -> Iterable[Ranger]:

async def lookupUser(self, searchTerm: str) -> IMSUser | None:
dms = self._dms
# call out to a more easily testable static method
return DMSDirectory._lookupUser(

Check warning on line 52 in src/ims/directory/clubhouse_db/_directory.py

View check run for this annotation

Codecov / codecov/patch

src/ims/directory/clubhouse_db/_directory.py#L52

Added line #L52 was not covered by tests
searchTerm,
tuple(await dms.personnel()),
tuple(await dms.positions()),
tuple(await dms.teams()),
)

# FIXME: a hash would be better (eg. rangersByHandle)
rangers = tuple(await dms.personnel())

for ranger in rangers:
if ranger.handle == searchTerm:
@staticmethod
def _lookupUser(
searchTerm: str,
rangers: tuple[Ranger, ...],
positions: tuple[Position, ...],
teams: tuple[Team, ...],
) -> IMSUser | None:
searchLower = searchTerm.lower()

ranger = None

for r in rangers:
if r.handle.lower() == searchLower:
ranger = r
for email in r.email:
if email.lower() == searchLower:
ranger = r
if ranger is not None:
break
else:
for ranger in rangers:
if searchTerm in ranger.email:
break
else:
return None

positions = tuple(await dms.positions())
teams = tuple(await dms.teams())
return None

assert ranger is not None

groups = tuple(
IMSGroupID(position.name)
Expand Down
77 changes: 75 additions & 2 deletions src/ims/directory/clubhouse_db/test/test_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,52 @@
Tests for L{ims.directory.clubhouse_db._directory}.
"""

from ims.directory import IMSUser
from ims.directory.clubhouse_db import DMSDirectory
from ims.directory.clubhouse_db._dms import Position, Team
from ims.ext.trial import TestCase
from ims.model import Ranger, RangerStatus


__all__ = ()


def _ranger_alpha() -> Ranger:
return Ranger(
handle="Alpha",
status=RangerStatus.active,
email=frozenset(["alpha@example.com"]),
onsite=False,
directoryID=None,
)


def _ranger_beta() -> Ranger:
return Ranger(
handle="Beta",
status=RangerStatus.active,
email=frozenset(["beta@example.com"]),
onsite=True,
directoryID=None,
)


def _position_delta() -> Position:
return Position(
positionID="ddd",
name="Delta",
members={_ranger_alpha()},
)


def _team_upsilon() -> Team:
return Team(
teamID="uuu",
name="Upsilon",
members={_ranger_beta()},
)


class DMSDirectoryTests(TestCase):
"""
Tests for :class:`DMSDirectory`
Expand All @@ -35,6 +75,39 @@ def test_personnel(self) -> None:
test_personnel.todo = "unimplemented" # type: ignore[attr-defined]

def test_lookupUser(self) -> None:
raise NotImplementedError()
def lookup(search: str) -> IMSUser | None:
return DMSDirectory._lookupUser(
search,
(_ranger_alpha(), _ranger_beta()),
(_position_delta(),),
(_team_upsilon(),),
)

# Case-insensitive matching against handles and email addresses
self.assertIsNotNone(lookup("alpha"))
self.assertIsNotNone(lookup("Alpha"))
self.assertIsNotNone(lookup("beta@example.com"))
self.assertIsNotNone(lookup("ALPHA@exAMple.com"))

# Failures to match against handle or email address
self.assertIsNone(lookup("NotARanger@example.com"))
self.assertIsNone(lookup("BetaWithSuffix"))

test_lookupUser.todo = "unimplemented" # type: ignore[attr-defined]
# Now check the various fields that come back, including
# positions and teams set up at the top of this test file.
alpha = lookup("alpha")
beta = lookup("BETA@EXAMPLE.COM")
self.assertIsNotNone(alpha)
# to appease mypy
assert alpha is not None
self.assertEqual(alpha.uid, "Alpha")
self.assertEqual(alpha.shortNames, ("Alpha",))
self.assertEqual(alpha.onsite, False)
self.assertEqual(alpha.groups, ("Delta",))
self.assertEqual(alpha.teams, ())
self.assertIsNotNone(beta)
# to appease mypy
assert beta is not None
self.assertEqual(beta.onsite, True)
self.assertEqual(beta.groups, ())
self.assertEqual(beta.teams, ("Upsilon",))

0 comments on commit 3511e9d

Please sign in to comment.