diff --git a/pkg/cluster/deploybaseresources.go b/pkg/cluster/deploybaseresources.go index 27395a44bb4..9acd50b5dbd 100644 --- a/pkg/cluster/deploybaseresources.go +++ b/pkg/cluster/deploybaseresources.go @@ -46,7 +46,7 @@ func (m *manager) createOIDC(ctx context.Context) error { blobContainerName := env.OIDCBlobContainerPrefix + m.doc.ID publicAccess := azstorage.PublicAccessNone - // Public access on OIDC Container needed for development environments because of no AFD avaialbility + // Public access on OIDC Container needed for development environments because of no AFD availability if m.env.IsLocalDevelopmentMode() { publicAccess = azstorage.PublicAccessBlob } diff --git a/pkg/cluster/deploybaseresources_test.go b/pkg/cluster/deploybaseresources_test.go index e2fc3c1ae59..942581a0027 100644 --- a/pkg/cluster/deploybaseresources_test.go +++ b/pkg/cluster/deploybaseresources_test.go @@ -1557,6 +1557,34 @@ func TestCreateOIDC(t *testing.T) { wantBoundServiceAccountSigningKey: false, wantErr: "generic error", }, + { + name: "Fail - OIDCBuilder fails to populate OIDC blob", + oc: &api.OpenShiftClusterDocument{ + Key: strings.ToLower(resourceID), + ID: clusterID, + OpenShiftCluster: &api.OpenShiftCluster{ + Properties: api.OpenShiftClusterProperties{ + ClusterProfile: api.ClusterProfile{ + ResourceGroupID: resourceGroup, + }, + PlatformWorkloadIdentityProfile: &api.PlatformWorkloadIdentityProfile{}, + }, + }, + }, + mocks: func(blob *mock_azblob.MockManager, env *mock_env.MockInterface, azblobClient *mock_azblob.MockAZBlobClient) { + env.EXPECT().OIDCStorageAccountName().AnyTimes().Return(oidcStorageAccountName) + env.EXPECT().IsLocalDevelopmentMode().Return(false) + env.EXPECT().ResourceGroup().Return(resourceGroupName) + env.EXPECT().Environment().Return(&azureclient.PublicCloud) + env.EXPECT().OIDCEndpoint().Return(afdEndpoint) + blob.EXPECT().CreateBlobContainer(gomock.Any(), resourceGroupName, oidcStorageAccountName, gomock.Any(), azstorage.PublicAccessNone).Return(nil) + azblobClient.EXPECT().UploadBuffer(gomock.Any(), "", oidcbuilder.DiscoveryDocumentKey, gomock.Any()).Return(nil) + azblobClient.EXPECT().UploadBuffer(gomock.Any(), "", oidcbuilder.JWKSKey, gomock.Any()).Return(errors.New("generic error")) + blob.EXPECT().GetAZBlobClient(gomock.Any(), &azblob.ClientOptions{}).Return(azblobClient, nil) + }, + wantBoundServiceAccountSigningKey: false, + wantErr: "generic error", + }, } { t.Run(tt.name, func(t *testing.T) { controller := gomock.NewController(t) diff --git a/pkg/util/azblob/manager.go b/pkg/util/azblob/manager.go index ff695a2591c..05883d6d461 100644 --- a/pkg/util/azblob/manager.go +++ b/pkg/util/azblob/manager.go @@ -39,8 +39,6 @@ func NewManager(environment *azureclient.AROEnvironment, subscriptionID string, } func (m *manager) CreateBlobContainer(ctx context.Context, resourceGroup string, accountName string, containerName string, publicAccess azstorage.PublicAccess) error { - needToCreateBlobContainer := false - _, err := m.blobContainer.Get( ctx, resourceGroup, @@ -48,14 +46,9 @@ func (m *manager) CreateBlobContainer(ctx context.Context, resourceGroup string, containerName, &azstorage.BlobContainersClientGetOptions{}, ) - if err != nil { - if !bloberror.HasCode(err, bloberror.ContainerNotFound) { - return err - } - needToCreateBlobContainer = true - } - - if !needToCreateBlobContainer { + if err != nil && !bloberror.HasCode(err, bloberror.ContainerNotFound) { + return err + } else if err == nil { return nil } diff --git a/pkg/util/azureclient/mgmt/storage/blobcontainers.go b/pkg/util/azureclient/mgmt/storage/blobcontainers.go deleted file mode 100644 index c53b155ab3f..00000000000 --- a/pkg/util/azureclient/mgmt/storage/blobcontainers.go +++ /dev/null @@ -1,31 +0,0 @@ -package storage - -// Copyright (c) Microsoft Corporation. -// Licensed under the Apache License 2.0. - -import ( - "context" - - mgmtstorage "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-06-01/storage" - "github.com/Azure/go-autorest/autorest" - - "github.com/Azure/ARO-RP/pkg/util/azureclient" -) - -type BlobContainersClient interface { - Get(ctx context.Context, resourceGroupName string, accountName string, containerName string) (mgmtstorage.BlobContainer, error) - Create(ctx context.Context, resourceGroupName string, accountName string, containerName string, blobContainer mgmtstorage.BlobContainer) (mgmtstorage.BlobContainer, error) - Delete(ctx context.Context, resourceGroupName string, accountName string, containerName string) (result autorest.Response, err error) -} - -type blobContainersClient struct { - mgmtstorage.BlobContainersClient -} - -func NewBlobContainersClient(environment *azureclient.AROEnvironment, subscriptionID string, authorizer autorest.Authorizer) *blobContainersClient { - client := mgmtstorage.NewBlobContainersClientWithBaseURI(environment.ResourceManagerEndpoint, subscriptionID) - client.Authorizer = authorizer - return &blobContainersClient{ - BlobContainersClient: client, - } -} diff --git a/pkg/util/oidcbuilder/jwks.go b/pkg/util/oidcbuilder/jwks.go index 365d2de642a..1436ff3c7ec 100644 --- a/pkg/util/oidcbuilder/jwks.go +++ b/pkg/util/oidcbuilder/jwks.go @@ -31,10 +31,10 @@ func CreateKeyPair() (encPrivateKey []byte, encPublicKey []byte, err error) { Bytes: x509.MarshalPKCS1PrivateKey(privateKey), }) - // Generate public key from private keypair + // Serialize public key into a byte array to prepare to store it in the OIDC storage blob pubKeyBytes, err := x509.MarshalPKIXPublicKey(&privateKey.PublicKey) if err != nil { - return nil, nil, errors.Wrapf(err, "failed to generate public key from private") + return nil, nil, errors.Wrapf(err, "failed to serialize public key") } encodedPublicKey := pem.EncodeToMemory(&pem.Block{ Type: "PUBLIC KEY", diff --git a/pkg/util/oidcbuilder/jwks_test.go b/pkg/util/oidcbuilder/jwks_test.go new file mode 100644 index 00000000000..63fd02dfdfd --- /dev/null +++ b/pkg/util/oidcbuilder/jwks_test.go @@ -0,0 +1,60 @@ +package oidcbuilder + +import ( + "crypto/rand" + "crypto/rsa" + "testing" + + "github.com/golang/mock/gomock" + + mock_azblob "github.com/Azure/ARO-RP/pkg/util/mocks/azblob" + utilerror "github.com/Azure/ARO-RP/test/util/error" +) + +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache License 2.0. + +func TestKeyIDFromPublicKey(t *testing.T) { + privateKey, err := rsa.GenerateKey(rand.Reader, 4096) + if err != nil { + t.Fatal(err) + } + + keyID, err := keyIDFromPublicKey(&privateKey.PublicKey) + if err != nil { + t.Fatal(err) + } + + invalidPublicKey := rsa.PublicKey{} + + for _, tt := range []struct { + name string + mocks func(*mock_azblob.MockAZBlobClient) + publicKey interface{} + wantkid string + wantErr string + }{ + { + name: "Success", + publicKey: &privateKey.PublicKey, + wantkid: keyID, + }, + { + name: "Failed to serialize public key", + publicKey: &invalidPublicKey, + wantErr: "Failed to serialize public key to DER format: asn1: structure error: empty integer", + }, + } { + t.Run(tt.name, func(t *testing.T) { + controller := gomock.NewController(t) + defer controller.Finish() + + kid, err := keyIDFromPublicKey(tt.publicKey) + utilerror.AssertErrorMessage(t, err, tt.wantErr) + + if tt.wantkid != kid { + t.Fatalf("Expected KeyID and returned KeyID doesn't match") + } + }) + } +} diff --git a/pkg/util/oidcbuilder/oidcbuilder_test.go b/pkg/util/oidcbuilder/oidcbuilder_test.go index 1a95caddec0..bdac62d3614 100644 --- a/pkg/util/oidcbuilder/oidcbuilder_test.go +++ b/pkg/util/oidcbuilder/oidcbuilder_test.go @@ -2,6 +2,8 @@ package oidcbuilder import ( "context" + "crypto/ecdsa" + "crypto/elliptic" "crypto/rand" "crypto/rsa" "crypto/x509" @@ -40,6 +42,14 @@ func TestEnsureOIDCDocs(t *testing.T) { Bytes: pubKeyBytes, }) + nonRSAPrivateKey, _ := ecdsa.GenerateKey(elliptic.P384(), rand.Reader) + nonRSAPubKeyBytes, err := x509.MarshalPKIXPublicKey(&nonRSAPrivateKey.PublicKey) + nonRSAEncodedPublicKey := pem.EncodeToMemory(&pem.Block{ + Type: "PUBLIC KEY", + Headers: nil, + Bytes: nonRSAPubKeyBytes, + }) + invalidKey := []byte("Invalid Key") for _, tt := range []struct { @@ -118,6 +128,16 @@ func TestEnsureOIDCDocs(t *testing.T) { }, wantErr: "generic error", }, + { + name: "Fail - Public key is not of type RSA", + oidcbuilder: &OIDCBuilder{ + privateKey: priKey, + publicKey: nonRSAEncodedPublicKey, + blobContainerURL: blobContainerURL, + endpointURL: endpointURL, + }, + wantErr: "Public key is not of type RSA", + }, } { t.Run(tt.name, func(t *testing.T) { controller := gomock.NewController(t)