You can now change someone's group again.

The background no longer randomly vanishes on Shadow.
Fixed the small margins on the paginator on Shadow.
Tweaked the padding for the paginator in the Control Panel on Shadow.
The footer is no longer 100% wide on Shadow.
Fixed a misplaced ':' in Shadow.
Added the BlankProfileReply function for tests.
Tests now run once more.
Made it easier to trace test errors which use recordMustExist and recordMustNotExist.

Added tests for profile reply deletion.
Added tests for GeneratePassword in addition to the existing ones for BcryptGeneratePassword.
Added tests for Auth.Authenticate.
Added tests for Auth.CreateSession.

Added a contributor convention for highly unstable builds.
This can be considered a stable build.
This commit is contained in:
Azareal 2018-05-28 16:27:12 +10:00
parent 55c10e0da2
commit 4d6a7bfda1
9 changed files with 123 additions and 31 deletions

View File

@ -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. 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 # 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. 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.

View File

@ -18,6 +18,7 @@ import (
"golang.org/x/crypto/bcrypt" "golang.org/x/crypto/bcrypt"
) )
// TODO: Write more authentication tests
var Auth AuthInt var Auth AuthInt
const SaltLength int = 32 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. // 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) { func (auth *DefaultAuth) Authenticate(username string, password string) (uid int, err error) {
var realPassword, salt string var realPassword, salt string
err = auth.login.QueryRow(username).Scan(&uid, &realPassword, &salt) err = auth.login.QueryRow(username).Scan(&uid, &realPassword, &salt)

View File

@ -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 // TODO: Write tests for this
func (reply *ProfileReply) Delete() error { func (reply *ProfileReply) Delete() error {
_, err := profileReplyStmts.delete.Exec(reply.ID) _, err := profileReplyStmts.delete.Exec(reply.ID)

View File

@ -54,21 +54,23 @@ type User struct {
} }
type UserStmts struct { type UserStmts struct {
activate *sql.Stmt activate *sql.Stmt
changeGroup *sql.Stmt changeGroup *sql.Stmt
delete *sql.Stmt delete *sql.Stmt
setAvatar *sql.Stmt setAvatar *sql.Stmt
setUsername *sql.Stmt setUsername *sql.Stmt
updateGroup *sql.Stmt incrementTopics *sql.Stmt
incrementTopics *sql.Stmt updateLevel *sql.Stmt
updateLevel *sql.Stmt
// TODO: Split these into a sub-struct
incrementScore *sql.Stmt incrementScore *sql.Stmt
incrementPosts *sql.Stmt incrementPosts *sql.Stmt
incrementBigposts *sql.Stmt incrementBigposts *sql.Stmt
incrementMegaposts *sql.Stmt incrementMegaposts *sql.Stmt
incrementLiked *sql.Stmt incrementLiked *sql.Stmt
decrementLiked *sql.Stmt
updateLastIP *sql.Stmt decrementLiked *sql.Stmt
updateLastIP *sql.Stmt
setPassword *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) { func (user *User) ChangeGroup(group int) (err error) {
_, err = userStmts.updateGroup.Exec(group, user.ID) return user.bindStmt(userStmts.changeGroup, group)
user.CacheRemove()
return err
} }
// ! Only updates the database not the *User for safety reasons // ! Only updates the database not the *User for safety reasons

View File

@ -229,6 +229,7 @@ func (mus *DefaultUserStore) Exists(id int) bool {
} }
// TODO: Change active to a 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) { func (mus *DefaultUserStore) Create(username string, password string, email string, group int, active bool) (int, error) {
// TODO: Strip spaces? // TODO: Strip spaces?

View File

@ -47,7 +47,7 @@ func ResetTables() (err error) {
func gloinit() (err error) { func gloinit() (err error) {
common.Dev.DebugMode = false common.Dev.DebugMode = false
//nogrouplog = true //nogrouplog = true
startTime = time.Now() common.StartTime = time.Now()
err = common.ProcessConfig() err = common.ProcessConfig()
if err != nil { if err != nil {

View File

@ -13,16 +13,20 @@ import (
func recordMustExist(t *testing.T, err error, errmsg string, args ...interface{}) { func recordMustExist(t *testing.T, err error, errmsg string, args ...interface{}) {
if err == ErrNoRows { if err == ErrNoRows {
debug.PrintStack()
t.Errorf(errmsg, args...) t.Errorf(errmsg, args...)
} else if err != nil { } else if err != nil {
debug.PrintStack()
t.Fatal(err) t.Fatal(err)
} }
} }
func recordMustNotExist(t *testing.T, err error, errmsg string, args ...interface{}) { func recordMustNotExist(t *testing.T, err error, errmsg string, args ...interface{}) {
if err == nil { if err == nil {
debug.PrintStack()
t.Errorf(errmsg, args...) t.Errorf(errmsg, args...)
} else if err != ErrNoRows { } else if err != ErrNoRows {
debug.PrintStack()
t.Fatal(err) t.Fatal(err)
} }
} }
@ -801,6 +805,11 @@ func TestProfileReplyStore(t *testing.T) {
_, err = common.Prstore.Get(1) _, err = common.Prstore.Get(1)
recordMustNotExist(t, err, "PRID #1 shouldn't exist") 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 var profileID = 1
prid, err := common.Prstore.Create(profileID, "Haha", 1, "::1") prid, err := common.Prstore.Create(profileID, "Haha", 1, "::1")
expect(t, err == nil, "Unable to create a profile reply") 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.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)) 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) err = profileReply.Delete()
//Create(profileID int, content string, createdBy int, ipaddress string) (id int, err error) 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) { func TestSlugs(t *testing.T) {
@ -856,25 +868,71 @@ func TestSlugs(t *testing.T) {
func TestAuth(t *testing.T) { func TestAuth(t *testing.T) {
// bcrypt likes doing stupid things, so this test will probably fail // bcrypt likes doing stupid things, so this test will probably fail
var realPassword string realPassword := "Madame Cassandra's Mystic Orb"
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"
t.Logf("Set realPassword to '%s'", realPassword) t.Logf("Set realPassword to '%s'", realPassword)
t.Log("Hashing the real password") t.Log("Hashing the real password with bcrypt")
hashedPassword, err = common.BcryptGeneratePassword(realPassword) hashedPassword, _, err := common.BcryptGeneratePassword(realPassword)
if err != nil { if err != nil {
t.Error(err) 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 password '%s'", password)
t.Logf("Testing salt '%s'", salt) t.Logf("Testing salt '%s'", salt)
err = common.CheckPassword(hashedPassword, password, salt) err := common.CheckPassword(hashedPassword, password, salt)
if err == common.ErrMismatchedHashAndPassword { if err == common.ErrMismatchedHashAndPassword {
t.Error("The two don't match") t.Error("The two don't match")
} else if err == common.ErrPasswordTooLong { } else if err == common.ErrPasswordTooLong {

View File

@ -35,6 +35,9 @@ body {
position: relative; position: relative;
top: -2px; top: -2px;
} }
#main {
padding-bottom: 5px;
}
ul { ul {
list-style-type: none; list-style-type: none;
@ -191,6 +194,9 @@ a {
font-size: 12px; font-size: 12px;
} }
.colstack {
display: flex;
}
.colstack_left, .colstack_right { .colstack_left, .colstack_right {
margin-left: 8px; margin-left: 8px;
} }
@ -455,6 +461,8 @@ textarea.large {
margin-top: 4px; margin-top: 4px;
display: flex; display: flex;
flex-direction: row; flex-direction: row;
margin-left: 8px;
margin-bottom: 8px;
} }
.pageitem { .pageitem {
background-color: var(--main-block-color); background-color: var(--main-block-color);
@ -559,9 +567,17 @@ input, select, textarea {
padding-left: 30px; padding-left: 30px;
} }
.footBlock {
margin-top: -2px;
display: flex;
}
.footer {
width: 70%;
margin-left: auto;
margin-right: auto;
}
#poweredByHolder { #poweredByHolder {
background-color: var(--main-block-color); background-color: var(--main-block-color);
margin-top: 5px;
padding: 10px; padding: 10px;
font-size: 14px; font-size: 14px;
padding-left: 13px; padding-left: 13px;
@ -1040,12 +1056,12 @@ input[type=checkbox]:checked + label.poll_option_label .sel {
text-overflow: ellipsis; text-overflow: ellipsis;
} }
#back { #back, .footer {
width: calc(100% - 20px); width: calc(100% - 20px);
} }
.opthead, .rowhead, .colstack_head { .opthead, .rowhead, .colstack_head {
padding-top: 0px !important; padding-top: 0px !important;
font-size 15px; font-size: 15px;
} }
.rowblock:not(.opthead):not(.colstack_head):not(.rowhead) .rowitem { .rowblock:not(.opthead):not(.colstack_head):not(.rowhead) .rowitem {
font-size: 14px; font-size: 14px;

View File

@ -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 { .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; stroke: hsl(359,98%,43%) !important;
}
.pageset {
margin-left: 0px;
margin-bottom: 0px;
}
.pageitem {
padding: 8px;
} }