Merge pull request #572 from fluxcd/generic-oicd-auth

Use registry host to get ACR access token and add new OIDCLogin function

oci/auth/aws/auth.go

@@ -27,6 +27,7 @@ import (
 
 	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/config"
+	"github.com/aws/aws-sdk-go-v2/feature/ec2/imds"
 	"github.com/aws/aws-sdk-go-v2/service/ecr"
 	"github.com/google/go-containerregistry/pkg/authn"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -78,7 +79,7 @@ func (c *Client) WithConfig(cfg *aws.Config) {
 // be the case if it's running in EKS, and may need additional setup
 // otherwise (visit https://aws.github.io/aws-sdk-go-v2/docs/configuring-sdk/
 // as a starting point).
-func (c *Client) getLoginAuth(ctx context.Context, awsEcrRegion string) (authn.AuthConfig, error) {
+func (c *Client) getLoginAuth(ctx context.Context) (authn.AuthConfig, error) {
 	// No caching of tokens is attempted; the quota for getting an
 	// auth token is high enough that getting a token every time you
 	// scan an image is viable for O(500) images per region. See
@@ -91,11 +92,20 @@ func (c *Client) getLoginAuth(ctx context.Context, awsEcrRegion string) (authn.A
 		cfg = c.config.Copy()
 	} else {
 		var err error
-		cfg, err = config.LoadDefaultConfig(ctx, config.WithRegion(awsEcrRegion))
+		cfg, err = config.LoadDefaultConfig(ctx)
 		if err != nil {
 			c.mu.Unlock()
 			return authConfig, fmt.Errorf("failed to load default configuration: %w", err)
 		}
+		// Query the current region from IMDS if it's not set yet.
+		if cfg.Region == "" {
+			client := imds.NewFromConfig(cfg)
+			resp, err := client.GetRegion(ctx, &imds.GetRegionInput{})
+			if err != nil {
+				return authConfig, err
+			}
+			cfg.Region = resp.Region
+		}
 		c.config = &cfg
 	}
 	c.mu.Unlock()
@@ -132,18 +142,11 @@ func (c *Client) getLoginAuth(ctx context.Context, awsEcrRegion string) (authn.A
 	return authConfig, nil
 }
 
-// Login attempts to get the authentication material for ECR. It extracts
-// the account and region information from the image URI. The caller can ensure
-// that the passed image is a valid ECR image using ParseRegistry().
+// Login attempts to get the authentication material for ECR.
 func (c *Client) Login(ctx context.Context, autoLogin bool, image string) (authn.Authenticator, error) {
 	if autoLogin {
 		ctrl.LoggerFrom(ctx).Info("logging in to AWS ECR for " + image)
-		_, awsEcrRegion, ok := ParseRegistry(image)
-		if !ok {
-			return nil, errors.New("failed to parse AWS ECR image, invalid ECR image")
-		}
-
-		authConfig, err := c.getLoginAuth(ctx, awsEcrRegion)
+		authConfig, err := c.getLoginAuth(ctx)
 		if err != nil {
 			return nil, err
 		}
@@ -153,3 +156,14 @@ func (c *Client) Login(ctx context.Context, autoLogin bool, image string) (authn
 	}
 	return nil, fmt.Errorf("ECR authentication failed: %w", oci.ErrUnconfiguredProvider)
 }
+
+// OIDCLogin attempts to get the authentication material for ECR.
+func (c *Client) OIDCLogin(ctx context.Context) (authn.Authenticator, error) {
+	authConfig, err := c.getLoginAuth(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	auth := authn.FromConfig(authConfig)
+	return auth, nil
+}

oci/auth/aws/auth_test.go

@@ -155,10 +155,13 @@ func TestGetLoginAuth(t *testing.T) {
 			cfg.EndpointResolverWithOptions = aws.EndpointResolverWithOptionsFunc(func(service, region string, options ...interface{}) (aws.Endpoint, error) {
 				return aws.Endpoint{URL: srv.URL}, nil
 			})
+			// set the region in the config since we are not using the `LoadDefaultConfig` function that sets the region
+			// by querying the instance metadata service(IMDS)
+			cfg.Region = "us-east-1"
 			cfg.Credentials = credentials.NewStaticCredentialsProvider("x", "y", "z")
 			ec.WithConfig(cfg)
 
-			a, err := ec.getLoginAuth(context.TODO(), "us-east-1")
+			a, err := ec.getLoginAuth(context.TODO())
 			g.Expect(err != nil).To(Equal(tt.wantErr))
 			if tt.statusCode == http.StatusOK {
 				g.Expect(a).To(Equal(tt.wantAuthConfig))
@@ -173,6 +176,7 @@ func TestLogin(t *testing.T) {
 		autoLogin  bool
 		image      string
 		statusCode int
+		testOIDC   bool
 		wantErr    bool
 	}{
 		{
@@ -187,19 +191,14 @@ func TestLogin(t *testing.T) {
 			autoLogin:  true,
 			image:      testValidECRImage,
 			statusCode: http.StatusOK,
+			testOIDC:   true,
 		},
 		{
 			name:       "login failure",
 			autoLogin:  true,
 			image:      testValidECRImage,
 			statusCode: http.StatusInternalServerError,
-			wantErr:    true,
-		},
-		{
-			name:       "non ECR image",
-			autoLogin:  true,
-			image:      "gcr.io/foo/bar:v1",
-			statusCode: http.StatusOK,
+			testOIDC:   true,
 			wantErr:    true,
 		},
 	}
@@ -228,6 +227,11 @@ func TestLogin(t *testing.T) {
 
 			_, err := ecrClient.Login(context.TODO(), tt.autoLogin, tt.image)
 			g.Expect(err != nil).To(Equal(tt.wantErr))
+
+			if tt.testOIDC {
+				_, err = ecrClient.OIDCLogin(context.TODO())
+				g.Expect(err != nil).To(Equal(tt.wantErr))
+			}
 		})
 	}
 }

oci/auth/azure/auth.go

@@ -58,8 +58,9 @@ func (c *Client) WithScheme(scheme string) *Client {
 }
 
 // getLoginAuth returns authentication for ACR. The details needed for authentication
-// are gotten from environment variable so there is not need to mount a host path.
-func (c *Client) getLoginAuth(ctx context.Context, ref name.Reference) (authn.AuthConfig, error) {
+// are gotten from environment variable so there is no need to mount a host path.
+// The endpoint is the registry server and will be queried for OAuth authorization token.
+func (c *Client) getLoginAuth(ctx context.Context, registryURL string) (authn.AuthConfig, error) {
 	var authConfig authn.AuthConfig
 
 	// Use default credentials if no token credential is provided.
@@ -83,8 +84,7 @@ func (c *Client) getLoginAuth(ctx context.Context, ref name.Reference) (authn.Au
 	}
 
 	// Obtain ACR access token using exchanger.
-	endpoint := fmt.Sprintf("%s://%s", c.scheme, ref.Context().RegistryStr())
-	ex := newExchanger(endpoint)
+	ex := newExchanger(registryURL)
 	accessToken, err := ex.ExchangeACRAccessToken(string(armToken.Token))
 	if err != nil {
 		return authConfig, fmt.Errorf("error exchanging token: %w", err)
@@ -114,7 +114,10 @@ func ValidHost(host string) bool {
 func (c *Client) Login(ctx context.Context, autoLogin bool, image string, ref name.Reference) (authn.Authenticator, error) {
 	if autoLogin {
 		ctrl.LoggerFrom(ctx).Info("logging in to Azure ACR for " + image)
-		authConfig, err := c.getLoginAuth(ctx, ref)
+		// get registry host from image
+		strArr := strings.SplitN(image, "/", 2)
+		endpoint := fmt.Sprintf("%s://%s", c.scheme, strArr[0])
+		authConfig, err := c.getLoginAuth(ctx, endpoint)
 		if err != nil {
 			ctrl.LoggerFrom(ctx).Info("error logging into ACR " + err.Error())
 			return nil, err
@@ -125,3 +128,18 @@ func (c *Client) Login(ctx context.Context, autoLogin bool, image string, ref na
 	}
 	return nil, fmt.Errorf("ACR authentication failed: %w", oci.ErrUnconfiguredProvider)
 }
+
+// OIDCLogin attempts to get an Authenticator for the provided ACR registry URL endpoint.
+//
+// If you want to construct an Authenticator based on an image reference,
+// you may want to use Login instead.
+func (c *Client) OIDCLogin(ctx context.Context, registryUrl string) (authn.Authenticator, error) {
+	authConfig, err := c.getLoginAuth(ctx, registryUrl)
+	if err != nil {
+		ctrl.LoggerFrom(ctx).Info("error logging into ACR " + err.Error())
+		return nil, err
+	}
+
+	auth := authn.FromConfig(authConfig)
+	return auth, nil
+}

oci/auth/azure/auth_test.go

@@ -78,19 +78,12 @@ func TestGetAzureLoginAuth(t *testing.T) {
 				srv.Close()
 			})
 
-			// Construct an image repo name against the test server.
-			u, err := url.Parse(srv.URL)
-			g.Expect(err).ToNot(HaveOccurred())
-			image := path.Join(u.Host, "foo/bar:v1")
-			ref, err := name.ParseReference(image)
-			g.Expect(err).ToNot(HaveOccurred())
-
 			// Configure new client with test token credential.
 			c := NewClient().
 				WithTokenCredential(tt.tokenCredential).
 				WithScheme("http")
 
-			auth, err := c.getLoginAuth(context.TODO(), ref)
+			auth, err := c.getLoginAuth(context.TODO(), srv.URL)
 			g.Expect(err != nil).To(Equal(tt.wantErr))
 			if tt.statusCode == http.StatusOK {
 				g.Expect(auth).To(Equal(tt.wantAuthConfig))
@@ -125,6 +118,7 @@ func TestLogin(t *testing.T) {
 		name       string
 		autoLogin  bool
 		statusCode int
+		testOIDC   bool
 		wantErr    bool
 	}{
 		{
@@ -136,12 +130,14 @@ func TestLogin(t *testing.T) {
 		{
 			name:       "with auto login",
 			autoLogin:  true,
+			testOIDC:   true,
 			statusCode: http.StatusOK,
 		},
 		{
 			name:       "login failure",
 			autoLogin:  true,
 			statusCode: http.StatusInternalServerError,
+			testOIDC:   true,
 			wantErr:    true,
 		},
 	}
@@ -171,8 +167,13 @@ func TestLogin(t *testing.T) {
 				WithTokenCredential(&FakeTokenCredential{Token: "foo"}).
 				WithScheme("http")
 
-			_, err = ac.Login(context.TODO(), tt.autoLogin, image, ref)
+			_, err = ac.Login(context.TODO(), tt.autoLogin, u.Host, ref)
 			g.Expect(err != nil).To(Equal(tt.wantErr))
+
+			if tt.testOIDC {
+				_, err = ac.OIDCLogin(context.TODO(), srv.URL)
+				g.Expect(err != nil).To(Equal(tt.wantErr))
+			}
 		})
 	}
 }

oci/auth/azure/exchanger.go

@@ -47,6 +47,7 @@ package azure
 import (
 	"encoding/json"
 	"fmt"
+	"io"
 	"net/http"
 	"net/url"
 	"path"
@@ -95,24 +96,28 @@ func (e *exchanger) ExchangeACRAccessToken(armToken string) (string, error) {
 	if err != nil {
 		return "", fmt.Errorf("failed to send token exchange request: %w", err)
 	}
+	defer resp.Body.Close()
 
+	b, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return "", fmt.Errorf("failed to read the body of the response: %w", err)
+	}
 	if resp.StatusCode != http.StatusOK {
 		// Parse the error response.
 		var errors []acrError
-		decoder := json.NewDecoder(resp.Body)
-		if err = decoder.Decode(&errors); err == nil {
+		if err = json.Unmarshal(b, &errors); err == nil {
 			return "", fmt.Errorf("unexpected status code %d from exchange request: %s",
 				resp.StatusCode, errors)
 		}
 
 		// Error response could not be parsed, return a generic error.
-		return "", fmt.Errorf("unexpected status code %d from exchange request", resp.StatusCode)
+		return "", fmt.Errorf("unexpected status code %d from exchange request, response body: %s",
+			resp.StatusCode, string(b))
 	}
 
 	var tokenResp tokenResponse
-	decoder := json.NewDecoder(resp.Body)
-	if err = decoder.Decode(&tokenResp); err != nil {
-		return "", fmt.Errorf("failed to decode the response: %w", err)
+	if err = json.Unmarshal(b, &tokenResp); err != nil {
+		return "", fmt.Errorf("failed to decode the response: %w, response body: %s", err, string(b))
 	}
 	return tokenResp.RefreshToken, nil
 }

oci/auth/gcp/auth.go

@@ -117,3 +117,15 @@ func (c *Client) Login(ctx context.Context, autoLogin bool, image string, ref na
 	}
 	return nil, fmt.Errorf("GCR authentication failed: %w", oci.ErrUnconfiguredProvider)
 }
+
+// OIDCLogin attempts to get the authentication material for GCR from the token url set in the client.
+func (c *Client) OIDCLogin(ctx context.Context) (authn.Authenticator, error) {
+	authConfig, err := c.getLoginAuth(ctx)
+	if err != nil {
+		ctrl.LoggerFrom(ctx).Info("error logging into GCP " + err.Error())
+		return nil, err
+	}
+
+	auth := authn.FromConfig(authConfig)
+	return auth, nil
+}

oci/auth/gcp/auth_test.go

@@ -111,6 +111,7 @@ func TestLogin(t *testing.T) {
 		autoLogin  bool
 		image      string
 		statusCode int
+		testOIDC   bool
 		wantErr    bool
 	}{
 		{
@@ -124,13 +125,15 @@ func TestLogin(t *testing.T) {
 			name:       "with auto login",
 			autoLogin:  true,
 			image:      testValidGCRImage,
+			testOIDC:   true,
 			statusCode: http.StatusOK,
 		},
 		{
 			name:       "login failure",
 			autoLogin:  true,
 			image:      testValidGCRImage,
 			statusCode: http.StatusInternalServerError,
+			testOIDC:   true,
 			wantErr:    true,
 		},
 		{
@@ -161,6 +164,11 @@ func TestLogin(t *testing.T) {
 
 			_, err = gc.Login(context.TODO(), tt.autoLogin, tt.image, ref)
 			g.Expect(err != nil).To(Equal(tt.wantErr))
+
+			if tt.testOIDC {
+				_, err = gc.OIDCLogin(context.TODO())
+				g.Expect(err != nil).To(Equal(tt.wantErr))
+			}
 		})
 	}
 }