MM-58349 Don't allow remote user to create sessions or reset password (…

…mattermost#27215)

Automatic Merge

server/channels/api4/user_test.go

@@ -3875,10 +3875,11 @@ func TestLogin(t *testing.T) {
 		ruser, appErr := th.App.CreateUser(th.Context, &user)
 		require.Nil(t, appErr)
 
+		// remote user cannot reset password
 		_, err := th.SystemAdminClient.UpdateUserPassword(context.Background(), ruser.Id, "", "password")
-		require.NoError(t, err)
+		require.Error(t, err)
 
-		_, _, err = th.Client.Login(context.Background(), ruser.Email, "password")
+		_, _, err = th.Client.Login(context.Background(), ruser.Email, "hello1")
 		CheckErrorID(t, err, "api.user.login.remote_users.login.error")
 	})
 

server/channels/app/oauth_test.go

@@ -116,7 +116,8 @@ func TestOAuthDeleteApp(t *testing.T) {
 	session.IsOAuth = true
 	th.App.ch.srv.platform.SetSessionExpireInHours(session, 24)
 
-	session, _ = th.App.CreateSession(th.Context, session)
+	session, appErr := th.App.CreateSession(th.Context, session)
+	require.Nil(t, appErr)
 
 	accessData := &model.AccessData{}
 	accessData.Token = session.Token

server/channels/app/session.go

@@ -27,6 +27,17 @@ func (a *App) CreateSession(c request.CTX, session *model.Session) (*model.Sessi
 		return nil, appErr
 	}
 
+	// remote/synthetic users cannot create sessions. This lookup will already be cached.
+	// Some unit tests rely on sessions being created for users that don't exist, therefore
+	// missing users are allowed.
+	user, appErr := a.GetUser(session.UserId)
+	if appErr != nil && appErr.StatusCode != http.StatusNotFound {
+		return nil, appErr
+	}
+	if user != nil && user.IsRemote() {
+		return nil, model.NewAppError("login", "api.user.login.remote_users.login.error", nil, "", http.StatusUnauthorized)
+	}
+
 	session, err := a.ch.srv.platform.CreateSession(c, session)
 	if err != nil {
 		var invErr *store.ErrInvalidInput

server/channels/app/user.go

@@ -1440,6 +1440,11 @@ func (a *App) UpdatePassword(rctx request.CTX, user *model.User, newPassword str
 		return err
 	}
 
+	// remote/synthetic users cannot update password via any mechanism
+	if user.IsRemote() {
+		return model.NewAppError("UpdatePassword", "api.user.update_password.failed.app_error", nil, "", http.StatusInternalServerError)
+	}
+
 	hashedPassword := model.HashPassword(newPassword)
 
 	if err := a.Srv().Store().User().UpdatePassword(user.Id, hashedPassword); err != nil {
@@ -1475,6 +1480,11 @@ func (a *App) UpdateHashedPasswordByUserId(userID, newHashedPassword string) *mo
 }
 
 func (a *App) UpdateHashedPassword(user *model.User, newHashedPassword string) *model.AppError {
+	// remote/synthetic users cannot update password via any mechanism
+	if user.IsRemote() {
+		return model.NewAppError("UpdatePassword", "api.user.update_password.failed.app_error", nil, "", http.StatusInternalServerError)
+	}
+
 	if err := a.Srv().Store().User().UpdatePassword(user.Id, newHashedPassword); err != nil {
 		return model.NewAppError("UpdatePassword", "api.user.update_password.failed.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
 	}
@@ -1520,6 +1530,11 @@ func (a *App) resetPasswordFromToken(c request.CTX, userSuppliedTokenString, new
 		return model.NewAppError("ResetPasswordFromCode", "api.user.reset_password.sso.app_error", nil, "userId="+user.Id, http.StatusBadRequest)
 	}
 
+	// don't allow password reset for remote/synthetic users
+	if user.IsRemote() {
+		return model.NewAppError("resetPassword", "api.user.reset_password.broken_token.app_error", nil, "", http.StatusBadRequest)
+	}
+
 	T := i18n.GetUserTranslations(user.Locale)
 
 	if err := a.UpdatePasswordSendEmail(c, user, newPassword, T("api.user.reset_password.method")); err != nil {
@@ -1539,6 +1554,11 @@ func (a *App) SendPasswordReset(rctx request.CTX, email string, siteURL string)
 		return false, nil
 	}
 
+	// don't allow password reset for remote/synthetic users
+	if user.IsRemote() {
+		return false, model.NewAppError("SendPasswordReset", "api.user.send_password_reset.send.app_error", nil, "userId="+user.Id, http.StatusBadRequest)
+	}
+
 	if user.AuthData != nil && *user.AuthData != "" {
 		return false, model.NewAppError("SendPasswordReset", "api.user.send_password_reset.sso.app_error", nil, "userId="+user.Id, http.StatusBadRequest)
 	}