From b8f18bd97d0404c16db068904bc9e339af356b40 Mon Sep 17 00:00:00 2001 From: Gani Georgiev Date: Wed, 20 Aug 2025 22:41:33 +0300 Subject: [PATCH] added more tests and extra debug log --- apis/record_auth_with_oauth2.go | 2 + apis/record_auth_with_oauth2_redirect.go | 11 ++++- apis/record_auth_with_oauth2_redirect_test.go | 41 +++++++++++++++++-- 3 files changed, 48 insertions(+), 6 deletions(-) diff --git a/apis/record_auth_with_oauth2.go b/apis/record_auth_with_oauth2.go index cbc5f400..c0589f9a 100644 --- a/apis/record_auth_with_oauth2.go +++ b/apis/record_auth_with_oauth2.go @@ -99,6 +99,8 @@ func recordAuthWithOAuth2(e *core.RequestEvent) error { if ok { e.App.Store().Remove(nameKey) authUser.Name = name + } else { + e.App.Logger().Debug("Missing or already removed Apple user's name") } } diff --git a/apis/record_auth_with_oauth2_redirect.go b/apis/record_auth_with_oauth2_redirect.go index 7115ce56..c4ac4583 100644 --- a/apis/record_auth_with_oauth2_redirect.go +++ b/apis/record_auth_with_oauth2_redirect.go @@ -2,6 +2,7 @@ package apis import ( "encoding/json" + "errors" "net/http" "strings" "time" @@ -58,7 +59,8 @@ func oauth2SubscriptionRedirect(e *core.RequestEvent) error { } defer client.Unsubscribe(oauth2SubscriptionTopic) - // see https://github.com/pocketbase/pocketbase/issues/7090 + // temporary store the Apple user's name so that it can be later retrieved with the authWithOAuth2 call + // (see https://github.com/pocketbase/pocketbase/issues/7090) if data.AppleUser != "" && data.Error == "" && data.Code != "" { nameErr := parseAndStoreAppleRedirectName( e.App, @@ -108,6 +110,11 @@ func parseAndStoreAppleRedirectName(app core.App, nameKey string, serializedName return nil } + // just in case to prevent storing large strings in memory + if len(nameKey) > 1000 { + return errors.New("nameKey is too large") + } + // https://developer.apple.com/documentation/signinwithapple/incorporating-sign-in-with-apple-into-other-platforms#Handle-the-response extracted := struct { Name struct { @@ -133,7 +140,7 @@ func parseAndStoreAppleRedirectName(app core.App, nameKey string, serializedName // store (and remove) app.Store().Set(nameKey, fullName) - time.AfterFunc(90*time.Second, func() { + time.AfterFunc(1*time.Minute, func() { app.Store().Remove(nameKey) }) diff --git a/apis/record_auth_with_oauth2_redirect_test.go b/apis/record_auth_with_oauth2_redirect_test.go index 5a09af30..fbf7ac32 100644 --- a/apis/record_auth_with_oauth2_redirect_test.go +++ b/apis/record_auth_with_oauth2_redirect_test.go @@ -268,11 +268,11 @@ func TestRecordAuthWithOAuth2Redirect(t *testing.T) { }, }, { - Name: "(POST) Apple user's name json", + Name: "(POST) Apple user's name json (nameKey error)", Method: http.MethodPost, URL: "/api/oauth2-redirect", Body: strings.NewReader(url.Values{ - "code": []string{"123"}, + "code": []string{strings.Repeat("a", 986)}, "state": []string{clientStubs[8]["c3"].Id()}, "user": []string{ `{"name":{"firstName":"aaa","lastName":"` + strings.Repeat("b", 200) + `"}}`, @@ -282,7 +282,7 @@ func TestRecordAuthWithOAuth2Redirect(t *testing.T) { "content-type": "application/x-www-form-urlencoded", }, BeforeTestFunc: beforeTestFunc(clientStubs[8], map[string][]string{ - "c3": {`"state":"` + clientStubs[8]["c3"].Id(), `"code":"123"`}, + "c3": {`"state":"` + clientStubs[8]["c3"].Id(), `"code":"` + strings.Repeat("a", 986) + `"`}, }), ExpectedStatus: http.StatusSeeOther, ExpectedEvents: map[string]int{"*": 0}, @@ -295,7 +295,40 @@ func TestRecordAuthWithOAuth2Redirect(t *testing.T) { t.Fatalf("Expected oauth2 subscription to be removed") } - storedName, _ := app.Store().Get("@redirect_name_123").(string) + if storedName := app.Store().Get("@redirect_name_" + strings.Repeat("a", 986)); storedName != nil { + t.Fatalf("Didn't expect stored name, got %q", storedName) + } + }, + }, + { + Name: "(POST) Apple user's name json", + Method: http.MethodPost, + URL: "/api/oauth2-redirect", + Body: strings.NewReader(url.Values{ + "code": []string{strings.Repeat("a", 985)}, + "state": []string{clientStubs[9]["c3"].Id()}, + "user": []string{ + `{"name":{"firstName":"aaa","lastName":"` + strings.Repeat("b", 200) + `"}}`, + }, + }.Encode()), + Headers: map[string]string{ + "content-type": "application/x-www-form-urlencoded", + }, + BeforeTestFunc: beforeTestFunc(clientStubs[9], map[string][]string{ + "c3": {`"state":"` + clientStubs[9]["c3"].Id(), `"code":"` + strings.Repeat("a", 985) + `"`}, + }), + ExpectedStatus: http.StatusSeeOther, + ExpectedEvents: map[string]int{"*": 0}, + AfterTestFunc: func(t testing.TB, app *tests.TestApp, res *http.Response) { + app.Store().Get("cancelFunc").(context.CancelFunc)() + + checkSuccessRedirect(t, app, res) + + if clientStubs[9]["c3"].HasSubscription("@oauth2") { + t.Fatalf("Expected oauth2 subscription to be removed") + } + + storedName, _ := app.Store().Get("@redirect_name_" + strings.Repeat("a", 985)).(string) expectedName := "aaa " + strings.Repeat("b", 146) if storedName != expectedName { t.Fatalf("Expected stored name\n%q\ngot\n%q", expectedName, storedName)