diff --git a/server/client.go b/server/client.go index 6e3adae340..f1c0348f82 100644 --- a/server/client.go +++ b/server/client.go @@ -1541,8 +1541,8 @@ func (c *client) processSub(argo []byte) (err error) { } else if kind == CLIENT && !c.canSubscribe(string(sub.subject)) { c.mu.Unlock() c.sendErr(fmt.Sprintf("Permissions Violation for Subscription to %q", sub.subject)) - c.Errorf("Subscription Violation - User %q, Subject %q, SID %s", - c.opts.Username, sub.subject, sub.sid) + c.Errorf("Subscription Violation - %s, Subject %q, SID %s", + c.getAuthUser(), sub.subject, sub.sid) return nil } @@ -2455,12 +2455,12 @@ sendToRoutes: func (c *client) pubPermissionViolation(subject []byte) { c.sendErr(fmt.Sprintf("Permissions Violation for Publish to %q", subject)) - c.Errorf("Publish Violation - User %q, Subject %q", c.opts.Username, subject) + c.Errorf("Publish Violation - %s, Subject %q", c.getAuthUser(), subject) } func (c *client) replySubjectViolation(reply []byte) { c.sendErr(fmt.Sprintf("Permissions Violation for Publish with Reply of %q", reply)) - c.Errorf("Publish Violation - User %q, Reply %q", c.opts.Username, reply) + c.Errorf("Publish Violation - %s, Reply %q", c.getAuthUser(), reply) } func (c *client) processPingTimer() { @@ -2610,14 +2610,7 @@ func (c *client) processSubsOnConfigReload(awcsti map[string]struct{}) { _removed [32]*subscription removed = _removed[:0] srv = c.srv - userInfo = c.opts.Nkey ) - if userInfo == "" { - userInfo = c.opts.Username - if userInfo == "" { - userInfo = fmt.Sprintf("%v", c.cid) - } - } if checkAcc { // We actually only want to check if stream imports have changed. if _, ok := awcsti[acc.Name]; !ok { @@ -2656,8 +2649,8 @@ func (c *client) processSubsOnConfigReload(awcsti map[string]struct{}) { c.unsubscribe(acc, sub, true) c.sendErr(fmt.Sprintf("Permissions Violation for Subscription to %q (sid %q)", sub.subject, sub.sid)) - srv.Noticef("Removed sub %q (sid %q) for user %q - not authorized", - sub.subject, sub.sid, userInfo) + srv.Noticef("Removed sub %q (sid %q) for %s - not authorized", + sub.subject, sub.sid, c.getAuthUser()) } } @@ -2901,6 +2894,18 @@ func (c *client) prunePerAccountCache() { } } +// getAuthUser returns the auth user for the client. +func (c *client) getAuthUser() string { + switch { + case c.opts.Nkey != "": + return fmt.Sprintf("Nkey %q", c.opts.Nkey) + case c.opts.Username != "": + return fmt.Sprintf("User %q", c.opts.Username) + default: + return `User "N/A"` + } +} + // Logging functionality scoped to a client or route. func (c *client) Errorf(format string, v ...interface{}) { diff --git a/server/client_test.go b/server/client_test.go index 573cc8fc20..7367ba61b7 100644 --- a/server/client_test.go +++ b/server/client_test.go @@ -1192,3 +1192,40 @@ func TestClientTraceRace(t *testing.T) { } wg.Wait() } + +func TestClientUserInfo(t *testing.T) { + pnkey := "UD6AYQSOIN2IN5OGC6VQZCR4H3UFMIOXSW6NNS6N53CLJA4PB56CEJJI" + c := &client{ + cid: 1024, + opts: clientOpts{ + Nkey: pnkey, + }, + } + got := c.getAuthUser() + expected := `Nkey "UD6AYQSOIN2IN5OGC6VQZCR4H3UFMIOXSW6NNS6N53CLJA4PB56CEJJI"` + if got != expected { + t.Errorf("Expected %q, got %q", expected, got) + } + + c = &client{ + cid: 1024, + opts: clientOpts{ + Username: "foo", + }, + } + got = c.getAuthUser() + expected = `User "foo"` + if got != expected { + t.Errorf("Expected %q, got %q", expected, got) + } + + c = &client{ + cid: 1024, + opts: clientOpts{}, + } + got = c.getAuthUser() + expected = `User "N/A"` + if got != expected { + t.Errorf("Expected %q, got %q", expected, got) + } +}