App Store Connect API client improvements (#256)

* Use jwt.RegisteredClaims instead of custom claims and set IssuedAt claim

* Add app store connect api client integration test

* Update integration tests

* Use check step from upgrade-golangci-lint branch

* Fix lint issues

* Use check step from master branch

* Log warning for too many requests response

* Update comment

* Add rate limit info to warning messages

Author: godrei

.gitignore

@@ -2,4 +2,5 @@ _tmp/
 .vscode/*
 .idea/*
 **/.idea/*
-.DS_Store
+.DS_Store
+.env

_integration_tests/appstoreconnect_tests/api_key_credential_helper.go

@@ -0,0 +1,69 @@
+package appstoreconnect_tests
+
+import (
+	"io"
+	"os"
+	"testing"
+
+	"github.com/bitrise-io/go-utils/v2/log"
+	"github.com/bitrise-io/go-utils/v2/retryhttp"
+	"github.com/bitrise-io/go-xcode/v2/_integration_tests"
+	"github.com/stretchr/testify/require"
+)
+
+func getAPIKey(t *testing.T) (string, string, []byte, bool) {
+	if os.Getenv("TEST_API_KEY") != "" {
+		return getLocalAPIKey(t)
+	}
+	return getRemoteAPIKey(t)
+}
+
+func getLocalAPIKey(t *testing.T) (string, string, []byte, bool) {
+	keyID := os.Getenv("TEST_API_KEY_ID")
+	require.NotEmpty(t, keyID)
+	issuerID := os.Getenv("TEST_API_KEY_ISSUER_ID")
+	require.NotEmpty(t, issuerID)
+	privateKey := os.Getenv("TEST_API_KEY")
+	require.NotEmpty(t, privateKey)
+	isEnterpriseAPIKey := os.Getenv("TEST_API_KEY_IS_ENTERPRISE") == "true"
+
+	return keyID, issuerID, []byte(privateKey), isEnterpriseAPIKey
+}
+
+func getRemoteAPIKey(t *testing.T) (string, string, []byte, bool) {
+	serviceAccountJSON := os.Getenv("GCS_SERVICE_ACCOUNT_JSON")
+	require.NotEmpty(t, serviceAccountJSON)
+	projectID := os.Getenv("GCS_PROJECT_ID")
+	require.NotEmpty(t, projectID)
+	bucketName := os.Getenv("GCS_BUCKET_NAME")
+	require.NotEmpty(t, bucketName)
+
+	secretAccessor, err := _integration_tests.NewSecretAccessor(serviceAccountJSON, projectID)
+	require.NoError(t, err)
+
+	bucketAccessor, err := _integration_tests.NewBucketAccessor(serviceAccountJSON, bucketName)
+	require.NoError(t, err)
+
+	keyID, err := secretAccessor.GetSecret("BITRISE_APPSTORECONNECT_API_KEY_ID")
+	require.NoError(t, err)
+
+	issuerID, err := secretAccessor.GetSecret("BITRISE_APPSTORECONNECT_API_KEY_ISSUER_ID")
+	require.NoError(t, err)
+
+	keyURL, err := secretAccessor.GetSecret("BITRISE_APPSTORECONNECT_API_KEY_URL")
+	require.NoError(t, err)
+
+	keyDownloadURL, err := bucketAccessor.GetExpiringURL(keyURL)
+	require.NoError(t, err)
+
+	logger := log.NewLogger()
+	logger.EnableDebugLog(false)
+	client := retryhttp.NewClient(logger)
+	resp, err := client.Get(keyDownloadURL)
+	require.NoError(t, err)
+
+	privateKey, err := io.ReadAll(resp.Body)
+	require.NoError(t, err)
+
+	return keyID, issuerID, privateKey, false
+}

_integration_tests/appstoreconnect_tests/appstoreconnect_test.go

@@ -0,0 +1,18 @@
+package appstoreconnect_tests
+
+import (
+	"testing"
+
+	"github.com/bitrise-io/go-xcode/v2/autocodesign/devportalclient/appstoreconnect"
+	"github.com/stretchr/testify/require"
+)
+
+func TestListBundleIDs(t *testing.T) {
+	keyID, issuerID, privateKey, enterpriseAccount := getAPIKey(t)
+
+	client := appstoreconnect.NewClient(appstoreconnect.NewRetryableHTTPClient(), keyID, issuerID, []byte(privateKey), enterpriseAccount)
+
+	response, err := client.Provisioning.ListBundleIDs(&appstoreconnect.ListBundleIDsOptions{})
+	require.NoError(t, err)
+	require.True(t, len(response.Data) > 0)
+}

_integration_tests/bucketaccessor.go

@@ -0,0 +1,46 @@
+package _integration_tests
+
+import (
+	"fmt"
+	"net/http"
+	"strings"
+	"time"
+
+	"cloud.google.com/go/storage"
+	"golang.org/x/oauth2/google"
+	"golang.org/x/oauth2/jwt"
+)
+
+// BucketAccessor ...
+type BucketAccessor struct {
+	jwtConfig    *jwt.Config
+	bucket       string
+	objectExpiry time.Duration
+}
+
+// NewBucketAccessor ...
+func NewBucketAccessor(serviceAccountJSONContent, bucket string) (*BucketAccessor, error) {
+	conf, err := google.JWTConfigFromJSON([]byte(serviceAccountJSONContent))
+	if err != nil {
+		return nil, err
+	}
+
+	return &BucketAccessor{
+		jwtConfig:    conf,
+		bucket:       bucket,
+		objectExpiry: 1 * time.Hour,
+	}, nil
+}
+
+// GetExpiringURL ...
+func (a BucketAccessor) GetExpiringURL(originalURL string) (string, error) {
+	artifactPath := strings.TrimPrefix(strings.TrimPrefix(originalURL, fmt.Sprintf("https://storage.googleapis.com/%s/", a.bucket)), fmt.Sprintf("https://storage.cloud.google.com/%s/", a.bucket))
+	opts := &storage.SignedURLOptions{
+		Method:         http.MethodGet,
+		GoogleAccessID: a.jwtConfig.Email,
+		PrivateKey:     a.jwtConfig.PrivateKey,
+		Expires:        time.Now().Add(a.objectExpiry),
+	}
+
+	return storage.SignedURL(a.bucket, artifactPath, opts)
+}

_integration_tests/secretaccessor.go

@@ -0,0 +1,62 @@
+package _integration_tests
+
+import (
+	"context"
+	"fmt"
+	"time"
+
+	secretmanager "cloud.google.com/go/secretmanager/apiv1"
+	secretmanagerpb "cloud.google.com/go/secretmanager/apiv1/secretmanagerpb"
+	"github.com/bitrise-io/go-utils/log"
+	"github.com/bitrise-io/go-utils/retry"
+	"google.golang.org/api/option"
+)
+
+// SecretAccessor ...
+type SecretAccessor struct {
+	ctx       context.Context
+	client    *secretmanager.Client
+	projectID string
+}
+
+// NewSecretAccessor ...
+func NewSecretAccessor(serviceAccountJSONContent, projectID string) (*SecretAccessor, error) {
+	ctx := context.Background()
+	client, err := secretmanager.NewClient(ctx, option.WithCredentialsJSON([]byte(serviceAccountJSONContent)))
+	if err != nil {
+		return nil, err
+	}
+
+	return &SecretAccessor{
+		ctx:       ctx,
+		client:    client,
+		projectID: projectID,
+	}, nil
+}
+
+// GetSecret ...
+func (m SecretAccessor) GetSecret(key string) (string, error) {
+	secretValue := ""
+	if err := retry.Times(3).Wait(30 * time.Second).Try(func(attempt uint) error {
+		if attempt > 0 {
+			log.Warnf("%d attempt failed", attempt)
+		}
+
+		name := fmt.Sprintf("projects/%s/secrets/%s/versions/latest", m.projectID, key)
+		req := &secretmanagerpb.AccessSecretVersionRequest{
+			Name: name,
+		}
+		result, err := m.client.AccessSecretVersion(m.ctx, req)
+		if err != nil {
+			log.Warnf("%s", err)
+			return err
+		}
+
+		secretValue = string(result.Payload.Data)
+		return nil
+	}); err != nil {
+		return "", err
+	}
+
+	return secretValue, nil
+}

autocodesign/devportalclient/appstoreconnect/appstoreconnect.go

@@ -37,9 +37,8 @@ var (
 	// A given token can be reused for up to 20 minutes:
 	// https://developer.apple.com/documentation/appstoreconnectapi/generating_tokens_for_api_requests
 	//
-	// Using 19 minutes to make sure time inaccuracies at token validation does not cause issues.
-	jwtDuration    = 19 * time.Minute
-	jwtReserveTime = 2 * time.Minute
+	// We use 18 minutes to make sure time inaccuracies at token validation does not cause issues.
+	jwtDuration = 18 * time.Minute
 )
 
 // HTTPClient ...
@@ -79,6 +78,23 @@ func NewRetryableHTTPClient() *http.Client {
 			return true, nil
 		}
 
+		if resp != nil && resp.StatusCode == http.StatusTooManyRequests {
+			message := "Received HTTP 429 Too Many Requests"
+			if rateLimit := resp.Header.Get("X-Rate-Limit"); rateLimit != "" {
+				message += " (" + rateLimit + ")"
+			}
+
+			if retryAfter := resp.Header.Get("Retry-After"); retryAfter != "" {
+				message += ", retrying the request in " + retryAfter + " seconds..."
+			} else {
+				message += ", retrying the request..."
+			}
+
+			log.Warnf(message)
+
+			return true, nil
+		}
+
 		shouldRetry, err := retryablehttp.DefaultRetryPolicy(ctx, resp, err)
 		if shouldRetry && resp != nil {
 			log.Debugf("Retry network error: %d", resp.StatusCode)
@@ -122,21 +138,12 @@ func NewClient(httpClient HTTPClient, keyID, issuerID string, privateKey []byte,
 // and return a signed key
 func (c *Client) ensureSignedToken() (string, error) {
 	if c.token != nil {
-		claim, ok := c.token.Claims.(claims)
-		if !ok {
-			return "", fmt.Errorf("failed to cast claim for token")
-		}
-		expiration := time.Unix(int64(claim.Expiration), 0)
-
-		// A given token can be reused for up to 20 minutes:
-		// https://developer.apple.com/documentation/appstoreconnectapi/generating_tokens_for_api_requests
-		//
-		// The step generates a new token 2 minutes before the expiry.
-		if time.Until(expiration) > jwtReserveTime {
+		err := c.token.Claims.Valid()
+		if err == nil {
 			return c.signedToken, nil
 		}
 
-		log.Debugf("JWT token expired, regenerating")
+		log.Debugf("JWT token is invalid: %s, regenerating...", err)
 	} else {
 		log.Debugf("Generating JWT token")
 	}

autocodesign/devportalclient/appstoreconnect/jwt.go

@@ -32,29 +32,22 @@ func signToken(token *jwt.Token, privateKeyContent []byte) (string, error) {
 
 // createToken creates a jwt.Token for the Apple API
 func createToken(keyID string, issuerID string, audience string) *jwt.Token {
-	payload := claims{
-		IssuerID:   issuerID,
-		Expiration: time.Now().Add(jwtDuration).Unix(),
-		Audience:   audience,
+	issuedAt := time.Now()
+	expirationTime := time.Now().Add(jwtDuration)
+
+	claims := jwt.RegisteredClaims{
+		Issuer:    issuerID,
+		IssuedAt:  jwt.NewNumericDate(issuedAt),
+		ExpiresAt: jwt.NewNumericDate(expirationTime),
+		Audience:  jwt.ClaimStrings{audience},
 	}
 
 	// registers headers: alg = ES256 and typ = JWT
-	token := jwt.NewWithClaims(jwt.SigningMethodES256, payload)
+	token := jwt.NewWithClaims(jwt.SigningMethodES256, claims)
 
 	header := token.Header
 	header["kid"] = keyID
+	token.Header = header
 
 	return token
 }
-
-// claims represents the JWT payload for the Apple API
-type claims struct {
-	IssuerID   string `json:"iss"`
-	Expiration int64  `json:"exp"`
-	Audience   string `json:"aud"`
-}
-
-// Valid implements the jwt.Claims interface
-func (c claims) Valid() error {
-	return nil
-}

codesign/codesign_test.go

@@ -217,7 +217,7 @@ func TestSelectConnectionCredentials(t *testing.T) {
 	localKeyPath := filepath.Join(t.TempDir(), "key.p8")
 	err := os.WriteFile(localKeyPath, []byte("private key contents"), 0700)
 	if err != nil {
-		t.Fatalf(err.Error())
+		t.Fatal(err.Error())
 	}
 	testInputs := ConnectionOverrideInputs{
 		APIKeyPath:     stepconf.Secret(localKeyPath),

codesign/inputparse_test.go

@@ -203,7 +203,7 @@ func Test_ParseConnectionOverrideConfig(t *testing.T) {
 	fileContent := "this is a private key"
 	err := os.WriteFile(path, []byte(fileContent), 0666)
 	if err != nil {
-		t.Errorf(err.Error())
+		t.Error(err.Error())
 	}
 
 	keyID := " ABC123   "
@@ -212,7 +212,7 @@ func Test_ParseConnectionOverrideConfig(t *testing.T) {
 	// When
 	connection, err := parseConnectionOverrideConfig(stepconf.Secret(path), keyID, keyIssuerID, true, log.NewLogger())
 	if err != nil {
-		t.Errorf(err.Error())
+		t.Error(err.Error())
 	}
 
 	// Then