fixed critical login TOTP bypass bug! whoops!!!!!
This commit is contained in:
parent
2e93c3c5e5
commit
1efe52a8cb
|
@ -134,7 +134,7 @@ func deleteAccountHandler(app *model.AppState) http.Handler {
|
|||
return
|
||||
}
|
||||
|
||||
if !r.Form.Has("password") || !r.Form.Has("totp") {
|
||||
if !r.Form.Has("password") {
|
||||
http.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
|
@ -153,23 +153,6 @@ func deleteAccountHandler(app *model.AppState) http.Handler {
|
|||
return
|
||||
}
|
||||
|
||||
totpMethod, err := controller.CheckTOTPForAccount(app.DB, session.Account.ID, r.Form.Get("totp"))
|
||||
if err != nil {
|
||||
fmt.Fprintf(os.Stderr, "Failed to fetch account: %v\n", err)
|
||||
controller.SetSessionError(app.DB, session, "Something went wrong. Please try again.")
|
||||
http.Redirect(w, r, "/admin/account", http.StatusFound)
|
||||
return
|
||||
}
|
||||
if totpMethod == nil {
|
||||
fmt.Printf(
|
||||
"[%s] WARN: Account \"%s\" attempted account deletion with incorrect TOTP.\n",
|
||||
time.Now().Format(time.UnixDate),
|
||||
session.Account.Username,
|
||||
)
|
||||
controller.SetSessionError(app.DB, session, "Incorrect TOTP.")
|
||||
http.Redirect(w, r, "/admin/account", http.StatusFound)
|
||||
}
|
||||
|
||||
err = controller.DeleteAccount(app.DB, session.Account.ID)
|
||||
if err != nil {
|
||||
fmt.Fprintf(os.Stderr, "Failed to delete account: %v\n", err)
|
||||
|
|
199
admin/http.go
199
admin/http.go
|
@ -31,20 +31,21 @@ func Handler(app *model.AppState) http.Handler {
|
|||
}))
|
||||
|
||||
mux.Handle("/login", loginHandler(app))
|
||||
mux.Handle("/logout", requireAccount(app, logoutHandler(app)))
|
||||
mux.Handle("/totp", loginTOTPHandler(app))
|
||||
mux.Handle("/logout", requireAccount(logoutHandler(app)))
|
||||
|
||||
mux.Handle("/register", registerAccountHandler(app))
|
||||
|
||||
mux.Handle("/account", requireAccount(app, accountIndexHandler(app)))
|
||||
mux.Handle("/account/", requireAccount(app, http.StripPrefix("/account", accountHandler(app))))
|
||||
mux.Handle("/account", requireAccount(accountIndexHandler(app)))
|
||||
mux.Handle("/account/", requireAccount(http.StripPrefix("/account", accountHandler(app))))
|
||||
|
||||
mux.Handle("/release/", requireAccount(app, http.StripPrefix("/release", serveRelease(app))))
|
||||
mux.Handle("/artist/", requireAccount(app, http.StripPrefix("/artist", serveArtist(app))))
|
||||
mux.Handle("/track/", requireAccount(app, http.StripPrefix("/track", serveTrack(app))))
|
||||
mux.Handle("/release/", requireAccount(http.StripPrefix("/release", serveRelease(app))))
|
||||
mux.Handle("/artist/", requireAccount(http.StripPrefix("/artist", serveArtist(app))))
|
||||
mux.Handle("/track/", requireAccount(http.StripPrefix("/track", serveTrack(app))))
|
||||
|
||||
mux.Handle("/static/", http.StripPrefix("/static", staticHandler()))
|
||||
|
||||
mux.Handle("/", requireAccount(app, AdminIndexHandler(app)))
|
||||
mux.Handle("/", requireAccount(AdminIndexHandler(app)))
|
||||
|
||||
// response wrapper to make sure a session cookie exists
|
||||
return enforceSession(app, mux)
|
||||
|
@ -243,7 +244,6 @@ func loginHandler(app *model.AppState) http.Handler {
|
|||
http.Redirect(w, r, "/admin", http.StatusFound)
|
||||
return
|
||||
}
|
||||
|
||||
render()
|
||||
return
|
||||
}
|
||||
|
@ -254,18 +254,15 @@ func loginHandler(app *model.AppState) http.Handler {
|
|||
return
|
||||
}
|
||||
|
||||
type LoginRequest struct {
|
||||
Username string `json:"username"`
|
||||
Password string `json:"password"`
|
||||
TOTP string `json:"totp"`
|
||||
}
|
||||
credentials := LoginRequest{
|
||||
Username: r.Form.Get("username"),
|
||||
Password: r.Form.Get("password"),
|
||||
TOTP: r.Form.Get("totp"),
|
||||
if !r.Form.Has("username") || !r.Form.Has("password") {
|
||||
http.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
|
||||
account, err := controller.GetAccountByUsername(app.DB, credentials.Username)
|
||||
username := r.FormValue("username")
|
||||
password := r.FormValue("password")
|
||||
|
||||
account, err := controller.GetAccountByUsername(app.DB, username)
|
||||
if err != nil {
|
||||
fmt.Fprintf(os.Stderr, "WARN: Failed to fetch account for login: %v\n", err)
|
||||
controller.SetSessionError(app.DB, session, "Invalid username or password.")
|
||||
|
@ -278,7 +275,7 @@ func loginHandler(app *model.AppState) http.Handler {
|
|||
return
|
||||
}
|
||||
|
||||
err = bcrypt.CompareHashAndPassword([]byte(account.Password), []byte(credentials.Password))
|
||||
err = bcrypt.CompareHashAndPassword([]byte(account.Password), []byte(password))
|
||||
if err != nil {
|
||||
fmt.Printf(
|
||||
"[%s] INFO: Account \"%s\" attempted login with incorrect password.\n",
|
||||
|
@ -290,68 +287,126 @@ func loginHandler(app *model.AppState) http.Handler {
|
|||
return
|
||||
}
|
||||
|
||||
var totpMethod *model.TOTP
|
||||
if len(credentials.TOTP) == 0 {
|
||||
// check if user has TOTP
|
||||
totps, err := controller.GetTOTPsForAccount(app.DB, account.ID)
|
||||
totps, err := controller.GetTOTPsForAccount(app.DB, account.ID)
|
||||
if err != nil {
|
||||
fmt.Fprintf(os.Stderr, "WARN: Failed to fetch TOTPs: %v\n", err)
|
||||
controller.SetSessionError(app.DB, session, "Something went wrong. Please try again.")
|
||||
render()
|
||||
return
|
||||
}
|
||||
|
||||
if len(totps) > 0 {
|
||||
err = controller.SetSessionAttemptAccount(app.DB, session, account)
|
||||
if err != nil {
|
||||
fmt.Fprintf(os.Stderr, "WARN: Failed to fetch TOTPs: %v\n", err)
|
||||
fmt.Fprintf(os.Stderr, "WARN: Failed to set attempt session: %v\n", err)
|
||||
controller.SetSessionError(app.DB, session, "Something went wrong. Please try again.")
|
||||
render()
|
||||
return
|
||||
}
|
||||
|
||||
if len(totps) > 0 {
|
||||
type loginTOTPData struct {
|
||||
Session *model.Session
|
||||
Username string
|
||||
Password string
|
||||
}
|
||||
err = loginTOTPTemplate.Execute(w, loginTOTPData{
|
||||
Session: session,
|
||||
Username: credentials.Username,
|
||||
Password: credentials.Password,
|
||||
})
|
||||
if err != nil {
|
||||
fmt.Fprintf(os.Stderr, "WARN: Failed to render login TOTP page: %v\n", err)
|
||||
http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
}
|
||||
} else {
|
||||
totpMethod, err = controller.CheckTOTPForAccount(app.DB, account.ID, credentials.TOTP)
|
||||
if err != nil {
|
||||
fmt.Fprintf(os.Stderr, "WARN: Failed to fetch TOTPs: %v\n", err)
|
||||
controller.SetSessionError(app.DB, session, "Something went wrong. Please try again.")
|
||||
render()
|
||||
return
|
||||
}
|
||||
if totpMethod == nil {
|
||||
controller.SetSessionError(app.DB, session, "Invalid TOTP.")
|
||||
render()
|
||||
return
|
||||
}
|
||||
http.Redirect(w, r, "/admin/totp", http.StatusFound)
|
||||
return
|
||||
}
|
||||
|
||||
if totpMethod != nil {
|
||||
fmt.Printf(
|
||||
"[%s] INFO: Account \"%s\" logged in with method \"%s\"\n",
|
||||
time.Now().Format(time.UnixDate),
|
||||
account.Username,
|
||||
totpMethod.Name,
|
||||
)
|
||||
} else {
|
||||
fmt.Printf(
|
||||
"[%s] INFO: Account \"%s\" logged in\n",
|
||||
time.Now().Format(time.UnixDate),
|
||||
account.Username,
|
||||
)
|
||||
}
|
||||
fmt.Printf(
|
||||
"[%s] INFO: Account \"%s\" logged in\n",
|
||||
time.Now().Format(time.UnixDate),
|
||||
account.Username,
|
||||
)
|
||||
|
||||
// TODO: log login activity to user
|
||||
|
||||
// login success!
|
||||
controller.SetSessionAccount(app.DB, session, account)
|
||||
err = controller.SetSessionAccount(app.DB, session, account)
|
||||
if err != nil {
|
||||
fmt.Fprintf(os.Stderr, "WARN: Failed to set session account: %v\n", err)
|
||||
controller.SetSessionError(app.DB, session, "Something went wrong. Please try again.")
|
||||
render()
|
||||
return
|
||||
}
|
||||
controller.SetSessionMessage(app.DB, session, "")
|
||||
controller.SetSessionError(app.DB, session, "")
|
||||
http.Redirect(w, r, "/admin", http.StatusFound)
|
||||
})
|
||||
}
|
||||
|
||||
func loginTOTPHandler(app *model.AppState) http.Handler {
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
session := r.Context().Value("session").(*model.Session)
|
||||
|
||||
if session.AttemptAccount == nil {
|
||||
http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized)
|
||||
return
|
||||
}
|
||||
|
||||
type loginTOTPData struct {
|
||||
Session *model.Session
|
||||
}
|
||||
|
||||
render := func() {
|
||||
err := loginTOTPTemplate.Execute(w, loginTOTPData{ Session: session })
|
||||
if err != nil {
|
||||
fmt.Fprintf(os.Stderr, "WARN: Failed to render login TOTP page: %v\n", err)
|
||||
http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
if r.Method == http.MethodGet {
|
||||
render()
|
||||
return
|
||||
}
|
||||
|
||||
if r.Method != http.MethodPost {
|
||||
http.NotFound(w, r)
|
||||
return
|
||||
}
|
||||
|
||||
r.ParseForm()
|
||||
|
||||
if !r.Form.Has("totp") {
|
||||
http.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
|
||||
totpCode := r.FormValue("totp")
|
||||
|
||||
if len(totpCode) != controller.TOTP_CODE_LENGTH {
|
||||
controller.SetSessionError(app.DB, session, "Invalid TOTP.")
|
||||
render()
|
||||
return
|
||||
}
|
||||
|
||||
totpMethod, err := controller.CheckTOTPForAccount(app.DB, session.AttemptAccount.ID, totpCode)
|
||||
if err != nil {
|
||||
fmt.Fprintf(os.Stderr, "WARN: Failed to check TOTPs: %v\n", err)
|
||||
controller.SetSessionError(app.DB, session, "Something went wrong. Please try again.")
|
||||
render()
|
||||
return
|
||||
}
|
||||
if totpMethod == nil {
|
||||
controller.SetSessionError(app.DB, session, "Invalid TOTP.")
|
||||
render()
|
||||
return
|
||||
}
|
||||
|
||||
fmt.Printf(
|
||||
"[%s] INFO: Account \"%s\" logged in with method \"%s\"\n",
|
||||
time.Now().Format(time.UnixDate),
|
||||
session.AttemptAccount.Username,
|
||||
totpMethod.Name,
|
||||
)
|
||||
|
||||
err = controller.SetSessionAccount(app.DB, session, session.AttemptAccount)
|
||||
if err != nil {
|
||||
fmt.Fprintf(os.Stderr, "WARN: Failed to set session account: %v\n", err)
|
||||
controller.SetSessionError(app.DB, session, "Something went wrong. Please try again.")
|
||||
render()
|
||||
return
|
||||
}
|
||||
err = controller.SetSessionAttemptAccount(app.DB, session, nil)
|
||||
if err != nil {
|
||||
fmt.Fprintf(os.Stderr, "WARN: Failed to clear attempt session: %v\n", err)
|
||||
}
|
||||
controller.SetSessionMessage(app.DB, session, "")
|
||||
controller.SetSessionError(app.DB, session, "")
|
||||
http.Redirect(w, r, "/admin", http.StatusFound)
|
||||
|
@ -387,7 +442,7 @@ func logoutHandler(app *model.AppState) http.Handler {
|
|||
})
|
||||
}
|
||||
|
||||
func requireAccount(app *model.AppState, next http.Handler) http.HandlerFunc {
|
||||
func requireAccount(next http.Handler) http.HandlerFunc {
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
session := r.Context().Value("session").(*model.Session)
|
||||
if session.Account == nil {
|
||||
|
@ -425,7 +480,7 @@ func enforceSession(app *model.AppState, next http.Handler) http.Handler {
|
|||
sessionCookie, err := r.Cookie(model.COOKIE_TOKEN)
|
||||
if err != nil && err != http.ErrNoCookie {
|
||||
fmt.Fprintf(os.Stderr, "WARN: Failed to retrieve session cookie: %v\n", err)
|
||||
http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
|
||||
http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
|
||||
|
|
|
@ -26,14 +26,19 @@ input {
|
|||
|
||||
{{define "content"}}
|
||||
<main>
|
||||
<form action="/admin/login" method="POST" id="login-totp">
|
||||
{{if .Session.Message.Valid}}
|
||||
<p id="message">{{html .Session.Message.String}}</p>
|
||||
{{end}}
|
||||
{{if .Session.Error.Valid}}
|
||||
<p id="error">{{html .Session.Error.String}}</p>
|
||||
{{end}}
|
||||
|
||||
<form action="/admin/totp" method="POST" id="login-totp">
|
||||
<h1>Two-Factor Authentication</h1>
|
||||
|
||||
<div>
|
||||
<label for="totp">TOTP</label>
|
||||
<input type="text" name="totp" value="" autocomplete="one-time-code" required autofocus>
|
||||
<input type="hidden" name="username" value="{{.Username}}">
|
||||
<input type="hidden" name="password" value="{{.Password}}">
|
||||
</div>
|
||||
|
||||
<button type="submit" class="save">Login</button>
|
||||
|
|
|
@ -49,6 +49,17 @@ func CreateSession(db *sqlx.DB, userAgent string) (*model.Session, error) {
|
|||
// return err
|
||||
// }
|
||||
|
||||
func SetSessionAttemptAccount(db *sqlx.DB, session *model.Session, account *model.Account) error {
|
||||
var err error
|
||||
session.AttemptAccount = account
|
||||
if account == nil {
|
||||
_, err = db.Exec("UPDATE session SET attempt_account=NULL WHERE token=$1", session.Token)
|
||||
} else {
|
||||
_, err = db.Exec("UPDATE session SET attempt_account=$2 WHERE token=$1", session.Token, account.ID)
|
||||
}
|
||||
return err
|
||||
}
|
||||
|
||||
func SetSessionAccount(db *sqlx.DB, session *model.Session, account *model.Account) error {
|
||||
var err error
|
||||
session.Account = account
|
||||
|
@ -89,7 +100,8 @@ func SetSessionError(db *sqlx.DB, session *model.Session, message string) error
|
|||
func GetSession(db *sqlx.DB, token string) (*model.Session, error) {
|
||||
type dbSession struct {
|
||||
model.Session
|
||||
AccountID sql.NullString `db:"account"`
|
||||
AttemptAccountID sql.NullString `db:"attempt_account"`
|
||||
AccountID sql.NullString `db:"account"`
|
||||
}
|
||||
|
||||
session := dbSession{}
|
||||
|
@ -109,6 +121,13 @@ func GetSession(db *sqlx.DB, token string) (*model.Session, error) {
|
|||
}
|
||||
}
|
||||
|
||||
if session.AttemptAccountID.Valid {
|
||||
session.AttemptAccount, err = GetAccountByID(db, session.AttemptAccountID.String)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
|
||||
return &session.Session, err
|
||||
}
|
||||
|
||||
|
|
|
@ -6,12 +6,13 @@ import (
|
|||
)
|
||||
|
||||
type Session struct {
|
||||
Token string `json:"token" db:"token"`
|
||||
Token string `json:"-" db:"token"`
|
||||
UserAgent string `json:"user_agent" db:"user_agent"`
|
||||
CreatedAt time.Time `json:"created_at" db:"created_at"`
|
||||
ExpiresAt time.Time `json:"expires_at" db:"expires_at"`
|
||||
ExpiresAt time.Time `json:"-" db:"expires_at"`
|
||||
|
||||
Account *Account `json:"-" db:"account"`
|
||||
Account *Account `json:"-" db:"-"`
|
||||
AttemptAccount *Account `json:"-" db:"-"`
|
||||
Message sql.NullString `json:"-" db:"message"`
|
||||
Error sql.NullString `json:"-" db:"error"`
|
||||
}
|
||||
|
|
|
@ -35,6 +35,7 @@ CREATE TABLE arimelody.session (
|
|||
created_at TIMESTAMP NOT NULL DEFAULT current_timestamp,
|
||||
expires_at TIMESTAMP DEFAULT NULL,
|
||||
account UUID,
|
||||
attempt_account UUID,
|
||||
message TEXT,
|
||||
error TEXT
|
||||
);
|
||||
|
@ -120,6 +121,7 @@ ALTER TABLE arimelody.musicreleasetrack ADD CONSTRAINT musicreleasetrack_pk PRIM
|
|||
|
||||
ALTER TABLE arimelody.privilege ADD CONSTRAINT privilege_account_fk FOREIGN KEY (account) REFERENCES account(id) ON DELETE CASCADE;
|
||||
ALTER TABLE arimelody.session ADD CONSTRAINT session_account_fk FOREIGN KEY (account) REFERENCES account(id) ON DELETE CASCADE;
|
||||
ALTER TABLE arimelody.session ADD CONSTRAINT session_attempt_account_fk FOREIGN KEY (account) REFERENCES account(id) ON DELETE CASCADE;
|
||||
ALTER TABLE arimelody.totp ADD CONSTRAINT totp_account_fk FOREIGN KEY (account) REFERENCES account(id) ON DELETE CASCADE;
|
||||
|
||||
ALTER TABLE arimelody.musiccredit ADD CONSTRAINT musiccredit_artist_fk FOREIGN KEY (artist) REFERENCES artist(id) ON DELETE CASCADE ON UPDATE CASCADE;
|
||||
|
|
|
@ -35,6 +35,7 @@ CREATE TABLE arimelody.session (
|
|||
created_at TIMESTAMP NOT NULL DEFAULT current_timestamp,
|
||||
expires_at TIMESTAMP DEFAULT NULL,
|
||||
account UUID,
|
||||
attempt_account UUID,
|
||||
message TEXT,
|
||||
error TEXT
|
||||
);
|
||||
|
@ -52,5 +53,6 @@ ALTER TABLE arimelody.totp ADD CONSTRAINT totp_pk PRIMARY KEY (account, name);
|
|||
|
||||
-- Foreign keys
|
||||
ALTER TABLE arimelody.privilege ADD CONSTRAINT privilege_account_fk FOREIGN KEY (account) REFERENCES account(id) ON DELETE CASCADE;
|
||||
ALTER TABLE arimelody.session ADD CONSTRAINT session FOREIGN KEY (account) REFERENCES account(id) ON DELETE CASCADE;
|
||||
ALTER TABLE arimelody.session ADD CONSTRAINT session_account_fk FOREIGN KEY (account) REFERENCES account(id) ON DELETE CASCADE;
|
||||
ALTER TABLE arimelody.session ADD CONSTRAINT session_attempt_account_fk FOREIGN KEY (account) REFERENCES account(id) ON DELETE CASCADE;
|
||||
ALTER TABLE arimelody.totp ADD CONSTRAINT totp_account_fk FOREIGN KEY (account) REFERENCES account(id) ON DELETE CASCADE;
|
||||
|
|
Loading…
Reference in a new issue