Fix sign in with Apple client_secret has missing sub, and invalid aud #…

…4985

ref DEV-2441

pkg/lib/oauthrelyingparty/apple/provider.go

@@ -94,10 +94,24 @@ func (Apple) scope() []string {
 }
 
 func (Apple) createClientSecret(deps oauthrelyingparty.Dependencies) (clientSecret string, err error) {
+	// See this documentation on how to create a client secret
+	// https://developer.apple.com/documentation/accountorganizationaldatasharing/creating-a-client-secret
+	//
+	// It was observed that Sign in with Apple has a weird behavior.
+	// When the client_id (Services ID) is Team A, and the account signed in is a managed account under Team A,
+	// client_secret IS NOT validated at all.
+	//
+	// For example, suppose the team is @mycompany.com.
+	// [email protected] (a managed Apple ID account under the team @mycompany.com) can sign in
+	// event client_secret is invalid.
+	//
+	// When client_secret is invalid, {"error": "invalid_client"} is returned.
+	// In that case, you need to refer to this documentation to resolve the problem.
+	// https://developer.apple.com/documentation/technotes/tn3107-resolving-sign-in-with-apple-response-errors#Possible-reasons-for-invalid-client-errors
+
 	teamID := ProviderConfig(deps.ProviderConfig).TeamID()
 	keyID := ProviderConfig(deps.ProviderConfig).KeyID()
 
-	// https://developer.apple.com/documentation/signinwithapplerestapi/generate_and_validate_tokens
 	key, err := crypto.ParseAppleP8PrivateKey([]byte(deps.ClientSecret))
 	if err != nil {
 		return
@@ -109,8 +123,12 @@ func (Apple) createClientSecret(deps oauthrelyingparty.Dependencies) (clientSecr
 	_ = payload.Set(jwt.IssuerKey, teamID)
 	_ = payload.Set(jwt.IssuedAtKey, now.Unix())
 	_ = payload.Set(jwt.ExpirationKey, now.Add(duration.Short).Unix())
+
+	// According to the documentation, aud is a string, not an array of string.
+	payload.Options().Enable(jwt.FlattenAudience)
 	_ = payload.Set(jwt.AudienceKey, "https://appleid.apple.com")
-	_ = payload.Set(jwt.SubjectKey, deps.ProviderConfig.ClientID)
+
+	_ = payload.Set(jwt.SubjectKey, deps.ProviderConfig.ClientID())
 
 	jwkKey, err := jwk.FromRaw(key)
 	if err != nil {

pkg/lib/oauthrelyingparty/apple/provider_test.go

@@ -7,10 +7,12 @@ import (
 	. "github.com/smartystreets/goconvey/convey"
 
 	"github.com/authgear/oauthrelyingparty/pkg/api/oauthrelyingparty"
+
+	"github.com/authgear/authgear-server/pkg/util/clock"
 )
 
 func TestAppleImpl(t *testing.T) {
-	Convey("AppleImpl", t, func() {
+	Convey("AppleImpl.GetAuthorizationURL", t, func() {
 		deps := oauthrelyingparty.Dependencies{
 			ProviderConfig: oauthrelyingparty.ProviderConfig{
 				"client_id": "client_id",
@@ -30,4 +32,37 @@ func TestAppleImpl(t *testing.T) {
 		So(err, ShouldBeNil)
 		So(u, ShouldEqual, "https://appleid.apple.com/auth/authorize?client_id=client_id&nonce=nonce&redirect_uri=https%3A%2F%2Flocalhost%2F&response_mode=form_post&response_type=code&scope=name+email&state=state")
 	})
+
+	Convey("AppleImpl.createClientSecret", t, func() {
+		deps := oauthrelyingparty.Dependencies{
+			ProviderConfig: oauthrelyingparty.ProviderConfig{
+				"client_id": "the_client_id",
+				"type":      Type,
+				"team_id":   "the_team_id",
+				"key_id":    "the_key_id",
+			},
+			// Generated with the following command
+			//   openssl genpkey -algorithm EC -pkeyopt ec_paramgen_curve:P-256 -out -
+			//
+			// In case you wonder why it is a P-256 key, it is observed that Apple generates such keys.
+			// See https://developer.apple.com/help/account/manage-keys/create-a-private-key/
+			ClientSecret: `-----BEGIN PRIVATE KEY-----
+MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgnDWXkNs9pRnFZwkm
+miwAePJd5JPUey25Bo8yNPPTovihRANCAARk0V61v/iATyYj3Qbj9ayQzDEVMAwp
+UyS+h/UyCBBNs4aRFSL76tZaeGAmGa62GINnZ4UH4etxoLa4PvNnc77t
+-----END PRIVATE KEY-----
+`,
+			Clock: clock.NewMockClockAt("2025-01-17T13:32:00+08:00"),
+		}
+		g := Apple{}
+
+		clientSecret, err := g.createClientSecret(deps)
+		So(err, ShouldBeNil)
+		// The signature algorithm is ES256, which is ECDSA with P-256 and SHA256, according to https://datatracker.ietf.org/doc/html/rfc7518#section-3.1
+		// ECDSA, by definition, use a cryptographically secure random number.
+		// You can see this nature by looking at the signature of https://pkg.go.dev/crypto/ecdsa#Sign
+		// So the signature is different in every generation.
+		// What we want to assert here is the header and the payload is of an expected shape.
+		So(clientSecret, ShouldStartWith, "eyJhbGciOiJFUzI1NiIsImtpZCI6InRoZV9rZXlfaWQiLCJ0eXAiOiJKV1QifQ.eyJhdWQiOiJodHRwczovL2FwcGxlaWQuYXBwbGUuY29tIiwiZXhwIjoxNzM3MDkyMjIwLCJpYXQiOjE3MzcwOTE5MjAsImlzcyI6InRoZV90ZWFtX2lkIiwic3ViIjoidGhlX2NsaWVudF9pZCJ9.")
+	})
 }