From f293c08dd8e237e44880044f0219566902882a0e Mon Sep 17 00:00:00 2001 From: Ian Booth Date: Thu, 17 Aug 2023 08:47:09 +1000 Subject: [PATCH] Ensure new Azure cred can access the Azure Vault Key app --- provider/azure/credentials.go | 27 ++++--- provider/azure/credentials_test.go | 3 + .../internal/azureauth/serviceprincipal.go | 80 +++++++++++++------ .../azureauth/serviceprincipal_test.go | 25 ++++++ 4 files changed, 99 insertions(+), 36 deletions(-) diff --git a/provider/azure/credentials.go b/provider/azure/credentials.go index 2209f85f014..2e9e871cfac 100644 --- a/provider/azure/credentials.go +++ b/provider/azure/credentials.go @@ -161,6 +161,19 @@ func (c environProviderCredentials) FinalizeCredential( switch authType := args.Credential.AuthType(); authType { case deviceCodeAuthType: subscriptionId := args.Credential.Attributes()[credAttrSubscriptionId] + + var azCloudName string + switch args.CloudName { + case "azure": + azCloudName = azureauth.AzureCloud + case "azure-china": + azCloudName = azureauth.AzureChinaCloud + case "azure-gov": + azCloudName = azureauth.AzureUSGovernment + default: + return nil, errors.Errorf("unknown Azure cloud name %q", args.CloudName) + } + if subscriptionId != "" { opts := azcore.ClientOptions{ Cloud: azureCloud(args.CloudName, args.CloudEndpoint, args.CloudIdentityEndpoint), @@ -173,21 +186,12 @@ func (c environProviderCredentials) FinalizeCredential( return nil, errors.Trace(err) } return c.deviceCodeCredential(ctx, args, azureauth.ServicePrincipalParams{ + CloudName: azCloudName, SubscriptionId: subscriptionId, TenantId: tenantID, }) } - var azCloudName string - switch args.CloudName { - case "azure": - azCloudName = "AzureCloud" - case "azure-china": - azCloudName = "AzureChinaCloud" - case "azure-gov": - azCloudName = "AzureUSGovernment" - default: - return nil, errors.Errorf("unknown Azure cloud name %q", args.CloudName) - } + params, err := c.getServicePrincipalParams(azCloudName) if err != nil { return nil, errors.Trace(err) @@ -295,6 +299,7 @@ func (c environProviderCredentials) getServicePrincipalParams(cloudName string) } } return azureauth.ServicePrincipalParams{ + CloudName: cloudName, SubscriptionId: acc.ID, TenantId: acc.AuthTenantId(), }, nil diff --git a/provider/azure/credentials_test.go b/provider/azure/credentials_test.go index 8d708d88bae..a05119931f3 100644 --- a/provider/azure/credentials_test.go +++ b/provider/azure/credentials_test.go @@ -256,6 +256,7 @@ func (s *credentialsSuite) TestFinalizeCredentialInteractive(c *gc.C) { in := cloud.NewCredential("interactive", map[string]string{"subscription-id": fakeSubscriptionId}) ctx := cmdtesting.Context(c) out, err := s.provider.FinalizeCredential(ctx, environs.FinalizeCredentialParams{ + CloudName: "azure", Credential: in, CloudEndpoint: "https://arm.invalid", CloudStorageEndpoint: "https://core.invalid", @@ -274,6 +275,7 @@ func (s *credentialsSuite) TestFinalizeCredentialInteractive(c *gc.C) { s.servicePrincipalCreator.CheckCallNames(c, "InteractiveCreate") args := s.servicePrincipalCreator.Calls()[0].Args c.Assert(args[2], jc.DeepEquals, azureauth.ServicePrincipalParams{ + CloudName: "AzureCloud", SubscriptionId: fakeSubscriptionId, TenantId: fakeTenantId, }) @@ -285,6 +287,7 @@ func (s *credentialsSuite) TestFinalizeCredentialInteractiveError(c *gc.C) { s.servicePrincipalCreator.SetErrors(errors.New("blargh")) ctx := cmdtesting.Context(c) _, err := s.provider.FinalizeCredential(ctx, environs.FinalizeCredentialParams{ + CloudName: "azure", Credential: in, CloudEndpoint: "https://arm.invalid", CloudIdentityEndpoint: "https://graph.invalid", diff --git a/provider/azure/internal/azureauth/serviceprincipal.go b/provider/azure/internal/azureauth/serviceprincipal.go index 51218e65be2..d38bcf73096 100644 --- a/provider/azure/internal/azureauth/serviceprincipal.go +++ b/provider/azure/internal/azureauth/serviceprincipal.go @@ -35,14 +35,33 @@ const ( // the application. jujuApplicationId = "60a04dc9-1857-425f-8076-5ba81ca53d66" - // JujuApplicationObjectId is the ObjectId of the Azure application. + // jujuApplicationObjectId is the ObjectId of the Azure application. jujuApplicationObjectId = "8b744cea-179d-4a73-9dff-20d52126030a" + // defaultAzureKeyVaultApplicationId is the default Azure Key Vault + // applicationID if not specified for a specific cloud type. + defaultAzureKeyVaultApplicationId = "cfa8b339-82a2-471a-a3c9-0fc0be7a4093" + // passwordExpiryDuration is how long the application password we // set will remain valid. passwordExpiryDuration = 365 * 24 * time.Hour ) +const ( + // These consts represent the Azure cloud types. + + AzureCloud = "AzureCloud" + AzureChinaCloud = "AzureChinaCloud" + AzureUSGovernment = "AzureUSGovernment" +) + +// cloudVaultApps holds the IDs of the Azure Key Vault Application +// for each cloud type. +var cloudVaultApps = map[string]string{ + AzureCloud: defaultAzureKeyVaultApplicationId, + AzureUSGovernment: "7e7c393b-45d0-48b1-a35e-2905ddf8183c", +} + // MaybeJujuApplicationObjectID returns the Juju Application Object ID // if the passed in application ID is the Juju Enterprise App. func MaybeJujuApplicationObjectID(appID string) (string, error) { @@ -54,6 +73,7 @@ func MaybeJujuApplicationObjectID(appID string) (string, error) { // ServicePrincipalParams are used when creating Juju service principal. type ServicePrincipalParams struct { + CloudName string // Credential is the authorization needed to contact the // Azure graph API. Credential azcore.TokenCredential @@ -119,7 +139,18 @@ func (c *ServicePrincipalCreator) Create(sdkCtx context.Context, params ServiceP if err != nil { return "", "", "", errors.Trace(err) } - servicePrincipalObjectId, password, err := c.createOrUpdateServicePrincipal(sdkCtx, client) + + // The user account must have a service principal for the Azure Key Vault application. + azureKeyVaultApplicationId, ok := cloudVaultApps[params.CloudName] + if !ok { + azureKeyVaultApplicationId = defaultAzureKeyVaultApplicationId + } + _, err = c.createOrUpdateServicePrincipal(sdkCtx, client, azureKeyVaultApplicationId, "Azure Key Vault application") + if err != nil { + return "", "", "", errors.Trace(err) + } + + servicePrincipalObjectId, password, err := c.createOrUpdateJujuServicePrincipal(sdkCtx, client) if err != nil { return "", "", "", errors.Trace(err) } @@ -129,7 +160,7 @@ func (c *ServicePrincipalCreator) Create(sdkCtx context.Context, params ServiceP return jujuApplicationId, servicePrincipalObjectId, password, nil } -func (c *ServicePrincipalCreator) createOrUpdateServicePrincipal(sdkCtx context.Context, client *msgraphsdkgo.GraphServiceClient) (servicePrincipalObjectId, password string, _ error) { +func (c *ServicePrincipalCreator) createOrUpdateJujuServicePrincipal(sdkCtx context.Context, client *msgraphsdkgo.GraphServiceClient) (servicePrincipalObjectId, password string, _ error) { passwordCredential, err := c.preparePasswordCredential() if err != nil { return "", "", errors.Annotate(err, "preparing password credential") @@ -155,24 +186,30 @@ func (c *ServicePrincipalCreator) createOrUpdateServicePrincipal(sdkCtx context. return toValue(servicePrincipal.GetId()), toValue(addPassword.GetSecretText()), nil } - // The service principal might already exist, so we need to query - // its object ID, and fetch the existing password credentials - // to update. - servicePrincipal, err := client.ServicePrincipalsWithAppId(to.Ptr(jujuApplicationId)).Get(sdkCtx, nil) + servicePrincipal, err := c.createOrUpdateServicePrincipal(sdkCtx, client, jujuApplicationId, "Juju Application") + if err != nil { + return "", "", errors.Trace(err) + } + id, password, err := addPassword(servicePrincipal) + if err != nil { + return "", "", errors.Annotate(err, "creating service principal password") + } + return id, password, nil +} + +func (c *ServicePrincipalCreator) createOrUpdateServicePrincipal(sdkCtx context.Context, client *msgraphsdkgo.GraphServiceClient, appId, label string) (models.ServicePrincipalable, error) { + // The service principal might already exist, so we need to query its application ID. + servicePrincipal, err := client.ServicePrincipalsWithAppId(to.Ptr(appId)).Get(sdkCtx, nil) if err == nil { - id, password, err := addPassword(servicePrincipal) - if err != nil { - return "", "", errors.Annotate(err, "creating service principal password") - } - return id, password, nil + return servicePrincipal, nil } if !isNotFound(err) { - return "", "", errors.Annotate(ReportableError(err), "looking for existing service principal") + return nil, errors.Annotatef(ReportableError(err), "looking for existing service principal for %s", label) } createServicePrincipal := func() error { requestBody := models.NewServicePrincipal() - requestBody.SetAppId(to.Ptr(jujuApplicationId)) + requestBody.SetAppId(to.Ptr(appId)) requestBody.SetAccountEnabled(to.Ptr(true)) servicePrincipal, err = client.ServicePrincipals().Post(sdkCtx, requestBody, nil) return errors.Annotate(ReportableError(err), "creating service principal") @@ -192,21 +229,14 @@ func (c *ServicePrincipalCreator) createOrUpdateServicePrincipal(sdkCtx context. } if err := retry.Call(retryArgs); err != nil { if !isAlreadyExists(err) { - return "", "", errors.Trace(err) + return nil, errors.Trace(err) } - // The service principal already exists, so we'll fall out - // and update the service principal's password credentials. - servicePrincipal, err = client.ServicePrincipalsWithAppId(to.Ptr(jujuApplicationId)).Get(sdkCtx, nil) + servicePrincipal, err = client.ServicePrincipalsWithAppId(to.Ptr(appId)).Get(sdkCtx, nil) if err != nil { - return "", "", errors.Annotate(ReportableError(err), "looking for service principal") + return nil, errors.Annotatef(ReportableError(err), "looking for service principal for %s", label) } } - - id, password, err := addPassword(servicePrincipal) - if err != nil { - return "", "", errors.Annotate(err, "creating service principal password") - } - return id, password, nil + return servicePrincipal, nil } func (c *ServicePrincipalCreator) preparePasswordCredential() (*models.PasswordCredential, error) { diff --git a/provider/azure/internal/azureauth/serviceprincipal_test.go b/provider/azure/internal/azureauth/serviceprincipal_test.go index 51b5e7f8464..6c712f8f462 100644 --- a/provider/azure/internal/azureauth/serviceprincipal_test.go +++ b/provider/azure/internal/azureauth/serviceprincipal_test.go @@ -142,6 +142,10 @@ func (s *InteractiveSuite) TestInteractive(c *gc.C) { mockAdaptor := &MockRequestAdaptor{NetHttpRequestAdapter: ra} mockAdaptor.results = []requestResult{{ + PathPattern: regexp.QuoteMeta("{+baseurl}/servicePrincipals(appId='{appId}')") + ".*", + Params: map[string]string{"appId": "cfa8b339-82a2-471a-a3c9-0fc0be7a4093"}, + Result: models.NewServicePrincipal(), + }, { PathPattern: regexp.QuoteMeta("{+baseurl}/servicePrincipals(appId='{appId}')") + ".*", Params: map[string]string{"appId": "60a04dc9-1857-425f-8076-5ba81ca53d66"}, Result: sp, @@ -167,6 +171,7 @@ func (s *InteractiveSuite) TestInteractive(c *gc.C) { sdkCtx := context.Background() appId, spObjectId, password, err := spc.InteractiveCreate(sdkCtx, &stderr, azureauth.ServicePrincipalParams{ + CloudName: "AzureCloud", SubscriptionId: subscriptionId, TenantId: fakeTenantId, Credential: &azuretesting.FakeCredential{}, @@ -193,6 +198,10 @@ func (s *InteractiveSuite) TestInteractiveRoleAssignmentAlreadyExists(c *gc.C) { mockAdaptor := &MockRequestAdaptor{NetHttpRequestAdapter: ra} mockAdaptor.results = []requestResult{{ + PathPattern: regexp.QuoteMeta("{+baseurl}/servicePrincipals(appId='{appId}')") + ".*", + Params: map[string]string{"appId": "cfa8b339-82a2-471a-a3c9-0fc0be7a4093"}, + Result: models.NewServicePrincipal(), + }, { PathPattern: regexp.QuoteMeta("{+baseurl}/servicePrincipals(appId='{appId}')") + ".*", Params: map[string]string{"appId": "60a04dc9-1857-425f-8076-5ba81ca53d66"}, Result: sp, @@ -218,6 +227,7 @@ func (s *InteractiveSuite) TestInteractiveRoleAssignmentAlreadyExists(c *gc.C) { sdkCtx := context.Background() appId, spObjectId, password, err := spc.InteractiveCreate(sdkCtx, &stderr, azureauth.ServicePrincipalParams{ + CloudName: "AzureCloud", SubscriptionId: subscriptionId, TenantId: fakeTenantId, Credential: &azuretesting.FakeCredential{}, @@ -254,6 +264,10 @@ func (s *InteractiveSuite) TestInteractiveServicePrincipalNotFound(c *gc.C) { mockAdaptor := &MockRequestAdaptor{NetHttpRequestAdapter: ra} mockAdaptor.results = []requestResult{{ + PathPattern: regexp.QuoteMeta("{+baseurl}/servicePrincipals(appId='{appId}')") + ".*", + Params: map[string]string{"appId": "cfa8b339-82a2-471a-a3c9-0fc0be7a4093"}, + Result: models.NewServicePrincipal(), + }, { PathPattern: regexp.QuoteMeta("{+baseurl}/servicePrincipals(appId='{appId}')") + ".*", Params: map[string]string{"appId": "60a04dc9-1857-425f-8076-5ba81ca53d66"}, Err: dataError("Request_ResourceNotFound"), @@ -282,6 +296,7 @@ func (s *InteractiveSuite) TestInteractiveServicePrincipalNotFound(c *gc.C) { sdkCtx := context.Background() appId, spObjectId, password, err := spc.InteractiveCreate(sdkCtx, &stderr, azureauth.ServicePrincipalParams{ + CloudName: "AzureCloud", SubscriptionId: subscriptionId, TenantId: fakeTenantId, Credential: &azuretesting.FakeCredential{}, @@ -304,6 +319,10 @@ func (s *InteractiveSuite) TestInteractiveServicePrincipalNotFoundRace(c *gc.C) mockAdaptor := &MockRequestAdaptor{NetHttpRequestAdapter: ra} mockAdaptor.results = []requestResult{{ + PathPattern: regexp.QuoteMeta("{+baseurl}/servicePrincipals(appId='{appId}')") + ".*", + Params: map[string]string{"appId": "cfa8b339-82a2-471a-a3c9-0fc0be7a4093"}, + Result: models.NewServicePrincipal(), + }, { PathPattern: regexp.QuoteMeta("{+baseurl}/servicePrincipals(appId='{appId}')") + ".*", Params: map[string]string{"appId": "60a04dc9-1857-425f-8076-5ba81ca53d66"}, Err: dataError("Request_ResourceNotFound"), @@ -335,6 +354,7 @@ func (s *InteractiveSuite) TestInteractiveServicePrincipalNotFoundRace(c *gc.C) sdkCtx := context.Background() appId, spObjectId, password, err := spc.InteractiveCreate(sdkCtx, &stderr, azureauth.ServicePrincipalParams{ + CloudName: "AzureCloud", SubscriptionId: subscriptionId, TenantId: fakeTenantId, Credential: &azuretesting.FakeCredential{}, }) @@ -356,6 +376,10 @@ func (s *InteractiveSuite) TestInteractiveRetriesRoleAssignment(c *gc.C) { mockAdaptor := &MockRequestAdaptor{NetHttpRequestAdapter: ra} mockAdaptor.results = []requestResult{{ + PathPattern: regexp.QuoteMeta("{+baseurl}/servicePrincipals(appId='{appId}')") + ".*", + Params: map[string]string{"appId": "cfa8b339-82a2-471a-a3c9-0fc0be7a4093"}, + Result: models.NewServicePrincipal(), + }, { PathPattern: regexp.QuoteMeta("{+baseurl}/servicePrincipals(appId='{appId}')") + ".*", Params: map[string]string{"appId": "60a04dc9-1857-425f-8076-5ba81ca53d66"}, Result: sp, @@ -381,6 +405,7 @@ func (s *InteractiveSuite) TestInteractiveRetriesRoleAssignment(c *gc.C) { subscriptionId := "22222222-2222-2222-2222-222222222222" sdkCtx := context.Background() appId, spObjectId, password, err := spc.InteractiveCreate(sdkCtx, &stderr, azureauth.ServicePrincipalParams{ + CloudName: "AzureCloud", SubscriptionId: subscriptionId, TenantId: fakeTenantId, Credential: &azuretesting.FakeCredential{}, })