Skip to content

Commit

Permalink
remove ** permissions from IMS, because they don't work
Browse files Browse the repository at this point in the history
This feature didn't actually work as of now, because the incident
and field report pages required user authentication. That made this
** ACLing pointless, because it was all about granting access when
there was no authenticated user. We might as well just remove this
sort of ACL, rather than have it in here, but have it broken.

More details in attached screenshot in this PR. It would take some
different architecture to make this work.
  • Loading branch information
srabraham committed Jan 22, 2025
1 parent 76df4fb commit dff3d9c
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 32 deletions.
16 changes: 9 additions & 7 deletions src/ims/auth/_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -336,19 +336,21 @@ def _matchACL(self, user: IMSUser | None, acl: Iterable[AccessEntry]) -> bool:
Match a user against a set of ACLs associated with an event's readers,
writers and reporters.
An ACL of "**" will always match, even for the None user.
An ACL of "*" matches all users other than the None user.
An ACL of "*" matches any authenticated user.
An ACL of the form "person:{user}" will match a user of one of the
user's short names equals {user}.
An ACL of the form "position:{group}" will match a user if the ID of
one of the groups that the user is a member of equals {group}.
"""

if "**" in [a.expression for a in acl]:
return True
# This form of wildcarding was previously intended to allow access to anyone,
# including the None user. This permitted non-Rangers (i.e. unauthenticated
# users) to create Field Reports at kiosks on-site. This feature hadn't been
# used for years, as of 2025, and it no longer actually works anyway, due to
# the authorization model that developed in recent years in IMS.
# if "**" in [a.expression for a in acl]:
# return True

if user is None:
return False
Expand All @@ -360,7 +362,7 @@ def _matchACL(self, user: IMSUser | None, acl: Iterable[AccessEntry]) -> bool:

assert a.validity == AccessValidity.always

if "*" in a.expression:
if a.expression == "*":
return True

for shortName in user.shortNames:
Expand Down
22 changes: 3 additions & 19 deletions src/ims/auth/test/test_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -551,37 +551,21 @@ def test_matchACL_none_user(self, user: IMSUser) -> None:

def test_matchACL_public_noUser(self) -> None:
"""
AuthProvider._matchACL matches public ("**") access with None user.
AuthProvider._matchACL does not match public ("**") access with None user,
because that sort of access isn't permitted in IMS anymore.
"""
provider = AuthProvider(
store=self.store(),
directory=self.directory(),
jsonWebKey=JSONWebKey.generate(),
)

self.assertTrue(
self.assertFalse(
provider._matchACL(
None, (AccessEntry(expression="**", validity=AccessValidity.always),)
)
)

@given(testUsers())
def test_matchACL_public_user(self, user: IMSUser) -> None:
"""
AuthProvider._matchACL matches public ("**") access with a user.
"""
provider = AuthProvider(
store=self.store(),
directory=self.directory(),
jsonWebKey=JSONWebKey.generate(),
)

self.assertTrue(
provider._matchACL(
user, (AccessEntry(expression="**", validity=AccessValidity.always),)
)
)

def test_matchACL_any_noUser(self) -> None:
"""
AuthProvider._matchACL does not match any ("*") access with None user.
Expand Down
9 changes: 3 additions & 6 deletions src/ims/element/static/admin_events.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,12 +145,9 @@ function addAccess(sender) {

if (newExpression === "**") {
const confirmed = confirm(
"DANGER: double-wildcard '**' ACLs can permit access to any requestor, even " +
"those who aren't logged in! You probably don't want this, except maybe for " +
"local testing.\n\n" +
"By comparison, a single-wildcard '*' ACL grants access to any " +
"authenticated user. You might be looking for that instead.\n\n" +
"Proceed with firing footgun?"
"Double-wildcard '**' ACLs are no longer supported, so this ACL will have " +
"no effect.\n\n" +
"Proceed with doing something pointless?"
);
if (!confirmed) {
sender.value = "";
Expand Down

0 comments on commit dff3d9c

Please sign in to comment.