From 869eb9b8390f5f5e25fabdf20ecfb3eb9da030c2 Mon Sep 17 00:00:00 2001 From: Juliusz Chroboczek Date: Wed, 27 Oct 2021 04:15:44 +0200 Subject: [PATCH] Move password checking into group.go. It used to be delegated to clients. --- diskwriter/diskwriter.go | 4 -- .../galene-password-generator.go | 2 +- group/client.go | 11 +++-- group/group.go | 36 ++++++++------ group/group_test.go | 48 ++++++------------- rtpconn/rtpconn_test.go | 1 - rtpconn/webclient.go | 27 +++++------ webserver/webserver.go | 28 +++-------- 8 files changed, 60 insertions(+), 97 deletions(-) diff --git a/diskwriter/diskwriter.go b/diskwriter/diskwriter.go index c92d998..f45baac 100644 --- a/diskwriter/diskwriter.go +++ b/diskwriter/diskwriter.go @@ -59,10 +59,6 @@ func (client *Client) Username() string { return "RECORDING" } -func (client *Client) Challenge(group string, cred group.ClientCredentials) bool { - return true -} - func (client *Client) SetPermissions(perms group.ClientPermissions) { return } diff --git a/galene-password-generator/galene-password-generator.go b/galene-password-generator/galene-password-generator.go index 0a55b82..413459e 100644 --- a/galene-password-generator/galene-password-generator.go +++ b/galene-password-generator/galene-password-generator.go @@ -56,7 +56,7 @@ func main() { } e := json.NewEncoder(os.Stdout) if username != "" { - creds := group.ClientCredentials{ + creds := group.ClientPattern{ Username: username, Password: &p, } diff --git a/group/client.go b/group/client.go index a97cc9a..7b0d0d3 100644 --- a/group/client.go +++ b/group/client.go @@ -76,7 +76,7 @@ func (p Password) MarshalJSON() ([]byte, error) { return json.Marshal(RawPassword(p)) } -type ClientCredentials struct { +type ClientPattern struct { Username string `json:"username,omitempty"` Password *Password `json:"password,omitempty"` } @@ -88,15 +88,16 @@ type ClientPermissions struct { System bool `json:"system,omitempty"` } -type Challengeable interface { - Username() string - Challenge(string, ClientCredentials) bool +type ClientCredentials struct { + System bool + Username string + Password string } type Client interface { Group() *Group Id() string - Challengeable + Username() string Permissions() ClientPermissions SetPermissions(ClientPermissions) Status() map[string]interface{} diff --git a/group/group.go b/group/group.go index 2e8b31c..bebb6ce 100644 --- a/group/group.go +++ b/group/group.go @@ -525,7 +525,7 @@ func deleteUnlocked(g *Group) bool { return true } -func AddClient(group string, c Client) (*Group, error) { +func AddClient(group string, c Client, creds ClientCredentials) (*Group, error) { g, err := Add(group, nil) if err != nil { return nil, err @@ -537,7 +537,7 @@ func AddClient(group string, c Client) (*Group, error) { clients := g.getClientsUnlocked(nil) if !c.Permissions().System { - perms, err := g.description.GetPermission(group, c) + perms, err := g.description.GetPermission(group, creds) if err != nil { return nil, err } @@ -788,12 +788,16 @@ func (g *Group) GetChatHistory() []ChatHistoryEntry { return h } -func matchClient(group string, c Challengeable, users []ClientCredentials) (bool, bool) { +func matchClient(group string, creds ClientCredentials, users []ClientPattern) (bool, bool) { matched := false for _, u := range users { - if u.Username == c.Username() { + if u.Username == creds.Username { matched = true - if c.Challenge(group, u) { + if u.Password == nil { + return true, true + } + m, _ := u.Password.Match(creds.Password) + if m { return true, true } } @@ -804,7 +808,11 @@ func matchClient(group string, c Challengeable, users []ClientCredentials) (bool for _, u := range users { if u.Username == "" { - if c.Challenge(group, u) { + if u.Password == nil { + return true, true + } + m, _ := u.Password.Match(creds.Password) + if m { return true, true } } @@ -865,13 +873,13 @@ type Description struct { Autokick bool `json:"autokick,omitempty"` // A list of logins for ops. - Op []ClientCredentials `json:"op,omitempty"` + Op []ClientPattern `json:"op,omitempty"` // A list of logins for presenters. - Presenter []ClientCredentials `json:"presenter,omitempty"` + Presenter []ClientPattern `json:"presenter,omitempty"` // A list of logins for non-presenting users. - Other []ClientCredentials `json:"other,omitempty"` + Other []ClientPattern `json:"other,omitempty"` // Codec preferences. If empty, a suitable default is chosen in // the APIFromNames function. @@ -970,12 +978,12 @@ func GetDescription(name string) (*Description, error) { return &desc, nil } -func (desc *Description) GetPermission(group string, c Challengeable) (ClientPermissions, error) { +func (desc *Description) GetPermission(group string, creds ClientCredentials) (ClientPermissions, error) { var p ClientPermissions - if !desc.AllowAnonymous && c.Username() == "" { + if !desc.AllowAnonymous && creds.Username == "" { return p, ErrAnonymousNotAuthorised } - if found, good := matchClient(group, c, desc.Op); found { + if found, good := matchClient(group, creds, desc.Op); found { if good { p.Op = true p.Present = true @@ -986,14 +994,14 @@ func (desc *Description) GetPermission(group string, c Challengeable) (ClientPer } return p, ErrNotAuthorised } - if found, good := matchClient(group, c, desc.Presenter); found { + if found, good := matchClient(group, creds, desc.Presenter); found { if good { p.Present = true return p, nil } return p, ErrNotAuthorised } - if found, good := matchClient(group, c, desc.Other); found { + if found, good := matchClient(group, creds, desc.Other); found { if good { return p, nil } diff --git a/group/group_test.go b/group/group_test.go index dd2dc1d..4873bcb 100644 --- a/group/group_test.go +++ b/group/group_test.go @@ -136,56 +136,36 @@ func TestDescriptionJSON(t *testing.T) { } } -type testClient struct { - username string - password string +var badClients = []ClientCredentials{ + {Username: "jch", Password: "foo"}, + {Username: "john", Password: "foo"}, + {Username: "james", Password: "foo"}, } -func (c testClient) Username() string { - return c.username -} - -func (c testClient) Challenge(g string, creds ClientCredentials) bool { - if creds.Password == nil { - return true - } - m, err := creds.Password.Match(c.password) - if err != nil { - return false - } - return m -} - -type testClientPerm struct { - c testClient +type credPerm struct { + c ClientCredentials p ClientPermissions } -var badClients = []testClient{ - testClient{"jch", "foo"}, - testClient{"john", "foo"}, - testClient{"james", "foo"}, -} - -var goodClients = []testClientPerm{ +var goodClients = []credPerm{ { - testClient{"jch", "topsecret"}, + ClientCredentials{Username: "jch", Password: "topsecret"}, ClientPermissions{Op: true, Present: true}, }, { - testClient{"john", "secret"}, + ClientCredentials{Username: "john", Password: "secret"}, ClientPermissions{Present: true}, }, { - testClient{"john", "secret2"}, + ClientCredentials{Username: "john", Password: "secret2"}, ClientPermissions{Present: true}, }, { - testClient{"james", "secret3"}, + ClientCredentials{Username: "james", Password: "secret3"}, ClientPermissions{}, }, { - testClient{"paul", "secret3"}, + ClientCredentials{Username: "paul", Password: "secret3"}, ClientPermissions{}, }, } @@ -198,7 +178,7 @@ func TestPermissions(t *testing.T) { } for _, c := range badClients { - t.Run("bad "+c.Username(), func(t *testing.T) { + t.Run("bad "+c.Username, func(t *testing.T) { p, err := d.GetPermission("test", c) if err != ErrNotAuthorised { t.Errorf("GetPermission %v: %v %v", c, err, p) @@ -207,7 +187,7 @@ func TestPermissions(t *testing.T) { } for _, cp := range goodClients { - t.Run("good "+cp.c.Username(), func(t *testing.T) { + t.Run("good "+cp.c.Username, func(t *testing.T) { p, err := d.GetPermission("test", cp.c) if err != nil { t.Errorf("GetPermission %v: %v", cp.c, err) diff --git a/rtpconn/rtpconn_test.go b/rtpconn/rtpconn_test.go index 179eb3a..36bd644 100644 --- a/rtpconn/rtpconn_test.go +++ b/rtpconn/rtpconn_test.go @@ -37,4 +37,3 @@ func TestDownTrackAtomics(t *testing.T) { t.Errorf("Expected %v, got %v", info, info2) } } - diff --git a/rtpconn/webclient.go b/rtpconn/webclient.go index 58ede34..b7e22cd 100644 --- a/rtpconn/webclient.go +++ b/rtpconn/webclient.go @@ -56,7 +56,6 @@ type webClient struct { group *group.Group id string username string - password string permissions group.ClientPermissions status map[string]interface{} requested map[string][]string @@ -83,18 +82,6 @@ func (c *webClient) Username() string { return c.username } -func (c *webClient) Challenge(group string, creds group.ClientCredentials) bool { - if creds.Password == nil { - return true - } - m, err := creds.Password.Match(c.password) - if err != nil { - log.Printf("Password match: %v", err) - return false - } - return m -} - func (c *webClient) Permissions() group.ClientPermissions { return c.permissions } @@ -1343,8 +1330,12 @@ func handleClientMessage(c *webClient, m clientMessage) error { return group.ProtocolError("cannot join multiple groups") } c.username = m.Username - c.password = m.Password - g, err := group.AddClient(m.Group, c) + g, err := group.AddClient(m.Group, c, + group.ClientCredentials{ + Username: m.Username, + Password: m.Password, + }, + ) if err != nil { var s string if os.IsNotExist(err) { @@ -1583,7 +1574,11 @@ func handleClientMessage(c *webClient, m clientMessage) error { } } disk := diskwriter.New(g) - _, err := group.AddClient(g.Name(), disk) + _, err := group.AddClient(g.Name(), disk, + group.ClientCredentials{ + System: true, + }, + ) if err != nil { disk.Close() return c.error(err) diff --git a/webserver/webserver.go b/webserver/webserver.go index 9f5469f..8e1ec2c 100644 --- a/webserver/webserver.go +++ b/webserver/webserver.go @@ -534,27 +534,6 @@ func handleGroupAction(w http.ResponseWriter, r *http.Request, group string) { } } -type httpClient struct { - username string - password string -} - -func (c httpClient) Username() string { - return c.username -} - -func (c httpClient) Challenge(group string, creds group.ClientCredentials) bool { - if creds.Password == nil { - return true - } - m, err := creds.Password.Match(c.password) - if err != nil { - log.Printf("Password match: %v", err) - return false - } - return m -} - func checkGroupPermissions(w http.ResponseWriter, r *http.Request, groupname string) bool { desc, err := group.GetDescription(groupname) if err != nil { @@ -566,7 +545,12 @@ func checkGroupPermissions(w http.ResponseWriter, r *http.Request, groupname str return false } - p, err := desc.GetPermission(groupname, httpClient{user, pass}) + p, err := desc.GetPermission(groupname, + group.ClientCredentials{ + Username: user, + Password: pass, + }, + ) if err != nil || !p.Record { if err == group.ErrNotAuthorised { time.Sleep(200 * time.Millisecond)