fix activation of primary email addresses #16385

lunny · 2021-07-09T05:10:37Z

Meano commented

2021-07-09 05:10:37 +00:00

(Migrated from github.com)

When RegisterEmailConfirm is enabled, the primary email should be activated together with the user account.

When RegisterEmailConfirm is enabled, the primary email should be activated together with the user account. Issue #16370

6543 (Migrated from github.com) approved these changes 2021-07-09 12:51:28 +00:00

lunny reviewed 2021-07-10 08:10:28 +00:00

routers/web/user/setting/account.go

						
				@ -112,1 +110,3 @@

								log.Error("Send activation: email not set for activation")

						id := ctx.QueryInt64("id")

						email, err := models.GetEmailAddressByID(ctx.User.ID, id)

lunny commented

2021-07-10 08:10:28 +00:00

Now all emails are stored in email_address table, we should get primary email address from that table if id is primary

6543 (Migrated from github.com) reviewed 2021-07-10 17:40:54 +00:00

6543 (Migrated from github.com) left a comment

as per lunny

Meano (Migrated from github.com) reviewed 2021-07-12 05:30:48 +00:00

routers/web/user/auth.go

Meano (Migrated from github.com) commented

2021-07-12 05:28:46 +00:00

I have no idea that whether primary email is required in any cases when calling handleAccountActivation.
If primary email required anyway,

	if err := models.ActivateUserEmail(user.ID, user.Email, true); err != nil {
		ctx.ServerError("ActivateUserEmail", err)
		log.Error(fmt.Sprintf("Error active user mail: %v", err))
		return
	}

return should be added.
Or, the handleAccountActivation could be:

func handleAccountActivation(ctx *context.Context, user *models.User) {
	if err := models.ActivateUserEmail(user.ID, user.Email, true); err != nil {
		if models.IsErrUserNotExist(err) {
			ctx.NotFound("UpdateUserCols", err)
		} else {
			ctx.ServerError("AccountActivation", err)
		}
		log.Error(fmt.Sprintf("Error active user mail: %v", err))
		return
	}

	user.IsActive = true
	log.Trace("User activated: %s", user.Name)

	if err := ctx.Session.Set("uid", user.ID); err != nil {
		log.Error(fmt.Sprintf("Error setting uid in session: %v", err))
	}
	if err := ctx.Session.Set("uname", user.Name); err != nil {
		log.Error(fmt.Sprintf("Error setting uname in session: %v", err))
	}
	if err := ctx.Session.Release(); err != nil {
		log.Error("Error storing session: %v", err)
	}

	ctx.Flash.Success(ctx.Tr("auth.account_activated"))
	ctx.Redirect(setting.AppSubURL + "/")
}

, for the ActivateUserEmail will activate user account if the email address is primary.

I have no idea that whether primary email is required in any cases when calling `handleAccountActivation`. If primary email required anyway, ```suggestion if err := models.ActivateUserEmail(user.ID, user.Email, true); err != nil { ctx.ServerError("ActivateUserEmail", err) log.Error(fmt.Sprintf("Error active user mail: %v", err)) return } ``` `return` should be added. Or, the `handleAccountActivation` could be: ```go func handleAccountActivation(ctx *context.Context, user *models.User) { if err := models.ActivateUserEmail(user.ID, user.Email, true); err != nil { if models.IsErrUserNotExist(err) { ctx.NotFound("UpdateUserCols", err) } else { ctx.ServerError("AccountActivation", err) } log.Error(fmt.Sprintf("Error active user mail: %v", err)) return } user.IsActive = true log.Trace("User activated: %s", user.Name) if err := ctx.Session.Set("uid", user.ID); err != nil { log.Error(fmt.Sprintf("Error setting uid in session: %v", err)) } if err := ctx.Session.Set("uname", user.Name); err != nil { log.Error(fmt.Sprintf("Error setting uname in session: %v", err)) } if err := ctx.Session.Release(); err != nil { log.Error("Error storing session: %v", err) } ctx.Flash.Success(ctx.Tr("auth.account_activated")) ctx.Redirect(setting.AppSubURL + "/") } ``` , for the `ActivateUserEmail` will activate user account if the email address is primary.

zeripath reviewed 2021-07-13 18:54:53 +00:00

routers/web/user/setting/account.go

						
				@ -112,1 +110,3 @@

								log.Error("Send activation: email not set for activation")

						id := ctx.QueryInt64("id")

						email, err := models.GetEmailAddressByID(ctx.User.ID, id)

zeripath commented

2021-07-13 18:54:53 +00:00

Do you mean remove the Email and IsActive fields from the User table? I agree but I think that could/should be another PR - either in 1.16 or straight after this one is merged.