[#7090] try to forward the Apple OAuth2 redirect user's name to the auth handler

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 - Regenerated JSVM types to include methods from structs with single generic parameter.
 
+- Fixed `RateLimitRule.Audience` code comment ([#7098](https://github.com/pocketbase/pocketbase/pull/7098); thanks @iustin05).
+
+- Try to forward Apple OAuth2 POST redirect user's name so that it can be returned (and eventually assigned) with the success response of the all-in-one auth call ([#7090](https://github.com/pocketbase/pocketbase/issues/7090)).
+
 
 ## v0.29.2
 

apis/record_auth_with_oauth2.go

@@ -15,6 +15,7 @@ import (
 	validation "github.com/go-ozzo/ozzo-validation/v4"
 	"github.com/pocketbase/dbx"
 	"github.com/pocketbase/pocketbase/core"
+	"github.com/pocketbase/pocketbase/tools/auth"
 	"github.com/pocketbase/pocketbase/tools/dbutils"
 	"github.com/pocketbase/pocketbase/tools/filesystem"
 	"golang.org/x/oauth2"
@@ -90,6 +91,17 @@ func recordAuthWithOAuth2(e *core.RequestEvent) error {
 		return firstApiError(err, e.BadRequestError("Failed to fetch OAuth2 user.", err))
 	}
 
+	// Apple currently returns the user's name only as part of the first redirect data response
+	// so we try to assign the [apis.oauth2SubscriptionRedirect] forwarded name.
+	if form.Provider == auth.NameApple && authUser.Name == "" {
+		nameKey := oauth2RedirectAppleNameStoreKeyPrefix + form.Code
+		name, ok := e.App.Store().Get(nameKey).(string)
+		if ok {
+			e.App.Store().Remove(nameKey)
+			authUser.Name = name
+		}
+	}
+
 	var authRecord *core.Record
 
 	// check for existing relation with the auth collection

apis/record_auth_with_oauth2_redirect.go

@@ -3,21 +3,27 @@ package apis
 import (
 	"encoding/json"
 	"net/http"
+	"strings"
+	"time"
 
 	"github.com/pocketbase/pocketbase/core"
 	"github.com/pocketbase/pocketbase/tools/subscriptions"
 )
 
 const (
-	oauth2SubscriptionTopic   string = "@oauth2"
+	oauth2SubscriptionTopic               string = "@oauth2"
-	oauth2RedirectFailurePath string = "../_/#/auth/oauth2-redirect-failure"
+	oauth2RedirectFailurePath             string = "../_/#/auth/oauth2-redirect-failure"
-	oauth2RedirectSuccessPath string = "../_/#/auth/oauth2-redirect-success"
+	oauth2RedirectSuccessPath             string = "../_/#/auth/oauth2-redirect-success"
+	oauth2RedirectAppleNameStoreKeyPrefix string = "@redirect_name_"
 )
 
 type oauth2RedirectData struct {
 	State string `form:"state" json:"state"`
 	Code  string `form:"code" json:"code"`
 	Error string `form:"error" json:"error,omitempty"`
+
+	// returned by Apple only
+	AppleUser string `form:"user" json:"-"`
 }
 
 func oauth2SubscriptionRedirect(e *core.RequestEvent) error {
@@ -52,6 +58,19 @@ func oauth2SubscriptionRedirect(e *core.RequestEvent) error {
 	}
 	defer client.Unsubscribe(oauth2SubscriptionTopic)
 
+	// see https://github.com/pocketbase/pocketbase/issues/7090
+	if data.AppleUser != "" && data.Error == "" && data.Code != "" {
+		nameErr := parseAndStoreAppleRedirectName(
+			e.App,
+			oauth2RedirectAppleNameStoreKeyPrefix+data.Code,
+			data.AppleUser,
+		)
+		if nameErr != nil {
+			// non-critical error
+			e.App.Logger().Debug("Failed to parse and load Apple Redirect name data", "error", nameErr)
+		}
+	}
+
 	encodedData, err := json.Marshal(data)
 	if err != nil {
 		e.App.Logger().Debug("Failed to marshalize OAuth2 redirect data", "error", err)
@@ -72,3 +91,51 @@ func oauth2SubscriptionRedirect(e *core.RequestEvent) error {
 
 	return e.Redirect(redirectStatusCode, oauth2RedirectSuccessPath)
 }
+
+// parseAndStoreAppleRedirectName extracts the first and last name
+// from serializedNameData and temporary store them in the app.Store.
+//
+// This is hacky workaround to forward safely and seamlessly the Apple
+// redirect user's name back to the OAuth2 auth handler.
+//
+// Note that currently Apple is the only provider that behaves like this and
+// for now it is unnecessary to check whether the redirect is coming from Apple or not.
+//
+// Ideally this shouldn't be needed and will be removed in the future
+// once Apple adds a dedicated userinfo endpoint.
+func parseAndStoreAppleRedirectName(app core.App, nameKey string, serializedNameData string) error {
+	if serializedNameData == "" {
+		return nil
+	}
+
+	// https://developer.apple.com/documentation/signinwithapple/incorporating-sign-in-with-apple-into-other-platforms#Handle-the-response
+	extracted := struct {
+		Name struct {
+			FirstName string `json:"firstName"`
+			LastName  string `json:"lastName"`
+		} `json:"name"`
+	}{}
+	if err := json.Unmarshal([]byte(serializedNameData), &extracted); err != nil {
+		return err
+	}
+
+	fullName := extracted.Name.FirstName + " " + extracted.Name.LastName
+
+	// truncate just in case to prevent storing large strings in memory
+	if len(fullName) > 150 {
+		fullName = fullName[:150]
+	}
+
+	fullName = strings.TrimSpace(fullName)
+	if fullName == "" {
+		return nil
+	}
+
+	// store (and remove)
+	app.Store().Set(nameKey, fullName)
+	time.AfterFunc(90*time.Second, func() {
+		app.Store().Remove(nameKey)
+	})
+
+	return nil
+}

apis/record_auth_with_oauth2_redirect_test.go

@@ -3,6 +3,7 @@ package apis_test
 import (
 	"context"
 	"net/http"
+	"net/url"
 	"strings"
 	"testing"
 	"time"
@@ -266,6 +267,41 @@ func TestRecordAuthWithOAuth2Redirect(t *testing.T) {
 				}
 			},
 		},
+		{
+			Name:   "(POST) Apple user's name json",
+			Method: http.MethodPost,
+			URL:    "/api/oauth2-redirect",
+			Body: strings.NewReader(url.Values{
+				"code":  []string{"123"},
+				"state": []string{clientStubs[8]["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[8], map[string][]string{
+				"c3": {`"state":"` + clientStubs[8]["c3"].Id(), `"code":"123"`},
+			}),
+			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[8]["c3"].HasSubscription("@oauth2") {
+					t.Fatalf("Expected oauth2 subscription to be removed")
+				}
+
+				storedName, _ := app.Store().Get("@redirect_name_123").(string)
+				expectedName := "aaa " + strings.Repeat("b", 146)
+				if storedName != expectedName {
+					t.Fatalf("Expected stored name\n%q\ngot\n%q", expectedName, storedName)
+				}
+			},
+		},
 	}
 
 	for _, scenario := range scenarios {

apis/record_auth_with_oauth2_test.go

@@ -1641,6 +1641,104 @@ func TestRecordAuthWithOAuth2(t *testing.T) {
 			ExpectedContent: []string{"TX_ERROR"},
 		},
 
+		// Apple AuthUser.Name assign checks
+		// -----------------------------------------------------------
+		{
+			Name:   "store name with Apple provider",
+			Method: http.MethodPost,
+			URL:    "/api/collections/users/auth-with-oauth2",
+			Body: strings.NewReader(`{
+				"provider": "apple",
+				"code":"test_code",
+				"redirectURL": "https://example.com"
+			}`),
+			BeforeTestFunc: func(t testing.TB, app *tests.TestApp, e *core.ServeEvent) {
+				users, err := app.FindCollectionByNameOrId("users")
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				// register the test provider
+				auth.Providers[auth.NameApple] = func() auth.Provider {
+					return &oauth2MockProvider{
+						AuthUser: &auth.AuthUser{Id: "test_id"},
+						Token:    &oauth2.Token{AccessToken: "abc"},
+					}
+				}
+
+				app.Store().Set("@redirect_name_test_code", "test_store_name")
+
+				// add the test provider in the collection
+				users.MFA.Enabled = false
+				users.OAuth2.Enabled = true
+				users.OAuth2.Providers = []core.OAuth2ProviderConfig{{
+					Name:         auth.NameApple,
+					ClientId:     "123",
+					ClientSecret: "456",
+				}}
+				if err := app.Save(users); err != nil {
+					t.Fatal(err)
+				}
+			},
+			ExpectedStatus: 200,
+			ExpectedContent: []string{
+				`"meta":{`,
+				`"name":"test_store_name"`,
+			},
+			AfterTestFunc: func(t testing.TB, app *tests.TestApp, res *http.Response) {
+				if app.Store().Has("@redirect_name_test_code") {
+					t.Fatal("Expected @redirect_name_test_code store key to be removed")
+				}
+			},
+		},
+		{
+			Name:   "store name with non-Apple provider",
+			Method: http.MethodPost,
+			URL:    "/api/collections/users/auth-with-oauth2",
+			Body: strings.NewReader(`{
+				"provider": "test",
+				"code":"test_code",
+				"redirectURL": "https://example.com"
+			}`),
+			BeforeTestFunc: func(t testing.TB, app *tests.TestApp, e *core.ServeEvent) {
+				users, err := app.FindCollectionByNameOrId("users")
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				// register the test provider
+				auth.Providers["test"] = func() auth.Provider {
+					return &oauth2MockProvider{
+						AuthUser: &auth.AuthUser{Id: "test_id"},
+						Token:    &oauth2.Token{AccessToken: "abc"},
+					}
+				}
+
+				app.Store().Set("@redirect_name_test_code", "test_store_name")
+
+				// add the test provider in the collection
+				users.MFA.Enabled = false
+				users.OAuth2.Enabled = true
+				users.OAuth2.Providers = []core.OAuth2ProviderConfig{{
+					Name:         "test",
+					ClientId:     "123",
+					ClientSecret: "456",
+				}}
+				if err := app.Save(users); err != nil {
+					t.Fatal(err)
+				}
+			},
+			ExpectedStatus: 200,
+			NotExpectedContent: []string{
+				`"name":"test_store_name"`,
+			},
+			AfterTestFunc: func(t testing.TB, app *tests.TestApp, res *http.Response) {
+				if !app.Store().Has("@redirect_name_test_code") {
+					t.Fatal("Expected @redirect_name_test_code store key to NOT be deleted")
+				}
+			},
+		},
+
 		// rate limit checks
 		// -----------------------------------------------------------
 		{

tools/auth/apple.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
-	"strings"
 
 	"github.com/golang-jwt/jwt/v5"
 	"github.com/pocketbase/pocketbase/tools/types"
@@ -23,7 +22,7 @@ const NameApple string = "apple"
 
 // Apple allows authentication via Apple OAuth2.
 //
-// [OIDC differences]: https://bitbucket.org/openid/connect/src/master/How-Sign-in-with-Apple-differs-from-OpenID-Connect.md
+// OIDC differences: https://bitbucket.org/openid/connect/src/master/How-Sign-in-with-Apple-differs-from-OpenID-Connect.md.
 type Apple struct {
 	BaseProvider
 
@@ -47,7 +46,7 @@ func NewAppleProvider() *Apple {
 
 // FetchAuthUser returns an AuthUser instance based on the provided token.
 //
-// API reference: https://developer.apple.com/documentation/sign_in_with_apple/tokenresponse.
+// API reference: https://developer.apple.com/documentation/signinwithapple/authenticating-users-with-sign-in-with-apple#Retrieve-the-users-information-from-Apple-ID-servers.
 func (p *Apple) FetchAuthUser(token *oauth2.Token) (*AuthUser, error) {
 	data, err := p.FetchRawUserInfo(token)
 	if err != nil {
@@ -60,16 +59,13 @@ func (p *Apple) FetchAuthUser(token *oauth2.Token) (*AuthUser, error) {
 	}
 
 	extracted := struct {
-		Id            string `json:"sub"`
-		Name          string `json:"name"`
-		Email         string `json:"email"`
 		EmailVerified any    `json:"email_verified"` // could be string or bool
-		User          struct {
+		Email         string `json:"email"`
-			Name struct {
+		Id            string `json:"sub"`
-				FirstName string `json:"firstName"`
+
-				LastName  string `json:"lastName"`
+		// not returned at the time of writing and it is usually
-			} `json:"name"`
+		// manually populated in apis.recordAuthWithOAuth2
-		} `json:"user"`
+		Name string `json:"name"`
 	}{}
 	if err := json.Unmarshal(data, &extracted); err != nil {
 		return nil, err
@@ -89,17 +85,13 @@ func (p *Apple) FetchAuthUser(token *oauth2.Token) (*AuthUser, error) {
 		user.Email = extracted.Email
 	}
 
-	if user.Name == "" {
-		user.Name = strings.TrimSpace(extracted.User.Name.FirstName + " " + extracted.User.Name.LastName)
-	}
-
 	return user, nil
 }
 
 // FetchRawUserInfo implements Provider.FetchRawUserInfo interface.
 //
-// Apple doesn't have a UserInfo endpoint and claims about users
+// Note that Apple doesn't have a UserInfo endpoint and claims about
-// are instead included in the "id_token" (https://openid.net/specs/openid-connect-core-1_0.html#id_tokenExample)
+// the users are included in the id_token (without the name - see #7090).
 func (p *Apple) FetchRawUserInfo(token *oauth2.Token) ([]byte, error) {
 	idToken, _ := token.Extra("id_token").(string)
 
@@ -108,18 +100,6 @@ func (p *Apple) FetchRawUserInfo(token *oauth2.Token) ([]byte, error) {
 		return nil, err
 	}
 
-	// Apple only returns the user object the first time the user authorizes the app
-	// https://developer.apple.com/documentation/sign_in_with_apple/sign_in_with_apple_js/configuring_your_webpage_for_sign_in_with_apple#3331292
-	rawUser, _ := token.Extra("user").(string)
-	if rawUser != "" {
-		user := map[string]any{}
-		err = json.Unmarshal([]byte(rawUser), &user)
-		if err != nil {
-			return nil, err
-		}
-		claims["user"] = user
-	}
-
 	return json.Marshal(claims)
 }