Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Force user to change password #4489

Merged
merged 39 commits into from
Sep 13, 2018
Merged
Show file tree
Hide file tree
Changes from 38 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
0bef325
redirect to login page after successfully activating account
adelowo Jul 15, 2018
68d57a1
Merge remote-tracking branch 'origin' into force_user_password_change…
adelowo Jul 21, 2018
23acb29
force users to change password if account was created by an admin
adelowo Jul 21, 2018
ff42bfd
force users to change password if account was created by an admin
adelowo Jul 21, 2018
26fccdf
fixed build
adelowo Jul 21, 2018
4562460
fixed build
adelowo Jul 21, 2018
5a2ea86
fix pending issues with translation and wrong routes
adelowo Jul 21, 2018
3b87fef
make sure path check is safe
adelowo Jul 21, 2018
7e67ecc
remove unneccessary newline
adelowo Jul 21, 2018
59432fa
make sure users that don't have to view the form get redirected
adelowo Jul 21, 2018
741ef66
move route to use /settings prefix so as to make sure unauthenticated…
adelowo Jul 21, 2018
9b4f70f
update as per @lafriks review
adelowo Jul 21, 2018
845c00b
add necessary comment
adelowo Jul 21, 2018
3b7258e
Merge branch 'master' into force_user_to_change_password
lafriks Jul 22, 2018
b6b39d3
remove unrelated changes
adelowo Jul 22, 2018
4ebdfc1
Merge branch 'force_user_to_change_password' of github.com:adelowo/gi…
adelowo Jul 22, 2018
e0f8fd8
support redirecting to location the user actually want to go to befor…
adelowo Jul 23, 2018
3e5ed18
Merge branch 'master' into force_user_to_change_password
adelowo Jul 27, 2018
1a75475
Fix merge connflicts
adelowo Jul 28, 2018
4402c56
Merge branch 'master' of https://github.com/go-gitea/gitea into force…
adelowo Jul 28, 2018
f7e1e08
run make fmt
adelowo Jul 28, 2018
4d96ba5
added tests
adelowo Jul 30, 2018
95f035d
improve assertions
adelowo Jul 30, 2018
8ea7cdc
add assertion
adelowo Jul 30, 2018
0181ebf
fix copyright year
adelowo Aug 1, 2018
d083410
Merge branch 'master' into force_user_to_change_password
adelowo Aug 1, 2018
d3febca
Merge branch 'master' of https://github.com/go-gitea/gitea into force…
adelowo Aug 6, 2018
dafa9c8
Merge branch 'master' into force_user_to_change_password
adelowo Aug 10, 2018
8c6e9c6
Merge branch 'master' into force_user_to_change_password
adelowo Aug 14, 2018
454cd3a
Merge branch 'master' into force_user_to_change_password
adelowo Aug 16, 2018
d7ee5f8
Merge branch 'master' into force_user_to_change_password
adelowo Aug 21, 2018
5441645
Merge branch 'master' into force_user_to_change_password
adelowo Aug 21, 2018
98933c6
Merge branch 'master' into force_user_to_change_password
adelowo Aug 23, 2018
d2ddc69
Merge branch 'master' into force_user_to_change_password
adelowo Aug 24, 2018
4c6658a
Merge branch 'master' into force_user_to_change_password
adelowo Aug 30, 2018
e2ef29b
Merge branch 'master' into force_user_to_change_password
adelowo Sep 4, 2018
16b5b61
Merge branch 'master' into force_user_to_change_password
techknowlogick Sep 5, 2018
a4152f2
Merge branch 'master' into force_user_to_change_password
adelowo Sep 10, 2018
a03c9a3
Merge branch 'master' into force_user_to_change_password
lafriks Sep 13, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions models/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 19 additions & 0 deletions models/migrations/v73.go
Original file line number Diff line number Diff line change
@@ -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))
}
29 changes: 17 additions & 12 deletions models/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
12 changes: 12 additions & 0 deletions modules/auth/user_form.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)"`
Expand Down
29 changes: 25 additions & 4 deletions modules/context/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,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
Expand Down
16 changes: 16 additions & 0 deletions routers/admin/main_test.go
Original file line number Diff line number Diff line change
@@ -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("..", ".."))
}
11 changes: 6 additions & 5 deletions routers/admin/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
adelowo marked this conversation as resolved.
Show resolved Hide resolved
}

if len(form.LoginType) > 0 {
Expand Down
50 changes: 50 additions & 0 deletions routers/admin/users_test.go
Original file line number Diff line number Diff line change
@@ -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 := "[email protected]"

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)
}
2 changes: 2 additions & 0 deletions routers/routes/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
73 changes: 72 additions & 1 deletion routers/user/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ import (
)

const (
// tplMustChangePassword template for updating a user's password
tplMustChangePassword = "user/auth/change_passwd"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: comment would be recommended here like it is for other constants below

// tplSignIn template for sign in page
tplSignIn base.TplName = "user/auth/signin"
// tplSignUp template path for sign up page
Expand Down Expand Up @@ -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
}
Expand All @@ -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 + "/")
}
7 changes: 7 additions & 0 deletions templates/user/auth/change_passwd.tmpl
Original file line number Diff line number Diff line change
@@ -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" .}}
26 changes: 26 additions & 0 deletions templates/user/auth/change_passwd_inner.tmpl
Original file line number Diff line number Diff line change
@@ -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>