Skip to content

Warn when a groups filter matches no users - #398

Merged
AaronAtDuo merged 2 commits into
masterfrom
warn-all-negated-groups
Jul 23, 2026
Merged

Warn when a groups filter matches no users#398
AaronAtDuo merged 2 commits into
masterfrom
warn-all-negated-groups

Conversation

@AaronAtDuo

Copy link
Copy Markdown
Contributor

Summary of the change

A groups pattern-list made up entirely of negated patterns matches no user, so the filter never selects anyone. Detect this at config load in both login_duo and pam_duo and log a warning, since the more likely intent is to require Duo for everyone except a group ("*,!group").

Adds unit coverage for the subpattern cases and login_duo integration tests for the warn/no-warn paths, plus a note in both man pages' PATTERNS sections.

Test Plan

New and existing tests pass

A groups pattern-list made up entirely of negated patterns matches no
user, so the filter never selects anyone. Detect this at config load in
both login_duo and pam_duo and log a warning, since the more likely
intent is to require Duo for everyone except a group ("*,!group").

Adds unit coverage for the subpattern cases and login_duo integration
tests for the warn/no-warn paths, plus a note in both man pages'
PATTERNS sections.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A comma-separated groups pattern-list may contain empty subpatterns
from a leading, trailing, or doubled comma (e.g. ",!wheel" or
"!wheel,,!admin"). An empty subpattern matches no group, so such a
list still matches no user. duo_groups_all_negated() previously
stopped at the first non-'!' character and so returned false for
these lists, suppressing the warning. Skip empty subpatterns instead
of treating them as a positive match, and cover both forms with
regression tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@AaronAtDuo
AaronAtDuo merged commit 050d854 into master Jul 23, 2026
3 checks passed
@AaronAtDuo
AaronAtDuo deleted the warn-all-negated-groups branch July 23, 2026 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants