added App.DeleteAllExternalAuthsByRecord

CHANGELOG.md

@@ -8,9 +8,12 @@
 
 - Fixed autocomplete selection not properly updating the underlying input value ([#7664](https://github.com/pocketbase/pocketbase/issues/7664)).
 
-- Adjusted Bitbucket, GitHub and Gitea/Forgejo OAuth2 providers to better reflect recent API updates and doc references.
-    _The providers also now always send an extra emails_list request to fetch only the explicitly verified primary email in order to eliminate eventual security issues caused by misconfigured onpremise setups._
+- Added dummy bcrypt password check for the failure auth path to minimize enumaration timing attacks.
 
+- Adjusted Bitbucket, GitHub and Gitea/Forgejo OAuth2 providers to better reflect recent API updates and doc references.
+    _The providers also now always send their respective dedicated emails_list request to fetch only the verified primary email in order to minimize eventual linking security issues caused by specific onpremise setups._
+
+- ⚠️ Fixed a pre-hijacking OAuth2 linking vulnerability ([#7662](https://github.com/pocketbase/pocketbase/discussions/7662); thanks @Alardiians for reporting it privately).
 
 
 ## v0.37.3

apis/record_auth_email_change_confirm_test.go

@@ -111,12 +111,51 @@ func TestRecordConfirmEmailChange(t *testing.T) {
 				"OnRecordUpdateExecute":             1,
 				"OnRecordAfterUpdateSuccess":        1,
 				"OnRecordValidate":                  1,
+				// unverified->verified external auths removal
+				"OnModelDelete":              2,
+				"OnModelDeleteExecute":       2,
+				"OnModelAfterDeleteSuccess":  2,
+				"OnRecordDelete":             2,
+				"OnRecordDeleteExecute":      2,
+				"OnRecordAfterDeleteSuccess": 2,
+			},
+			BeforeTestFunc: func(t testing.TB, app *tests.TestApp, e *core.ServeEvent) {
+				user, err := app.FindAuthRecordByEmail("users", "test@example.com")
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				if user.Verified() {
+					t.Fatalf("Expected the user to be unverified before the confirmation")
+				}
+
+				// ensure that there is at least one pre-existing OAuth2 link
+				externalAuths, err := app.FindAllExternalAuthsByRecord(user)
+				if err != nil {
+					t.Fatal(err)
+				}
+				if len(externalAuths) == 0 {
+					t.Fatal("Expected at least one external auths")
+				}
 			},
 			AfterTestFunc: func(t testing.TB, app *tests.TestApp, res *http.Response) {
-				_, err := app.FindAuthRecordByEmail("users", "change@example.com")
+				user, err := app.FindAuthRecordByEmail("users", "change@example.com")
 				if err != nil {
 					t.Fatalf("Expected to find user with email %q, got error: %v", "change@example.com", err)
 				}
+
+				if !user.Verified() {
+					t.Fatalf("Expected the user to be verified after the confirmation")
+				}
+
+				// ensure that all pre-existing OAuth2 links are cleared
+				externalAuths, err := app.FindAllExternalAuthsByRecord(user)
+				if err != nil {
+					t.Fatal(err)
+				}
+				if len(externalAuths) > 0 {
+					t.Fatalf("Expected all external auths to be cleared, found %d", len(externalAuths))
+				}
 			},
 		},
 		{

apis/record_auth_password_reset_confirm_test.go

@@ -114,11 +114,18 @@ func TestRecordConfirmPasswordReset(t *testing.T) {
 				"OnModelUpdate":                       1,
 				"OnModelUpdateExecute":                1,
 				"OnModelAfterUpdateSuccess":           1,
-				"OnModelValidate":                     1,
 				"OnRecordUpdate":                      1,
 				"OnRecordUpdateExecute":               1,
 				"OnRecordAfterUpdateSuccess":          1,
+				"OnModelValidate":                     1,
 				"OnRecordValidate":                    1,
+				// ---
+				"OnModelDelete":              2, // pre-existing OAuth2 links
+				"OnModelDeleteExecute":       2,
+				"OnModelAfterDeleteSuccess":  2,
+				"OnRecordDelete":             2,
+				"OnRecordDeleteExecute":      2,
+				"OnRecordAfterDeleteSuccess": 2,
 			},
 			BeforeTestFunc: func(t testing.TB, app *tests.TestApp, e *core.ServeEvent) {
 				user, err := app.FindAuthRecordByEmail("users", "test@example.com")
@@ -151,6 +158,15 @@ func TestRecordConfirmPasswordReset(t *testing.T) {
 				if !user.ValidatePassword("1234567!") {
 					t.Fatal("Password wasn't changed")
 				}
+
+				// ensure that all pre-existing OAuth2 links are cleared
+				externalAuths, err := app.FindAllExternalAuthsByRecord(user)
+				if err != nil {
+					t.Fatal(err)
+				}
+				if len(externalAuths) > 0 {
+					t.Fatalf("Expected all external auths to be cleared, found %d", len(externalAuths))
+				}
 			},
 		},
 		{
@@ -221,6 +237,15 @@ func TestRecordConfirmPasswordReset(t *testing.T) {
 				if !user.ValidatePassword("1234567!") {
 					t.Fatal("Password wasn't changed")
 				}
+
+				// ensure that all pre-existing OAuth2 were NOT deleted
+				externalAuths, err := app.FindAllExternalAuthsByRecord(user)
+				if err != nil {
+					t.Fatal(err)
+				}
+				if len(externalAuths) != 2 {
+					t.Fatalf("Expected 2 external auths, found %d", len(externalAuths))
+				}
 			},
 		},
 		{
@@ -251,11 +276,20 @@ func TestRecordConfirmPasswordReset(t *testing.T) {
 					t.Fatalf("Failed to fetch confirm password user: %v", err)
 				}
 
+				oldTokenKey := user.TokenKey()
+
 				// ensure that the user is already verified
 				user.SetVerified(true)
 				if err := app.Save(user); err != nil {
 					t.Fatalf("Failed to update user verified state")
 				}
+
+				// resave with the old token key since the verified change above
+				// would refresh it and will make the password token invalid
+				user.SetTokenKey(oldTokenKey)
+				if err = app.Save(user); err != nil {
+					t.Fatalf("Failed to restore original user tokenKey: %v", err)
+				}
 			},
 			AfterTestFunc: func(t testing.TB, app *tests.TestApp, res *http.Response) {
 				_, err := app.FindAuthRecordByToken(

apis/record_auth_verification_confirm_test.go

@@ -105,6 +105,51 @@ func TestRecordConfirmVerification(t *testing.T) {
 				"OnRecordValidate":                   1,
 				"OnRecordUpdateExecute":              1,
 				"OnRecordAfterUpdateSuccess":         1,
+				// unverified->verified external auths removal
+				"OnModelDelete":              2,
+				"OnModelDeleteExecute":       2,
+				"OnModelAfterDeleteSuccess":  2,
+				"OnRecordDelete":             2,
+				"OnRecordDeleteExecute":      2,
+				"OnRecordAfterDeleteSuccess": 2,
+			},
+			BeforeTestFunc: func(t testing.TB, app *tests.TestApp, e *core.ServeEvent) {
+				user, err := app.FindAuthRecordByEmail("users", "test@example.com")
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				if user.Verified() {
+					t.Fatalf("Expected the user to be unverified before the confirmation")
+				}
+
+				// ensure that there is at least one pre-existing OAuth2 link
+				externalAuths, err := app.FindAllExternalAuthsByRecord(user)
+				if err != nil {
+					t.Fatal(err)
+				}
+				if len(externalAuths) == 0 {
+					t.Fatal("Expected at least one external auths")
+				}
+			},
+			AfterTestFunc: func(t testing.TB, app *tests.TestApp, res *http.Response) {
+				user, err := app.FindAuthRecordByEmail("users", "test@example.com")
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				if !user.Verified() {
+					t.Fatalf("Expected the user to be verified after the confirmation")
+				}
+
+				// ensure that all pre-existing OAuth2 links are cleared
+				externalAuths, err := app.FindAllExternalAuthsByRecord(user)
+				if err != nil {
+					t.Fatal(err)
+				}
+				if len(externalAuths) > 0 {
+					t.Fatalf("Expected all external auths to be cleared, found %d", len(externalAuths))
+				}
 			},
 		},
 		{

apis/record_auth_with_oauth2.go

@@ -338,26 +338,43 @@ func oauth2Submit(e *core.RecordAuthWithOAuth2RequestEvent, optExternalAuth *cor
 				e.Auth.Id == e.Record.Id &&
 				e.Auth.Collection().Id == e.Record.Collection().Id
 
-			// set random password for users with unverified email
-			// (this is in case a malicious actor has registered previously with the user email)
-			if !isLoggedAuthRecord && e.Record.Email() != "" && !e.Record.Verified() {
-				e.Record.SetRandomPassword()
+			// prevent pre-hijacking with password auth
+			//
+			// reset the unverified user password in case the record was precreated by a malicious actor
+			if !isLoggedAuthRecord && !e.Record.Verified() {
 				needUpdate = true
+				e.Record.SetRandomPassword()
+			}
+
+			// prevent pre-hijacking with different OAuth2 provider
+			//
+			// delete all other previous OAuth2 record links for the cases
+			// when the user was precreated by malicious OAuth2 auth with custom payload data
+			//
+			// while this would be also done automatically on unverified -> verified upgrade,
+			// doing it manually here ensures that a single unverified record could have
+			// max 1 OAuth2 link to prevent further abuse when mixed with other auth flows
+			if !e.Record.Verified() {
+				err := txApp.DeleteAllExternalAuthsByRecord(e.Record)
+				if err != nil {
+					return err
+				}
+				optExternalAuth = nil // clear to allow recreate below
 			}
 
 			// update the existing auth record empty email if the data.OAuth2User has one
 			// (this is in case previously the auth record was created
 			// with an OAuth2 provider that didn't return an email address)
 			if e.Record.Email() == "" && e.OAuth2User.Email != "" {
-				e.Record.SetEmail(e.OAuth2User.Email)
 				needUpdate = true
+				e.Record.SetEmail(e.OAuth2User.Email)
 			}
 
 			// update the existing auth record verified state
 			// (only if the auth record doesn't have an email or the auth record email match with the one in data.OAuth2User)
 			if !e.Record.Verified() && (e.Record.Email() == "" || e.Record.Email() == e.OAuth2User.Email) {
-				e.Record.SetVerified(true)
 				needUpdate = true
+				e.Record.SetVerified(true)
 			}
 
 			if needUpdate {

apis/record_auth_with_oauth2_test.go

@@ -178,6 +178,20 @@ func TestRecordAuthWithOAuth2(t *testing.T) {
 					t.Fatal(err)
 				}
 
+				// ensure that there is at least one other external auth different than test
+				// so that later we can verify that it was deleted
+				var hasAtLeastOneOtherEA = false
+				externalAuths, _ := app.FindAllExternalAuthsByRecord(user)
+				for _, rel := range externalAuths {
+					if rel.Id != ea.Id {
+						hasAtLeastOneOtherEA = true
+						break
+					}
+				}
+				if !hasAtLeastOneOtherEA {
+					t.Fatal("Expected at least one non-test external auth linked")
+				}
+
 				// test at least once that the correct request info context is properly loaded
 				app.OnRecordAuthRequest().BindFunc(func(e *core.RecordAuthRequestEvent) error {
 					info, err := e.RequestInfo()
@@ -213,12 +227,12 @@ func TestRecordAuthWithOAuth2(t *testing.T) {
 				"OnRecordAuthRequest":           1,
 				"OnRecordEnrich":                1,
 				// ---
-				"OnModelCreate":              1,
-				"OnModelCreateExecute":       1,
-				"OnModelAfterCreateSuccess":  1,
-				"OnRecordCreate":             1,
-				"OnRecordCreateExecute":      1,
-				"OnRecordAfterCreateSuccess": 1,
+				"OnModelCreate":              2, // user + recreated external auth
+				"OnModelCreateExecute":       2,
+				"OnModelAfterCreateSuccess":  2,
+				"OnRecordCreate":             2,
+				"OnRecordCreateExecute":      2,
+				"OnRecordAfterCreateSuccess": 2,
 				// ---
 				"OnModelUpdate":              1,
 				"OnModelUpdateExecute":       1,
@@ -227,8 +241,15 @@ func TestRecordAuthWithOAuth2(t *testing.T) {
 				"OnRecordUpdateExecute":      1,
 				"OnRecordAfterUpdateSuccess": 1,
 				// ---
-				"OnModelValidate":  2, // create + update
-				"OnRecordValidate": 2,
+				"OnModelDelete":              3, // pre-existing external auths
+				"OnModelDeleteExecute":       3,
+				"OnModelAfterDeleteSuccess":  3,
+				"OnRecordDelete":             3,
+				"OnRecordDeleteExecute":      3,
+				"OnRecordAfterDeleteSuccess": 3,
+				// ---
+				"OnModelValidate":  3, // user create/update + recreated external auth
+				"OnRecordValidate": 3,
 			},
 			AfterTestFunc: func(t testing.TB, app *tests.TestApp, res *http.Response) {
 				user, err := app.FindAuthRecordByEmail("users", "test@example.com")
@@ -248,6 +269,24 @@ func TestRecordAuthWithOAuth2(t *testing.T) {
 				if len(devices) != 1 {
 					t.Fatalf("Expected only 1 auth origin to be created, got %d (%v)", len(devices), err)
 				}
+
+				// ensure that other linked external auths have been deleted
+				externalAuths, _ := app.FindAllExternalAuthsByRecord(user)
+				if len(externalAuths) != 1 {
+					t.Fatalf("Expected only 1 external auth to remain, got %d", len(externalAuths))
+				}
+				if provider := externalAuths[0].Provider(); provider != "test" {
+					t.Fatalf("Expected %q external auth, got %q", "test", provider)
+				}
+				if providerId := externalAuths[0].ProviderId(); providerId != "test_id" {
+					t.Fatalf("Expected %q providerId, got %q", "test_id", providerId)
+				}
+				if recordRef := externalAuths[0].RecordRef(); recordRef != user.Id {
+					t.Fatalf("Expected %q recordRef, got %q", user.Id, recordRef)
+				}
+				if collectionRef := externalAuths[0].CollectionRef(); collectionRef != user.Collection().Id {
+					t.Fatalf("Expected %q collectionRef, got %q", user.Collection().Id, collectionRef)
+				}
 			},
 		},
 		{
@@ -343,7 +382,7 @@ func TestRecordAuthWithOAuth2(t *testing.T) {
 				}
 
 				if !user.ValidatePassword("1234567890") {
-					t.Fatalf("Expected old password %q to be valid", "1234567890")
+					t.Fatalf("Expected old password %q to remain valid", "1234567890")
 				}
 
 				devices, err := app.FindAllAuthOriginsByRecord(user)
@@ -353,7 +392,7 @@ func TestRecordAuthWithOAuth2(t *testing.T) {
 			},
 		},
 		{
-			Name:   "link by email",
+			Name:   "link by email (unverified user)",
 			Method: http.MethodPost,
 			URL:    "/api/collections/users/auth-with-oauth2",
 			Body: strings.NewReader(`{
@@ -376,6 +415,20 @@ func TestRecordAuthWithOAuth2(t *testing.T) {
 					t.Fatalf("Expected password %q to be valid", "1234567890")
 				}
 
+				// ensure that there is at least one other external auth different than test
+				// so that later we can verify that it was deleted
+				var hasAtLeastOneOtherEA = false
+				externalAuths, _ := app.FindAllExternalAuthsByRecord(user)
+				for _, rel := range externalAuths {
+					if rel.Provider() != "test" {
+						hasAtLeastOneOtherEA = true
+						break
+					}
+				}
+				if !hasAtLeastOneOtherEA {
+					t.Fatal("Expected at least one non-test external auth linked")
+				}
+
 				// register the test provider
 				auth.Providers["test"] = func() auth.Provider {
 					return &oauth2MockProvider{
@@ -432,6 +485,13 @@ func TestRecordAuthWithOAuth2(t *testing.T) {
 				"OnRecordUpdateExecute":      1,
 				"OnRecordAfterUpdateSuccess": 1,
 				// ---
+				"OnModelDelete":              2, // pre-existing external auths
+				"OnModelDeleteExecute":       2,
+				"OnModelAfterDeleteSuccess":  2,
+				"OnRecordDelete":             2,
+				"OnRecordDeleteExecute":      2,
+				"OnRecordAfterDeleteSuccess": 2,
+				// ---
 				"OnModelValidate":  3, // record + authOrigins + externalAuths
 				"OnRecordValidate": 3,
 			},
@@ -449,6 +509,145 @@ func TestRecordAuthWithOAuth2(t *testing.T) {
 				if len(devices) != 1 {
 					t.Fatalf("Expected only 1 auth origin to be created, got %d (%v)", len(devices), err)
 				}
+
+				// ensure that other linked external auths have been deleted
+				externalAuths, _ := app.FindAllExternalAuthsByRecord(user)
+				if len(externalAuths) != 1 {
+					t.Fatalf("Expected only 1 external auth to remain, got %d", len(externalAuths))
+				}
+				if provider := externalAuths[0].Provider(); provider != "test" {
+					t.Fatalf("Expected %q external auth, got %q", "test", provider)
+				}
+				if providerId := externalAuths[0].ProviderId(); providerId != "test_id" {
+					t.Fatalf("Expected %q providerId, got %q", "test_id", providerId)
+				}
+				if recordRef := externalAuths[0].RecordRef(); recordRef != user.Id {
+					t.Fatalf("Expected %q recordRef, got %q", user.Id, recordRef)
+				}
+				if collectionRef := externalAuths[0].CollectionRef(); collectionRef != user.Collection().Id {
+					t.Fatalf("Expected %q collectionRef, got %q", user.Collection().Id, collectionRef)
+				}
+			},
+		},
+		{
+			Name:   "link by email (verified user)",
+			Method: http.MethodPost,
+			URL:    "/api/collections/users/auth-with-oauth2",
+			Body: strings.NewReader(`{
+				"provider": "test",
+				"code":"123",
+				"redirectURL": "https://example.com"
+			}`),
+			BeforeTestFunc: func(t testing.TB, app *tests.TestApp, e *core.ServeEvent) {
+				user, err := app.FindAuthRecordByEmail("users", "test3@example.com")
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				if !user.Verified() {
+					t.Fatalf("Expected user %q to be verified", user.Email())
+				}
+
+				// ensure that the old password works
+				if !user.ValidatePassword("1234567890") {
+					t.Fatalf("Expected password %q to be valid", "1234567890")
+				}
+
+				// register the test provider
+				auth.Providers["test"] = func() auth.Provider {
+					return &oauth2MockProvider{
+						AuthUser: &auth.AuthUser{Id: "test_id", Email: "test3@example.com"},
+						Token:    &oauth2.Token{AccessToken: "abc"},
+					}
+				}
+
+				// ensure that there is at least one other external auth different than test
+				// so that later we can verify that they are not deleted
+				var hasAtLeastOneOtherEA = false
+				externalAuths, _ := app.FindAllExternalAuthsByRecord(user)
+				for _, rel := range externalAuths {
+					if rel.Provider() != "test" {
+						hasAtLeastOneOtherEA = true
+						break
+					}
+				}
+				if !hasAtLeastOneOtherEA {
+					t.Fatal("Expected at least one non-test external auth linked")
+				}
+
+				// add the test provider in the collection
+				user.Collection().MFA.Enabled = false
+				user.Collection().OAuth2.Enabled = true
+				user.Collection().OAuth2.Providers = []core.OAuth2ProviderConfig{{
+					Name:         "test",
+					ClientId:     "123",
+					ClientSecret: "456",
+				}}
+				if err := app.Save(user.Collection()); err != nil {
+					t.Fatal(err)
+				}
+			},
+			ExpectedStatus: 200,
+			ExpectedContent: []string{
+				`"record":{`,
+				`"token":"`,
+				`"meta":{`,
+				`"isNew":false`,
+				`"email":"test3@example.com"`,
+				`"id":"bgs820n361vj1qd"`,
+				`"id":"test_id"`,
+				`"verified":true`,
+			},
+			NotExpectedContent: []string{
+				// hidden fields
+				`"tokenKey"`,
+				`"password"`,
+			},
+			ExpectedEvents: map[string]int{
+				"*":                             0,
+				"OnRecordAuthWithOAuth2Request": 1,
+				"OnRecordAuthRequest":           1,
+				"OnRecordEnrich":                1,
+				// ---
+				"OnModelCreate":              2, // authOrigins + externalAuths
+				"OnModelCreateExecute":       2,
+				"OnModelAfterCreateSuccess":  2,
+				"OnRecordCreate":             2,
+				"OnRecordCreateExecute":      2,
+				"OnRecordAfterCreateSuccess": 2,
+				// ---
+				"OnModelValidate":  2, // authOrigins + externalAuths
+				"OnRecordValidate": 2,
+			},
+			AfterTestFunc: func(t testing.TB, app *tests.TestApp, res *http.Response) {
+				user, err := app.FindAuthRecordByEmail("users", "test3@example.com")
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				if !user.ValidatePassword("1234567890") {
+					t.Fatalf("Expected old password %q to remain valid", "1234567890")
+				}
+
+				devices, err := app.FindAllAuthOriginsByRecord(user)
+				if len(devices) != 1 {
+					t.Fatalf("Expected only 1 auth origin to be created, got %d (%v)", len(devices), err)
+				}
+
+				var hasTestEA = false
+				externalAuths, err := app.FindAllExternalAuthsByRecord(user)
+				if len(externalAuths) <= 1 {
+					t.Fatalf("Expected to have 2+ ExternalAuth records, got %d (%v)", len(externalAuths), err)
+				}
+				for _, rel := range externalAuths {
+					if rel.Provider() == "test" {
+						hasTestEA = true
+						break
+					}
+				}
+				if !hasTestEA {
+					t.Fatal("Expected test external auth to be linked")
+				}
 			},
 		},
 		{
@@ -531,6 +730,13 @@ func TestRecordAuthWithOAuth2(t *testing.T) {
 				"OnRecordCreateExecute":      2,
 				"OnRecordAfterCreateSuccess": 2,
 				// ---
+				"OnModelDelete":              2, // pre-existing external auths
+				"OnModelDeleteExecute":       2,
+				"OnModelAfterDeleteSuccess":  2,
+				"OnRecordDelete":             2,
+				"OnRecordDeleteExecute":      2,
+				"OnRecordAfterDeleteSuccess": 2,
+				// ---
 				"OnModelValidate":  2,
 				"OnRecordValidate": 2,
 			},
@@ -541,7 +747,7 @@ func TestRecordAuthWithOAuth2(t *testing.T) {
 				}
 
 				if !user.ValidatePassword("1234567890") {
-					t.Fatalf("Expected password %q not to be changed", "1234567890")
+					t.Fatalf("Expected old password %q to remain valid", "1234567890")
 				}
 
 				devices, err := app.FindAllAuthOriginsByRecord(user)
@@ -652,6 +858,13 @@ func TestRecordAuthWithOAuth2(t *testing.T) {
 				"OnRecordUpdateExecute":      1,
 				"OnRecordAfterUpdateSuccess": 1,
 				// ---
+				"OnModelDelete":              2, // pre-existing external auths
+				"OnModelDeleteExecute":       2,
+				"OnModelAfterDeleteSuccess":  2,
+				"OnRecordDelete":             2,
+				"OnRecordDeleteExecute":      2,
+				"OnRecordAfterDeleteSuccess": 2,
+				// ---
 				"OnModelValidate":  3, // record + authOrigins + externalAuths
 				"OnRecordValidate": 3,
 			},
@@ -662,7 +875,7 @@ func TestRecordAuthWithOAuth2(t *testing.T) {
 				}
 
 				if !user.ValidatePassword("1234567890") {
-					t.Fatalf("Expected password %q not to be changed", "1234567890")
+					t.Fatalf("Expected old password %q to remain valid", "1234567890")
 				}
 
 				devices, err := app.FindAllAuthOriginsByRecord(user)
@@ -758,6 +971,13 @@ func TestRecordAuthWithOAuth2(t *testing.T) {
 				"OnRecordUpdateExecute":      1,
 				"OnRecordAfterUpdateSuccess": 1,
 				// ---
+				"OnModelDelete":              2, // pre-existing external auths
+				"OnModelDeleteExecute":       2,
+				"OnModelAfterDeleteSuccess":  2,
+				"OnRecordDelete":             2,
+				"OnRecordDeleteExecute":      2,
+				"OnRecordAfterDeleteSuccess": 2,
+				// ---
 				"OnModelValidate":  3, // record + authOrigins + externalAuths
 				"OnRecordValidate": 3,
 			},
@@ -768,7 +988,7 @@ func TestRecordAuthWithOAuth2(t *testing.T) {
 				}
 
 				if !user.ValidatePassword("1234567890") {
-					t.Fatalf("Expected password %q not to be changed", "1234567890")
+					t.Fatalf("Expected old password %q to remain valid", "1234567890")
 				}
 
 				devices, err := app.FindAllAuthOriginsByRecord(user)

apis/record_auth_with_otp.go

@@ -71,8 +71,15 @@ func recordAuthWithOTP(e *core.RequestEvent) error {
 		otpSentTo := e.OTP.SentTo()
 		if !e.Record.Verified() && otpSentTo != "" && e.Record.Email() == otpSentTo {
 			e.Record.SetVerified(true)
-			err = e.App.Save(e.Record)
-			if err != nil {
+
+			// this is technically not required but we enforce password
+			// reset on verified upgrades in case the OTP is used on its own
+			// since this makes it less error prone to pre-hijacking attacks
+			if !e.Record.Collection().MFA.Enabled {
+				e.Record.SetRandomPassword()
+			}
+
+			if err := e.App.Save(e.Record); err != nil {
 				e.App.Logger().Error("Failed to update record verified state after successful OTP validation",
 					"error", err,
 					"otpId", e.OTP.Id,

apis/record_auth_with_otp_test.go

@@ -327,6 +327,15 @@ func TestRecordAuthWithOTP(t *testing.T) {
 				if user.Verified() {
 					t.Fatal("Expected the user to remain unverified because sentTo != email")
 				}
+
+				// ensure that all pre-existing OAuth2 were NOT deleted
+				externalAuths, err := app.FindAllExternalAuthsByRecord(user)
+				if err != nil {
+					t.Fatal(err)
+				}
+				if len(externalAuths) != 2 {
+					t.Fatalf("Expected 2 external auths, found %d", len(externalAuths))
+				}
 			},
 		},
 		{
@@ -364,6 +373,15 @@ func TestRecordAuthWithOTP(t *testing.T) {
 				if err := app.Save(otp); err != nil {
 					t.Fatal(err)
 				}
+
+				// verify that there are at least one pre-existing OAuth2 link
+				externalAuths, err := app.FindAllExternalAuthsByRecord(user)
+				if err != nil {
+					t.Fatal(err)
+				}
+				if len(externalAuths) == 0 {
+					t.Fatal("Expected at least one external auth")
+				}
 			},
 			ExpectedStatus: 200,
 			ExpectedContent: []string{
@@ -388,10 +406,10 @@ func TestRecordAuthWithOTP(t *testing.T) {
 				"OnModelCreate":             1,
 				"OnModelCreateExecute":      1,
 				"OnModelAfterCreateSuccess": 1,
-				// OTP delete
-				"OnModelDelete":             1,
-				"OnModelDeleteExecute":      1,
-				"OnModelAfterDeleteSuccess": 1,
+				// OTP delete + 2 ExternalAuth delete
+				"OnModelDelete":             3,
+				"OnModelDeleteExecute":      3,
+				"OnModelAfterDeleteSuccess": 3,
 				// user verified update
 				"OnModelUpdate":             1,
 				"OnModelUpdateExecute":      1,
@@ -401,9 +419,9 @@ func TestRecordAuthWithOTP(t *testing.T) {
 				"OnRecordCreate":             1,
 				"OnRecordCreateExecute":      1,
 				"OnRecordAfterCreateSuccess": 1,
-				"OnRecordDelete":             1,
-				"OnRecordDeleteExecute":      1,
-				"OnRecordAfterDeleteSuccess": 1,
+				"OnRecordDelete":             3,
+				"OnRecordDeleteExecute":      3,
+				"OnRecordAfterDeleteSuccess": 3,
 				"OnRecordUpdate":             1,
 				"OnRecordUpdateExecute":      1,
 				"OnRecordAfterUpdateSuccess": 1,
@@ -417,6 +435,15 @@ func TestRecordAuthWithOTP(t *testing.T) {
 				if !user.Verified() {
 					t.Fatal("Expected the user to be marked as verified")
 				}
+
+				// ensure that all pre-existing OAuth2 links are cleared
+				externalAuths, err := app.FindAllExternalAuthsByRecord(user)
+				if err != nil {
+					t.Fatal(err)
+				}
+				if len(externalAuths) > 0 {
+					t.Fatalf("Expected all external auths to be cleared, found %d", len(externalAuths))
+				}
 			},
 		},
 		{

core/app.go

@@ -502,6 +502,11 @@ type App interface {
 	// ExternalAuth model that satisfies the non-nil expression.
 	FindFirstExternalAuthByExpr(expr dbx.Expression) (*ExternalAuth, error)
 
+	// DeleteAllExternalAuthsByRecord deletes all ExternalAuth models associated with the provided record.
+	//
+	// Returns a combined error with the failed deletes.
+	DeleteAllExternalAuthsByRecord(authRecord *Record) error
+
 	// ---------------------------------------------------------------
 
 	// FindAllMFAsByRecord returns all MFA models linked to the provided auth record.

core/external_auth_query.go

@@ -1,6 +1,8 @@
 package core
 
 import (
+	"errors"
+
 	"github.com/pocketbase/dbx"
 )
 
@@ -59,3 +61,25 @@ func (app *BaseApp) FindFirstExternalAuthByExpr(expr dbx.Expression) (*ExternalA
 
 	return model, nil
 }
+
+// DeleteAllExternalAuthsByRecord deletes all ExternalAuth models associated with the provided record.
+//
+// Returns a combined error with the failed deletes.
+func (app *BaseApp) DeleteAllExternalAuthsByRecord(authRecord *Record) error {
+	models, err := app.FindAllExternalAuthsByRecord(authRecord)
+	if err != nil {
+		return err
+	}
+
+	var errs []error
+	for _, m := range models {
+		if err := app.Delete(m); err != nil {
+			errs = append(errs, err)
+		}
+	}
+	if len(errs) > 0 {
+		return errors.Join(errs...)
+	}
+
+	return nil
+}

core/external_auth_query_test.go

@@ -2,6 +2,7 @@ package core_test
 
 import (
 	"fmt"
+	"slices"
 	"testing"
 
 	"github.com/pocketbase/dbx"
@@ -174,3 +175,68 @@ func TestFindFirstExternalAuthByExpr(t *testing.T) {
 		})
 	}
 }
+
+func TestDeleteAllExternalAuthsByRecord(t *testing.T) {
+	t.Parallel()
+
+	testApp, _ := tests.NewTestApp()
+	defer testApp.Cleanup()
+
+	demo1, err := testApp.FindRecordById("demo1", "84nmscqy84lsi1t")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	user1, err := testApp.FindAuthRecordByEmail("users", "test@example.com")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	client1, err := testApp.FindAuthRecordByEmail("clients", "test@example.com")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	client2, err := testApp.FindAuthRecordByEmail("clients", "test2@example.com")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	scenarios := []struct {
+		record     *core.Record
+		deletedIds []string
+	}{
+		{demo1, nil}, // non-auth record
+		{user1, []string{"dlmflokuq1xl342", "clmflokuq1xl341"}},
+		{client1, []string{"f1z5b3843pzc964"}},
+		{client2, nil},
+	}
+
+	for i, s := range scenarios {
+		t.Run(fmt.Sprintf("%d_%s_%s", i, s.record.Collection().Name, s.record.Id), func(t *testing.T) {
+			app, _ := tests.NewTestApp()
+			defer app.Cleanup()
+
+			deletedIds := []string{}
+			app.OnRecordDelete().BindFunc(func(e *core.RecordEvent) error {
+				deletedIds = append(deletedIds, e.Record.Id)
+				return e.Next()
+			})
+
+			err := app.DeleteAllExternalAuthsByRecord(s.record)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if len(deletedIds) != len(s.deletedIds) {
+				t.Fatalf("Expected deleted ids\n%v\ngot\n%v", s.deletedIds, deletedIds)
+			}
+
+			for _, id := range s.deletedIds {
+				if !slices.Contains(deletedIds, id) {
+					t.Errorf("Expected to find deleted id %q in %v", id, deletedIds)
+				}
+			}
+		})
+	}
+}

core/record_model.go

@@ -1427,22 +1427,31 @@ func onRecordValidate(e *RecordEvent) error {
 }
 
 func onRecordSaveExecute(e *RecordEvent) error {
+	var needToDeleteExternalAuths bool
+
 	if e.Record.Collection().IsAuth() {
-		// ensure that the token key is regenerated on password change or email change
+		// auth resets to prevent (pre)hijacking vulnerabilities
 		if !e.Record.IsNew() {
 			lastSavedRecord, err := e.App.FindRecordById(e.Record.Collection(), e.Record.Id)
 			if err != nil {
 				return err
 			}
 
+			// ensure that the token key is regenerated on password change or email change
 			if lastSavedRecord.TokenKey() == e.Record.TokenKey() &&
 				(lastSavedRecord.Get(FieldNamePassword) != e.Record.Get(FieldNamePassword) ||
 					lastSavedRecord.Email() != e.Record.Email()) {
 				e.Record.RefreshTokenKey()
 			}
+
+			// in case upgrading from "unverified" -> "verified" mark all pre-existing OAuth2 links
+			// for deletion since there is no reliable way to verify that they weren't created by an attacker
+			if !lastSavedRecord.Verified() && e.Record.Verified() {
+				needToDeleteExternalAuths = true
+			}
 		}
 
-		// cross-check that the auth record id is unique across all auth collections.
+		// cross-check that the auth record id is unique across all auth collections
 		authCollections, err := e.App.FindAllCollections(CollectionTypeAuth)
 		if err != nil {
 			return fmt.Errorf("unable to fetch the auth collections for cross-id unique check: %w", err)
@@ -1460,16 +1469,45 @@ func onRecordSaveExecute(e *RecordEvent) error {
 		}
 	}
 
-	err := e.Next()
-	if err == nil {
-		return nil
+	finalizer := func() error {
+		err := e.Next()
+		if err == nil {
+			return nil
+		}
+
+		return validators.NormalizeUniqueIndexError(
+			err,
+			e.Record.Collection().Name,
+			e.Record.Collection().Fields.FieldNames(),
+		)
 	}
 
-	return validators.NormalizeUniqueIndexError(
-		err,
-		e.Record.Collection().Name,
-		e.Record.Collection().Fields.FieldNames(),
-	)
+	if needToDeleteExternalAuths {
+		originalApp := e.App
+
+		return e.App.RunInTransaction(func(txApp App) error {
+			e.App = txApp
+			defer func() { e.App = originalApp }()
+
+			externalAuths, err := txApp.FindAllExternalAuthsByRecord(e.Record)
+			if err != nil {
+				return err
+			}
+			if len(externalAuths) > 0 {
+				// delete all pre-existing external auths
+				if err := txApp.DeleteAllExternalAuthsByRecord(e.Record); err != nil {
+					return err
+				}
+
+				// force refresh tokens reset (if not already)
+				e.Record.RefreshTokenKey()
+			}
+
+			return finalizer()
+		})
+	}
+
+	return finalizer()
 }
 
 func onRecordDeleteExecute(e *RecordEvent) error {