Email confirmation fix (#1231)

* Email confirmation fix

* Added test for incorrect auth user confirming e-mail change

server/datastore/datastore_email_changes_test.go

@@ -21,13 +21,26 @@ func testChangeEmail(t *testing.T, ds kolide.Datastore) {
 	require.Nil(t, err)
 	err = ds.PendingEmailChange(user.ID, "xxxx@yyy.com", "abcd12345")
 	require.Nil(t, err)
-	newMail, err := ds.ConfirmPendingEmailChange("abcd12345")
+	newMail, err := ds.ConfirmPendingEmailChange(user.ID, "abcd12345")
 	require.Nil(t, err)
 	assert.Equal(t, "xxxx@yyy.com", newMail)
 	user, err = ds.UserByID(user.ID)
 	require.Nil(t, err)
 	assert.Equal(t, "xxxx@yyy.com", user.Email)
 	// this should fail because it doesn't exist
-	newMail, err = ds.ConfirmPendingEmailChange("abcd12345")
+	newMail, err = ds.ConfirmPendingEmailChange(user.ID, "abcd12345")
 	assert.NotNil(t, err)
+
+	// test that wrong user can't confirm e-mail change
+	err = ds.PendingEmailChange(user.ID, "other@bob.com", "uniquetoken")
+	require.Nil(t, err)
+	otheruser, err := ds.NewUser(&kolide.User{
+		Username: "fred",
+		Password: []byte("supersecret"),
+		Email:    "other@bobcom",
+	})
+	require.Nil(t, err)
+	_, err = ds.ConfirmPendingEmailChange(otheruser.ID, "uniquetoken")
+	assert.NotNil(t, err)
+
 }

server/datastore/inmem/email_changes.go

@@ -4,6 +4,6 @@ func (ds *Datastore) PendingEmailChange(uid uint, newEmail, token string) error
 	panic("deprecated")
 }
 
-func (ds *Datastore) ConfirmPendingEmailChange(token string) (string, error) {
+func (ds *Datastore) ConfirmPendingEmailChange(uid uint, token string) (string, error) {
 	panic("deprecated")
 }

server/datastore/mysql/email_changes.go

@@ -25,7 +25,7 @@ func (ds *Datastore) PendingEmailChange(uid uint, newEmail, token string) error
 
 // ConfirmPendingEmailChange finds email change record, updates user with new email,
 // then deletes change record if everything succeeds.
-func (ds *Datastore) ConfirmPendingEmailChange(token string) (newEmail string, err error) {
+func (ds *Datastore) ConfirmPendingEmailChange(id uint, token string) (newEmail string, err error) {
 	var (
 		tx      *sqlx.Tx
 		success bool // indicates all db operations success if true
@@ -36,7 +36,7 @@ func (ds *Datastore) ConfirmPendingEmailChange(token string) (newEmail string, e
 		Token    string
 		NewEmail string `db:"new_email"`
 	}{}
-	err = ds.db.Get(&changeRecord, "SELECT * FROM email_changes WHERE token = ?", token)
+	err = ds.db.Get(&changeRecord, "SELECT * FROM email_changes WHERE token = ? AND user_id = ?", token, id)
 	if err != nil {
 		if err == sql.ErrNoRows {
 			return "", notFound("email change with token")

server/kolide/users.go

@@ -24,8 +24,9 @@ type UserStore interface {
 	// with a link that they can use to confirm the change.
 	PendingEmailChange(userID uint, newEmail, token string) error
 	// ConfirmPendingEmailChange will confirm new email address identified by token is valid.
-	// The new email will be written to user record.
+	// The new email will be written to user record. userID is the ID of the
-	ConfirmPendingEmailChange(token string) (string, error)
+	// user whose e-mail is being changed.
+	ConfirmPendingEmailChange(userID uint, token string) (string, error)
 }
 
 // UserService contains methods for managing a Kolide User.

server/mock/datastore_users.go

@@ -20,7 +20,7 @@ type SaveUserFunc func(user *kolide.User) error
 
 type PendingEmailChangeFunc func(userID uint, newEmail string, token string) error
 
-type ConfirmPendingEmailChangeFunc func(token string) (string, error)
+type ConfirmPendingEmailChangeFunc func(userID uint, token string) (string, error)
 
 type UserStore struct {
 	NewUserFunc        NewUserFunc
@@ -83,7 +83,7 @@ func (s *UserStore) PendingEmailChange(userID uint, newEmail string, token strin
 	return s.PendingEmailChangeFunc(userID, newEmail, token)
 }
 
-func (s *UserStore) ConfirmPendingEmailChange(token string) (string, error) {
+func (s *UserStore) ConfirmPendingEmailChange(userID uint, token string) (string, error) {
 	s.ConfirmPendingEmailChangeFuncInvoked = true
-	return s.ConfirmPendingEmailChangeFunc(token)
+	return s.ConfirmPendingEmailChangeFunc(userID, token)
 }

server/service/service_users.go

@@ -159,7 +159,11 @@ func (svc service) modifyEmailAddress(ctx context.Context, user *kolide.User, em
 }
 
 func (svc service) ChangeUserEmail(ctx context.Context, token string) (string, error) {
-	return svc.ds.ConfirmPendingEmailChange(token)
+	vc, ok := viewer.FromContext(ctx)
+	if !ok {
+		return "", errNoContext
+	}
+	return svc.ds.ConfirmPendingEmailChange(vc.UserID(), token)
 }
 
 func (svc service) User(ctx context.Context, id uint) (*kolide.User, error) {