From ced52616787f4dac3423fb4c7ed4132e507c7184 Mon Sep 17 00:00:00 2001 From: Glenn Wilkinson Date: Mon, 7 Dec 2020 15:56:05 +0100 Subject: [PATCH] Added functionality to lock accounts (+bug fix) (#2060) * Added functionality to lock accounts * Fixed typo and added test case for locked account --- controllers/api/user.go | 12 +++++++----- controllers/controllers_test.go | 8 ++++++++ controllers/route.go | 12 ++++++++---- controllers/route_test.go | 11 +++++++++++ .../20201201000000_0.11.0_account_locked.sql | 7 +++++++ .../20201201000000_0.11.0_account_locked.sql | 6 ++++++ models/user.go | 1 + static/js/src/app/users.js | 8 ++++++-- templates/users.html | 4 ++++ 9 files changed, 58 insertions(+), 11 deletions(-) create mode 100644 db/db_mysql/migrations/20201201000000_0.11.0_account_locked.sql create mode 100644 db/db_sqlite3/migrations/20201201000000_0.11.0_account_locked.sql diff --git a/controllers/api/user.go b/controllers/api/user.go index c76fd38a..61dd2ff2 100644 --- a/controllers/api/user.go +++ b/controllers/api/user.go @@ -33,6 +33,7 @@ type userRequest struct { Password string `json:"password"` Role string `json:"role"` PasswordChangeRequired bool `json:"password_change_required"` + AccountLocked bool `json:"account_locked"` } func (ur *userRequest) Validate(existingUser *models.User) error { @@ -102,11 +103,12 @@ func (as *Server) Users(w http.ResponseWriter, r *http.Request) { return } user := models.User{ - Username: ur.Username, - Hash: hash, - ApiKey: auth.GenerateSecureKey(auth.APIKeyLength), - Role: role, - RoleID: role.ID, + Username: ur.Username, + Hash: hash, + ApiKey: auth.GenerateSecureKey(auth.APIKeyLength), + Role: role, + RoleID: role.ID, + PasswordChangeRequired: ur.PasswordChangeRequired, } err = models.PutUser(&user) if err != nil { diff --git a/controllers/controllers_test.go b/controllers/controllers_test.go index 96221aa1..31de71f1 100644 --- a/controllers/controllers_test.go +++ b/controllers/controllers_test.go @@ -49,6 +49,14 @@ func setupTest(t *testing.T) *testContext { if err != nil { t.Fatalf("error getting first user from database: %v", err) } + + // Create a second user to test account locked status + u2 := models.User{Username: "houdini", Hash: hash, AccountLocked: true} + models.PutUser(&u2) + if err != nil { + t.Fatalf("error creating new user: %v", err) + } + ctx.apiKey = u.ApiKey // Start the phishing server ctx.phishServer = httptest.NewUnstartedServer(NewPhishingServer(ctx.config.PhishConf).server.Handler) diff --git a/controllers/route.go b/controllers/route.go index 6ea299c8..58b1b974 100644 --- a/controllers/route.go +++ b/controllers/route.go @@ -304,9 +304,9 @@ func (as *AdminServer) nextOrIndex(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, next, 302) } -func (as *AdminServer) handleInvalidLogin(w http.ResponseWriter, r *http.Request) { +func (as *AdminServer) handleInvalidLogin(w http.ResponseWriter, r *http.Request, message string) { session := ctx.Get(r, "session").(*sessions.Session) - Flash(w, r, "danger", "Invalid Username/Password") + Flash(w, r, "danger", message) params := struct { User models.User Title string @@ -376,14 +376,18 @@ func (as *AdminServer) Login(w http.ResponseWriter, r *http.Request) { u, err := models.GetUserByUsername(username) if err != nil { log.Error(err) - as.handleInvalidLogin(w, r) + as.handleInvalidLogin(w, r, "Invalid Username/Password") return } // Validate the user's password err = auth.ValidatePassword(password, u.Hash) if err != nil { log.Error(err) - as.handleInvalidLogin(w, r) + as.handleInvalidLogin(w, r, "Invalid Username/Password") + return + } + if u.AccountLocked == true { + as.handleInvalidLogin(w, r, "Account Locked") return } u.LastLogin = time.Now().UTC() diff --git a/controllers/route_test.go b/controllers/route_test.go index c3bd186d..560f25dc 100644 --- a/controllers/route_test.go +++ b/controllers/route_test.go @@ -117,3 +117,14 @@ func TestSuccessfulRedirect(t *testing.T) { t.Fatalf("unexpected Location header received. expected %s got %s", next, url.Path) } } + +func TestAccountLocked(t *testing.T) { + ctx := setupTest(t) + defer tearDown(t, ctx) + resp := attemptLogin(t, ctx, nil, "houdini", "gophish", "") + got := resp.StatusCode + expected := http.StatusUnauthorized + if got != expected { + t.Fatalf("invalid status code received. expected %d got %d", expected, got) + } +} diff --git a/db/db_mysql/migrations/20201201000000_0.11.0_account_locked.sql b/db/db_mysql/migrations/20201201000000_0.11.0_account_locked.sql new file mode 100644 index 00000000..eb158adb --- /dev/null +++ b/db/db_mysql/migrations/20201201000000_0.11.0_account_locked.sql @@ -0,0 +1,7 @@ + +-- +goose Up +-- SQL in section 'Up' is executed when this migration is applied +ALTER TABLE `users` ADD COLUMN account_locked BOOLEAN; + +-- +goose Down +-- SQL section 'Down' is executed when this migration is rolled back diff --git a/db/db_sqlite3/migrations/20201201000000_0.11.0_account_locked.sql b/db/db_sqlite3/migrations/20201201000000_0.11.0_account_locked.sql new file mode 100644 index 00000000..74ac31d1 --- /dev/null +++ b/db/db_sqlite3/migrations/20201201000000_0.11.0_account_locked.sql @@ -0,0 +1,6 @@ +-- +goose Up +-- SQL in section 'Up' is executed when this migration is applied +ALTER TABLE users ADD COLUMN account_locked BOOLEAN; + +-- +goose Down +-- SQL section 'Down' is executed when this migration is rolled back diff --git a/models/user.go b/models/user.go index c1de9a29..ece94234 100644 --- a/models/user.go +++ b/models/user.go @@ -21,6 +21,7 @@ type User struct { Role Role `json:"role" gorm:"association_autoupdate:false;association_autocreate:false"` RoleID int64 `json:"-"` PasswordChangeRequired bool `json:"password_change_required"` + AccountLocked bool `json:"account_locked"` LastLogin time.Time `json:"last_login"` } diff --git a/static/js/src/app/users.js b/static/js/src/app/users.js index 01418be5..48d581cc 100644 --- a/static/js/src/app/users.js +++ b/static/js/src/app/users.js @@ -11,7 +11,8 @@ const save = (id) => { username: $("#username").val(), password: $("#password").val(), role: $("#role").val(), - password_change_required: $("#force_password_change_checkbox").prop('checked') + password_change_required: $("#force_password_change_checkbox").prop('checked'), + account_locked: $("#account_locked_checkbox").prop('checked') } // Submit the user if (id != -1) { @@ -49,6 +50,8 @@ const dismiss = () => { $("#password").val("") $("#confirm_password").val("") $("#role").val("") + $("#force_password_change_checkbox").prop('checked', true) + $("#account_locked_checkbox").prop('checked', false) $("#modal\\.flashes").empty() } @@ -66,7 +69,8 @@ const edit = (id) => { $("#username").val(user.username) $("#role").val(user.role.slug) $("#role").trigger("change") - $("#force_password_change_checkbox").prop('checked', false) + $("#force_password_change_checkbox").prop('checked', user.password_change_required) + $("#account_locked_checkbox").prop('checked', user.account_locked) }) .error(function () { errorFlash("Error fetching user") diff --git a/templates/users.html b/templates/users.html index d3db1f63..0626d714 100644 --- a/templates/users.html +++ b/templates/users.html @@ -67,6 +67,10 @@ +
+ + +