diff options
author | Lanre Adelowo <adelowomailbox@gmail.com> | 2018-09-13 13:04:25 +0100 |
---|---|---|
committer | Lauris BH <lauris@nix.lv> | 2018-09-13 15:04:25 +0300 |
commit | 126ba796dcc9ccdf9c25ed7d441786478be2825b (patch) | |
tree | 63f0ceb0a89495cd86cf664b9ceba6b4cdca589b | |
parent | 10a2a904d7938e26f6d64fe9a9788185b802d4df (diff) | |
download | gitea-126ba796dcc9ccdf9c25ed7d441786478be2825b.tar.gz gitea-126ba796dcc9ccdf9c25ed7d441786478be2825b.zip |
Force user to change password (#4489)
* redirect to login page after successfully activating account
* force users to change password if account was created by an admin
* force users to change password if account was created by an admin
* fixed build
* fixed build
* fix pending issues with translation and wrong routes
* make sure path check is safe
* remove unneccessary newline
* make sure users that don't have to view the form get redirected
* move route to use /settings prefix so as to make sure unauthenticated users can't view the page
* update as per @lafriks review
* add necessary comment
* remove unrelated changes
* support redirecting to location the user actually want to go to before being forced to change his/her password
* run make fmt
* added tests
* improve assertions
* add assertion
* fix copyright year
Signed-off-by: Lanre Adelowo <yo@lanre.wtf>
-rw-r--r-- | models/migrations/migrations.go | 2 | ||||
-rw-r--r-- | models/migrations/v73.go | 19 | ||||
-rw-r--r-- | models/user.go | 29 | ||||
-rw-r--r-- | modules/auth/user_form.go | 12 | ||||
-rw-r--r-- | modules/context/auth.go | 29 | ||||
-rw-r--r-- | options/locale/locale_en-US.ini | 1 | ||||
-rw-r--r-- | routers/admin/main_test.go | 16 | ||||
-rw-r--r-- | routers/admin/users.go | 11 | ||||
-rw-r--r-- | routers/admin/users_test.go | 50 | ||||
-rw-r--r-- | routers/routes/routes.go | 2 | ||||
-rw-r--r-- | routers/user/auth.go | 73 | ||||
-rw-r--r-- | templates/user/auth/change_passwd.tmpl | 7 | ||||
-rw-r--r-- | templates/user/auth/change_passwd_inner.tmpl | 26 |
13 files changed, 255 insertions, 22 deletions
diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 15bb0723c0..6ac5004eb1 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -198,6 +198,8 @@ var migrations = []Migration{ NewMigration("protect each scratch token", addScratchHash), // v72 -> v73 NewMigration("add review", addReview), + // v73 -> v74 + NewMigration("add must_change_password column for users table", addMustChangePassword), } // Migrate database to current version diff --git a/models/migrations/v73.go b/models/migrations/v73.go new file mode 100644 index 0000000000..1265b4519e --- /dev/null +++ b/models/migrations/v73.go @@ -0,0 +1,19 @@ +// Copyright 2018 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "github.com/go-xorm/xorm" +) + +func addMustChangePassword(x *xorm.Engine) error { + // User see models/user.go + type User struct { + ID int64 `xorm:"pk autoincr"` + MustChangePassword bool `xorm:"NOT NULL DEFAULT false"` + } + + return x.Sync2(new(User)) +} diff --git a/models/user.go b/models/user.go index 11cbdb2f45..01c7f50489 100644 --- a/models/user.go +++ b/models/user.go @@ -83,18 +83,23 @@ type User struct { Email string `xorm:"NOT NULL"` KeepEmailPrivate bool Passwd string `xorm:"NOT NULL"` - LoginType LoginType - LoginSource int64 `xorm:"NOT NULL DEFAULT 0"` - LoginName string - Type UserType - OwnedOrgs []*User `xorm:"-"` - Orgs []*User `xorm:"-"` - Repos []*Repository `xorm:"-"` - Location string - Website string - Rands string `xorm:"VARCHAR(10)"` - Salt string `xorm:"VARCHAR(10)"` - Language string `xorm:"VARCHAR(5)"` + + // MustChangePassword is an attribute that determines if a user + // is to change his/her password after registration. + MustChangePassword bool `xorm:"NOT NULL DEFAULT false"` + + LoginType LoginType + LoginSource int64 `xorm:"NOT NULL DEFAULT 0"` + LoginName string + Type UserType + OwnedOrgs []*User `xorm:"-"` + Orgs []*User `xorm:"-"` + Repos []*Repository `xorm:"-"` + Location string + Website string + Rands string `xorm:"VARCHAR(10)"` + Salt string `xorm:"VARCHAR(10)"` + Language string `xorm:"VARCHAR(5)"` CreatedUnix util.TimeStamp `xorm:"INDEX created"` UpdatedUnix util.TimeStamp `xorm:"INDEX updated"` diff --git a/modules/auth/user_form.go b/modules/auth/user_form.go index 959a8ac417..43ddb29c76 100644 --- a/modules/auth/user_form.go +++ b/modules/auth/user_form.go @@ -84,6 +84,18 @@ func (f *RegisterForm) Validate(ctx *macaron.Context, errs binding.Errors) bindi return validate(errs, ctx.Data, f, ctx.Locale) } +// MustChangePasswordForm form for updating your password after account creation +// by an admin +type MustChangePasswordForm struct { + Password string `binding:"Required;MaxSize(255)"` + Retype string +} + +// Validate valideates the fields +func (f *MustChangePasswordForm) Validate(ctx *macaron.Context, errs binding.Errors) binding.Errors { + return validate(errs, ctx.Data, f, ctx.Locale) +} + // SignInForm form for signing in with user/password type SignInForm struct { UserName string `binding:"Required;MaxSize(254)"` diff --git a/modules/context/auth.go b/modules/context/auth.go index c38cc3948d..110122cb66 100644 --- a/modules/context/auth.go +++ b/modules/context/auth.go @@ -31,10 +31,31 @@ func Toggle(options *ToggleOptions) macaron.Handler { } // Check prohibit login users. - if ctx.IsSigned && ctx.User.ProhibitLogin { - ctx.Data["Title"] = ctx.Tr("auth.prohibit_login") - ctx.HTML(200, "user/auth/prohibit_login") - return + if ctx.IsSigned { + + if ctx.User.ProhibitLogin { + ctx.Data["Title"] = ctx.Tr("auth.prohibit_login") + ctx.HTML(200, "user/auth/prohibit_login") + return + } + + // prevent infinite redirection + // also make sure that the form cannot be accessed by + // users who don't need this + if ctx.Req.URL.Path == setting.AppSubURL+"/user/settings/change_password" { + if !ctx.User.MustChangePassword { + ctx.Redirect(setting.AppSubURL + "/") + } + return + } + + if ctx.User.MustChangePassword { + ctx.Data["Title"] = ctx.Tr("auth.must_change_password") + ctx.Data["ChangePasscodeLink"] = setting.AppSubURL + "/user/change_password" + ctx.SetCookie("redirect_to", url.QueryEscape(setting.AppSubURL+ctx.Req.RequestURI), 0, setting.AppSubURL) + ctx.Redirect(setting.AppSubURL + "/user/settings/change_password") + return + } } // Redirect to dashboard if user tries to visit any non-login page. diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index c20b6624b0..e163a7e46d 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -205,6 +205,7 @@ forgot_password = Forgot password? sign_up_now = Need an account? Register now. sign_up_successful = Account was successfully created. confirmation_mail_sent_prompt = A new confirmation email has been sent to <b>%s</b>. Please check your inbox within the next %s to complete the registration process. +must_change_password = Update your password reset_password_mail_sent_prompt = A confirmation email has been sent to <b>%s</b>. Please check your inbox within the next %s to complete the password reset process. active_your_account = Activate Your Account account_activated = Account has been activated diff --git a/routers/admin/main_test.go b/routers/admin/main_test.go new file mode 100644 index 0000000000..9a7191d471 --- /dev/null +++ b/routers/admin/main_test.go @@ -0,0 +1,16 @@ +// Copyright 2018 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package admin + +import ( + "path/filepath" + "testing" + + "code.gitea.io/gitea/models" +) + +func TestMain(m *testing.M) { + models.MainTest(m, filepath.Join("..", "..")) +} diff --git a/routers/admin/users.go b/routers/admin/users.go index 9aa78db103..ae8882ac12 100644 --- a/routers/admin/users.go +++ b/routers/admin/users.go @@ -77,11 +77,12 @@ func NewUserPost(ctx *context.Context, form auth.AdminCreateUserForm) { } u := &models.User{ - Name: form.UserName, - Email: form.Email, - Passwd: form.Password, - IsActive: true, - LoginType: models.LoginPlain, + Name: form.UserName, + Email: form.Email, + Passwd: form.Password, + IsActive: true, + LoginType: models.LoginPlain, + MustChangePassword: true, } if len(form.LoginType) > 0 { diff --git a/routers/admin/users_test.go b/routers/admin/users_test.go new file mode 100644 index 0000000000..8f6859940d --- /dev/null +++ b/routers/admin/users_test.go @@ -0,0 +1,50 @@ +// Copyright 2017 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package admin + +import ( + "testing" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/auth" + "code.gitea.io/gitea/modules/test" + "github.com/stretchr/testify/assert" +) + +func TestNewUserPost_MustChangePassword(t *testing.T) { + + models.PrepareTestEnv(t) + ctx := test.MockContext(t, "admin/users/new") + + u := models.AssertExistsAndLoadBean(t, &models.User{ + IsAdmin: true, + ID: 2, + }).(*models.User) + + ctx.User = u + + username := "gitea" + email := "gitea@gitea.io" + + form := auth.AdminCreateUserForm{ + LoginType: "local", + LoginName: "local", + UserName: username, + Email: email, + Password: "xxxxxxxx", + SendNotify: false, + } + + NewUserPost(ctx, form) + + assert.NotEmpty(t, ctx.Flash.SuccessMsg) + + u, err := models.GetUserByName(username) + + assert.NoError(t, err) + assert.Equal(t, username, u.Name) + assert.Equal(t, email, u.Email) + assert.True(t, u.MustChangePassword) +} diff --git a/routers/routes/routes.go b/routers/routes/routes.go index e5476fd227..bc4879b51a 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -230,6 +230,8 @@ func RegisterRoutes(m *macaron.Macaron) { m.Group("/user/settings", func() { m.Get("", userSetting.Profile) m.Post("", bindIgnErr(auth.UpdateProfileForm{}), userSetting.ProfilePost) + m.Get("/change_password", user.MustChangePassword) + m.Post("/change_password", bindIgnErr(auth.MustChangePasswordForm{}), user.MustChangePasswordPost) m.Post("/avatar", binding.MultipartForm(auth.AvatarForm{}), userSetting.AvatarPost) m.Post("/avatar/delete", userSetting.DeleteAvatar) m.Group("/account", func() { diff --git a/routers/user/auth.go b/routers/user/auth.go index da4663f452..a4a0ee3e6a 100644 --- a/routers/user/auth.go +++ b/routers/user/auth.go @@ -28,6 +28,8 @@ import ( ) const ( + // tplMustChangePassword template for updating a user's password + tplMustChangePassword = "user/auth/change_passwd" // tplSignIn template for sign in page tplSignIn base.TplName = "user/auth/signin" // tplSignUp template path for sign up page @@ -1178,7 +1180,8 @@ func ResetPasswdPost(ctx *context.Context) { return } u.HashPassword(passwd) - if err := models.UpdateUserCols(u, "passwd", "rands", "salt"); err != nil { + u.MustChangePassword = false + if err := models.UpdateUserCols(u, "must_change_password", "passwd", "rands", "salt"); err != nil { ctx.ServerError("UpdateUser", err) return } @@ -1191,3 +1194,71 @@ func ResetPasswdPost(ctx *context.Context) { ctx.Data["IsResetFailed"] = true ctx.HTML(200, tplResetPassword) } + +// MustChangePassword renders the page to change a user's password +func MustChangePassword(ctx *context.Context) { + ctx.Data["Title"] = ctx.Tr("auth.must_change_password") + ctx.Data["ChangePasscodeLink"] = setting.AppSubURL + "/user/settings/change_password" + + ctx.HTML(200, tplMustChangePassword) +} + +// MustChangePasswordPost response for updating a user's password after his/her +// account was created by an admin +func MustChangePasswordPost(ctx *context.Context, cpt *captcha.Captcha, form auth.MustChangePasswordForm) { + ctx.Data["Title"] = ctx.Tr("auth.must_change_password") + + ctx.Data["ChangePasscodeLink"] = setting.AppSubURL + "/user/settings/change_password" + + if ctx.HasError() { + ctx.HTML(200, tplMustChangePassword) + return + } + + u := ctx.User + + // Make sure only requests for users who are eligible to change their password via + // this method passes through + if !u.MustChangePassword { + ctx.ServerError("MustUpdatePassword", errors.New("cannot update password.. Please visit the settings page")) + return + } + + if form.Password != form.Retype { + ctx.Data["Err_Password"] = true + ctx.RenderWithErr(ctx.Tr("form.password_not_match"), tplMustChangePassword, &form) + return + } + + if len(form.Password) < setting.MinPasswordLength { + ctx.Data["Err_Password"] = true + ctx.RenderWithErr(ctx.Tr("auth.password_too_short", setting.MinPasswordLength), tplMustChangePassword, &form) + return + } + + var err error + if u.Salt, err = models.GetUserSalt(); err != nil { + ctx.ServerError("UpdateUser", err) + return + } + + u.HashPassword(form.Password) + u.MustChangePassword = false + + if err := models.UpdateUserCols(u, "must_change_password", "passwd", "salt"); err != nil { + ctx.ServerError("UpdateUser", err) + return + } + + ctx.Flash.Success(ctx.Tr("settings.change_password_success")) + + log.Trace("User updated password: %s", u.Name) + + if redirectTo, _ := url.QueryUnescape(ctx.GetCookie("redirect_to")); len(redirectTo) > 0 && !util.IsExternalURL(redirectTo) { + ctx.SetCookie("redirect_to", "", -1, setting.AppSubURL) + ctx.RedirectToFirst(redirectTo) + return + } + + ctx.Redirect(setting.AppSubURL + "/") +} diff --git a/templates/user/auth/change_passwd.tmpl b/templates/user/auth/change_passwd.tmpl new file mode 100644 index 0000000000..d84796348e --- /dev/null +++ b/templates/user/auth/change_passwd.tmpl @@ -0,0 +1,7 @@ +{{template "base/head" .}} +<div class="user signin{{if .LinkAccountMode}} icon{{end}}"> + <div class="ui container"> + {{template "user/auth/change_passwd_inner" .}} + </div> +</div> +{{template "base/footer" .}} diff --git a/templates/user/auth/change_passwd_inner.tmpl b/templates/user/auth/change_passwd_inner.tmpl new file mode 100644 index 0000000000..60d4a210ee --- /dev/null +++ b/templates/user/auth/change_passwd_inner.tmpl @@ -0,0 +1,26 @@ + {{if or (not .LinkAccountMode) (and .LinkAccountMode .LinkAccountModeSignIn)}} + {{template "base/alert" .}} + {{end}} + <h4 class="ui top attached header center"> + {{.i18n.Tr "settings.change_password"}} + </h4> + <div class="ui attached segment"> + <form class="ui form" action="{{.ChangePasscodeLink}}" method="post"> + {{.CsrfTokenHtml}} + <div class="required inline field {{if and (.Err_Password) (or (not .LinkAccountMode) (and .LinkAccountMode .LinkAccountModeSignIn))}}error{{end}}"> + <label for="password">{{.i18n.Tr "password"}}</label> + <input id="password" name="password" type="password" value="{{.password}}" autocomplete="off" required> + </div> + + + <div class="required inline field {{if and (.Err_Password) (or (not .LinkAccountMode) (and .LinkAccountMode .LinkAccountModeRegister))}}error{{end}}"> + <label for="retype">{{.i18n.Tr "re_type"}}</label> + <input id="retype" name="retype" type="password" autocomplete="off" required> + </div> + + <div class="inline field"> + <label></label> + <button class="ui green button">{{.i18n.Tr "settings.change_password" }}</button> + </div> + </form> + </div> |