diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0f59dd8e..1e16e851 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -4,6 +4,8 @@ It's mainly there to deal with any legal issues which come our way and to switch Other uses may arise in the future, e.g. commercial licensing, although that's currently uncertain. +Try to prefix commits which introduce a lot of bugs or otherwise has a large impact on the usability of Gosora with UNSTABLE. + # Coding Standards All code must be unit tested where ever possible with the exception of JavaScript which is untestable with our current technologies, tread with caution there. diff --git a/common/auth.go b/common/auth.go index 7d3c9949..c70be49a 100644 --- a/common/auth.go +++ b/common/auth.go @@ -18,6 +18,7 @@ import ( "golang.org/x/crypto/bcrypt" ) +// TODO: Write more authentication tests var Auth AuthInt const SaltLength int = 32 @@ -83,6 +84,7 @@ func NewDefaultAuth() (*DefaultAuth, error) { } // Authenticate checks if a specific username and password is valid and returns the UID for the corresponding user, if so. Otherwise, a user safe error. +// TODO: Find a better way of handling errors we don't want to reach the user func (auth *DefaultAuth) Authenticate(username string, password string) (uid int, err error) { var realPassword, salt string err = auth.login.QueryRow(username).Scan(&uid, &realPassword, &salt) diff --git a/common/profile_reply.go b/common/profile_reply.go index ab56f463..e397d101 100644 --- a/common/profile_reply.go +++ b/common/profile_reply.go @@ -39,6 +39,11 @@ func init() { }) } +// Mostly for tests, so we don't wind up with out-of-date profile reply initialisation logic there +func BlankProfileReply(id int) *ProfileReply { + return &ProfileReply{ID: id} +} + // TODO: Write tests for this func (reply *ProfileReply) Delete() error { _, err := profileReplyStmts.delete.Exec(reply.ID) diff --git a/common/user.go b/common/user.go index 0112a1a7..71194ffb 100644 --- a/common/user.go +++ b/common/user.go @@ -54,21 +54,23 @@ type User struct { } type UserStmts struct { - activate *sql.Stmt - changeGroup *sql.Stmt - delete *sql.Stmt - setAvatar *sql.Stmt - setUsername *sql.Stmt - updateGroup *sql.Stmt - incrementTopics *sql.Stmt - updateLevel *sql.Stmt + activate *sql.Stmt + changeGroup *sql.Stmt + delete *sql.Stmt + setAvatar *sql.Stmt + setUsername *sql.Stmt + incrementTopics *sql.Stmt + updateLevel *sql.Stmt + + // TODO: Split these into a sub-struct incrementScore *sql.Stmt incrementPosts *sql.Stmt incrementBigposts *sql.Stmt incrementMegaposts *sql.Stmt incrementLiked *sql.Stmt - decrementLiked *sql.Stmt - updateLastIP *sql.Stmt + + decrementLiked *sql.Stmt + updateLastIP *sql.Stmt setPassword *sql.Stmt } @@ -242,9 +244,7 @@ func (user *User) ChangeAvatar(avatar string) (err error) { } func (user *User) ChangeGroup(group int) (err error) { - _, err = userStmts.updateGroup.Exec(group, user.ID) - user.CacheRemove() - return err + return user.bindStmt(userStmts.changeGroup, group) } // ! Only updates the database not the *User for safety reasons diff --git a/common/user_store.go b/common/user_store.go index 6387d077..4de39f94 100644 --- a/common/user_store.go +++ b/common/user_store.go @@ -229,6 +229,7 @@ func (mus *DefaultUserStore) Exists(id int) bool { } // TODO: Change active to a bool? +// TODO: Use unique keys for the usernames func (mus *DefaultUserStore) Create(username string, password string, email string, group int, active bool) (int, error) { // TODO: Strip spaces? diff --git a/general_test.go b/general_test.go index f1b457f5..6519d2be 100644 --- a/general_test.go +++ b/general_test.go @@ -47,7 +47,7 @@ func ResetTables() (err error) { func gloinit() (err error) { common.Dev.DebugMode = false //nogrouplog = true - startTime = time.Now() + common.StartTime = time.Now() err = common.ProcessConfig() if err != nil { diff --git a/misc_test.go b/misc_test.go index a4fd7a28..4ee3961f 100644 --- a/misc_test.go +++ b/misc_test.go @@ -13,16 +13,20 @@ import ( func recordMustExist(t *testing.T, err error, errmsg string, args ...interface{}) { if err == ErrNoRows { + debug.PrintStack() t.Errorf(errmsg, args...) } else if err != nil { + debug.PrintStack() t.Fatal(err) } } func recordMustNotExist(t *testing.T, err error, errmsg string, args ...interface{}) { if err == nil { + debug.PrintStack() t.Errorf(errmsg, args...) } else if err != ErrNoRows { + debug.PrintStack() t.Fatal(err) } } @@ -801,6 +805,11 @@ func TestProfileReplyStore(t *testing.T) { _, err = common.Prstore.Get(1) recordMustNotExist(t, err, "PRID #1 shouldn't exist") + // ? - Commented this one out as strong constraints like this put an unreasonable load on the database, we only want errors if a delete which should succeed fails + //profileReply := common.BlankProfileReply(1) + //err = profileReply.Delete() + //expect(t,err != nil,"You shouldn't be able to delete profile replies which don't exist") + var profileID = 1 prid, err := common.Prstore.Create(profileID, "Haha", 1, "::1") expect(t, err == nil, "Unable to create a profile reply") @@ -814,9 +823,12 @@ func TestProfileReplyStore(t *testing.T) { expect(t, profileReply.CreatedBy == 1, fmt.Sprintf("The profile reply's creator should be 1 not %d", profileReply.CreatedBy)) expect(t, profileReply.IPAddress == "::1", fmt.Sprintf("The profile reply's IP Address should be '::1' not '%s'", profileReply.IPAddress)) - //Get(id int) (*Reply, error) - //Create(profileID int, content string, createdBy int, ipaddress string) (id int, err error) + err = profileReply.Delete() + expectNilErr(t, err) + _, err = common.Prstore.Get(1) + expect(t, err != nil, "PRID #1 shouldn't exist after being deleted") + // TODO: Test profileReply.SetBody() and profileReply.Creator() } func TestSlugs(t *testing.T) { @@ -856,25 +868,71 @@ func TestSlugs(t *testing.T) { func TestAuth(t *testing.T) { // bcrypt likes doing stupid things, so this test will probably fail - var realPassword string - var hashedPassword string - var password string - var salt string - var err error - - /* No extra salt tests, we might not need this extra salt, as bcrypt has it's own? */ - realPassword = "Madame Cassandra's Mystic Orb" + realPassword := "Madame Cassandra's Mystic Orb" t.Logf("Set realPassword to '%s'", realPassword) - t.Log("Hashing the real password") - hashedPassword, err = common.BcryptGeneratePassword(realPassword) + t.Log("Hashing the real password with bcrypt") + hashedPassword, _, err := common.BcryptGeneratePassword(realPassword) if err != nil { t.Error(err) } + passwordTest(t, realPassword, hashedPassword) + // TODO: Peek at the prefix to verify this is a bcrypt hash - password = realPassword + t.Log("Hashing the real password") + hashedPassword2, _, err := common.GeneratePassword(realPassword) + if err != nil { + t.Error(err) + } + passwordTest(t, realPassword, hashedPassword2) + // TODO: Peek at the prefix to verify this is a bcrypt hash + + _, err = common.Auth.Authenticate("None", "password") + errmsg := "Username None shouldn't exist" + if err != nil { + errmsg += "\n" + err.Error() + } + expect(t, err == common.ErrNoUserByName, errmsg) + + uid, err := common.Auth.Authenticate("Admin", "password") + expectNilErr(t, err) + expect(t, uid == 1, fmt.Sprintf("Default admin uid should be 1 not %d", uid)) + + _, err = common.Auth.Authenticate("Sam", "ReallyBadPassword") + errmsg = "Username Sam shouldn't exist" + if err != nil { + errmsg += "\n" + err.Error() + } + expect(t, err == common.ErrNoUserByName, errmsg) + + admin, err := common.Users.Get(1) + expectNilErr(t, err) + // TODO: Move this into the user store tests to provide better coverage? E.g. To see if the installer and the user creator initialise the field differently + expect(t, admin.Session == "", "Admin session should be blank") + + session, err := common.Auth.CreateSession(1) + expectNilErr(t, err) + expect(t, session != "", "Admin session shouldn't be blank") + // TODO: Test the actual length set in the setting in addition to this "too short" test + // TODO: We might be able to push up this minimum requirement + expect(t, len(session) > 10, "Admin session shouldn't be too short") + expect(t, admin.Session != session, "Old session should not match new one") + admin, err = common.Users.Get(1) + expectNilErr(t, err) + expect(t, admin.Session == session, "Sessions should match") + + // TODO: Tests for SessionCheck, GetCookies, and ForceLogout +} + +// TODO: Vary the salts? Keep in mind that some algorithms store the salt in the hash therefore the salt string may be blank +func passwordTest(t *testing.T, realPassword string, hashedPassword string) { + if len(hashedPassword) < 10 { + t.Error("Hash too short") + } + salt := "" + password := realPassword t.Logf("Testing password '%s'", password) t.Logf("Testing salt '%s'", salt) - err = common.CheckPassword(hashedPassword, password, salt) + err := common.CheckPassword(hashedPassword, password, salt) if err == common.ErrMismatchedHashAndPassword { t.Error("The two don't match") } else if err == common.ErrPasswordTooLong { diff --git a/themes/shadow/public/main.css b/themes/shadow/public/main.css index d7552ec1..e93565c3 100644 --- a/themes/shadow/public/main.css +++ b/themes/shadow/public/main.css @@ -35,6 +35,9 @@ body { position: relative; top: -2px; } +#main { + padding-bottom: 5px; +} ul { list-style-type: none; @@ -191,6 +194,9 @@ a { font-size: 12px; } +.colstack { + display: flex; +} .colstack_left, .colstack_right { margin-left: 8px; } @@ -455,6 +461,8 @@ textarea.large { margin-top: 4px; display: flex; flex-direction: row; + margin-left: 8px; + margin-bottom: 8px; } .pageitem { background-color: var(--main-block-color); @@ -559,9 +567,17 @@ input, select, textarea { padding-left: 30px; } +.footBlock { + margin-top: -2px; + display: flex; +} +.footer { + width: 70%; + margin-left: auto; + margin-right: auto; +} #poweredByHolder { background-color: var(--main-block-color); - margin-top: 5px; padding: 10px; font-size: 14px; padding-left: 13px; @@ -1040,12 +1056,12 @@ input[type=checkbox]:checked + label.poll_option_label .sel { text-overflow: ellipsis; } - #back { + #back, .footer { width: calc(100% - 20px); } .opthead, .rowhead, .colstack_head { padding-top: 0px !important; - font-size 15px; + font-size: 15px; } .rowblock:not(.opthead):not(.colstack_head):not(.rowhead) .rowitem { font-size: 14px; diff --git a/themes/shadow/public/panel.css b/themes/shadow/public/panel.css index 0d8d3b17..2aee7b23 100644 --- a/themes/shadow/public/panel.css +++ b/themes/shadow/public/panel.css @@ -95,4 +95,12 @@ } .ct-series-a .ct-bar, .ct-series-a .ct-line, .ct-series-a .ct-point, .ct-series-a .ct-slice-donut { stroke: hsl(359,98%,43%) !important; +} + +.pageset { + margin-left: 0px; + margin-bottom: 0px; +} +.pageitem { + padding: 8px; } \ No newline at end of file