From 75a1edcff3bc4faecd60c98144f555e2468c62e4 Mon Sep 17 00:00:00 2001 From: Umputun Date: Sun, 9 Sep 2018 16:59:33 -0500 Subject: [PATCH] feature/adm_mongo (#201) * add admin akd key mongo implementations * wire admin and key stores to mongo * lint: wrong comment on keys store * typos and comments corrections * fix failed test on avatar resizer * get rid of ineffectual assignment in test * add tests for encoding fallback and mongo's keys & admin * empty encode test adjusted --- backend/app/cmd/server.go | 15 +++++++ backend/app/cmd/server_test.go | 3 +- backend/app/migrator/remark_test.go | 2 +- backend/app/rest/api/admin_test.go | 3 +- backend/app/store/admin/admin.go | 51 +++++++++++++++++++++++- backend/app/store/admin/admin_test.go | 43 ++++++++++++++++++++ backend/app/store/avatar/gridfs.go | 2 +- backend/app/store/avatar/localfs.go | 2 +- backend/app/store/avatar/localfs_test.go | 2 +- backend/app/store/formatter.go | 2 +- backend/app/store/formatter_test.go | 4 +- backend/app/store/keys/keys.go | 32 ++++++++++++++- backend/app/store/keys/keys_test.go | 38 ++++++++++++++++++ backend/app/store/user.go | 23 ++++++----- backend/app/store/user_test.go | 24 ++++++++++- 15 files changed, 223 insertions(+), 23 deletions(-) diff --git a/backend/app/cmd/server.go b/backend/app/cmd/server.go index ebb7167b..b1bd03f5 100644 --- a/backend/app/cmd/server.go +++ b/backend/app/cmd/server.go @@ -357,12 +357,20 @@ func (s *ServerCommand) makeKeyStore() (keys.Store, error) { switch s.Key.Type { case "shared": return keys.NewStaticStore(s.SharedSecret), nil + case "mongo": + mgServer, e := s.makeMongo() + if e != nil { + return nil, errors.Wrap(e, "failed to create mongo server") + } + conn := mongo.NewConnection(mgServer, s.Mongo.DB, "admin") + return keys.NewMongoStore(conn), nil default: return nil, errors.Errorf("unsupported key store type %s", s.Key.Type) } } func (s *ServerCommand) makeAdminStore() (admin.Store, error) { + switch s.Admin.Type { case "shared": if s.Admin.Shared.Email == "" { // no admin email, use admin@domain @@ -371,6 +379,13 @@ func (s *ServerCommand) makeAdminStore() (admin.Store, error) { } } return admin.NewStaticStore(s.Admin.Shared.Admins, s.Admin.Shared.Email), nil + case "mongo": + mgServer, e := s.makeMongo() + if e != nil { + return nil, errors.Wrap(e, "failed to create mongo server") + } + conn := mongo.NewConnection(mgServer, s.Mongo.DB, "admin") + return admin.NewMongoStore(conn), nil default: return nil, errors.Errorf("unsupported admin store type %s", s.Key.Type) } diff --git a/backend/app/cmd/server_test.go b/backend/app/cmd/server_test.go index 93fcb4c7..5016f7c1 100644 --- a/backend/app/cmd/server_test.go +++ b/backend/app/cmd/server_test.go @@ -91,7 +91,8 @@ func TestServerApp_WithMongo(t *testing.T) { // prepare options p := flags.NewParser(&opts, flags.Default) _, err := p.ParseArgs([]string{"--dev-passwd=password", "--cache.type=mongo", "--store.type=mongo", - "--avatar.type=mongo", "--mongo.url=" + mongoURL, "--mongo.db=test_remark", "--port=12345"}) + "--avatar.type=mongo", "--mongo.url=" + mongoURL, "--mongo.db=test_remark", "--port=12345", + "--key.type=mongo", "--admin.type=mongo"}) require.Nil(t, err) opts.Auth.Github.CSEC, opts.Auth.Github.CID = "csec", "cid" opts.BackupLocation = "/tmp" diff --git a/backend/app/migrator/remark_test.go b/backend/app/migrator/remark_test.go index 572060d5..7c0621de 100644 --- a/backend/app/migrator/remark_test.go +++ b/backend/app/migrator/remark_test.go @@ -31,7 +31,7 @@ func TestRemark_Export(t *testing.T) { c1, err := buf.ReadString('\n') assert.Nil(t, err) log.Print(c1) - exp := `{"id":"efbc17f177ee1a1c0ee6e1e025749966ec071adc","pid":"","text":"some text, link","user":{"name":"user name","id":"user1","picture":"","admin":false},"locator":{"site":"radio-t","url":"https://radio-t.com"},"score":0,"votes":{},"time":"2017-12-20T15:18:22-06:00"}` + "\n" + exp := `{"id":"efbc17f177ee1a1c0ee6e1e025749966ec071adc","pid":"","text":"some text, link","user":{"name":"user name","id":"user1","picture":"","ip":"293ec5b0cf154855258824ec7fac5dc63d176915","admin":false},"locator":{"site":"radio-t","url":"https://radio-t.com"},"score":0,"votes":{},"time":"2017-12-20T15:18:22-06:00"}` + "\n" assert.Equal(t, exp, c1) } diff --git a/backend/app/rest/api/admin_test.go b/backend/app/rest/api/admin_test.go index 567256d9..9921a99d 100644 --- a/backend/app/rest/api/admin_test.go +++ b/backend/app/rest/api/admin_test.go @@ -618,7 +618,8 @@ func TestAdmin_GetUserInfo(t *testing.T) { u := store.User{} err = json.Unmarshal([]byte(body), &u) assert.Nil(t, err) - assert.Equal(t, store.User{Name: "user1 name", ID: "user1", Picture: "", IP: "", Admin: false, Blocked: false, Verified: false}, u) + assert.Equal(t, store.User{Name: "user1 name", ID: "user1", Picture: "", IP: "823688dafca7393d24c871a2da98a84d8732e927", + Admin: false, Blocked: false, Verified: false}, u) _, code = get(t, fmt.Sprintf("%s/api/v1/admin/user/user1?site=radio-t&url=https://radio-t.com/blah", ts.URL)) assert.Equal(t, 401, code, "no auth") diff --git a/backend/app/store/admin/admin.go b/backend/app/store/admin/admin.go index e75d0ec2..05068e3e 100644 --- a/backend/app/store/admin/admin.go +++ b/backend/app/store/admin/admin.go @@ -1,6 +1,12 @@ package admin -import "log" +import ( + "log" + + "github.com/globalsign/mgo" + "github.com/globalsign/mgo/bson" + "github.com/go-pkgz/mongo" +) // Store defines interface returning admins info for given site type Store interface { @@ -29,3 +35,46 @@ func (s *StaticStore) Admins(string) (ids []string) { func (s *StaticStore) Email(string) (email string) { return s.email } + +// MongoStore implements admin.Store with mongo backend +type MongoStore struct { + connection *mongo.Connection +} + +// NewMongoStore makes admin Store for mongo's connection +func NewMongoStore(conn *mongo.Connection) *MongoStore { + log.Printf("[DEBUG] make mongo admin store with %+v", conn) + return &MongoStore{connection: conn} +} + +// Admins executes find by siteID and returns admins ids +func (m *MongoStore) Admins(siteID string) (ids []string) { + resp := struct { + SiteID string `bson:"site"` + IDs []string `bson:"admin_ids"` + Email string `bson:"admin_email"` + }{} + err := m.connection.WithCollection(func(coll *mgo.Collection) error { + return coll.Find(bson.M{"site": siteID}).One(&resp) + }) + if err != nil { + return []string{} + } + return resp.IDs +} + +// Email executes find by siteID and returns admin's email +func (m *MongoStore) Email(siteID string) (email string) { + resp := struct { + SiteID string `bson:"site"` + IDs []string `bson:"admin_ids"` + Email string `bson:"admin_email"` + }{} + err := m.connection.WithCollection(func(coll *mgo.Collection) error { + return coll.Find(bson.M{"site": siteID}).One(&resp) + }) + if err != nil { + return "" + } + return resp.Email +} diff --git a/backend/app/store/admin/admin_test.go b/backend/app/store/admin/admin_test.go index acd4df2c..73cf5db3 100644 --- a/backend/app/store/admin/admin_test.go +++ b/backend/app/store/admin/admin_test.go @@ -3,7 +3,10 @@ package admin import ( "testing" + "github.com/globalsign/mgo" + "github.com/go-pkgz/mongo" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestStaticStore_Get(t *testing.T) { @@ -15,3 +18,43 @@ func TestStaticStore_Get(t *testing.T) { email := ks.Email("blah") assert.Equal(t, "aa@example.com", email) } + +func TestMongoStore_Get(t *testing.T) { + conn, err := mongo.MakeTestConnection(t) + require.NoError(t, err) + var ms Store = NewMongoStore(conn) + + recs := []struct { + SiteID string `bson:"site"` + IDs []string `bson:"admin_ids"` + Email string `bson:"admin_email"` + }{ + {"site1", []string{"i11", "i12"}, "e1"}, + {"site2", []string{"i21", "i22"}, "e2"}, + } + err = conn.WithCollection(func(coll *mgo.Collection) error { + if e1 := coll.Insert(recs[0]); e1 != nil { + return e1 + } + if e2 := coll.Insert(recs[1]); e2 != nil { + return e2 + } + return nil + }) + require.NoError(t, err) + + admins := ms.Admins("site1") + assert.Equal(t, []string{"i11", "i12"}, admins) + email := ms.Email("site1") + assert.Equal(t, "e1", email) + + admins = ms.Admins("site2") + assert.Equal(t, []string{"i21", "i22"}, admins) + email = ms.Email("site2") + assert.Equal(t, "e2", email) + + admins = ms.Admins("no-site-in-db") + assert.Equal(t, []string{}, admins) + email = ms.Email("no-site-in-db") + assert.Equal(t, "", email) +} diff --git a/backend/app/store/avatar/gridfs.go b/backend/app/store/avatar/gridfs.go index 0110b2e1..38a10617 100644 --- a/backend/app/store/avatar/gridfs.go +++ b/backend/app/store/avatar/gridfs.go @@ -40,7 +40,7 @@ func (gf *GridFS) Put(userID string, reader io.Reader) (avatar string, err error // Trying to resize avatar. if reader = resize(reader, gf.resizeLimit); reader == nil { - return errors.New("avatar reader is nil") + return errors.New("avatar resize reader is nil") } _, e = io.Copy(fh, reader) return e diff --git a/backend/app/store/avatar/localfs.go b/backend/app/store/avatar/localfs.go index 1c077b0f..306c0221 100644 --- a/backend/app/store/avatar/localfs.go +++ b/backend/app/store/avatar/localfs.go @@ -53,7 +53,7 @@ func (fs *LocalFS) Put(userID string, reader io.Reader) (avatar string, err erro // Trying to resize avatar. if reader = resize(reader, fs.resizeLimit); reader == nil { - return "", errors.New("avatar reader is nil") + return "", errors.New("avatar resize reader is nil") } if _, err = io.Copy(fh, reader); err != nil { diff --git a/backend/app/store/avatar/localfs_test.go b/backend/app/store/avatar/localfs_test.go index 51833f30..2b9d1de7 100644 --- a/backend/app/store/avatar/localfs_test.go +++ b/backend/app/store/avatar/localfs_test.go @@ -19,7 +19,7 @@ func TestAvatarStoreFS_Put(t *testing.T) { avatar, err := p.Put("user1", nil) assert.Equal(t, "", avatar) - assert.EqualError(t, err, "avatar reader is nil") + assert.EqualError(t, err, "avatar resize reader is nil") avatar, err = p.Put("user1", strings.NewReader("some picture bin data")) require.Nil(t, err) diff --git a/backend/app/store/formatter.go b/backend/app/store/formatter.go index d1dec9c5..3543395a 100644 --- a/backend/app/store/formatter.go +++ b/backend/app/store/formatter.go @@ -38,7 +38,7 @@ func (f *CommentFormatter) Format(c Comment) Comment { return c } -// FormatText formatting line +// FormatText converts text with markdown processor, applies external converters and shortens links func (f *CommentFormatter) FormatText(txt string) (res string) { mdExt := blackfriday.NoIntraEmphasis | blackfriday.Tables | blackfriday.FencedCode | blackfriday.Strikethrough | blackfriday.SpaceHeadings | blackfriday.HardLineBreak | diff --git a/backend/app/store/formatter_test.go b/backend/app/store/formatter_test.go index 11f8b609..95959ca3 100644 --- a/backend/app/store/formatter_test.go +++ b/backend/app/store/formatter_test.go @@ -28,12 +28,12 @@ func TestFormatter_FormatText(t *testing.T) { } } -func TestFormatter_FormatTextNoConvertor(t *testing.T) { +func TestFormatter_FormatTextNoConverter(t *testing.T) { f := NewCommentFormatter() assert.Equal(t, "

12345

\n", f.FormatText("12345")) } -func TestFormatter_FormatTextConvertorFunc(t *testing.T) { +func TestFormatter_FormatTextConverterFunc(t *testing.T) { fn := CommentConverterFunc(func(text string) string { return "zz!" + text }) f := NewCommentFormatter(fn) assert.Equal(t, "zz!

12345

\n", f.FormatText("12345")) diff --git a/backend/app/store/keys/keys.go b/backend/app/store/keys/keys.go index 36353a6c..588bf5c6 100644 --- a/backend/app/store/keys/keys.go +++ b/backend/app/store/keys/keys.go @@ -1,6 +1,13 @@ package keys -import "github.com/pkg/errors" +import ( + "log" + + "github.com/globalsign/mgo" + "github.com/globalsign/mgo/bson" + "github.com/go-pkgz/mongo" + "github.com/pkg/errors" +) // Store defines interface returning key for given site // this key used for JWT and HMAC hashes @@ -25,3 +32,26 @@ func (s *StaticStore) Get(siteID string) (key string, err error) { } return s.key, nil } + +// MongoStore implements keys.Store with mongo backend +type MongoStore struct { + connection *mongo.Connection +} + +// NewMongoStore makes keys Store for mongo's connection +func NewMongoStore(conn *mongo.Connection) *MongoStore { + log.Printf("[DEBUG] make mongo keys store with %+v", conn) + return &MongoStore{connection: conn} +} + +// Get executes find by siteID and returns substructure with secret key +func (m *MongoStore) Get(siteID string) (key string, err error) { + resp := struct { + SiteID string `bson:"site"` + SecretKey string `bson:"secret"` + }{} + err = m.connection.WithCollection(func(coll *mgo.Collection) error { + return coll.Find(bson.M{"site": siteID}).One(&resp) + }) + return resp.SecretKey, errors.Wrapf(err, "can't get secret for site %s", siteID) +} diff --git a/backend/app/store/keys/keys_test.go b/backend/app/store/keys/keys_test.go index c4b47eb2..0364c200 100644 --- a/backend/app/store/keys/keys_test.go +++ b/backend/app/store/keys/keys_test.go @@ -3,7 +3,10 @@ package keys import ( "testing" + "github.com/globalsign/mgo" + "github.com/go-pkgz/mongo" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestStaticStore_Get(t *testing.T) { @@ -18,3 +21,38 @@ func TestStaticStore_Get(t *testing.T) { _, err = ks.Get("any") assert.NotNil(t, err, "invalid (empty key) store") } + +func TestMongoStore_Get(t *testing.T) { + conn, err := mongo.MakeTestConnection(t) + require.NoError(t, err) + var ms Store = NewMongoStore(conn) + + recs := []struct { + SiteID string `bson:"site"` + SecretKey string `bson:"secret"` + }{ + {"site1", "secret1"}, + {"site2", "secret2"}, + } + err = conn.WithCollection(func(coll *mgo.Collection) error { + if e1 := coll.Insert(recs[0]); e1 != nil { + return e1 + } + if e2 := coll.Insert(recs[1]); e2 != nil { + return e2 + } + return nil + }) + require.NoError(t, err) + + r, err := ms.Get("site1") + assert.NoError(t, err) + assert.Equal(t, "secret1", r) + + r, err = ms.Get("site2") + assert.NoError(t, err) + assert.Equal(t, "secret2", r) + + _, err = ms.Get("no-site-in-db") + assert.Error(t, err, "can't get secret for site no-site-in-db") +} diff --git a/backend/app/store/user.go b/backend/app/store/user.go index 626521b4..a0e0cd42 100644 --- a/backend/app/store/user.go +++ b/backend/app/store/user.go @@ -24,6 +24,7 @@ type User struct { } var reValidSha = regexp.MustCompile("^[a-fA-F0-9]{40}$") +var reValidCrc64 = regexp.MustCompile("^[a-fA-F0-9]{16}$") // HashIP replace IP field with hashed hmac func (u *User) HashIP(secret string) { @@ -32,29 +33,29 @@ func (u *User) HashIP(secret string) { // HashValue makes hmac with secret func HashValue(val string, secret string) string { - if val == "" || reValidSha.MatchString(val) { - return val // already hashed or empty - } key := []byte(secret) - h := hmac.New(sha1.New, key) - return hashWithFallback(h, val) + return hashWithFallback(hmac.New(sha1.New, key), val) } // EncodeID hashes id to sha1. The function intentionally left outside of User struct because in some cases // we need hashing for parts of id, in some others hashing for non-User values. func EncodeID(id string) string { - if reValidSha.MatchString(id) { - return id // already hashed or empty - } - h := sha1.New() - return hashWithFallback(h, id) + return hashWithFallback(sha1.New(), id) } -// hashWithFallback tries to has val with hash.Hash and failback to crc if needed +// hashWithFallback tries to has val with hash.Hash and fallback to crc if needed func hashWithFallback(h hash.Hash, val string) string { + + if reValidSha.MatchString(val) { + return val // already hashed or empty + } + if _, err := io.WriteString(h, val); err != nil { // fail back to crc64 log.Printf("[WARN] can't hash id %s, %s", val, err) + if reValidCrc64.MatchString(val) { + return val // already crced + } return fmt.Sprintf("%x", crc64.Checksum([]byte(val), crc64.MakeTable(crc64.ECMA))) } return hex.EncodeToString(h.Sum(nil)) diff --git a/backend/app/store/user_test.go b/backend/app/store/user_test.go index 63f49f60..34a3077a 100644 --- a/backend/app/store/user_test.go +++ b/backend/app/store/user_test.go @@ -1,6 +1,8 @@ package store import ( + "crypto/sha1" + "errors" "testing" "github.com/stretchr/testify/assert" @@ -30,7 +32,7 @@ func TestUser_HashIP(t *testing.T) { {"127.0.0.1", "ae12fe3b5f129b5cc4cdd2b136b7b7947c4d2741", "dbc7c999343f003f189f70aaf52cc04443f90790"}, {"8.8.8.8", "8cee77c27e32a2b5aec95c29888ac9946618d9a2", "70a46afce9633f010b06e129b8ad08243a1c4da9"}, {"8cee77c27e32a2b5aec95c29888ac9946618d9a2", "8cee77c27e32a2b5aec95c29888ac9946618d9a2", "8cee77c27e32a2b5aec95c29888ac9946618d9a2"}, - {"", "", ""}, + {"", "fbdb1d1b18aa6c08324b7d64b71fb76370690e1d", "823688dafca7393d24c871a2da98a84d8732e927"}, } for i, tt := range tbl { @@ -43,3 +45,23 @@ func TestUser_HashIP(t *testing.T) { assert.Equal(t, tt.hash2, u.IP, "case #%d", i) } } + +func TestUser_HashFailed(t *testing.T) { + r := hashWithFallback(mockHash{}, "123456789") + assert.Equal(t, "995dc9bbdf1939fa", r) + + r = hashWithFallback(mockHash{}, "995dc9bbdf1939fa") + assert.Equal(t, "995dc9bbdf1939fa", r) + + r = hashWithFallback(sha1.New(), "123456789") + assert.Equal(t, "f7c3bc1d808e04732adf679965ccc34ca7ae3441", r) + +} + +type mockHash struct{} + +func (mock mockHash) Sum(b []byte) []byte { return nil } +func (mock mockHash) Reset() {} +func (mock mockHash) Size() int { return 0 } +func (mock mockHash) BlockSize() int { return 0 } +func (mock mockHash) Write(p []byte) (n int, err error) { return 0, errors.New("error") }