Repository: sopel-irc/sopel

Issue:
url: `.urlban` privilege check is broken
### Description

Non-admin users hit a broken channel-privilege check if they try to use `.urlban`, which throws an uncaught exception.

### Reproduction steps

0. Be a non-admin user, i.e. not listed in Sopel's `core.admins` or `core.owner` settings
1. Do `.urlban *`
2. Sopel spits out an unexpected `AttributeError`

### Expected behavior

Use of the `.urlban` command by non-admins should be correctly allowed or denied based on their channel privilege level and the bot's `url.exclude_required_access` setting.

### Relevant logs

```shell
[2025-01-24 02:51:44,490] sopel.bot            ERROR    - Unexpected AttributeError (module 'sopel.privileges' has no attribute 'OP') from msv. Message was: .urlban *
Traceback (most recent call last):
  File "/home/sopel/.local/lib/python3.12/site-packages/sopel/bot.py", line 698, in call_rule
    rule.execute(sopel, trigger)
  File "/home/sopel/.local/lib/python3.12/site-packages/sopel/plugins/rules.py", line 1273, in execute
    exit_code = self._handler(bot, trigger)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/sopel/.local/lib/python3.12/site-packages/sopel/builtins/url.py", line 183, in url_ban
    if not _user_can_change_excludes(bot, trigger):
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/sopel/.local/lib/python3.12/site-packages/sopel/builtins/url.py", line 162, in _user_can_change_excludes
    if user_access >= getattr(privileges, required_access):
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: module 'sopel.privileges' has no attribute 'OP'
```

### Notes

Someone reported this bug on our IRC last night. @SnoopJ correctly deduced that it was related to the switch to `privileges.AccessLevel`. We can safely say #2540 was responsible for this breakage; as is often the case, it was my fault. I forgot about #2352 when making that change.

`sopel.privileges.names` is a new shortcut to `sopel.privileges.AccessLevel._member_names_`, which is quite a mouthful. (I wanted to create `sopel.privileges.AccessLevel.names` as a `@property`, but that doesn't work and I don't want to monkeypatch our subclass of `enum.IntFlag` too much.)

### Sopel version

8.0.1, master

### Installation method

`pip install`

### Python version

3.12.7

### Operating system

_No response_

### IRCd

_No response_

### Relevant plugins

url

Discussion:
@dgw: Note: I started working on this in [the `privilege-names` branch](https://github.com/sopel-irc/sopel/tree/privilege-names).

The solution there is adding a `sopel.privileges.names` attribute as a shortcut to `sopel.privileges.AccessLevel._member_names_`, though using the `AccessLevel` attr directly would obviously work without the shortcut.

I'm holding off on opening a PR because of unresolved CI issues, though. See #2647.@dgw: Fixed by #2650, just not correctly linked/closed automatically.

Solution patch:
diff --git a/sopel/builtins/url.py b/sopel/builtins/url.py
index 66864d2e..585de712 100644
--- a/sopel/builtins/url.py
+++ b/sopel/builtins/url.py
@@ -64,7 +64,7 @@ class UrlSection(types.StaticSection):
     """A list of regular expressions to match URLs for which the title should not be shown."""
     exclude_required_access = types.ChoiceAttribute(
         'exclude_required_access',
-        choices=privileges.__all__,
+        choices=[level.name for level in privileges.AccessLevel],
         default='OP',
     )
     """Minimum channel access level required to edit ``exclude`` list using chat commands."""
@@ -168,7 +168,7 @@ def _user_can_change_excludes(bot: SopelWrapper, trigger: Trigger) -> bool:
     channel = bot.channels[trigger.sender]
     user_access = channel.privileges[trigger.nick]
 
-    if user_access >= getattr(privileges, required_access):
+    if user_access >= getattr(privileges.AccessLevel, required_access):
         return True
 
     return False


Test patch:
diff --git a/test/builtins/test_builtins_url.py b/test/builtins/test_builtins_url.py
index eedce541..fcdabb7a 100644
--- a/test/builtins/test_builtins_url.py
+++ b/test/builtins/test_builtins_url.py
@@ -76,6 +76,22 @@ def mockbot(configfactory):
     return sopel
 
 
+PRELOADED_CONFIG = """
+[core]
+owner = testnick
+nick = TestBot
+enable =
+    coretasks
+    url
+"""
+
+
+@pytest.fixture
+def preloadedbot(configfactory, botfactory):
+    tmpconfig = configfactory('preloaded.cfg', PRELOADED_CONFIG)
+    return botfactory.preloaded(tmpconfig, ['url'])
+
+
 @pytest.mark.parametrize("site", INVALID_URLS)
 def test_find_title_invalid(site):
     # All local for invalid ones
@@ -114,3 +130,30 @@ def test_url_triggers_rules_and_auto_title(mockbot):
     labels = sorted(result[0].get_rule_label() for result in results)
     expected = ['handle_urls_https', 'title_auto']
     assert labels == expected
+
+
+@pytest.mark.parametrize('level, result', (
+    ('NOTHING', False),
+    ('VOICE', False),
+    ('HALFOP', False),
+    ('OP', True),
+    ('ADMIN', True),
+    ('OWNER', True),
+))
+def test_url_ban_privilege(preloadedbot, ircfactory, triggerfactory, level, result):
+    """Make sure the urlban command privilege check functions correctly."""
+    irc = ircfactory(preloadedbot)
+    irc.channel_joined('#test', [
+        'Unothing', 'Uvoice', 'Uhalfop', 'Uop', 'Uadmin', 'Uowner'])
+    irc.mode_set('#test', '+vhoaq', [
+        'Uvoice', 'Uhalfop', 'Uop', 'Uadmin', 'Uowner'])
+
+    nick = f'U{level.title()}'
+    user = level.lower()
+    line = f':{nick}!{user}@example.com PRIVMSG #test :.urlban *'
+    wrapper = triggerfactory.wrapper(preloadedbot, line)
+    match = triggerfactory(preloadedbot, line)
+
+    # parameter matrix assumes the default `url.exclude_required_access` config
+    # value, which was 'OP' at the time of test creation
+    assert url._user_can_change_excludes(wrapper, match) == result