From d3815fbe61de12688143b408e2ac1a59b18f933d Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Fri, 25 May 2018 00:38:20 -0400 Subject: [PATCH] review fixes and updates --- irc/chanserv.go | 89 +++++++++++++++++++++++++---------------------- irc/errors.go | 1 + irc/getters.go | 34 ------------------ irc/modes.go | 76 ++++++++++++++++++++++++++++++++++++++++ irc/modes_test.go | 14 ++++++++ oragono.yaml | 1 + 6 files changed, 140 insertions(+), 75 deletions(-) diff --git a/irc/chanserv.go b/irc/chanserv.go index cd6cdd6f..3decf726 100644 --- a/irc/chanserv.go +++ b/irc/chanserv.go @@ -52,8 +52,7 @@ For example, $bAMODE #channel +o dan$b grants the the holder of the "dan" account the +o operator mode every time they join #channel. To list current accounts and modes, use $bAMODE #channel$b. Note that users are always referenced by their registered account names, not their nicknames.`, - helpShort: `$bAMODE$b modifies persistent mode settings for channel members.`, - authRequired: true, + helpShort: `$bAMODE$b modifies persistent mode settings for channel members.`, }, } ) @@ -70,61 +69,69 @@ func csAmodeHandler(server *Server, client *Client, command, params string, rb * if channel == nil { csNotice(rb, client.t("Channel does not exist")) return - } - - clientAccount := client.Account() - if clientAccount == "" || clientAccount != channel.Founder() { - csNotice(rb, client.t("You must be the channel founder to use AMODE")) + } else if channel.Founder() == "" { + csNotice(rb, client.t("Channel is not registered")) return } modeChanges, unknown := modes.ParseChannelModeChanges(strings.Fields(modeChange)...) - + var change modes.ModeChange if len(modeChanges) > 1 || len(unknown) > 0 { csNotice(rb, client.t("Invalid mode change")) return + } else if len(modeChanges) == 1 { + change = modeChanges[0] + } else { + change = modes.ModeChange{Op: modes.List} } - if len(modeChanges) == 0 || modeChanges[0].Op == modes.List { - persistentModes := channel.AccountToUmode() - // sort the persistent modes in descending order of priority, i.e., - // ascending order of their index in the ChannelUserModes list - sort.Slice(persistentModes, func(i, j int) bool { - index := func(modeChange modes.ModeChange) int { - for idx, mode := range modes.ChannelUserModes { - if modeChange.Mode == mode { - return idx - } - } - return len(modes.ChannelUserModes) - } - return index(persistentModes[i]) < index(persistentModes[j]) - }) - csNotice(rb, fmt.Sprintf(client.t("Channel %s has %d persistent modes set"), channelName, len(persistentModes))) - for _, modeChange := range persistentModes { - csNotice(rb, fmt.Sprintf(client.t("Account %s receives mode +%s"), modeChange.Arg, string(modeChange.Mode))) - } - return - } - + // normalize and validate the account argument accountIsValid := false - change := modeChanges[0] - // Arg is the account name, casefold it here change.Arg, _ = CasefoldName(change.Arg) - if change.Arg != "" { - _, err := server.accounts.LoadAccount(change.Arg) - accountIsValid = (err == nil) + switch change.Op { + case modes.List: + accountIsValid = true + case modes.Add: + // if we're adding a mode, the account must exist + if change.Arg != "" { + _, err := server.accounts.LoadAccount(change.Arg) + accountIsValid = (err == nil) + } + case modes.Remove: + // allow removal of accounts that may have been deleted + accountIsValid = (change.Arg != "") } if !accountIsValid { csNotice(rb, client.t("Account does not exist")) return } - applied := channel.ApplyAccountToUmodeChange(change) - if applied { - csNotice(rb, fmt.Sprintf(client.t("Successfully set mode %s"), change.String())) - go server.channelRegistry.StoreChannel(channel, IncludeLists) - } else { - csNotice(rb, client.t("Change was a no-op")) + + affectedModes, err := channel.ProcessAccountToUmodeChange(client, change) + + if err == errInsufficientPrivs { + csNotice(rb, client.t("Insufficient privileges")) + return + } else if err != nil { + csNotice(rb, client.t("Internal error")) + return + } + + switch change.Op { + case modes.List: + // sort the persistent modes in descending order of priority + sort.Slice(affectedModes, func(i, j int) bool { + return umodeGreaterThan(affectedModes[i].Mode, affectedModes[j].Mode) + }) + csNotice(rb, fmt.Sprintf(client.t("Channel %s has %d persistent modes set"), channelName, len(affectedModes))) + for _, modeChange := range affectedModes { + csNotice(rb, fmt.Sprintf(client.t("Account %s receives mode +%s"), modeChange.Arg, string(modeChange.Mode))) + } + case modes.Add, modes.Remove: + if len(affectedModes) > 0 { + csNotice(rb, fmt.Sprintf(client.t("Successfully set mode %s"), change.String())) + } else { + csNotice(rb, client.t("Change was a no-op")) + } } } diff --git a/irc/errors.go b/irc/errors.go index fbd9f3fd..761f6a00 100644 --- a/irc/errors.go +++ b/irc/errors.go @@ -34,6 +34,7 @@ var ( errNoExistingBan = errors.New("Ban does not exist") errNoSuchChannel = errors.New("No such channel") errRenamePrivsNeeded = errors.New("Only chanops can rename channels") + errInsufficientPrivs = errors.New("Insufficient privileges") errSaslFail = errors.New("SASL failed") ) diff --git a/irc/getters.go b/irc/getters.go index f30378df..1972f53d 100644 --- a/irc/getters.go +++ b/irc/getters.go @@ -303,37 +303,3 @@ func (channel *Channel) Founder() string { defer channel.stateMutex.RUnlock() return channel.registeredFounder } - -func (channel *Channel) AccountToUmode() (result []modes.ModeChange) { - channel.stateMutex.RLock() - defer channel.stateMutex.RUnlock() - for account, mode := range channel.accountToUMode { - result = append(result, modes.ModeChange{ - Mode: mode, - Arg: account, - Op: modes.Add, - }) - } - - return -} - -func (channel *Channel) ApplyAccountToUmodeChange(change modes.ModeChange) (applied bool) { - channel.stateMutex.Lock() - defer channel.stateMutex.Unlock() - - current := channel.accountToUMode[change.Arg] - switch change.Op { - case modes.Add: - applied = (current != change.Mode) - if applied { - channel.accountToUMode[change.Arg] = change.Mode - } - case modes.Remove: - applied = (current == change.Mode) - if applied { - delete(channel.accountToUMode, change.Arg) - } - } - return -} diff --git a/irc/modes.go b/irc/modes.go index 576d0375..265f118d 100644 --- a/irc/modes.go +++ b/irc/modes.go @@ -240,3 +240,79 @@ func (channel *Channel) ApplyChannelModeChanges(client *Client, isSamode bool, c return applied } + +// tests whether l > r, in the channel-user mode ordering (e.g., Halfop > Voice) +func umodeGreaterThan(l modes.Mode, r modes.Mode) bool { + for _, mode := range modes.ChannelUserModes { + if l == mode && r != mode { + return true + } else if r == mode { + return false + } + } + return false +} + +// ProcessAccountToUmodeChange processes Add/Remove/List operations for channel persistent usermodes. +func (channel *Channel) ProcessAccountToUmodeChange(client *Client, change modes.ModeChange) (results []modes.ModeChange, err error) { + umodeGEQ := func(l modes.Mode, r modes.Mode) bool { + return l == r || umodeGreaterThan(l, r) + } + + account := client.Account() + isOperChange := client.HasRoleCapabs("chanreg") + + channel.stateMutex.Lock() + defer channel.stateMutex.Unlock() + + clientMode := channel.accountToUMode[account] + targetModeNow := channel.accountToUMode[change.Arg] + var targetModeAfter modes.Mode + if change.Op == modes.Add { + targetModeAfter = change.Mode + } + + // operators and founders can do anything + hasPrivs := isOperChange || (account != "" && account == channel.registeredFounder) + // halfop and up can list, and do add/removes at levels <= their own + if change.Op == modes.List && umodeGEQ(clientMode, modes.Halfop) { + hasPrivs = true + } else if umodeGEQ(clientMode, modes.Halfop) && umodeGEQ(clientMode, targetModeNow) && umodeGEQ(clientMode, targetModeAfter) { + hasPrivs = true + } + if !hasPrivs { + return nil, errInsufficientPrivs + } + + switch change.Op { + case modes.Add: + if targetModeNow != targetModeAfter { + channel.accountToUMode[change.Arg] = change.Mode + go client.server.channelRegistry.StoreChannel(channel, IncludeLists) + return []modes.ModeChange{change}, nil + } + return nil, nil + case modes.Remove: + if targetModeNow == change.Mode { + delete(channel.accountToUMode, change.Arg) + go client.server.channelRegistry.StoreChannel(channel, IncludeLists) + return []modes.ModeChange{change}, nil + } + return nil, nil + case modes.List: + result := make([]modes.ModeChange, len(channel.accountToUMode)) + pos := 0 + for account, mode := range channel.accountToUMode { + result[pos] = modes.ModeChange{ + Mode: mode, + Arg: account, + Op: modes.Add, + } + pos++ + } + return result, nil + default: + // shouldn't happen + return nil, errInvalidCharacter + } +} diff --git a/irc/modes_test.go b/irc/modes_test.go index c4a042b9..a929aa10 100644 --- a/irc/modes_test.go +++ b/irc/modes_test.go @@ -36,3 +36,17 @@ func TestParseDefaultChannelModes(t *testing.T) { } } } + +func TestUmodeGreaterThan(t *testing.T) { + if !umodeGreaterThan(modes.Halfop, modes.Voice) { + t.Errorf("expected Halfop > Voice") + } + + if !umodeGreaterThan(modes.Voice, modes.Mode(0)) { + t.Errorf("expected Voice > 0 (the zero value of modes.Mode)") + } + + if umodeGreaterThan(modes.ChannelAdmin, modes.ChannelAdmin) { + t.Errorf("modes should not be greater than themselves") + } +} diff --git a/oragono.yaml b/oragono.yaml index 020d5226..815b52f8 100644 --- a/oragono.yaml +++ b/oragono.yaml @@ -283,6 +283,7 @@ oper-classes: - "unregister" - "samode" - "vhosts" + - "chanreg" # ircd operators opers: