Merge pull request #1102 from slingamn/issue1027_remove_timeout.2

fix #1027

irc/accounts.go

@@ -873,8 +873,6 @@ func (am *AccountManager) Verify(client *Client, account string, code string) er
 		}
 		if method == NickEnforcementStrict {
 			am.server.RandomlyRename(currentClient)
-		} else if method == NickEnforcementWithTimeout {
-			currentClient.nickTimer.Touch(nil)
 		}
 	}
 	return nil
@@ -1738,8 +1736,6 @@ func (am *AccountManager) applyVhostToClients(account string, result VHostInfo)
 func (am *AccountManager) Login(client *Client, account ClientAccount) {
 	client.Login(account)
 
-	client.nickTimer.Touch(nil)
-
 	am.applyVHostInfo(client, account.VHost)
 
 	casefoldedAccount := client.Account()
@@ -1757,7 +1753,7 @@ func (am *AccountManager) Logout(client *Client) {
 		return
 	}
 
-	am.logoutOfAccount(client)
+	client.Logout()
 
 	clients := am.accountToClients[casefoldedAccount]
 	if len(clients) <= 1 {
@@ -1934,15 +1930,3 @@ type rawClientAccount struct {
 	VHost           string
 	Settings        string
 }
-
-// logoutOfAccount logs the client out of their current account.
-// TODO(#1027) delete this entire method and just use client.Logout()
-func (am *AccountManager) logoutOfAccount(client *Client) {
-	if client.Account() == "" {
-		// already logged out
-		return
-	}
-
-	client.Logout()
-	go client.nickTimer.Touch(nil)
-}

irc/client.go

@@ -67,7 +67,6 @@ type Client struct {
 	nickCasefolded     string
 	nickMaskCasefolded string
 	nickMaskString     string // cache for nickmask string since it's used with lots of replies
-	nickTimer          NickTimer
 	oper               *Oper
 	preregNick         string
 	proxiedIP          net.IP // actual remote IP if using the PROXY protocol
@@ -604,9 +603,6 @@ func (client *Client) run(session *Session) {
 		} else {
 			client.playReattachMessages(session)
 		}
-	} else {
-		// don't reset the nick timer during a reattach
-		client.nickTimer.Initialize(client)
 	}
 
 	firstLine := !isReattach
@@ -1353,7 +1349,6 @@ func (client *Client) destroy(session *Session) {
 	client.server.clients.Remove(client)
 
 	// clean up self
-	client.nickTimer.Stop()
 	client.brbTimer.Disable()
 
 	client.server.accounts.Logout(client)

irc/config.go

@@ -320,7 +320,6 @@ const (
 	// else be fixed up by a schema change)
 	NickEnforcementOptional NickEnforcementMethod = iota
 	NickEnforcementNone
-	NickEnforcementWithTimeout
 	NickEnforcementStrict
 )
 
@@ -330,8 +329,6 @@ func nickReservationToString(method NickEnforcementMethod) string {
 		return "default"
 	case NickEnforcementNone:
 		return "none"
-	case NickEnforcementWithTimeout:
-		return "timeout"
 	case NickEnforcementStrict:
 		return "strict"
 	default:
@@ -347,8 +344,6 @@ func nickReservationFromString(method string) (NickEnforcementMethod, error) {
 		return NickEnforcementOptional, nil
 	case "none":
 		return NickEnforcementNone, nil
-	case "timeout":
-		return NickEnforcementWithTimeout, nil
 	case "strict":
 		return NickEnforcementStrict, nil
 	default:

irc/database.go

@@ -23,7 +23,7 @@ const (
 	// 'version' of the database schema
 	keySchemaVersion = "db.version"
 	// latest schema of the db
-	latestDbSchema = "11"
+	latestDbSchema = "12"
 
 	keyCloakSecret = "crypto.cloak_secret"
 )
@@ -650,6 +650,42 @@ func schemaChangeV10ToV11(config *Config, tx *buntdb.Tx) error {
 	return err
 }
 
+// #1027: NickEnforcementTimeout (2) was removed,
+// NickEnforcementStrict was 3 and is now 2
+func schemaChangeV11ToV12(config *Config, tx *buntdb.Tx) error {
+	prefix := "account.settings "
+	var accounts, rawSettings []string
+	tx.AscendGreaterOrEqual("", prefix, func(key, value string) bool {
+		if !strings.HasPrefix(key, prefix) {
+			return false
+		}
+		account := strings.TrimPrefix(key, prefix)
+		accounts = append(accounts, account)
+		rawSettings = append(rawSettings, value)
+		return true
+	})
+
+	for i, account := range accounts {
+		var settings AccountSettings
+		err := json.Unmarshal([]byte(rawSettings[i]), &settings)
+		if err != nil {
+			log.Printf("corrupt account settings entry for %s: %v\n", account, err)
+			continue
+		}
+		// upgrade NickEnforcementTimeout (which was 2) to NickEnforcementStrict (currently 2),
+		// fix up the old value of NickEnforcementStrict (3) to the current value (2)
+		if int(settings.NickEnforcement) == 3 {
+			settings.NickEnforcement = NickEnforcementMethod(2)
+			text, err := json.Marshal(settings)
+			if err != nil {
+				return err
+			}
+			tx.Set(prefix+account, string(text), nil)
+		}
+	}
+	return nil
+}
+
 func init() {
 	allChanges := []SchemaChange{
 		{
@@ -702,6 +738,11 @@ func init() {
 			TargetVersion:  "11",
 			Changer:        schemaChangeV10ToV11,
 		},
+		{
+			InitialVersion: "11",
+			TargetVersion:  "12",
+			Changer:        schemaChangeV11ToV12,
+		},
 	}
 
 	// build the index

irc/idletimer.go

@@ -6,10 +6,8 @@ package irc
 import (
 	"fmt"
 	"sync"
-	"sync/atomic"
 	"time"
 
-	"github.com/goshuirc/irc-go/ircfmt"
 	"github.com/oragono/oragono/irc/caps"
 )
 
@@ -179,134 +177,6 @@ func (it *IdleTimer) quitMessage(state TimerState) string {
 	}
 }
 
-// NickTimer manages timing out of clients who are squatting reserved nicks
-type NickTimer struct {
-	sync.Mutex // tier 1
-
-	// immutable after construction
-	client *Client
-
-	// mutable
-	nick           string
-	accountForNick string
-	account        string
-	timeout        time.Duration
-	timer          *time.Timer
-	enabled        uint32
-}
-
-// Initialize sets up a NickTimer, based on server config settings.
-func (nt *NickTimer) Initialize(client *Client) {
-	if nt.client == nil {
-		nt.client = client // placate the race detector
-	}
-
-	config := &client.server.Config().Accounts.NickReservation
-	enabled := config.Enabled && (config.Method == NickEnforcementWithTimeout || config.AllowCustomEnforcement)
-
-	nt.Lock()
-	defer nt.Unlock()
-	nt.timeout = config.RenameTimeout
-	if enabled {
-		atomic.StoreUint32(&nt.enabled, 1)
-	} else {
-		nt.stopInternal()
-	}
-}
-
-func (nt *NickTimer) Enabled() bool {
-	return atomic.LoadUint32(&nt.enabled) == 1
-}
-
-func (nt *NickTimer) Timeout() (timeout time.Duration) {
-	nt.Lock()
-	timeout = nt.timeout
-	nt.Unlock()
-	return
-}
-
-// Touch records a nick change and updates the timer as necessary
-func (nt *NickTimer) Touch(rb *ResponseBuffer) {
-	if !nt.Enabled() {
-		return
-	}
-
-	var session *Session
-	if rb != nil {
-		session = rb.session
-	}
-
-	cfnick, skeleton := nt.client.uniqueIdentifiers()
-	account := nt.client.Account()
-	accountForNick, method := nt.client.server.accounts.EnforcementStatus(cfnick, skeleton)
-	enforceTimeout := method == NickEnforcementWithTimeout
-
-	var shouldWarn, shouldRename bool
-
-	func() {
-		nt.Lock()
-		defer nt.Unlock()
-
-		// the timer will not reset as long as the squatter is targeting the same account
-		accountChanged := accountForNick != nt.accountForNick
-		// change state
-		nt.nick = cfnick
-		nt.account = account
-		nt.accountForNick = accountForNick
-		delinquent := accountForNick != "" && accountForNick != account
-
-		if nt.timer != nil && (!enforceTimeout || !delinquent || accountChanged) {
-			nt.timer.Stop()
-			nt.timer = nil
-		}
-		if enforceTimeout && delinquent && (accountChanged || nt.timer == nil) {
-			nt.timer = time.AfterFunc(nt.timeout, nt.processTimeout)
-			shouldWarn = true
-		} else if method == NickEnforcementStrict && delinquent {
-			shouldRename = true // this can happen if reservation was enabled by rehash
-		}
-	}()
-
-	if shouldWarn {
-		tnick := nt.client.Nick()
-		message := fmt.Sprintf(ircfmt.Unescape(nt.client.t(nsTimeoutNotice)), nt.Timeout())
-		// #449
-		for _, mSession := range nt.client.Sessions() {
-			if mSession == session {
-				rb.Add(nil, nsPrefix, "NOTICE", tnick, message)
-				rb.Add(nil, nt.client.server.name, "WARN", "*", "ACCOUNT_REQUIRED", message)
-			} else {
-				mSession.Send(nil, nsPrefix, "NOTICE", tnick, message)
-				mSession.Send(nil, nt.client.server.name, "WARN", "*", "ACCOUNT_REQUIRED", message)
-			}
-		}
-	} else if shouldRename {
-		nt.client.Notice(nt.client.t("Nickname is reserved by a different account"))
-		nt.client.server.RandomlyRename(nt.client)
-	}
-}
-
-// Stop stops counting time and cleans up the timer
-func (nt *NickTimer) Stop() {
-	nt.Lock()
-	defer nt.Unlock()
-	nt.stopInternal()
-}
-
-func (nt *NickTimer) stopInternal() {
-	if nt.timer != nil {
-		nt.timer.Stop()
-		nt.timer = nil
-	}
-	atomic.StoreUint32(&nt.enabled, 0)
-}
-
-func (nt *NickTimer) processTimeout() {
-	baseMsg := "Nick is reserved and authentication timeout expired: %v"
-	nt.client.Notice(fmt.Sprintf(nt.client.t(baseMsg), nt.Timeout()))
-	nt.client.server.RandomlyRename(nt.client)
-}
-
 // BrbTimer is a timer on the client as a whole (not an individual session) for implementing
 // the BRB command and related functionality (where a client can remain online without
 // having any connected sessions).

irc/nickname.go

@@ -97,7 +97,6 @@ func performNickChange(server *Server, client *Client, target *Client, session *
 			client.server.monitorManager.AlertAbout(details.nick, details.nickCasefolded, false)
 			client.server.monitorManager.AlertAbout(assignedNickname, newCfnick, true)
 		}
-		target.nickTimer.Touch(rb)
 	} // else: these will be deferred to the end of registration (see #572)
 	return nil
 }

irc/nickserv.go

@@ -37,11 +37,6 @@ func servCmdRequiresBouncerEnabled(config *Config) bool {
 
 const (
 	nsPrefix = "NickServ!NickServ@localhost"
-	// ZNC's nickserv module will not detect this unless it is:
-	// 1. sent with prefix `nickserv`
-	// 2. contains the string "identify"
-	// 3. contains at least one of several other magic strings ("msg" works)
-	nsTimeoutNotice = `This nickname is reserved. Please login within %v (using $b/msg NickServ IDENTIFY <password>$b or SASL), or switch to a different nickname.`
 )
 
 const nickservHelp = `NickServ lets you register, log in to, and manage an account.`
@@ -249,10 +244,9 @@ SET modifies your account settings. The following settings are available:`,
 'enforce' lets you specify a custom enforcement mechanism for your registered
 nicknames. Your options are:
 1. 'none'    [no enforcement, overriding the server default]
-2. 'timeout' [anyone using the nick must authenticate before a deadline,
               or else they will be renamed]
-3. 'strict'  [you must already be authenticated to use the nick]
-4. 'default' [use the server default]`,
+2. 'strict'  [you must already be authenticated to use the nick]
+3. 'default' [use the server default]`,
 
 				`$bMULTICLIENT$b
 If 'multiclient' is enabled and you are already logged in and using a nick, a

irc/server.go

@@ -320,9 +320,6 @@ func (server *Server) playRegistrationBurst(session *Session) {
 	if server.logger.IsLoggingRawIO() {
 		session.Send(nil, c.server.name, "NOTICE", d.nick, c.t("This server is in debug mode and is logging all user I/O. If you do not wish for everything you send to be readable by the server owner(s), please disconnect."))
 	}
-
-	// #572: defer nick warnings to the end of the registration burst
-	session.client.nickTimer.Touch(nil)
 }
 
 // RplISupport outputs our ISUPPORT lines to the client. This is used on connection and in VERSION responses.
@@ -629,13 +626,6 @@ func (server *Server) applyConfig(config *Config) (err error) {
 			if sendRawOutputNotice {
 				sClient.Notice(sClient.t("This server is in debug mode and is logging all user I/O. If you do not wish for everything you send to be readable by the server owner(s), please disconnect."))
 			}
-
-			if !oldConfig.Accounts.NickReservation.Enabled && config.Accounts.NickReservation.Enabled {
-				sClient.nickTimer.Initialize(sClient)
-				sClient.nickTimer.Touch(nil)
-			} else if oldConfig.Accounts.NickReservation.Enabled && !config.Accounts.NickReservation.Enabled {
-				sClient.nickTimer.Stop()
-			}
 		}
 	}