Skip to content

Commit 126ba79

Browse files
adelowolafriks
adelowo
authored and
lafriks
committed
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>
1 parent 10a2a90 commit 126ba79

File tree

13 files changed

+255
-22
lines changed

13 files changed

+255
-22
lines changed

‎models/migrations/migrations.go

+2
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,8 @@ var migrations = []Migration{
198198
NewMigration("protect each scratch token", addScratchHash),
199199
// v72 -> v73
200200
NewMigration("add review", addReview),
201+
// v73 -> v74
202+
NewMigration("add must_change_password column for users table", addMustChangePassword),
201203
}
202204

203205
// Migrate database to current version

‎models/migrations/v73.go

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// Copyright 2018 The Gitea Authors. All rights reserved.
2+
// Use of this source code is governed by a MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
package migrations
6+
7+
import (
8+
"github.com/go-xorm/xorm"
9+
)
10+
11+
func addMustChangePassword(x *xorm.Engine) error {
12+
// User see models/user.go
13+
type User struct {
14+
ID int64 `xorm:"pk autoincr"`
15+
MustChangePassword bool `xorm:"NOT NULL DEFAULT false"`
16+
}
17+
18+
return x.Sync2(new(User))
19+
}

‎models/user.go

+17-12
Original file line numberDiff line numberDiff line change
@@ -83,18 +83,23 @@ type User struct {
8383
Email string `xorm:"NOT NULL"`
8484
KeepEmailPrivate bool
8585
Passwd string `xorm:"NOT NULL"`
86-
LoginType LoginType
87-
LoginSource int64 `xorm:"NOT NULL DEFAULT 0"`
88-
LoginName string
89-
Type UserType
90-
OwnedOrgs []*User `xorm:"-"`
91-
Orgs []*User `xorm:"-"`
92-
Repos []*Repository `xorm:"-"`
93-
Location string
94-
Website string
95-
Rands string `xorm:"VARCHAR(10)"`
96-
Salt string `xorm:"VARCHAR(10)"`
97-
Language string `xorm:"VARCHAR(5)"`
86+
87+
// MustChangePassword is an attribute that determines if a user
88+
// is to change his/her password after registration.
89+
MustChangePassword bool `xorm:"NOT NULL DEFAULT false"`
90+
91+
LoginType LoginType
92+
LoginSource int64 `xorm:"NOT NULL DEFAULT 0"`
93+
LoginName string
94+
Type UserType
95+
OwnedOrgs []*User `xorm:"-"`
96+
Orgs []*User `xorm:"-"`
97+
Repos []*Repository `xorm:"-"`
98+
Location string
99+
Website string
100+
Rands string `xorm:"VARCHAR(10)"`
101+
Salt string `xorm:"VARCHAR(10)"`
102+
Language string `xorm:"VARCHAR(5)"`
98103

99104
CreatedUnix util.TimeStamp `xorm:"INDEX created"`
100105
UpdatedUnix util.TimeStamp `xorm:"INDEX updated"`

‎modules/auth/user_form.go

+12
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,18 @@ func (f *RegisterForm) Validate(ctx *macaron.Context, errs binding.Errors) bindi
8484
return validate(errs, ctx.Data, f, ctx.Locale)
8585
}
8686

87+
// MustChangePasswordForm form for updating your password after account creation
88+
// by an admin
89+
type MustChangePasswordForm struct {
90+
Password string `binding:"Required;MaxSize(255)"`
91+
Retype string
92+
}
93+
94+
// Validate valideates the fields
95+
func (f *MustChangePasswordForm) Validate(ctx *macaron.Context, errs binding.Errors) binding.Errors {
96+
return validate(errs, ctx.Data, f, ctx.Locale)
97+
}
98+
8799
// SignInForm form for signing in with user/password
88100
type SignInForm struct {
89101
UserName string `binding:"Required;MaxSize(254)"`

‎modules/context/auth.go

+25-4
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,31 @@ func Toggle(options *ToggleOptions) macaron.Handler {
3131
}
3232

3333
// Check prohibit login users.
34-
if ctx.IsSigned && ctx.User.ProhibitLogin {
35-
ctx.Data["Title"] = ctx.Tr("auth.prohibit_login")
36-
ctx.HTML(200, "user/auth/prohibit_login")
37-
return
34+
if ctx.IsSigned {
35+
36+
if ctx.User.ProhibitLogin {
37+
ctx.Data["Title"] = ctx.Tr("auth.prohibit_login")
38+
ctx.HTML(200, "user/auth/prohibit_login")
39+
return
40+
}
41+
42+
// prevent infinite redirection
43+
// also make sure that the form cannot be accessed by
44+
// users who don't need this
45+
if ctx.Req.URL.Path == setting.AppSubURL+"/user/settings/change_password" {
46+
if !ctx.User.MustChangePassword {
47+
ctx.Redirect(setting.AppSubURL + "/")
48+
}
49+
return
50+
}
51+
52+
if ctx.User.MustChangePassword {
53+
ctx.Data["Title"] = ctx.Tr("auth.must_change_password")
54+
ctx.Data["ChangePasscodeLink"] = setting.AppSubURL + "/user/change_password"
55+
ctx.SetCookie("redirect_to", url.QueryEscape(setting.AppSubURL+ctx.Req.RequestURI), 0, setting.AppSubURL)
56+
ctx.Redirect(setting.AppSubURL + "/user/settings/change_password")
57+
return
58+
}
3859
}
3960

4061
// Redirect to dashboard if user tries to visit any non-login page.

‎options/locale/locale_en-US.ini

+1
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@ forgot_password = Forgot password?
205205
sign_up_now = Need an account? Register now.
206206
sign_up_successful = Account was successfully created.
207207
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.
208+
must_change_password = Update your password
208209
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.
209210
active_your_account = Activate Your Account
210211
account_activated = Account has been activated

‎routers/admin/main_test.go

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Copyright 2018 The Gitea Authors. All rights reserved.
2+
// Use of this source code is governed by a MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
package admin
6+
7+
import (
8+
"path/filepath"
9+
"testing"
10+
11+
"code.gitea.io/gitea/models"
12+
)
13+
14+
func TestMain(m *testing.M) {
15+
models.MainTest(m, filepath.Join("..", ".."))
16+
}

‎routers/admin/users.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -77,11 +77,12 @@ func NewUserPost(ctx *context.Context, form auth.AdminCreateUserForm) {
7777
}
7878

7979
u := &models.User{
80-
Name: form.UserName,
81-
Email: form.Email,
82-
Passwd: form.Password,
83-
IsActive: true,
84-
LoginType: models.LoginPlain,
80+
Name: form.UserName,
81+
Email: form.Email,
82+
Passwd: form.Password,
83+
IsActive: true,
84+
LoginType: models.LoginPlain,
85+
MustChangePassword: true,
8586
}
8687

8788
if len(form.LoginType) > 0 {

‎routers/admin/users_test.go

+50
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// Copyright 2017 The Gitea Authors. All rights reserved.
2+
// Use of this source code is governed by a MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
package admin
6+
7+
import (
8+
"testing"
9+
10+
"code.gitea.io/gitea/models"
11+
"code.gitea.io/gitea/modules/auth"
12+
"code.gitea.io/gitea/modules/test"
13+
"github.com/stretchr/testify/assert"
14+
)
15+
16+
func TestNewUserPost_MustChangePassword(t *testing.T) {
17+
18+
models.PrepareTestEnv(t)
19+
ctx := test.MockContext(t, "admin/users/new")
20+
21+
u := models.AssertExistsAndLoadBean(t, &models.User{
22+
IsAdmin: true,
23+
ID: 2,
24+
}).(*models.User)
25+
26+
ctx.User = u
27+
28+
username := "gitea"
29+
email := "gitea@gitea.io"
30+
31+
form := auth.AdminCreateUserForm{
32+
LoginType: "local",
33+
LoginName: "local",
34+
UserName: username,
35+
Email: email,
36+
Password: "xxxxxxxx",
37+
SendNotify: false,
38+
}
39+
40+
NewUserPost(ctx, form)
41+
42+
assert.NotEmpty(t, ctx.Flash.SuccessMsg)
43+
44+
u, err := models.GetUserByName(username)
45+
46+
assert.NoError(t, err)
47+
assert.Equal(t, username, u.Name)
48+
assert.Equal(t, email, u.Email)
49+
assert.True(t, u.MustChangePassword)
50+
}

‎routers/routes/routes.go

+2
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,8 @@ func RegisterRoutes(m *macaron.Macaron) {
230230
m.Group("/user/settings", func() {
231231
m.Get("", userSetting.Profile)
232232
m.Post("", bindIgnErr(auth.UpdateProfileForm{}), userSetting.ProfilePost)
233+
m.Get("/change_password", user.MustChangePassword)
234+
m.Post("/change_password", bindIgnErr(auth.MustChangePasswordForm{}), user.MustChangePasswordPost)
233235
m.Post("/avatar", binding.MultipartForm(auth.AvatarForm{}), userSetting.AvatarPost)
234236
m.Post("/avatar/delete", userSetting.DeleteAvatar)
235237
m.Group("/account", func() {

‎routers/user/auth.go

+72-1
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ import (
2828
)
2929

3030
const (
31+
// tplMustChangePassword template for updating a user's password
32+
tplMustChangePassword = "user/auth/change_passwd"
3133
// tplSignIn template for sign in page
3234
tplSignIn base.TplName = "user/auth/signin"
3335
// tplSignUp template path for sign up page
@@ -1178,7 +1180,8 @@ func ResetPasswdPost(ctx *context.Context) {
11781180
return
11791181
}
11801182
u.HashPassword(passwd)
1181-
if err := models.UpdateUserCols(u, "passwd", "rands", "salt"); err != nil {
1183+
u.MustChangePassword = false
1184+
if err := models.UpdateUserCols(u, "must_change_password", "passwd", "rands", "salt"); err != nil {
11821185
ctx.ServerError("UpdateUser", err)
11831186
return
11841187
}
@@ -1191,3 +1194,71 @@ func ResetPasswdPost(ctx *context.Context) {
11911194
ctx.Data["IsResetFailed"] = true
11921195
ctx.HTML(200, tplResetPassword)
11931196
}
1197+
1198+
// MustChangePassword renders the page to change a user's password
1199+
func MustChangePassword(ctx *context.Context) {
1200+
ctx.Data["Title"] = ctx.Tr("auth.must_change_password")
1201+
ctx.Data["ChangePasscodeLink"] = setting.AppSubURL + "/user/settings/change_password"
1202+
1203+
ctx.HTML(200, tplMustChangePassword)
1204+
}
1205+
1206+
// MustChangePasswordPost response for updating a user's password after his/her
1207+
// account was created by an admin
1208+
func MustChangePasswordPost(ctx *context.Context, cpt *captcha.Captcha, form auth.MustChangePasswordForm) {
1209+
ctx.Data["Title"] = ctx.Tr("auth.must_change_password")
1210+
1211+
ctx.Data["ChangePasscodeLink"] = setting.AppSubURL + "/user/settings/change_password"
1212+
1213+
if ctx.HasError() {
1214+
ctx.HTML(200, tplMustChangePassword)
1215+
return
1216+
}
1217+
1218+
u := ctx.User
1219+
1220+
// Make sure only requests for users who are eligible to change their password via
1221+
// this method passes through
1222+
if !u.MustChangePassword {
1223+
ctx.ServerError("MustUpdatePassword", errors.New("cannot update password.. Please visit the settings page"))
1224+
return
1225+
}
1226+
1227+
if form.Password != form.Retype {
1228+
ctx.Data["Err_Password"] = true
1229+
ctx.RenderWithErr(ctx.Tr("form.password_not_match"), tplMustChangePassword, &form)
1230+
return
1231+
}
1232+
1233+
if len(form.Password) < setting.MinPasswordLength {
1234+
ctx.Data["Err_Password"] = true
1235+
ctx.RenderWithErr(ctx.Tr("auth.password_too_short", setting.MinPasswordLength), tplMustChangePassword, &form)
1236+
return
1237+
}
1238+
1239+
var err error
1240+
if u.Salt, err = models.GetUserSalt(); err != nil {
1241+
ctx.ServerError("UpdateUser", err)
1242+
return
1243+
}
1244+
1245+
u.HashPassword(form.Password)
1246+
u.MustChangePassword = false
1247+
1248+
if err := models.UpdateUserCols(u, "must_change_password", "passwd", "salt"); err != nil {
1249+
ctx.ServerError("UpdateUser", err)
1250+
return
1251+
}
1252+
1253+
ctx.Flash.Success(ctx.Tr("settings.change_password_success"))
1254+
1255+
log.Trace("User updated password: %s", u.Name)
1256+
1257+
if redirectTo, _ := url.QueryUnescape(ctx.GetCookie("redirect_to")); len(redirectTo) > 0 && !util.IsExternalURL(redirectTo) {
1258+
ctx.SetCookie("redirect_to", "", -1, setting.AppSubURL)
1259+
ctx.RedirectToFirst(redirectTo)
1260+
return
1261+
}
1262+
1263+
ctx.Redirect(setting.AppSubURL + "/")
1264+
}

‎templates/user/auth/change_passwd.tmpl

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{{template "base/head" .}}
2+
<div class="user signin{{if .LinkAccountMode}} icon{{end}}">
3+
<div class="ui container">
4+
{{template "user/auth/change_passwd_inner" .}}
5+
</div>
6+
</div>
7+
{{template "base/footer" .}}

‎templates/user/auth/change_passwd_inner.tmpl

+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
{{if or (not .LinkAccountMode) (and .LinkAccountMode .LinkAccountModeSignIn)}}
2+
{{template "base/alert" .}}
3+
{{end}}
4+
<h4 class="ui top attached header center">
5+
{{.i18n.Tr "settings.change_password"}}
6+
</h4>
7+
<div class="ui attached segment">
8+
<form class="ui form" action="{{.ChangePasscodeLink}}" method="post">
9+
{{.CsrfTokenHtml}}
10+
<div class="required inline field {{if and (.Err_Password) (or (not .LinkAccountMode) (and .LinkAccountMode .LinkAccountModeSignIn))}}error{{end}}">
11+
<label for="password">{{.i18n.Tr "password"}}</label>
12+
<input id="password" name="password" type="password" value="{{.password}}" autocomplete="off" required>
13+
</div>
14+
15+
16+
<div class="required inline field {{if and (.Err_Password) (or (not .LinkAccountMode) (and .LinkAccountMode .LinkAccountModeRegister))}}error{{end}}">
17+
<label for="retype">{{.i18n.Tr "re_type"}}</label>
18+
<input id="retype" name="retype" type="password" autocomplete="off" required>
19+
</div>
20+
21+
<div class="inline field">
22+
<label></label>
23+
<button class="ui green button">{{.i18n.Tr "settings.change_password" }}</button>
24+
</div>
25+
</form>
26+
</div>

0 commit comments

Comments
 (0)