From 126ba796dcc9ccdf9c25ed7d441786478be2825b Mon Sep 17 00:00:00 2001
From: Lanre Adelowo <adelowomailbox@gmail.com>
Date: Thu, 13 Sep 2018 13:04:25 +0100
Subject: [PATCH] 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>
---
 models/migrations/migrations.go              |  2 +
 models/migrations/v73.go                     | 19 +++++
 models/user.go                               | 29 ++++----
 modules/auth/user_form.go                    | 12 ++++
 modules/context/auth.go                      | 29 ++++++--
 options/locale/locale_en-US.ini              |  1 +
 routers/admin/main_test.go                   | 16 +++++
 routers/admin/users.go                       | 11 +--
 routers/admin/users_test.go                  | 50 ++++++++++++++
 routers/routes/routes.go                     |  2 +
 routers/user/auth.go                         | 73 +++++++++++++++++++-
 templates/user/auth/change_passwd.tmpl       |  7 ++
 templates/user/auth/change_passwd_inner.tmpl | 26 +++++++
 13 files changed, 255 insertions(+), 22 deletions(-)
 create mode 100644 models/migrations/v73.go
 create mode 100644 routers/admin/main_test.go
 create mode 100644 routers/admin/users_test.go
 create mode 100644 templates/user/auth/change_passwd.tmpl
 create mode 100644 templates/user/auth/change_passwd_inner.tmpl

diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go
index 15bb0723c..6ac5004eb 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 000000000..1265b4519
--- /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 11cbdb2f4..01c7f5048 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 959a8ac41..43ddb29c7 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 c38cc3948..110122cb6 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 c20b6624b..e163a7e46 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 000000000..9a7191d47
--- /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 9aa78db10..ae8882ac1 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 000000000..8f6859940
--- /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 e5476fd22..bc4879b51 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 da4663f45..a4a0ee3e6 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 000000000..d84796348
--- /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 000000000..60d4a210e
--- /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>