diff --git a/admin/http.go b/admin/http.go index bdacad3..b16c209 100644 --- a/admin/http.go +++ b/admin/http.go @@ -274,20 +274,11 @@ func loginHandler(app *model.AppState) http.Handler { render() return } - if account.Locked { - controller.SetSessionError(app.DB, session, "This account is locked.") - render() - return - } err = bcrypt.CompareHashAndPassword([]byte(account.Password), []byte(password)) if err != nil { app.Log.Warn(log.TYPE_ACCOUNT, "\"%s\" attempted login with incorrect password. (%s)", account.Username, controller.ResolveIP(app, r)) - if locked := handleFailedLogin(app, account); locked { - controller.SetSessionError(app.DB, session, "Too many failed attempts. This account is now locked.") - } else { - controller.SetSessionError(app.DB, session, "Invalid username or password.") - } + controller.SetSessionError(app.DB, session, "Invalid username or password.") render() return } @@ -308,8 +299,6 @@ func loginHandler(app *model.AppState) http.Handler { render() return } - controller.SetSessionMessage(app.DB, session, "") - controller.SetSessionError(app.DB, session, "") http.Redirect(w, r, "/admin/totp", http.StatusFound) return } @@ -388,14 +377,8 @@ func loginTOTPHandler(app *model.AppState) http.Handler { return } if totpMethod == nil { - app.Log.Warn(log.TYPE_ACCOUNT, "\"%s\" failed login (Incorrect TOTP). (%s)", session.AttemptAccount.Username, controller.ResolveIP(app, r)) - if locked := handleFailedLogin(app, session.AttemptAccount); locked { - controller.SetSessionError(app.DB, session, "Too many failed attempts. This account is now locked.") - controller.SetSessionAttemptAccount(app.DB, session, nil) - http.Redirect(w, r, "/admin", http.StatusFound) - } else { - controller.SetSessionError(app.DB, session, "Incorrect TOTP.") - } + app.Log.Warn(log.TYPE_ACCOUNT, "\"%s\" failed login (Invalid TOTP). (%s)", session.AttemptAccount.Username, controller.ResolveIP(app, r)) + controller.SetSessionError(app.DB, session, "Invalid TOTP.") render() return } @@ -483,7 +466,7 @@ func staticHandler() http.Handler { func enforceSession(app *model.AppState, next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - session, err := controller.GetSessionFromRequest(app, r) + session, err := controller.GetSessionFromRequest(app.DB, r) if err != nil { fmt.Fprintf(os.Stderr, "WARN: Failed to retrieve session: %v\n", err) http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) @@ -513,29 +496,3 @@ func enforceSession(app *model.AppState, next http.Handler) http.Handler { next.ServeHTTP(w, r.WithContext(ctx)) }) } - -func handleFailedLogin(app *model.AppState, account *model.Account) bool { - locked, err := controller.IncrementAccountFails(app.DB, account.ID) - if err != nil { - fmt.Fprintf( - os.Stderr, - "WARN: Failed to increment login failures for \"%s\": %v\n", - account.Username, - err, - ) - app.Log.Warn( - log.TYPE_ACCOUNT, - "Failed to increment login failures for \"%s\"", - account.Username, - ) - } - if locked { - app.Log.Warn( - log.TYPE_ACCOUNT, - "Account \"%s\" was locked: %d failed login attempts", - account.Username, - model.MAX_LOGIN_FAIL_ATTEMPTS, - ) - } - return locked -} diff --git a/api/release.go b/api/release.go index 5cb87b0..efed8dd 100644 --- a/api/release.go +++ b/api/release.go @@ -20,7 +20,7 @@ func ServeRelease(app *model.AppState, release *model.Release) http.Handler { // only allow authorised users to view hidden releases privileged := false if !release.Visible { - session, err := controller.GetSessionFromRequest(app, r) + session, err := controller.GetSessionFromRequest(app.DB, r) if err != nil { fmt.Fprintf(os.Stderr, "WARN: Failed to retrieve session: %v\n", err) http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) diff --git a/controller/account.go b/controller/account.go index e4f5dc4..9c7c1e1 100644 --- a/controller/account.go +++ b/controller/account.go @@ -110,26 +110,3 @@ func DeleteAccount(db *sqlx.DB, accountID string) error { _, err := db.Exec("DELETE FROM account WHERE id=$1", accountID) return err } - -func IncrementAccountFails(db *sqlx.DB, accountID string) (bool, error) { - failAttempts := 0 - err := db.Get(&failAttempts, "UPDATE account SET fail_attempts = fail_attempts + 1 WHERE id=$1 RETURNING fail_attempts", accountID) - if err != nil { return false, err } - locked := false - if failAttempts >= model.MAX_LOGIN_FAIL_ATTEMPTS { - err = LockAccount(db, accountID) - if err != nil { return false, err } - locked = true - } - return locked, err -} - -func LockAccount(db *sqlx.DB, accountID string) error { - _, err := db.Exec("UPDATE account SET locked = true WHERE id=$1", accountID) - return err -} - -func UnlockAccount(db *sqlx.DB, accountID string) error { - _, err := db.Exec("UPDATE account SET locked = false, fail_attempts = 0 WHERE id=$1", accountID) - return err -} diff --git a/controller/migrator.go b/controller/migrator.go index b970a1b..b053a27 100644 --- a/controller/migrator.go +++ b/controller/migrator.go @@ -8,7 +8,7 @@ import ( "github.com/jmoiron/sqlx" ) -const DB_VERSION int = 4 +const DB_VERSION int = 3 func CheckDBVersionAndMigrate(db *sqlx.DB) { db.MustExec("CREATE SCHEMA IF NOT EXISTS arimelody") @@ -45,10 +45,6 @@ func CheckDBVersionAndMigrate(db *sqlx.DB) { ApplyMigration(db, "002-audit-logs") oldDBVersion = 3 - case 3: - ApplyMigration(db, "003-fail-lock") - oldDBVersion = 4 - } } diff --git a/controller/session.go b/controller/session.go index dce7ad0..cf423fe 100644 --- a/controller/session.go +++ b/controller/session.go @@ -8,7 +8,6 @@ import ( "strings" "time" - "arimelody-web/log" "arimelody-web/model" "github.com/jmoiron/sqlx" @@ -16,7 +15,7 @@ import ( const TOKEN_LEN = 64 -func GetSessionFromRequest(app *model.AppState, r *http.Request) (*model.Session, error) { +func GetSessionFromRequest(db *sqlx.DB, r *http.Request) (*model.Session, error) { sessionCookie, err := r.Cookie(model.COOKIE_TOKEN) if err != nil && err != http.ErrNoCookie { return nil, errors.New(fmt.Sprintf("Failed to retrieve session cookie: %v", err)) @@ -26,26 +25,14 @@ func GetSessionFromRequest(app *model.AppState, r *http.Request) (*model.Session if sessionCookie != nil { // fetch existing session - session, err = GetSession(app.DB, sessionCookie.Value) + session, err = GetSession(db, sessionCookie.Value) if err != nil && !strings.Contains(err.Error(), "no rows") { return nil, errors.New(fmt.Sprintf("Failed to retrieve session: %v", err)) } if session != nil { - if session.UserAgent != r.UserAgent() { - msg := "Session user agent mismatch. A cookie may have been hijacked!" - if session.Account != nil { - account, _ := GetAccountByID(app.DB, session.Account.ID) - msg += " (Account \"" + account.Username + "\")" - } - app.Log.Warn(log.TYPE_ACCOUNT, msg) - err = DeleteSession(app.DB, session.Token) - if err != nil { - app.Log.Warn(log.TYPE_ACCOUNT, "Failed to delete affected session") - } - return nil, nil - } + // TODO: consider running security checks here (i.e. user agent mismatches) } } diff --git a/main.go b/main.go index 360f7ac..2252622 100644 --- a/main.go +++ b/main.go @@ -276,13 +276,11 @@ func main() { "User: %s\n" + "\tID: %s\n" + "\tEmail: %s\n" + - "\tCreated: %s\n" + - "\tLocked: %t\n", + "\tCreated: %s\n", account.Username, account.ID, email, account.CreatedAt, - account.Locked, ) } return @@ -357,64 +355,6 @@ func main() { fmt.Printf("Account \"%s\" deleted successfully.\n", account.Username) return - case "lockAccount": - if len(os.Args) < 3 { - fmt.Fprintf(os.Stderr, "FATAL: `username` must be specified for lockAccount\n") - os.Exit(1) - } - username := os.Args[2] - fmt.Printf("Unlocking account \"%s\"...\n", username) - - account, err := controller.GetAccountByUsername(app.DB, username) - if err != nil { - fmt.Fprintf(os.Stderr, "FATAL: Failed to fetch account \"%s\": %v\n", username, err) - os.Exit(1) - } - - if account == nil { - fmt.Fprintf(os.Stderr, "FATAL: Account \"%s\" does not exist.\n", username) - os.Exit(1) - } - - err = controller.LockAccount(app.DB, account.ID) - if err != nil { - fmt.Fprintf(os.Stderr, "FATAL: Failed to lock account: %v\n", err) - os.Exit(1) - } - - app.Log.Info(log.TYPE_ACCOUNT, "Account '%s' locked via config utility.", account.Username) - fmt.Printf("Account \"%s\" locked successfully.\n", account.Username) - return - - case "unlockAccount": - if len(os.Args) < 3 { - fmt.Fprintf(os.Stderr, "FATAL: `username` must be specified for unlockAccount\n") - os.Exit(1) - } - username := os.Args[2] - fmt.Printf("Unlocking account \"%s\"...\n", username) - - account, err := controller.GetAccountByUsername(app.DB, username) - if err != nil { - fmt.Fprintf(os.Stderr, "FATAL: Failed to fetch account \"%s\": %v\n", username, err) - os.Exit(1) - } - - if account == nil { - fmt.Fprintf(os.Stderr, "FATAL: Account \"%s\" does not exist.\n", username) - os.Exit(1) - } - - err = controller.UnlockAccount(app.DB, account.ID) - if err != nil { - fmt.Fprintf(os.Stderr, "FATAL: Failed to unlock account: %v\n", err) - os.Exit(1) - } - - app.Log.Info(log.TYPE_ACCOUNT, "Account '%s' unlocked via config utility.", account.Username) - fmt.Printf("Account \"%s\" unlocked successfully.\n", account.Username) - return - case "logs": // TODO: add log search parameters logs, err := app.Log.Search([]log.LogLevel{}, []string{}, "", 100, 0) @@ -449,9 +389,7 @@ func main() { "createInvite:\n\tCreates an invite code to register new accounts.\n" + "purgeInvites:\n\tDeletes all available invite codes.\n" + "listAccounts:\n\tLists all active accounts.\n", - "deleteAccount :\n\tDeletes the account under `username`.\n", - "unlockAccount :\n\tUnlocks the account under `username`.\n", - "logs:\n\tShows system logs.\n", + "deleteAccount :\n\tDeletes an account with a given `username`.\n", ) return } diff --git a/model/account.go b/model/account.go index ad65b82..166e880 100644 --- a/model/account.go +++ b/model/account.go @@ -6,18 +6,15 @@ import ( ) const COOKIE_TOKEN string = "AM_SESSION" -const MAX_LOGIN_FAIL_ATTEMPTS int = 3 type ( - Account struct { - ID string `json:"id" db:"id"` - Username string `json:"username" db:"username"` - Password string `json:"password" db:"password"` - Email sql.NullString `json:"email" db:"email"` - AvatarURL sql.NullString `json:"avatar_url" db:"avatar_url"` - CreatedAt time.Time `json:"created_at" db:"created_at"` - FailAttempts int `json:"fail_attempts" db:"fail_attempts"` - Locked bool `json:"locked" db:"locked"` + Account struct { + ID string `json:"id" db:"id"` + Username string `json:"username" db:"username"` + Password string `json:"password" db:"password"` + Email sql.NullString `json:"email" db:"email"` + AvatarURL sql.NullString `json:"avatar_url" db:"avatar_url"` + CreatedAt time.Time `json:"created_at" db:"created_at"` Privileges []AccountPrivilege `json:"privileges"` } diff --git a/schema-migration/000-init.sql b/schema-migration/000-init.sql index 2174109..b56c6b6 100644 --- a/schema-migration/000-init.sql +++ b/schema-migration/000-init.sql @@ -19,8 +19,6 @@ CREATE TABLE arimelody.account ( email TEXT, avatar_url TEXT, created_at TIMESTAMP NOT NULL DEFAULT current_timestamp - fail_attempts INT NOT NULL DEFAULT 0, - locked BOOLEAN DEFAULT false, ); ALTER TABLE arimelody.account ADD CONSTRAINT account_pk PRIMARY KEY (id); diff --git a/schema-migration/003-fail-lock.sql b/schema-migration/003-fail-lock.sql deleted file mode 100644 index 32d19bf..0000000 --- a/schema-migration/003-fail-lock.sql +++ /dev/null @@ -1,3 +0,0 @@ --- it would be nice to prevent brute-forcing -ALTER TABLE arimelody.account ADD COLUMN fail_attempts INT NOT NULL DEFAULT 0; -ALTER TABLE arimelody.account ADD COLUMN locked BOOLEAN DEFAULT false; diff --git a/view/music.go b/view/music.go index dfe884e..2d40ef0 100644 --- a/view/music.go +++ b/view/music.go @@ -60,7 +60,7 @@ func ServeGateway(app *model.AppState, release *model.Release) http.Handler { // only allow authorised users to view hidden releases privileged := false if !release.Visible { - session, err := controller.GetSessionFromRequest(app, r) + session, err := controller.GetSessionFromRequest(app.DB, r) if err != nil { fmt.Fprintf(os.Stderr, "WARN: Failed to retrieve session: %v\n", err) http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)