From ebce851806afccfcead3918121dafb883eed1fc4 Mon Sep 17 00:00:00 2001 From: Michael Golowka <72365+pcman312@users.noreply.github.com> Date: Mon, 27 Sep 2021 14:51:37 -0600 Subject: [PATCH 01/39] Remove bare returns --- api/application_msgraph.go | 46 ++++++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/api/application_msgraph.go b/api/application_msgraph.go index 4ff7329..610680d 100644 --- a/api/application_msgraph.go +++ b/api/application_msgraph.go @@ -90,6 +90,7 @@ func (c *AppClient) CreateApplication(ctx context.Context, displayName string) ( result, err = c.createApplicationResponder(resp) if err != nil { return result, autorest.NewErrorWithError(err, "provider", "CreateApplication", resp, "Failure responding to request") + } return result, nil @@ -100,12 +101,13 @@ func (c *AppClient) CreateApplication(ctx context.Context, displayName string) ( func (c *AppClient) DeleteApplication(ctx context.Context, applicationObjectID string) error { req, err := c.deleteApplicationPreparer(ctx, applicationObjectID) if err != nil { - return autorest.NewErrorWithError(err, "provider", "DeleteApplication", nil, "Failure preparing request") + return result, autorest.NewErrorWithError(err, "provider", "DeleteApplication", nil, "Failure preparing request") } resp, err := c.deleteApplicationSender(req) if err != nil { - return autorest.NewErrorWithError(err, "provider", "DeleteApplication", resp, "Failure sending request") + result = autorest.Response{Response: resp} + return result, autorest.NewErrorWithError(err, "provider", "DeleteApplication", resp, "Failure sending request") } err = autorest.Respond( @@ -115,10 +117,10 @@ func (c *AppClient) DeleteApplication(ctx context.Context, applicationObjectID s autorest.ByClosing()) if err != nil { - return autorest.NewErrorWithError(err, "provider", "DeleteApplication", resp, "Failure responding to request") + return result, autorest.NewErrorWithError(err, "provider", "DeleteApplication", resp, "Failure responding to request") } - return nil + return result, nil } func (c *AppClient) AddApplicationPassword(ctx context.Context, applicationObjectID string, displayName string, endDateTime date.Time) (PasswordCredentialResult, error) { @@ -148,20 +150,21 @@ func (c *AppClient) AddApplicationPassword(ctx context.Context, applicationObjec func (c *AppClient) RemoveApplicationPassword(ctx context.Context, applicationObjectID string, keyID string) error { req, err := c.removePasswordPreparer(ctx, applicationObjectID, keyID) if err != nil { - return autorest.NewErrorWithError(err, "provider", "RemoveApplicationPassword", nil, "Failure preparing request") + return result, autorest.NewErrorWithError(err, "provider", "RemoveApplicationPassword", nil, "Failure preparing request") } resp, err := c.removePasswordSender(req) if err != nil { - return autorest.NewErrorWithError(err, "provider", "RemoveApplicationPassword", resp, "Failure sending request") + result = autorest.Response{Response: resp} + return result, autorest.NewErrorWithError(err, "provider", "RemoveApplicationPassword", resp, "Failure sending request") } _, err = c.removePasswordResponder(resp) if err != nil { - return autorest.NewErrorWithError(err, "provider", "RemoveApplicationPassword", resp, "Failure responding to request") + return result, autorest.NewErrorWithError(err, "provider", "RemoveApplicationPassword", resp, "Failure responding to request") } - return nil + return result, nil } func (c AppClient) getApplicationPreparer(ctx context.Context, applicationObjectID string) (*http.Request, error) { @@ -232,7 +235,9 @@ func (c AppClient) addPasswordResponder(resp *http.Response) (PasswordCredential azure.WithErrorUnlessStatusCode(http.StatusOK), autorest.ByUnmarshallingJSON(&result), autorest.ByClosing()) - result.Response = autorest.Response{Response: resp} + result = PasswordCredentialResult{ + Response: autorest.Response{Response: resp}, + } return result, err } @@ -270,7 +275,9 @@ func (c AppClient) removePasswordResponder(resp *http.Response) (autorest.Respon azure.WithErrorUnlessStatusCode(http.StatusNoContent), autorest.ByUnmarshallingJSON(&result), autorest.ByClosing()) - result.Response = resp + result = autorest.Response{ + Response: resp, + } return result, err } @@ -304,8 +311,10 @@ func (c AppClient) createApplicationResponder(resp *http.Response) (ApplicationR azure.WithErrorUnlessStatusCode(http.StatusCreated), autorest.ByUnmarshallingJSON(&result), autorest.ByClosing()) - result.Response = autorest.Response{Response: resp} - return result, err + result = ApplicationResult{ + Response: autorest.Response{Response: resp}, + } + return result, nil } func (c AppClient) deleteApplicationPreparer(ctx context.Context, applicationObjectID string) (*http.Request, error) { @@ -327,6 +336,19 @@ func (c AppClient) deleteApplicationSender(req *http.Request) (*http.Response, e return autorest.SendWithSender(c.client, req, sd...) } +func (c AppClient) deleteApplicationResponder(resp *http.Response) (result autorest.Response, err error) { + err = autorest.Respond( + resp, + c.client.ByInspecting(), + azure.WithErrorUnlessStatusCode(http.StatusNoContent), + autorest.ByUnmarshallingJSON(&result), + autorest.ByClosing()) + result = autorest.Response{ + Response: resp, + } + return result, err +} + func (c AppClient) AddGroupMember(ctx context.Context, groupObjectID string, memberObjectID string) error { if groupObjectID == "" { return fmt.Errorf("missing groupObjectID") From d6bb52ec4b66a03e8fdc5a154a1e076e3b454101 Mon Sep 17 00:00:00 2001 From: Michael Golowka <72365+pcman312@users.noreply.github.com> Date: Mon, 27 Sep 2021 16:29:05 -0600 Subject: [PATCH 02/39] Readability cleanup Removed bare returns Removed unused return values Small readability improvements --- api/application_aad.go | 8 ++++---- api/application_msgraph.go | 39 +++++++++----------------------------- 2 files changed, 13 insertions(+), 34 deletions(-) diff --git a/api/application_aad.go b/api/application_aad.go index e8a5a9d..d4c62b2 100644 --- a/api/application_aad.go +++ b/api/application_aad.go @@ -106,7 +106,7 @@ func (a *ActiveDirectoryApplicationClient) AddApplicationPassword(ctx context.Co return PasswordCredentialResult{}, fmt.Errorf("error updating credentials: %w", err) } - result := PasswordCredentialResult{ + result = PasswordCredentialResult{ PasswordCredential: PasswordCredential{ DisplayName: to.StringPtr(displayName), StartDate: &now, @@ -118,11 +118,11 @@ func (a *ActiveDirectoryApplicationClient) AddApplicationPassword(ctx context.Co return result, nil } -func (a *ActiveDirectoryApplicationClient) RemoveApplicationPassword(ctx context.Context, applicationObjectID string, keyID string) error { +func (a *ActiveDirectoryApplicationClient) RemoveApplicationPassword(ctx context.Context, applicationObjectID string, keyID string) (err error) { // Load current credentials resp, err := a.Client.ListPasswordCredentials(ctx, applicationObjectID) if err != nil { - return fmt.Errorf("error fetching credentials: %w", err) + return errwrap.Wrapf("error fetching credentials: {{err}}", err) } curCreds := *resp.Value @@ -149,7 +149,7 @@ func (a *ActiveDirectoryApplicationClient) RemoveApplicationPassword(ctx context }, ) if err != nil { - return fmt.Errorf("error updating credentials: %w", err) + return errwrap.Wrapf("error updating credentials: {{err}}", err) } return nil diff --git a/api/application_msgraph.go b/api/application_msgraph.go index 610680d..8472ddb 100644 --- a/api/application_msgraph.go +++ b/api/application_msgraph.go @@ -98,29 +98,22 @@ func (c *AppClient) CreateApplication(ctx context.Context, displayName string) ( // DeleteApplication deletes an Azure application object. // This will in turn remove the service principal (but not the role assignments). -func (c *AppClient) DeleteApplication(ctx context.Context, applicationObjectID string) error { +func (c *AppClient) DeleteApplication(ctx context.Context, applicationObjectID string) (err error) { req, err := c.deleteApplicationPreparer(ctx, applicationObjectID) if err != nil { - return result, autorest.NewErrorWithError(err, "provider", "DeleteApplication", nil, "Failure preparing request") + return autorest.NewErrorWithError(err, "provider", "DeleteApplication", nil, "Failure preparing request") } resp, err := c.deleteApplicationSender(req) if err != nil { - result = autorest.Response{Response: resp} - return result, autorest.NewErrorWithError(err, "provider", "DeleteApplication", resp, "Failure sending request") + return autorest.NewErrorWithError(err, "provider", "DeleteApplication", resp, "Failure sending request") } - err = autorest.Respond( + return autorest.Respond( resp, c.client.ByInspecting(), azure.WithErrorUnlessStatusCode(http.StatusNoContent, http.StatusNotFound), autorest.ByClosing()) - - if err != nil { - return result, autorest.NewErrorWithError(err, "provider", "DeleteApplication", resp, "Failure responding to request") - } - - return result, nil } func (c *AppClient) AddApplicationPassword(ctx context.Context, applicationObjectID string, displayName string, endDateTime date.Time) (PasswordCredentialResult, error) { @@ -147,24 +140,23 @@ func (c *AppClient) AddApplicationPassword(ctx context.Context, applicationObjec return result, nil } -func (c *AppClient) RemoveApplicationPassword(ctx context.Context, applicationObjectID string, keyID string) error { +func (c *AppClient) RemoveApplicationPassword(ctx context.Context, applicationObjectID string, keyID string) (err error) { req, err := c.removePasswordPreparer(ctx, applicationObjectID, keyID) if err != nil { - return result, autorest.NewErrorWithError(err, "provider", "RemoveApplicationPassword", nil, "Failure preparing request") + return autorest.NewErrorWithError(err, "provider", "RemoveApplicationPassword", nil, "Failure preparing request") } resp, err := c.removePasswordSender(req) if err != nil { - result = autorest.Response{Response: resp} - return result, autorest.NewErrorWithError(err, "provider", "RemoveApplicationPassword", resp, "Failure sending request") + return autorest.NewErrorWithError(err, "provider", "RemoveApplicationPassword", resp, "Failure sending request") } _, err = c.removePasswordResponder(resp) if err != nil { - return result, autorest.NewErrorWithError(err, "provider", "RemoveApplicationPassword", resp, "Failure responding to request") + return autorest.NewErrorWithError(err, "provider", "RemoveApplicationPassword", resp, "Failure responding to request") } - return result, nil + return nil } func (c AppClient) getApplicationPreparer(ctx context.Context, applicationObjectID string) (*http.Request, error) { @@ -336,19 +328,6 @@ func (c AppClient) deleteApplicationSender(req *http.Request) (*http.Response, e return autorest.SendWithSender(c.client, req, sd...) } -func (c AppClient) deleteApplicationResponder(resp *http.Response) (result autorest.Response, err error) { - err = autorest.Respond( - resp, - c.client.ByInspecting(), - azure.WithErrorUnlessStatusCode(http.StatusNoContent), - autorest.ByUnmarshallingJSON(&result), - autorest.ByClosing()) - result = autorest.Response{ - Response: resp, - } - return result, err -} - func (c AppClient) AddGroupMember(ctx context.Context, groupObjectID string, memberObjectID string) error { if groupObjectID == "" { return fmt.Errorf("missing groupObjectID") From 6d07ce0f6547318bf0755312ee4cce0d8f8cf6ab Mon Sep 17 00:00:00 2001 From: Michael Golowka <72365+pcman312@users.noreply.github.com> Date: Mon, 27 Sep 2021 16:46:51 -0600 Subject: [PATCH 03/39] Remove errwrap --- api/application_aad.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/application_aad.go b/api/application_aad.go index d4c62b2..009b0ac 100644 --- a/api/application_aad.go +++ b/api/application_aad.go @@ -122,7 +122,7 @@ func (a *ActiveDirectoryApplicationClient) RemoveApplicationPassword(ctx context // Load current credentials resp, err := a.Client.ListPasswordCredentials(ctx, applicationObjectID) if err != nil { - return errwrap.Wrapf("error fetching credentials: {{err}}", err) + return fmt.Errorf("error fetching credentials: %w", err) } curCreds := *resp.Value @@ -149,7 +149,7 @@ func (a *ActiveDirectoryApplicationClient) RemoveApplicationPassword(ctx context }, ) if err != nil { - return errwrap.Wrapf("error updating credentials: {{err}}", err) + return fmt.Errorf("error updating credentials: %w", err) } return nil From b6addec63868fa3926b688be6bcb88f31476d370 Mon Sep 17 00:00:00 2001 From: Michael Golowka <72365+pcman312@users.noreply.github.com> Date: Mon, 27 Sep 2021 17:01:25 -0600 Subject: [PATCH 04/39] Make tests happy again --- api/application_msgraph.go | 12 +++--------- provider_mock_test.go | 3 ++- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/api/application_msgraph.go b/api/application_msgraph.go index 8472ddb..46276c7 100644 --- a/api/application_msgraph.go +++ b/api/application_msgraph.go @@ -227,9 +227,7 @@ func (c AppClient) addPasswordResponder(resp *http.Response) (PasswordCredential azure.WithErrorUnlessStatusCode(http.StatusOK), autorest.ByUnmarshallingJSON(&result), autorest.ByClosing()) - result = PasswordCredentialResult{ - Response: autorest.Response{Response: resp}, - } + result.Response = autorest.Response{Response: resp} return result, err } @@ -267,9 +265,7 @@ func (c AppClient) removePasswordResponder(resp *http.Response) (autorest.Respon azure.WithErrorUnlessStatusCode(http.StatusNoContent), autorest.ByUnmarshallingJSON(&result), autorest.ByClosing()) - result = autorest.Response{ - Response: resp, - } + result.Response = resp return result, err } @@ -303,9 +299,7 @@ func (c AppClient) createApplicationResponder(resp *http.Response) (ApplicationR azure.WithErrorUnlessStatusCode(http.StatusCreated), autorest.ByUnmarshallingJSON(&result), autorest.ByClosing()) - result = ApplicationResult{ - Response: autorest.Response{Response: resp}, - } + result.Response = autorest.Response{Response: resp} return result, nil } diff --git a/provider_mock_test.go b/provider_mock_test.go index 4b3ba1d..2e40b4a 100644 --- a/provider_mock_test.go +++ b/provider_mock_test.go @@ -10,6 +10,7 @@ import ( "time" "github.com/Azure/azure-sdk-for-go/profiles/latest/authorization/mgmt/authorization" + "github.com/Azure/azure-sdk-for-go/profiles/latest/compute/mgmt/compute" "github.com/Azure/go-autorest/autorest/date" "github.com/Azure/go-autorest/autorest/to" "github.com/hashicorp/vault-plugin-secrets-azure/api" @@ -137,7 +138,7 @@ func (m *mockProvider) AddApplicationPassword(_ context.Context, _ string, displ }, nil } -func (m *mockProvider) RemoveApplicationPassword(_ context.Context, _ string, keyID string) error { +func (m *mockProvider) RemoveApplicationPassword(_ context.Context, _ string, keyID string) (err error) { m.lock.Lock() defer m.lock.Unlock() From ebb03251adabdf1bed51199dbe6bc4d69a8a2ff1 Mon Sep 17 00:00:00 2001 From: Michael Golowka <72365+pcman312@users.noreply.github.com> Date: Mon, 4 Oct 2021 16:29:50 -0600 Subject: [PATCH 05/39] Add rotate-root endpoint --- api/api.go | 18 ++- api/application_aad.go | 41 +++++- api/application_msgraph.go | 30 ++++- backend.go | 1 + backend_test.go | 9 +- client.go | 4 +- go.mod | 1 + go.sum | 1 + mock_provider_test.go | 260 +++++++++++++++++++++++++++++++++++++ path_rotate_root.go | 248 +++++++++++++++++++++++++++++++++++ path_rotate_root_test.go | 238 +++++++++++++++++++++++++++++++++ provider.go | 17 ++- provider_mock_test.go | 8 +- wal.go | 10 +- 14 files changed, 854 insertions(+), 32 deletions(-) create mode 100644 mock_provider_test.go create mode 100644 path_rotate_root.go create mode 100644 path_rotate_root_test.go diff --git a/api/api.go b/api/api.go index 6af79dd..ed292e5 100644 --- a/api/api.go +++ b/api/api.go @@ -2,6 +2,7 @@ package api import ( "context" + "time" "github.com/Azure/azure-sdk-for-go/profiles/latest/authorization/mgmt/authorization" "github.com/Azure/go-autorest/autorest" @@ -31,20 +32,17 @@ type ApplicationsClient interface { GetApplication(ctx context.Context, applicationObjectID string) (ApplicationResult, error) CreateApplication(ctx context.Context, displayName string) (ApplicationResult, error) DeleteApplication(ctx context.Context, applicationObjectID string) error - AddApplicationPassword(ctx context.Context, applicationObjectID string, displayName string, endDateTime date.Time) (PasswordCredentialResult, error) + ListApplications(ctx context.Context, filter string) ([]ApplicationResult, error) + AddApplicationPassword(ctx context.Context, applicationObjectID string, displayName string, endDateTime time.Time) (PasswordCredentialResult, error) RemoveApplicationPassword(ctx context.Context, applicationObjectID string, keyID string) error } type PasswordCredential struct { - DisplayName *string `json:"displayName"` - // StartDate - Start date. - StartDate *date.Time `json:"startDateTime,omitempty"` - // EndDate - End date. - EndDate *date.Time `json:"endDateTime,omitempty"` - // KeyID - Key ID. - KeyID *string `json:"keyId,omitempty"` - // Value - Key value. - SecretText *string `json:"secretText,omitempty"` + DisplayName *string `json:"displayName"` + StartDate *date.Time `json:"startDateTime,omitempty"` + EndDate *date.Time `json:"endDateTime,omitempty"` + KeyID *string `json:"keyId,omitempty"` + SecretText *string `json:"secretText,omitempty"` } type PasswordCredentialResult struct { diff --git a/api/application_aad.go b/api/application_aad.go index 009b0ac..c1403bd 100644 --- a/api/application_aad.go +++ b/api/application_aad.go @@ -31,7 +31,40 @@ func (a *ActiveDirectoryApplicationClient) GetApplication(ctx context.Context, a }, nil } -func (a *ActiveDirectoryApplicationClient) CreateApplication(ctx context.Context, displayName string) (ApplicationResult, error) { +func (a *ActiveDirectoryApplicationClient) ListApplications(ctx context.Context, filter string) ([]ApplicationResult, error) { + resp, err := a.Client.List(ctx, filter) + if err != nil { + return nil, err + } + + results := []ApplicationResult{} + for resp.NotDone() { + for _, app := range resp.Values() { + passCreds := []*PasswordCredential{} + for _, rawPC := range *app.PasswordCredentials { + pc := &PasswordCredential{ + StartDate: rawPC.StartDate, + EndDate: rawPC.EndDate, + KeyID: rawPC.KeyID, + } + passCreds = append(passCreds, pc) + } + appResult := ApplicationResult{ + AppID: app.AppID, + ID: app.ObjectID, + PasswordCredentials: passCreds, + } + results = append(results, appResult) + } + err = resp.NextWithContext(ctx) + if err != nil { + return results, fmt.Errorf("failed to get all results: %w", err) + } + } + return results, nil +} + +func (a *ActiveDirectoryApplicationClient) CreateApplication(ctx context.Context, displayName string) (result ApplicationResult, err error) { appURL := fmt.Sprintf("https://%s", displayName) app, err := a.Client.Create(ctx, graphrbac.ApplicationCreateParameters{ @@ -62,7 +95,7 @@ func (a *ActiveDirectoryApplicationClient) DeleteApplication(ctx context.Context return nil } -func (a *ActiveDirectoryApplicationClient) AddApplicationPassword(ctx context.Context, applicationObjectID string, displayName string, endDateTime date.Time) (PasswordCredentialResult, error) { +func (a *ActiveDirectoryApplicationClient) AddApplicationPassword(ctx context.Context, applicationObjectID string, displayName string, endDateTime time.Time) (result PasswordCredentialResult, err error) { keyID, err := uuid.GenerateUUID() if err != nil { return PasswordCredentialResult{}, err @@ -80,7 +113,7 @@ func (a *ActiveDirectoryApplicationClient) AddApplicationPassword(ctx context.Co now := date.Time{Time: time.Now().UTC()} cred := graphrbac.PasswordCredential{ StartDate: &now, - EndDate: &endDateTime, + EndDate: &date.Time{endDateTime}, KeyID: to.StringPtr(keyID), Value: to.StringPtr(password), } @@ -110,7 +143,7 @@ func (a *ActiveDirectoryApplicationClient) AddApplicationPassword(ctx context.Co PasswordCredential: PasswordCredential{ DisplayName: to.StringPtr(displayName), StartDate: &now, - EndDate: &endDateTime, + EndDate: &date.Time{endDateTime}, KeyID: to.StringPtr(keyID), SecretText: to.StringPtr(password), }, diff --git a/api/application_msgraph.go b/api/application_msgraph.go index 46276c7..316caaa 100644 --- a/api/application_msgraph.go +++ b/api/application_msgraph.go @@ -72,6 +72,30 @@ func (c *AppClient) GetApplication(ctx context.Context, applicationObjectID stri return result, nil } +type listApplicationsResponse struct { + Value []ApplicationResult +} + +func (c *AppClient) ListApplications(ctx context.Context, filter string) ([]ApplicationResult, error) { + filterArgs := url.Values{} + if filter != "" { + filterArgs.Set("$filter", filter) + } + preparer := c.GetPreparer( + autorest.AsGet(), + autorest.WithPath(fmt.Sprintf("/v1.0/applications?%s", filterArgs.Encode())), + ) + listAppResp := listApplicationsResponse{} + err := c.SendRequest(ctx, preparer, + azure.WithErrorUnlessStatusCode(http.StatusOK), + autorest.ByUnmarshallingJSON(&listAppResp), + ) + if err != nil { + return nil, err + } + return listAppResp.Value, nil +} + // CreateApplication create a new Azure application object. func (c *AppClient) CreateApplication(ctx context.Context, displayName string) (ApplicationResult, error) { var result ApplicationResult @@ -116,10 +140,8 @@ func (c *AppClient) DeleteApplication(ctx context.Context, applicationObjectID s autorest.ByClosing()) } -func (c *AppClient) AddApplicationPassword(ctx context.Context, applicationObjectID string, displayName string, endDateTime date.Time) (PasswordCredentialResult, error) { - var result PasswordCredentialResult - - req, err := c.addPasswordPreparer(ctx, applicationObjectID, displayName, endDateTime) +func (c *AppClient) AddApplicationPassword(ctx context.Context, applicationObjectID string, displayName string, endDateTime time.Time) (result PasswordCredentialResult, err error) { + req, err := c.addPasswordPreparer(ctx, applicationObjectID, displayName, date.Time{endDateTime}) if err != nil { return PasswordCredentialResult{}, autorest.NewErrorWithError(err, "provider", "AddApplicationPassword", nil, "Failure preparing request") } diff --git a/backend.go b/backend.go index 2815d03..f886475 100644 --- a/backend.go +++ b/backend.go @@ -48,6 +48,7 @@ func backend() *azureSecretBackend { []*framework.Path{ pathConfig(&b), pathServicePrincipal(&b), + pathRotateRoot(&b), }, ), Secrets: []*framework.Secret{ diff --git a/backend_test.go b/backend_test.go index c22cef3..96e43a2 100644 --- a/backend_test.go +++ b/backend_test.go @@ -18,6 +18,11 @@ const ( defaultTestMaxTTL = 3600 ) +var ( + testClientID = "testClientId" + testClientSecret = "testClientSecret" +) + func getTestBackend(t *testing.T, initConfig bool) (*azureSecretBackend, logical.Storage) { b := backend() @@ -44,8 +49,8 @@ func getTestBackend(t *testing.T, initConfig bool) (*azureSecretBackend, logical cfg := map[string]interface{}{ "subscription_id": generateUUID(), "tenant_id": generateUUID(), - "client_id": "testClientId", - "client_secret": "testClientSecret", + "client_id": testClientID, + "client_secret": testClientSecret, "environment": "AZURECHINACLOUD", "ttl": defaultTestTTL, "max_ttl": defaultTestMaxTTL, diff --git a/client.go b/client.go index 239227d..37ce4b7 100644 --- a/client.go +++ b/client.go @@ -11,7 +11,6 @@ import ( "github.com/Azure/azure-sdk-for-go/profiles/latest/authorization/mgmt/authorization" "github.com/Azure/go-autorest/autorest/azure" - "github.com/Azure/go-autorest/autorest/date" "github.com/Azure/go-autorest/autorest/to" multierror "github.com/hashicorp/go-multierror" uuid "github.com/hashicorp/go-uuid" @@ -44,6 +43,7 @@ func (c *client) Valid() bool { // An Application is a needed to create service principals used by // the caller for authentication. func (c *client) createApp(ctx context.Context) (app *api.ApplicationResult, err error) { + // TODO: Make this name customizable with the same logic as username customization name, err := uuid.GenerateUUID() if err != nil { return nil, err @@ -95,7 +95,7 @@ func (c *client) createSP( // addAppPassword adds a new password to an App's credentials list. func (c *client) addAppPassword(ctx context.Context, appObjID string, expiresIn time.Duration) (string, string, error) { - exp := date.Time{Time: time.Now().Add(expiresIn)} + exp := time.Now().Add(expiresIn) resp, err := c.provider.AddApplicationPassword(ctx, appObjID, "vault-plugin-secrets-azure", exp) if err != nil { if strings.Contains(err.Error(), "size of the object has exceeded its limit") { diff --git a/go.mod b/go.mod index de1e9d0..6bb9444 100644 --- a/go.mod +++ b/go.mod @@ -12,6 +12,7 @@ require ( github.com/fatih/color v1.9.0 // indirect github.com/frankban/quicktest v1.10.0 // indirect github.com/go-test/deep v1.0.2-0.20181118220953-042da051cf31 + github.com/golang/mock v1.1.1 github.com/hashicorp/go-hclog v0.14.1 github.com/hashicorp/go-multierror v1.1.0 github.com/hashicorp/go-retryablehttp v0.6.6 // indirect diff --git a/go.sum b/go.sum index 6f91d51..1cc0814 100644 --- a/go.sum +++ b/go.sum @@ -62,6 +62,7 @@ github.com/go-test/deep v1.0.2-0.20181118220953-042da051cf31 h1:28FVBuwkwowZMjbA github.com/go-test/deep v1.0.2-0.20181118220953-042da051cf31/go.mod h1:wGDj63lr65AM2AQyKZd/NYHGb0R+1RLqB8NKt3aSFNA= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b h1:VKtxabqXZkF25pY9ekfRL6a582T4P37/31XEstQ5p58= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= +github.com/golang/mock v1.1.1 h1:G5FRp8JnTd7RQH5kemVNlMeyXQAztQ3mOWV95KxsXH8= github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= diff --git a/mock_provider_test.go b/mock_provider_test.go new file mode 100644 index 0000000..3f8ca73 --- /dev/null +++ b/mock_provider_test.go @@ -0,0 +1,260 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/hashicorp/vault-plugin-secrets-azure/api (interfaces: AzureProvider) + +// Package azuresecrets is a generated GoMock package. +package azuresecrets + +import ( + context "context" + reflect "reflect" + time "time" + + authorization "github.com/Azure/azure-sdk-for-go/services/authorization/mgmt/2015-07-01/authorization" + gomock "github.com/golang/mock/gomock" + api "github.com/hashicorp/vault-plugin-secrets-azure/api" +) + +// MockAzureProvider is a mock of AzureProvider interface. +type MockAzureProvider struct { + ctrl *gomock.Controller + recorder *MockAzureProviderMockRecorder +} + +// MockAzureProviderMockRecorder is the mock recorder for MockAzureProvider. +type MockAzureProviderMockRecorder struct { + mock *MockAzureProvider +} + +// NewMockAzureProvider creates a new mock instance. +func NewMockAzureProvider(ctrl *gomock.Controller) *MockAzureProvider { + mock := &MockAzureProvider{ctrl: ctrl} + mock.recorder = &MockAzureProviderMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockAzureProvider) EXPECT() *MockAzureProviderMockRecorder { + return m.recorder +} + +// AddApplicationPassword mocks base method. +func (m *MockAzureProvider) AddApplicationPassword(arg0 context.Context, arg1, arg2 string, arg3 time.Time) (api.PasswordCredentialResult, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AddApplicationPassword", arg0, arg1, arg2, arg3) + ret0, _ := ret[0].(api.PasswordCredentialResult) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// AddApplicationPassword indicates an expected call of AddApplicationPassword. +func (mr *MockAzureProviderMockRecorder) AddApplicationPassword(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddApplicationPassword", reflect.TypeOf((*MockAzureProvider)(nil).AddApplicationPassword), arg0, arg1, arg2, arg3) +} + +// AddGroupMember mocks base method. +func (m *MockAzureProvider) AddGroupMember(arg0 context.Context, arg1, arg2 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AddGroupMember", arg0, arg1, arg2) + ret0, _ := ret[0].(error) + return ret0 +} + +// AddGroupMember indicates an expected call of AddGroupMember. +func (mr *MockAzureProviderMockRecorder) AddGroupMember(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddGroupMember", reflect.TypeOf((*MockAzureProvider)(nil).AddGroupMember), arg0, arg1, arg2) +} + +// CreateApplication mocks base method. +func (m *MockAzureProvider) CreateApplication(arg0 context.Context, arg1 string) (api.ApplicationResult, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CreateApplication", arg0, arg1) + ret0, _ := ret[0].(api.ApplicationResult) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CreateApplication indicates an expected call of CreateApplication. +func (mr *MockAzureProviderMockRecorder) CreateApplication(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateApplication", reflect.TypeOf((*MockAzureProvider)(nil).CreateApplication), arg0, arg1) +} + +// CreateRoleAssignment mocks base method. +func (m *MockAzureProvider) CreateRoleAssignment(arg0 context.Context, arg1, arg2 string, arg3 authorization.RoleAssignmentCreateParameters) (authorization.RoleAssignment, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CreateRoleAssignment", arg0, arg1, arg2, arg3) + ret0, _ := ret[0].(authorization.RoleAssignment) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CreateRoleAssignment indicates an expected call of CreateRoleAssignment. +func (mr *MockAzureProviderMockRecorder) CreateRoleAssignment(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateRoleAssignment", reflect.TypeOf((*MockAzureProvider)(nil).CreateRoleAssignment), arg0, arg1, arg2, arg3) +} + +// CreateServicePrincipal mocks base method. +func (m *MockAzureProvider) CreateServicePrincipal(arg0 context.Context, arg1 string, arg2, arg3 time.Time) (string, string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CreateServicePrincipal", arg0, arg1, arg2, arg3) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(string) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// CreateServicePrincipal indicates an expected call of CreateServicePrincipal. +func (mr *MockAzureProviderMockRecorder) CreateServicePrincipal(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateServicePrincipal", reflect.TypeOf((*MockAzureProvider)(nil).CreateServicePrincipal), arg0, arg1, arg2, arg3) +} + +// DeleteApplication mocks base method. +func (m *MockAzureProvider) DeleteApplication(arg0 context.Context, arg1 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteApplication", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeleteApplication indicates an expected call of DeleteApplication. +func (mr *MockAzureProviderMockRecorder) DeleteApplication(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteApplication", reflect.TypeOf((*MockAzureProvider)(nil).DeleteApplication), arg0, arg1) +} + +// DeleteRoleAssignmentByID mocks base method. +func (m *MockAzureProvider) DeleteRoleAssignmentByID(arg0 context.Context, arg1 string) (authorization.RoleAssignment, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteRoleAssignmentByID", arg0, arg1) + ret0, _ := ret[0].(authorization.RoleAssignment) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// DeleteRoleAssignmentByID indicates an expected call of DeleteRoleAssignmentByID. +func (mr *MockAzureProviderMockRecorder) DeleteRoleAssignmentByID(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteRoleAssignmentByID", reflect.TypeOf((*MockAzureProvider)(nil).DeleteRoleAssignmentByID), arg0, arg1) +} + +// GetApplication mocks base method. +func (m *MockAzureProvider) GetApplication(arg0 context.Context, arg1 string) (api.ApplicationResult, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetApplication", arg0, arg1) + ret0, _ := ret[0].(api.ApplicationResult) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetApplication indicates an expected call of GetApplication. +func (mr *MockAzureProviderMockRecorder) GetApplication(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetApplication", reflect.TypeOf((*MockAzureProvider)(nil).GetApplication), arg0, arg1) +} + +// GetGroup mocks base method. +func (m *MockAzureProvider) GetGroup(arg0 context.Context, arg1 string) (api.Group, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetGroup", arg0, arg1) + ret0, _ := ret[0].(api.Group) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetGroup indicates an expected call of GetGroup. +func (mr *MockAzureProviderMockRecorder) GetGroup(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetGroup", reflect.TypeOf((*MockAzureProvider)(nil).GetGroup), arg0, arg1) +} + +// GetRoleDefinitionByID mocks base method. +func (m *MockAzureProvider) GetRoleDefinitionByID(arg0 context.Context, arg1 string) (authorization.RoleDefinition, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetRoleDefinitionByID", arg0, arg1) + ret0, _ := ret[0].(authorization.RoleDefinition) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetRoleDefinitionByID indicates an expected call of GetRoleDefinitionByID. +func (mr *MockAzureProviderMockRecorder) GetRoleDefinitionByID(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRoleDefinitionByID", reflect.TypeOf((*MockAzureProvider)(nil).GetRoleDefinitionByID), arg0, arg1) +} + +// ListApplications mocks base method. +func (m *MockAzureProvider) ListApplications(arg0 context.Context, arg1 string) ([]api.ApplicationResult, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListApplications", arg0, arg1) + ret0, _ := ret[0].([]api.ApplicationResult) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListApplications indicates an expected call of ListApplications. +func (mr *MockAzureProviderMockRecorder) ListApplications(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListApplications", reflect.TypeOf((*MockAzureProvider)(nil).ListApplications), arg0, arg1) +} + +// ListGroups mocks base method. +func (m *MockAzureProvider) ListGroups(arg0 context.Context, arg1 string) ([]api.Group, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListGroups", arg0, arg1) + ret0, _ := ret[0].([]api.Group) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListGroups indicates an expected call of ListGroups. +func (mr *MockAzureProviderMockRecorder) ListGroups(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListGroups", reflect.TypeOf((*MockAzureProvider)(nil).ListGroups), arg0, arg1) +} + +// ListRoleDefinitions mocks base method. +func (m *MockAzureProvider) ListRoleDefinitions(arg0 context.Context, arg1, arg2 string) ([]authorization.RoleDefinition, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListRoleDefinitions", arg0, arg1, arg2) + ret0, _ := ret[0].([]authorization.RoleDefinition) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListRoleDefinitions indicates an expected call of ListRoleDefinitions. +func (mr *MockAzureProviderMockRecorder) ListRoleDefinitions(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListRoleDefinitions", reflect.TypeOf((*MockAzureProvider)(nil).ListRoleDefinitions), arg0, arg1, arg2) +} + +// RemoveApplicationPassword mocks base method. +func (m *MockAzureProvider) RemoveApplicationPassword(arg0 context.Context, arg1, arg2 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RemoveApplicationPassword", arg0, arg1, arg2) + ret0, _ := ret[0].(error) + return ret0 +} + +// RemoveApplicationPassword indicates an expected call of RemoveApplicationPassword. +func (mr *MockAzureProviderMockRecorder) RemoveApplicationPassword(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveApplicationPassword", reflect.TypeOf((*MockAzureProvider)(nil).RemoveApplicationPassword), arg0, arg1, arg2) +} + +// RemoveGroupMember mocks base method. +func (m *MockAzureProvider) RemoveGroupMember(arg0 context.Context, arg1, arg2 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RemoveGroupMember", arg0, arg1, arg2) + ret0, _ := ret[0].(error) + return ret0 +} + +// RemoveGroupMember indicates an expected call of RemoveGroupMember. +func (mr *MockAzureProviderMockRecorder) RemoveGroupMember(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveGroupMember", reflect.TypeOf((*MockAzureProvider)(nil).RemoveGroupMember), arg0, arg1, arg2) +} diff --git a/path_rotate_root.go b/path_rotate_root.go new file mode 100644 index 0000000..4f89efa --- /dev/null +++ b/path_rotate_root.go @@ -0,0 +1,248 @@ +package azuresecrets + +import ( + "context" + "fmt" + "time" + + "github.com/hashicorp/vault-plugin-secrets-azure/api" + + "github.com/mitchellh/mapstructure" + + "github.com/hashicorp/go-multierror" + + "github.com/hashicorp/go-uuid" + + "github.com/hashicorp/vault/sdk/framework" + "github.com/hashicorp/vault/sdk/logical" +) + +func pathRotateRoot(b *azureSecretBackend) *framework.Path { + return &framework.Path{ + Pattern: "rotate-root", + Fields: map[string]*framework.FieldSchema{ + // None + }, + Operations: map[logical.Operation]framework.OperationHandler{ + logical.UpdateOperation: &framework.PathOperation{ + Callback: b.pathRotateRoot, + ForwardPerformanceSecondary: true, + ForwardPerformanceStandby: true, + }, + }, + + HelpSynopsis: "Attempt to rotate the root credentials used to communicate with Azure.", + HelpDescription: "This path will attempt to generate new root credentials for the user used to access and manipulate Azure.\n" + + "The new credentials will not be returned from this endpoint, nor the read config endpoint.", + } +} + +func (b *azureSecretBackend) pathRotateRoot(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { + expiration := time.Now().AddDate(0, 6, 0) + passCred, warnings, err := b.rotateRootCredentials(ctx, req.Storage, expiration) + if err != nil { + return nil, err + } + + resultData := map[string]interface{}{ + "client_id": *passCred.KeyID, + "end_date": *passCred.EndDate, + } + if passCred.DisplayName != nil && *passCred.DisplayName != "" { + resultData["display_name"] = *passCred.DisplayName + } + + resp := &logical.Response{ + Data: resultData, + Warnings: warnings, + } + + return resp, nil +} + +func (b *azureSecretBackend) rotateRootCredentials(ctx context.Context, storage logical.Storage, expiration time.Time) (cred api.PasswordCredential, warnings []string, err error) { + cfg, err := b.getConfig(ctx, storage) + if err != nil { + return api.PasswordCredential{}, nil, err + } + + client, err := b.getClient(ctx, storage) + if err != nil { + return api.PasswordCredential{}, nil, err + } + + // We need to use List instead of Get here because we don't have the Object ID + // (which is different from the Application/Client ID) + apps, err := client.provider.ListApplications(ctx, fmt.Sprintf("appId eq '%s'", cfg.ClientID)) + if err != nil { + return api.PasswordCredential{}, nil, err + } + + if len(apps) == 0 { + return api.PasswordCredential{}, nil, fmt.Errorf("no application found") + } + if len(apps) > 1 { + return api.PasswordCredential{}, nil, fmt.Errorf("multiple applications found - double check your client_id") + } + + app := apps[0] + + // Get a list of the current passwords to delete if adding a new credential succeeds. This list will not + // include the ID of the new password created via AddApplicationPassword + credsToDelete := []string{} + for _, cred := range app.PasswordCredentials { + credsToDelete = append(credsToDelete, *cred.KeyID) + } + + uniqueID, err := uuid.GenerateUUID() + if err != nil { + return api.PasswordCredential{}, nil, fmt.Errorf("failed to generate UUID: %w", err) + } + + // This could have the same username customization logic put on it if we really wanted it here + passwordDisplayName := fmt.Sprintf("vault-plugin-secrets-azure-%s", uniqueID) + + newPasswordResp, err := client.provider.AddApplicationPassword(ctx, *app.ID, passwordDisplayName, expiration) + if err != nil { + return api.PasswordCredential{}, nil, fmt.Errorf("failed to add new password: %w", err) + } + + cfg.ClientSecret = *newPasswordResp.PasswordCredential.SecretText + + err = b.saveConfig(ctx, cfg, storage) + if err != nil { + // Remove the key since we failed to save it to Vault storage. It's reasonable to assume that this call + // to Azure will succeed since the AddApplicationPassword call succeeded above. If it doesn't, we aren't going + // to do any retries via a WAL here since the current configuration will continue to work. + azureErr := client.provider.RemoveApplicationPassword(ctx, *app.ID, *newPasswordResp.PasswordCredential.KeyID) + merr := multierror.Append(err, azureErr) + return api.PasswordCredential{}, nil, merr + } + + err = removeApplicationPasswords(ctx, storage, client.provider, *app.ID, credsToDelete...) + if err != nil { + warnings = append(warnings, fmt.Sprintf("failed to clean up all other credentials for root user. Will attempt again later.\n%s", err.Error())) + } + + return newPasswordResp.PasswordCredential, warnings, nil +} + +const walRemoveCreds = "removeCreds" +const walRemoveCredsExpiration = 24 * time.Hour + +type removeCredsWAL struct { + AppID string + KeyIDsToRemove []string + Expiration time.Time +} + +type passwordRemover interface { + RemoveApplicationPassword(ctx context.Context, applicationObjectID string, keyID string) error +} + +func removeApplicationPasswords(ctx context.Context, storage logical.Storage, passRemover passwordRemover, appID string, passwordKeyIDs ...string) (err error) { + wal := removeCredsWAL{ + AppID: appID, + KeyIDsToRemove: passwordKeyIDs, + Expiration: time.Now().Add(walRemoveCredsExpiration), + } + walID, walErr := framework.PutWAL(ctx, storage, walRemoveCreds, wal) + + // If writing the WAL failed, continue to try to remove the creds from Azure and report the WAL error later if + // the removal failed + merr := new(multierror.Error) + var remainingCreds []string + for _, keyID := range passwordKeyIDs { + // Attempt to remove all of them, don't fail early + err := passRemover.RemoveApplicationPassword(ctx, appID, keyID) + if err != nil { + merr = multierror.Append(merr, err) + remainingCreds = append(remainingCreds, keyID) + } + } + + if len(remainingCreds) == 0 { + // If walErr != nil we failed to write the WAL in the first place, so we don't need to remove it + if walErr == nil { + err := framework.DeleteWAL(ctx, storage, walID) + if err != nil { + return fmt.Errorf("passwords removed, but WAL failed to be removed: %w", err) + } + } + return nil + } + + return merr.ErrorOrNil() +} + +func (b *azureSecretBackend) rollbackCredsWAL(ctx context.Context, req *logical.Request, data interface{}) error { + entry := removeCredsWAL{} + + d, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ + DecodeHook: mapstructure.StringToTimeHookFunc(time.RFC3339), + Result: &entry, + }) + if err != nil { + return err + } + err = d.Decode(data) + if err != nil { + return err + } + + if time.Now().After(entry.Expiration) { + b.Logger().Info("WAL for removing dangling credentials for root user has expired") + return nil + } + + client, err := b.getClient(ctx, req.Storage) + if err != nil { + return err + } + + app, err := client.provider.GetApplication(ctx, entry.AppID) + if err != nil { + return fmt.Errorf("failed to retrieve existing application: %w", err) + } + + actualKeys := []string{} + for _, pc := range app.PasswordCredentials { + actualKeys = append(actualKeys, *pc.KeyID) + } + keysToRemove := intersectStrings(entry.KeyIDsToRemove, actualKeys) + if len(keysToRemove) == 0 { + return nil + } + + b.Logger().Debug("Attempting to remove dangling credentials for root user") + + merr := new(multierror.Error) + for _, keyID := range keysToRemove { + // Attempt to remove all of them, don't fail early + err := client.provider.RemoveApplicationPassword(ctx, entry.AppID, keyID) + if err != nil { + merr = multierror.Append(merr, err) + } + } + return merr.ErrorOrNil() +} + +func intersectStrings(a []string, b []string) []string { + if len(a) == 0 || len(b) == 0 { + return []string{} + } + + aMap := map[string]struct{}{} + for _, aStr := range a { + aMap[aStr] = struct{}{} + } + + result := []string{} + for _, bStr := range b { + if _, exists := aMap[bStr]; exists { + result = append(result, bStr) + continue + } + } + return result +} diff --git a/path_rotate_root_test.go b/path_rotate_root_test.go new file mode 100644 index 0000000..6b4914d --- /dev/null +++ b/path_rotate_root_test.go @@ -0,0 +1,238 @@ +package azuresecrets + +import ( + "context" + "fmt" + "reflect" + "testing" + "time" + + "github.com/Azure/go-autorest/autorest/date" + "github.com/golang/mock/gomock" + "github.com/hashicorp/vault-plugin-secrets-azure/api" + "github.com/hashicorp/vault/sdk/framework" +) + +func TestRotateRootCredentials(t *testing.T) { + type testCase struct { + rawConfig map[string]interface{} + } + + tests := map[string]testCase{ + "AAD": { + rawConfig: map[string]interface{}{ + "subscription_id": generateUUID(), + "tenant_id": generateUUID(), + "client_id": testClientID, + "client_secret": testClientSecret, + "environment": "AZURECHINACLOUD", + "ttl": defaultTestTTL, + "max_ttl": defaultTestMaxTTL, + }, + }, + "MS-Graph": { + rawConfig: map[string]interface{}{ + "subscription_id": generateUUID(), + "tenant_id": generateUUID(), + "client_id": testClientID, + "client_secret": testClientSecret, + "environment": "AZURECHINACLOUD", + "ttl": defaultTestTTL, + "max_ttl": defaultTestMaxTTL, + "use_microsoft_graph_api": true, + }, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + b, storage := getTestBackend(t, false) + testConfigCreate(t, b, storage, test.rawConfig) + + ctx := context.Background() + + originalCfg, err := b.getConfig(ctx, storage) + assertErrorIsNil(t, err) + + now := time.Now() + + objID := "test-client-uuid" + keyID := "original-credential" + + apps := []api.ApplicationResult{ + { + AppID: &testClientID, + ID: &objID, + PasswordCredentials: []*api.PasswordCredential{ + { + DisplayName: strPtr("test-credential-01"), + StartDate: &date.Time{now.Add(-1 * time.Hour)}, + EndDate: &date.Time{now.Add(1 * time.Hour)}, + KeyID: &keyID, + }, + }, + }, + } + + expiration := now.Add(6 * time.Hour) + + mockProvider := NewMockAzureProvider(ctrl) + mockProvider.EXPECT().ListApplications(gomock.Any(), fmt.Sprintf("appId eq '%s'", testClientID)). + Return(apps, nil) + + newPasswordResult := api.PasswordCredentialResult{ + PasswordCredential: api.PasswordCredential{ + DisplayName: strPtr("vault-plugin-secrets-azure-someuuid"), + StartDate: &date.Time{now}, + EndDate: &date.Time{expiration}, + KeyID: strPtr("new-credential"), + SecretText: strPtr("myreallysecurepassword"), + }, + } + mockProvider.EXPECT().AddApplicationPassword(gomock.Any(), objID, gomock.Any(), expiration). + Return(newPasswordResult, nil) + + mockProvider.EXPECT().RemoveApplicationPassword(gomock.Any(), objID, keyID).Return(nil) + + b.getProvider = func(_ *clientSettings, _ bool, _ api.Passwords) (api.AzureProvider, error) { + return mockProvider, nil + } + + client, err := b.getClient(ctx, storage) + assertErrorIsNil(t, err) + assertNotNil(t, client) + + passCred, warnings, err := b.rotateRootCredentials(ctx, storage, expiration) + assertErrorIsNil(t, err) + assertStrSliceIsEmpty(t, warnings) + + expectedCred := newPasswordResult.PasswordCredential + + if !reflect.DeepEqual(passCred, expectedCred) { + t.Fatalf("Expected: %#v\nActual: %#v", expectedCred, passCred) + } + + updatedCfg, err := b.getConfig(ctx, storage) + assertErrorIsNil(t, err) + + if reflect.DeepEqual(updatedCfg, originalCfg) { + t.Fatalf("New config should not equal the original config") + } + + if updatedCfg.ClientSecret != *newPasswordResult.PasswordCredential.SecretText { + t.Fatalf("Expected client secret: %s Actual client secret: %s", *newPasswordResult.PasswordCredential.SecretText, updatedCfg.ClientSecret) + } + + wals, err := framework.ListWAL(ctx, storage) + assertErrorIsNil(t, err) + + assertStrSliceIsEmpty(t, wals) + }) + } +} + +func TestIntersectStrings(t *testing.T) { + type testCase struct { + a []string + b []string + expect []string + } + + tests := map[string]testCase{ + "nil slices": { + a: nil, + b: nil, + expect: []string{}, + }, + "a is nil": { + a: nil, + b: []string{"foo"}, + expect: []string{}, + }, + "b is nil": { + a: []string{"foo"}, + b: nil, + expect: []string{}, + }, + "a is empty": { + a: []string{}, + b: []string{"foo"}, + expect: []string{}, + }, + "b is empty": { + a: []string{"foo"}, + b: []string{}, + expect: []string{}, + }, + "a equals b": { + a: []string{"foo"}, + b: []string{"foo"}, + expect: []string{"foo"}, + }, + "a equals b (many)": { + a: []string{"foo", "bar", "baz", "qux", "quux", "quuz"}, + b: []string{"foo", "bar", "baz", "qux", "quux", "quuz"}, + expect: []string{"foo", "bar", "baz", "qux", "quux", "quuz"}, + }, + "a equals b but out of order": { + a: []string{"foo", "bar", "baz", "qux", "quux", "quuz"}, + b: []string{"quuz", "bar", "qux", "foo", "quux", "baz"}, + expect: []string{"quuz", "bar", "qux", "foo", "quux", "baz"}, + }, + "a is superset": { + a: []string{"foo", "bar", "baz"}, + b: []string{"foo"}, + expect: []string{"foo"}, + }, + "a is superset out of order": { + a: []string{"bar", "foo", "baz"}, + b: []string{"foo"}, + expect: []string{"foo"}, + }, + "b is superset": { + a: []string{"foo"}, + b: []string{"foo", "bar", "baz"}, + expect: []string{"foo"}, + }, + "b is superset out of order": { + a: []string{"foo"}, + b: []string{"bar", "foo", "baz"}, + expect: []string{"foo"}, + }, + "a not equal to b": { + a: []string{"foo", "bar", "baz"}, + b: []string{"qux", "quux", "quuz"}, + expect: []string{}, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + actual := intersectStrings(test.a, test.b) + if !reflect.DeepEqual(actual, test.expect) { + t.Fatalf("Actual: %#v\nExpected: %#v\n", actual, test.expect) + } + }) + } +} + +func assertNotNil(t *testing.T, val interface{}) { + t.Helper() + if val == nil { + t.Fatalf("expected not nil, but was nil") + } +} + +func assertStrSliceIsEmpty(t *testing.T, strs []string) { + t.Helper() + if strs != nil && len(strs) > 0 { + t.Fatalf("string slice is not empty") + } +} + +func strPtr(str string) *string { + return &str +} diff --git a/provider.go b/provider.go index a5533d3..b573a1e 100644 --- a/provider.go +++ b/provider.go @@ -9,7 +9,6 @@ import ( "github.com/Azure/azure-sdk-for-go/services/graphrbac/1.6/graphrbac" "github.com/Azure/go-autorest/autorest" "github.com/Azure/go-autorest/autorest/azure/auth" - "github.com/Azure/go-autorest/autorest/date" "github.com/hashicorp/vault-plugin-secrets-azure/api" "github.com/hashicorp/vault/sdk/helper/useragent" "github.com/hashicorp/vault/sdk/version" @@ -33,11 +32,6 @@ type provider struct { // newAzureProvider creates an azureProvider, backed by Azure client objects for underlying services. func newAzureProvider(settings *clientSettings, useMsGraphApi bool, passwords api.Passwords) (api.AzureProvider, error) { // build clients that use the GraphRBAC endpoint - graphAuthorizer, err := getAuthorizer(settings, settings.Environment.GraphEndpoint) - if err != nil { - return nil, err - } - userAgent := getUserAgent(settings) var appClient api.ApplicationsClient @@ -58,6 +52,11 @@ func newAzureProvider(settings *clientSettings, useMsGraphApi bool, passwords ap groupsClient = msGraphAppClient spClient = msGraphAppClient } else { + graphAuthorizer, err := getAuthorizer(settings, settings.Environment.GraphEndpoint) + if err != nil { + return nil, err + } + aadGraphClient := graphrbac.NewApplicationsClient(settings.TenantID) aadGraphClient.Authorizer = graphAuthorizer aadGraphClient.AddToUserAgent(userAgent) @@ -164,13 +163,17 @@ func (p *provider) GetApplication(ctx context.Context, applicationObjectID strin return p.appClient.GetApplication(ctx, applicationObjectID) } +func (p *provider) ListApplications(ctx context.Context, filter string) ([]api.ApplicationResult, error) { + return p.appClient.ListApplications(ctx, filter) +} + // DeleteApplication deletes an Azure application object. // This will in turn remove the service principal (but not the role assignments). func (p *provider) DeleteApplication(ctx context.Context, applicationObjectID string) error { return p.appClient.DeleteApplication(ctx, applicationObjectID) } -func (p *provider) AddApplicationPassword(ctx context.Context, applicationObjectID string, displayName string, endDateTime date.Time) (result api.PasswordCredentialResult, err error) { +func (p *provider) AddApplicationPassword(ctx context.Context, applicationObjectID string, displayName string, endDateTime time.Time) (result api.PasswordCredentialResult, err error) { return p.appClient.AddApplicationPassword(ctx, applicationObjectID, displayName, endDateTime) } diff --git a/provider_mock_test.go b/provider_mock_test.go index 2e40b4a..c9ee418 100644 --- a/provider_mock_test.go +++ b/provider_mock_test.go @@ -114,17 +114,21 @@ func (m *mockProvider) GetApplication(_ context.Context, _ string) (api.Applicat }, nil } +func (m *mockProvider) ListApplications(_ context.Context, _ string) ([]api.ApplicationResult, error) { + return nil, fmt.Errorf("not implemented") +} + func (m *mockProvider) DeleteApplication(_ context.Context, applicationObjectID string) error { delete(m.applications, applicationObjectID) return nil } -func (m *mockProvider) AddApplicationPassword(_ context.Context, _ string, displayName string, endDateTime date.Time) (api.PasswordCredentialResult, error) { +func (m *mockProvider) AddApplicationPassword(_ context.Context, _ string, displayName string, endDateTime time.Time) (result api.PasswordCredentialResult, err error) { keyID := generateUUID() cred := api.PasswordCredential{ DisplayName: to.StringPtr(displayName), StartDate: &date.Time{Time: time.Now()}, - EndDate: &endDateTime, + EndDate: &date.Time{endDateTime}, KeyID: to.StringPtr(keyID), SecretText: to.StringPtr(generateUUID()), } diff --git a/wal.go b/wal.go index f3dc4ed..9d1da8f 100644 --- a/wal.go +++ b/wal.go @@ -21,9 +21,17 @@ type walApp struct { } func (b *azureSecretBackend) walRollback(ctx context.Context, req *logical.Request, kind string, data interface{}) error { - if kind != walAppKey { + switch kind { + case walAppKey: + return b.rollbackAppWAL(ctx, req, data) + case walRemoveCreds: + return b.rollbackCredsWAL(ctx, req, data) + default: return fmt.Errorf("unknown rollback type %q", kind) } +} + +func (b *azureSecretBackend) rollbackAppWAL(ctx context.Context, req *logical.Request, data interface{}) error { // Decode the WAL data var entry walApp d, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ From 41bd7eb75f4c387c7f42addcc8b71cd7257f90a1 Mon Sep 17 00:00:00 2001 From: Michael Golowka <72365+pcman312@users.noreply.github.com> Date: Tue, 5 Oct 2021 14:06:23 -0600 Subject: [PATCH 06/39] Use correct response value --- path_rotate_root.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/path_rotate_root.go b/path_rotate_root.go index 4f89efa..e619d50 100644 --- a/path_rotate_root.go +++ b/path_rotate_root.go @@ -45,7 +45,7 @@ func (b *azureSecretBackend) pathRotateRoot(ctx context.Context, req *logical.Re } resultData := map[string]interface{}{ - "client_id": *passCred.KeyID, + "secret_id": *passCred.KeyID, "end_date": *passCred.EndDate, } if passCred.DisplayName != nil && *passCred.DisplayName != "" { From 56227f5878729c26abdbcaa645e840301e1cfa2d Mon Sep 17 00:00:00 2001 From: Michael Golowka <72365+pcman312@users.noreply.github.com> Date: Tue, 5 Oct 2021 14:17:22 -0600 Subject: [PATCH 07/39] Fix merge failure --- go.mod | 5 +---- go.sum | 31 ++++++++++++++++++++++--------- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/go.mod b/go.mod index 6bb9444..a6140a9 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( github.com/fatih/color v1.9.0 // indirect github.com/frankban/quicktest v1.10.0 // indirect github.com/go-test/deep v1.0.2-0.20181118220953-042da051cf31 - github.com/golang/mock v1.1.1 + github.com/golang/mock v1.6.0 github.com/hashicorp/go-hclog v0.14.1 github.com/hashicorp/go-multierror v1.1.0 github.com/hashicorp/go-retryablehttp v0.6.6 // indirect @@ -27,9 +27,6 @@ require ( github.com/mitchellh/mapstructure v1.3.2 github.com/pierrec/lz4 v2.5.2+incompatible // indirect golang.org/x/crypto v0.0.0-20200604202706-70a84ac30bf9 // indirect - golang.org/x/net v0.0.0-20200602114024-627f9648deb9 // indirect - golang.org/x/sys v0.0.0-20200602225109-6fdc65e7d980 // indirect - golang.org/x/text v0.3.2 // indirect golang.org/x/time v0.0.0-20200416051211-89c76fbcd5d1 // indirect google.golang.org/protobuf v1.24.0 // indirect gopkg.in/square/go-jose.v2 v2.5.1 // indirect diff --git a/go.sum b/go.sum index 1cc0814..6168d0f 100644 --- a/go.sum +++ b/go.sum @@ -62,8 +62,9 @@ github.com/go-test/deep v1.0.2-0.20181118220953-042da051cf31 h1:28FVBuwkwowZMjbA github.com/go-test/deep v1.0.2-0.20181118220953-042da051cf31/go.mod h1:wGDj63lr65AM2AQyKZd/NYHGb0R+1RLqB8NKt3aSFNA= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b h1:VKtxabqXZkF25pY9ekfRL6a582T4P37/31XEstQ5p58= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= -github.com/golang/mock v1.1.1 h1:G5FRp8JnTd7RQH5kemVNlMeyXQAztQ3mOWV95KxsXH8= github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A= +github.com/golang/mock v1.6.0 h1:ErTB+efbowRARo13NNdxyJji2egdxLGQhRaY+DUumQc= +github.com/golang/mock v1.6.0/go.mod h1:p6yTPP+5HYm5mzsMV8JkE6ZKdX+/wYM6Hr+LicevLPs= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.4.0-rc.1/go.mod h1:ceaxUfeHdC40wWswd/P6IGgMaK3YpKi5j83Wpe3EHw8= @@ -182,28 +183,33 @@ github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXf github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/tv42/httpunix v0.0.0-20150427012821-b75d8614f926/go.mod h1:9ESjWnEqriFuLhtthL60Sar/7RFoluCcXsuvEwTV5KM= +github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20190418165655-df01cb2cc480/go.mod h1:WFFai1msRO1wXaEeE5yQxYXgSfI8pQAWXbQop6sCtWE= +golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200604202706-70a84ac30bf9 h1:vEg9joUBmeBcK9iSJftGNf3coIG4HqZElCPehJsfAYM= golang.org/x/crypto v0.0.0-20200604202706-70a84ac30bf9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU= golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= +golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20181201002055-351d144fa1fc/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190213061140-3a22650c66bd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= +golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20190813141303-74dc4d7220e7/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= -golang.org/x/net v0.0.0-20200602114024-627f9648deb9 h1:pNX+40auqi2JqRfOP1akLGtYcn15TUbkhwuCO3foqqM= -golang.org/x/net v0.0.0-20200602114024-627f9648deb9/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A= +golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4 h1:4nGaVu0QrbjT/AK2PRLuQfQuh6DJve+pELhqTdAj3x0= +golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20180823144017-11551d06cbcc/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190129075346-302c3dd5f1cc/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -215,12 +221,14 @@ golang.org/x/sys v0.0.0-20191008105621-543471e840be/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200116001909-b77594299b42/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200223170610-d5e6a3e2c0ae/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20200602225109-6fdc65e7d980 h1:OjiUf46hAmXblsZdnoSXsEUSKU8r1UEzcL5RVZ4gO9Y= -golang.org/x/sys v0.0.0-20200602225109-6fdc65e7d980/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210510120138-977fb7262007 h1:gG67DSER+11cZvqIMb8S8bt0vZtiN6xWYARwirrOSfE= +golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= -golang.org/x/text v0.3.2 h1:tW2bmiBqwgJj/UpqtC8EpXEZVYOwU0yG4iWbprSVAcs= -golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= +golang.org/x/text v0.3.3 h1:cokOdA+Jmi5PJGXLlLllQSgYigAEfHXJAERHVMaCc2k= +golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20200416051211-89c76fbcd5d1 h1:NusfzzA6yGQ+ua51ck7E3omNUX/JuqbFSaRGqU8CcLI= golang.org/x/time v0.0.0-20200416051211-89c76fbcd5d1/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= @@ -229,8 +237,13 @@ golang.org/x/tools v0.0.0-20190114222345-bf090417da8b/go.mod h1:n7NCudcB/nEzxVGm golang.org/x/tools v0.0.0-20190226205152-f727befe758c/go.mod h1:9Yl7xja0Znq3iFh3HoIrodX9oNMXvdceNzlUR8zjMvY= golang.org/x/tools v0.0.0-20190311212946-11955173bddd/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs= golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q= -golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= +golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= +golang.org/x/tools v0.1.1/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= +golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE= +golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM= google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4= google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8/go.mod h1:JiN7NxoALGmiZfu7CAH4rXhgtRTLTxftemlI0sWmxmc= From 59884a2bef362221cbac49a308691ebb5bb15c49 Mon Sep 17 00:00:00 2001 From: Michael Golowka <72365+pcman312@users.noreply.github.com> Date: Thu, 7 Oct 2021 14:05:41 -0600 Subject: [PATCH 08/39] Add additional AAD warnings; Respond to code review --- path_config.go | 35 ++++++++++++++++++++++++++++------- path_roles.go | 33 +++++++++++++++++++++------------ path_rotate_root.go | 32 ++++++++++++++++++++------------ 3 files changed, 69 insertions(+), 31 deletions(-) diff --git a/path_config.go b/path_config.go index e405aa0..852cd8b 100644 --- a/path_config.go +++ b/path_config.go @@ -65,11 +65,19 @@ func pathConfig(b *azureSecretBackend) *framework.Path { Description: "Enable usage of the Microsoft Graph API over the deprecated Azure AD Graph API.", }, }, - Callbacks: map[logical.Operation]framework.OperationFunc{ - logical.ReadOperation: b.pathConfigRead, - logical.CreateOperation: b.pathConfigWrite, - logical.UpdateOperation: b.pathConfigWrite, - logical.DeleteOperation: b.pathConfigDelete, + Operations: map[logical.Operation]framework.OperationHandler{ + logical.ReadOperation: &framework.PathOperation{ + Callback: b.pathConfigRead, + }, + logical.CreateOperation: &framework.PathOperation{ + Callback: b.pathConfigWrite, + }, + logical.UpdateOperation: &framework.PathOperation{ + Callback: b.pathConfigWrite, + }, + logical.DeleteOperation: &framework.PathOperation{ + Callback: b.pathConfigDelete, + }, }, ExistenceCheck: b.pathConfigExistenceCheck, HelpSynopsis: confHelpSyn, @@ -129,7 +137,20 @@ func (b *azureSecretBackend) pathConfigWrite(ctx context.Context, req *logical.R err = b.saveConfig(ctx, config, req.Storage) - return nil, err + return addAADWarning(nil, config), err +} + +func addAADWarning(resp *logical.Response, config *azureConfig) *logical.Response { + if !config.UseMsGraphAPI { + return resp + } + if resp == nil { + resp = &logical.Response{} + } + resp.AddWarning("This configuration is using the Azure Active Directory API which is being " + + "removed soon. Please migrate to using the Microsoft Graph API using the " + + "use_microsoft_graph_api configuration parameter.") + return resp } func (b *azureSecretBackend) pathConfigRead(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { @@ -152,7 +173,7 @@ func (b *azureSecretBackend) pathConfigRead(ctx context.Context, req *logical.Re "use_microsoft_graph_api": config.UseMsGraphAPI, }, } - return resp, nil + return addAADWarning(resp, config), nil } func (b *azureSecretBackend) pathConfigDelete(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { diff --git a/path_roles.go b/path_roles.go index 1bc1811..cffc37d 100644 --- a/path_roles.go +++ b/path_roles.go @@ -124,6 +124,11 @@ func pathsRole(b *azureSecretBackend) []*framework.Path { func (b *azureSecretBackend) pathRoleUpdate(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { var resp *logical.Response + config, err := b.getConfig(ctx, req.Storage) + if err != nil { + return nil, err + } + client, err := b.getClient(ctx, req.Storage) if err != nil { return nil, err @@ -278,14 +283,17 @@ func (b *azureSecretBackend) pathRoleUpdate(ctx context.Context, req *logical.Re return nil, fmt.Errorf("error storing role: %w", err) } - return resp, nil + return addAADWarning(resp, config), nil } func (b *azureSecretBackend) pathRoleRead(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - var data = make(map[string]interface{}) - name := d.Get("name").(string) + config, err := b.getConfig(ctx, req.Storage) + if err != nil { + return nil, err + } + r, err := getRole(ctx, name, req.Storage) if err != nil { return nil, fmt.Errorf("error reading role: %w", err) @@ -295,15 +303,16 @@ func (b *azureSecretBackend) pathRoleRead(ctx context.Context, req *logical.Requ return nil, nil } - data["ttl"] = r.TTL / time.Second - data["max_ttl"] = r.MaxTTL / time.Second - data["azure_roles"] = r.AzureRoles - data["azure_groups"] = r.AzureGroups - data["application_object_id"] = r.ApplicationObjectID - - return &logical.Response{ - Data: data, - }, nil + resp := &logical.Response{ + Data: map[string]interface{}{ + "ttl": r.TTL / time.Second, + "max_ttl": r.MaxTTL / time.Second, + "azure_roles": r.AzureRoles, + "azure_groups": r.AzureGroups, + "application_object_id": r.ApplicationObjectID, + }, + } + return addAADWarning(resp, config), nil } func (b *azureSecretBackend) pathRoleList(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { diff --git a/path_rotate_root.go b/path_rotate_root.go index e619d50..ba8868f 100644 --- a/path_rotate_root.go +++ b/path_rotate_root.go @@ -20,8 +20,14 @@ import ( func pathRotateRoot(b *azureSecretBackend) *framework.Path { return &framework.Path{ Pattern: "rotate-root", - Fields: map[string]*framework.FieldSchema{ - // None + Fields: map[string]*framework.FieldSchema{ + "expiration": { + Type: framework.TypeDurationSecond, + // 28 weeks (~6 months) -> days -> hours + Default: 28 * 7 * 24 * time.Hour, + Description: "The expiration date of the new credentials in Azure", + Required: false, + }, }, Operations: map[logical.Operation]framework.OperationHandler{ logical.UpdateOperation: &framework.PathOperation{ @@ -38,8 +44,15 @@ func pathRotateRoot(b *azureSecretBackend) *framework.Path { } func (b *azureSecretBackend) pathRotateRoot(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - expiration := time.Now().AddDate(0, 6, 0) - passCred, warnings, err := b.rotateRootCredentials(ctx, req.Storage, expiration) + expirationDur := data.Get("expiration").(time.Duration) + expiration := time.Now().Add(expirationDur) + + config, err := b.getConfig(ctx, req.Storage) + if err != nil { + return nil, err + } + + passCred, warnings, err := b.rotateRootCredentials(ctx, req.Storage, config, expiration) if err != nil { return nil, err } @@ -57,15 +70,10 @@ func (b *azureSecretBackend) pathRotateRoot(ctx context.Context, req *logical.Re Warnings: warnings, } - return resp, nil + return addAADWarning(resp, config), nil } -func (b *azureSecretBackend) rotateRootCredentials(ctx context.Context, storage logical.Storage, expiration time.Time) (cred api.PasswordCredential, warnings []string, err error) { - cfg, err := b.getConfig(ctx, storage) - if err != nil { - return api.PasswordCredential{}, nil, err - } - +func (b *azureSecretBackend) rotateRootCredentials(ctx context.Context, storage logical.Storage, cfg *azureConfig, expiration time.Time) (cred api.PasswordCredential, warnings []string, err error) { client, err := b.getClient(ctx, storage) if err != nil { return api.PasswordCredential{}, nil, err @@ -100,7 +108,7 @@ func (b *azureSecretBackend) rotateRootCredentials(ctx context.Context, storage } // This could have the same username customization logic put on it if we really wanted it here - passwordDisplayName := fmt.Sprintf("vault-plugin-secrets-azure-%s", uniqueID) + passwordDisplayName := fmt.Sprintf("vault-%s", uniqueID) newPasswordResp, err := client.provider.AddApplicationPassword(ctx, *app.ID, passwordDisplayName, expiration) if err != nil { From 5916c4b60892a9d6472fea383e5e86a2cd9c4bb2 Mon Sep 17 00:00:00 2001 From: Michael Golowka <72365+pcman312@users.noreply.github.com> Date: Thu, 7 Oct 2021 14:13:35 -0600 Subject: [PATCH 09/39] Fix test --- path_rotate_root_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/path_rotate_root_test.go b/path_rotate_root_test.go index 6b4914d..2769727 100644 --- a/path_rotate_root_test.go +++ b/path_rotate_root_test.go @@ -105,7 +105,7 @@ func TestRotateRootCredentials(t *testing.T) { assertErrorIsNil(t, err) assertNotNil(t, client) - passCred, warnings, err := b.rotateRootCredentials(ctx, storage, expiration) + passCred, warnings, err := b.rotateRootCredentials(ctx, storage, originalCfg, expiration) assertErrorIsNil(t, err) assertStrSliceIsEmpty(t, warnings) From 714aff4835fee399948efdc934cafa721d046dd6 Mon Sep 17 00:00:00 2001 From: Michael Golowka <72365+pcman312@users.noreply.github.com> Date: Thu, 7 Oct 2021 14:18:59 -0600 Subject: [PATCH 10/39] Don't pass config as a pointer so it gets a copy --- path_rotate_root.go | 6 +++--- path_rotate_root_test.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/path_rotate_root.go b/path_rotate_root.go index ba8868f..2df7497 100644 --- a/path_rotate_root.go +++ b/path_rotate_root.go @@ -52,7 +52,7 @@ func (b *azureSecretBackend) pathRotateRoot(ctx context.Context, req *logical.Re return nil, err } - passCred, warnings, err := b.rotateRootCredentials(ctx, req.Storage, config, expiration) + passCred, warnings, err := b.rotateRootCredentials(ctx, req.Storage, *config, expiration) if err != nil { return nil, err } @@ -73,7 +73,7 @@ func (b *azureSecretBackend) pathRotateRoot(ctx context.Context, req *logical.Re return addAADWarning(resp, config), nil } -func (b *azureSecretBackend) rotateRootCredentials(ctx context.Context, storage logical.Storage, cfg *azureConfig, expiration time.Time) (cred api.PasswordCredential, warnings []string, err error) { +func (b *azureSecretBackend) rotateRootCredentials(ctx context.Context, storage logical.Storage, cfg azureConfig, expiration time.Time) (cred api.PasswordCredential, warnings []string, err error) { client, err := b.getClient(ctx, storage) if err != nil { return api.PasswordCredential{}, nil, err @@ -117,7 +117,7 @@ func (b *azureSecretBackend) rotateRootCredentials(ctx context.Context, storage cfg.ClientSecret = *newPasswordResp.PasswordCredential.SecretText - err = b.saveConfig(ctx, cfg, storage) + err = b.saveConfig(ctx, &cfg, storage) if err != nil { // Remove the key since we failed to save it to Vault storage. It's reasonable to assume that this call // to Azure will succeed since the AddApplicationPassword call succeeded above. If it doesn't, we aren't going diff --git a/path_rotate_root_test.go b/path_rotate_root_test.go index 2769727..f3dc167 100644 --- a/path_rotate_root_test.go +++ b/path_rotate_root_test.go @@ -105,7 +105,7 @@ func TestRotateRootCredentials(t *testing.T) { assertErrorIsNil(t, err) assertNotNil(t, client) - passCred, warnings, err := b.rotateRootCredentials(ctx, storage, originalCfg, expiration) + passCred, warnings, err := b.rotateRootCredentials(ctx, storage, *originalCfg, expiration) assertErrorIsNil(t, err) assertStrSliceIsEmpty(t, warnings) From c3615a88f42bccb21ed754bd87c255f7515de784 Mon Sep 17 00:00:00 2001 From: Michael Golowka <72365+pcman312@users.noreply.github.com> Date: Fri, 8 Oct 2021 13:46:58 -0600 Subject: [PATCH 11/39] Fix expiration date logic; fix inverted warning logic --- path_config.go | 17 ++++++++---- path_config_test.go | 63 +++++++++++++++++++++++++++++++++++++++++++++ path_rotate_root.go | 20 +++++++------- 3 files changed, 85 insertions(+), 15 deletions(-) diff --git a/path_config.go b/path_config.go index 852cd8b..ac81f9e 100644 --- a/path_config.go +++ b/path_config.go @@ -136,20 +136,27 @@ func (b *azureSecretBackend) pathConfigWrite(ctx context.Context, req *logical.R } err = b.saveConfig(ctx, config, req.Storage) + if err != nil { + return nil, err + } - return addAADWarning(nil, config), err + resp := addAADWarning(nil, config) + + return resp, nil } +const aadWarning = "This configuration is using the Azure Active Directory API which is being " + + "removed soon. Please migrate to using the Microsoft Graph API using the " + + "use_microsoft_graph_api configuration parameter." + func addAADWarning(resp *logical.Response, config *azureConfig) *logical.Response { - if !config.UseMsGraphAPI { + if config.UseMsGraphAPI { return resp } if resp == nil { resp = &logical.Response{} } - resp.AddWarning("This configuration is using the Azure Active Directory API which is being " + - "removed soon. Please migrate to using the Microsoft Graph API using the " + - "use_microsoft_graph_api configuration parameter.") + resp.AddWarning(aadWarning) return resp } diff --git a/path_config_test.go b/path_config_test.go index a43c471..817488f 100644 --- a/path_config_test.go +++ b/path_config_test.go @@ -2,6 +2,7 @@ package azuresecrets import ( "context" + "reflect" "testing" "github.com/hashicorp/vault/sdk/logical" @@ -90,6 +91,68 @@ func TestConfigDelete(t *testing.T) { testConfigRead(t, b, s, config) } +func TestAddAADWarning(t *testing.T) { + type testCase struct { + useMSGraphAPI bool + resp *logical.Response + expectedResp *logical.Response + } + + tests := map[string]testCase{ + "nil resp, using AAD": { + useMSGraphAPI: false, + resp: nil, + expectedResp: &logical.Response{ + Warnings: []string{aadWarning}, + }, + }, + "nil resp, using ms-graph": { + useMSGraphAPI: true, + resp: nil, + expectedResp: nil, + }, + "non-nil resp, using AAD": { + useMSGraphAPI: false, + resp: &logical.Response{ + Data: map[string]interface{}{ + "foo": "bar", + }, + }, + expectedResp: &logical.Response{ + Data: map[string]interface{}{ + "foo": "bar", + }, + Warnings: []string{aadWarning}, + }, + }, + "non-nil resp, using ms-graph": { + useMSGraphAPI: true, + resp: &logical.Response{ + Data: map[string]interface{}{ + "foo": "bar", + }, + }, + expectedResp: &logical.Response{ + Data: map[string]interface{}{ + "foo": "bar", + }, + }, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + cfg := &azureConfig{ + UseMsGraphAPI: test.useMSGraphAPI, + } + actualResp := addAADWarning(test.resp, cfg) + if !reflect.DeepEqual(actualResp, test.expectedResp) { + t.Fatalf("Actual: %#v\nExpected: %#v", actualResp, test.expectedResp) + } + }) + } +} + func testConfigCreate(t *testing.T, b logical.Backend, s logical.Storage, d map[string]interface{}) { t.Helper() testConfigCreateUpdate(t, b, logical.CreateOperation, s, d) diff --git a/path_rotate_root.go b/path_rotate_root.go index 2df7497..8ad00dc 100644 --- a/path_rotate_root.go +++ b/path_rotate_root.go @@ -5,16 +5,13 @@ import ( "fmt" "time" - "github.com/hashicorp/vault-plugin-secrets-azure/api" - - "github.com/mitchellh/mapstructure" - "github.com/hashicorp/go-multierror" - "github.com/hashicorp/go-uuid" - + "github.com/hashicorp/vault-plugin-secrets-azure/api" "github.com/hashicorp/vault/sdk/framework" + "github.com/hashicorp/vault/sdk/helper/parseutil" "github.com/hashicorp/vault/sdk/logical" + "github.com/mitchellh/mapstructure" ) func pathRotateRoot(b *azureSecretBackend) *framework.Path { @@ -22,10 +19,10 @@ func pathRotateRoot(b *azureSecretBackend) *framework.Path { Pattern: "rotate-root", Fields: map[string]*framework.FieldSchema{ "expiration": { - Type: framework.TypeDurationSecond, + Type: framework.TypeString, // 28 weeks (~6 months) -> days -> hours - Default: 28 * 7 * 24 * time.Hour, - Description: "The expiration date of the new credentials in Azure", + Default: (28 * 7 * 24 * time.Hour).String(), + Description: "The expiration date of the new credentials in Azure. This can be either a number of seconds or a time formatted duration (ex: 24h)", Required: false, }, }, @@ -44,7 +41,10 @@ func pathRotateRoot(b *azureSecretBackend) *framework.Path { } func (b *azureSecretBackend) pathRotateRoot(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - expirationDur := data.Get("expiration").(time.Duration) + expirationDur, err := parseutil.ParseDurationSecond(data.Get("expiration").(string)) + if err != nil { + return nil, fmt.Errorf("invalid expiration: %w", err) + } expiration := time.Now().Add(expirationDur) config, err := b.getConfig(ctx, req.Storage) From 5eb9e4cd3273377c180619f2ee87193df8db7f90 Mon Sep 17 00:00:00 2001 From: Michael Golowka <72365+pcman312@users.noreply.github.com> Date: Wed, 13 Oct 2021 11:07:24 -0600 Subject: [PATCH 12/39] Minor code review tweaks --- api/application_msgraph.go | 2 +- path_rotate_root.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/api/application_msgraph.go b/api/application_msgraph.go index 316caaa..dc4d5b6 100644 --- a/api/application_msgraph.go +++ b/api/application_msgraph.go @@ -73,7 +73,7 @@ func (c *AppClient) GetApplication(ctx context.Context, applicationObjectID stri } type listApplicationsResponse struct { - Value []ApplicationResult + Value []ApplicationResult `json:"value"` } func (c *AppClient) ListApplications(ctx context.Context, filter string) ([]ApplicationResult, error) { diff --git a/path_rotate_root.go b/path_rotate_root.go index 8ad00dc..f4bf112 100644 --- a/path_rotate_root.go +++ b/path_rotate_root.go @@ -249,7 +249,6 @@ func intersectStrings(a []string, b []string) []string { for _, bStr := range b { if _, exists := aMap[bStr]; exists { result = append(result, bStr) - continue } } return result From 4085253523624d384b12d2ecf526e633c1d7bf43 Mon Sep 17 00:00:00 2001 From: Michael Golowka <72365+pcman312@users.noreply.github.com> Date: Wed, 13 Oct 2021 11:15:43 -0600 Subject: [PATCH 13/39] Move expiration to config --- path_config.go | 23 ++++++++++++++++------- path_rotate_root.go | 23 ++++++++--------------- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/path_config.go b/path_config.go index ac81f9e..a154dc2 100644 --- a/path_config.go +++ b/path_config.go @@ -3,6 +3,7 @@ package azuresecrets import ( "context" "errors" + "time" "github.com/Azure/go-autorest/autorest/azure" multierror "github.com/hashicorp/go-multierror" @@ -18,13 +19,14 @@ const ( // defaults for roles. The zero value is useful and results in // environments variable and system defaults being used. type azureConfig struct { - SubscriptionID string `json:"subscription_id"` - TenantID string `json:"tenant_id"` - ClientID string `json:"client_id"` - ClientSecret string `json:"client_secret"` - Environment string `json:"environment"` - PasswordPolicy string `json:"password_policy"` - UseMsGraphAPI bool `json:"use_microsoft_graph_api"` + SubscriptionID string `json:"subscription_id"` + TenantID string `json:"tenant_id"` + ClientID string `json:"client_id"` + ClientSecret string `json:"client_secret"` + Environment string `json:"environment"` + PasswordPolicy string `json:"password_policy"` + UseMsGraphAPI bool `json:"use_microsoft_graph_api"` + DefaultExpiration time.Duration `json:"default_expiration"` } func pathConfig(b *azureSecretBackend) *framework.Path { @@ -64,6 +66,13 @@ func pathConfig(b *azureSecretBackend) *framework.Path { Type: framework.TypeBool, Description: "Enable usage of the Microsoft Graph API over the deprecated Azure AD Graph API.", }, + "default_expiration": &framework.FieldSchema{ + Type: framework.TypeString, + // 28 weeks (~6 months) -> days -> hours + Default: (28 * 7 * 24 * time.Hour).String(), + Description: "The expiration date of the new credentials in Azure. This can be either a number of seconds or a time formatted duration (ex: 24h)", + Required: false, + }, }, Operations: map[logical.Operation]framework.OperationHandler{ logical.ReadOperation: &framework.PathOperation{ diff --git a/path_rotate_root.go b/path_rotate_root.go index f4bf112..dd7de58 100644 --- a/path_rotate_root.go +++ b/path_rotate_root.go @@ -9,7 +9,6 @@ import ( "github.com/hashicorp/go-uuid" "github.com/hashicorp/vault-plugin-secrets-azure/api" "github.com/hashicorp/vault/sdk/framework" - "github.com/hashicorp/vault/sdk/helper/parseutil" "github.com/hashicorp/vault/sdk/logical" "github.com/mitchellh/mapstructure" ) @@ -17,14 +16,8 @@ import ( func pathRotateRoot(b *azureSecretBackend) *framework.Path { return &framework.Path{ Pattern: "rotate-root", - Fields: map[string]*framework.FieldSchema{ - "expiration": { - Type: framework.TypeString, - // 28 weeks (~6 months) -> days -> hours - Default: (28 * 7 * 24 * time.Hour).String(), - Description: "The expiration date of the new credentials in Azure. This can be either a number of seconds or a time formatted duration (ex: 24h)", - Required: false, - }, + Fields: map[string]*framework.FieldSchema{ + // None }, Operations: map[logical.Operation]framework.OperationHandler{ logical.UpdateOperation: &framework.PathOperation{ @@ -41,17 +34,17 @@ func pathRotateRoot(b *azureSecretBackend) *framework.Path { } func (b *azureSecretBackend) pathRotateRoot(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - expirationDur, err := parseutil.ParseDurationSecond(data.Get("expiration").(string)) - if err != nil { - return nil, fmt.Errorf("invalid expiration: %w", err) - } - expiration := time.Now().Add(expirationDur) - config, err := b.getConfig(ctx, req.Storage) if err != nil { return nil, err } + expDur := config.DefaultExpiration + if expDur == 0 { + expDur = 28 * 7 * 24 * time.Hour + } + expiration := time.Now().Add(expDur) + passCred, warnings, err := b.rotateRootCredentials(ctx, req.Storage, *config, expiration) if err != nil { return nil, err From 938122f00569a89e95f8d80616b6265c02fe9dea Mon Sep 17 00:00:00 2001 From: Michael Golowka <72365+pcman312@users.noreply.github.com> Date: Wed, 13 Oct 2021 15:31:56 -0600 Subject: [PATCH 14/39] Don't error if there isn't an error --- api/application_msgraph.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/api/application_msgraph.go b/api/application_msgraph.go index dc4d5b6..53853ae 100644 --- a/api/application_msgraph.go +++ b/api/application_msgraph.go @@ -122,7 +122,7 @@ func (c *AppClient) CreateApplication(ctx context.Context, displayName string) ( // DeleteApplication deletes an Azure application object. // This will in turn remove the service principal (but not the role assignments). -func (c *AppClient) DeleteApplication(ctx context.Context, applicationObjectID string) (err error) { +func (c *AppClient) DeleteApplication(ctx context.Context, applicationObjectID string) error { req, err := c.deleteApplicationPreparer(ctx, applicationObjectID) if err != nil { return autorest.NewErrorWithError(err, "provider", "DeleteApplication", nil, "Failure preparing request") @@ -138,6 +138,10 @@ func (c *AppClient) DeleteApplication(ctx context.Context, applicationObjectID s c.client.ByInspecting(), azure.WithErrorUnlessStatusCode(http.StatusNoContent, http.StatusNotFound), autorest.ByClosing()) + if err != nil { + return autorest.NewErrorWithError(err, "provider", "DeleteApplication", resp, "Failure responding to request") + } + return nil } func (c *AppClient) AddApplicationPassword(ctx context.Context, applicationObjectID string, displayName string, endDateTime time.Time) (result PasswordCredentialResult, err error) { From e5cc93ae0813c367789b4173ff00ca959892f5bb Mon Sep 17 00:00:00 2001 From: Michael Golowka <72365+pcman312@users.noreply.github.com> Date: Wed, 13 Oct 2021 16:10:55 -0600 Subject: [PATCH 15/39] Update the config & remove old passwords in the WAL --- path_rotate_root.go | 106 +++++++++++++++++++-------------------- path_rotate_root_test.go | 36 ++++++++++--- wal.go | 4 +- 3 files changed, 82 insertions(+), 64 deletions(-) diff --git a/path_rotate_root.go b/path_rotate_root.go index dd7de58..c874264 100644 --- a/path_rotate_root.go +++ b/path_rotate_root.go @@ -45,7 +45,7 @@ func (b *azureSecretBackend) pathRotateRoot(ctx context.Context, req *logical.Re } expiration := time.Now().Add(expDur) - passCred, warnings, err := b.rotateRootCredentials(ctx, req.Storage, *config, expiration) + passCred, err := b.rotateRootCredentials(ctx, req.Storage, *config, expiration) if err != nil { return nil, err } @@ -59,31 +59,30 @@ func (b *azureSecretBackend) pathRotateRoot(ctx context.Context, req *logical.Re } resp := &logical.Response{ - Data: resultData, - Warnings: warnings, + Data: resultData, } return addAADWarning(resp, config), nil } -func (b *azureSecretBackend) rotateRootCredentials(ctx context.Context, storage logical.Storage, cfg azureConfig, expiration time.Time) (cred api.PasswordCredential, warnings []string, err error) { +func (b *azureSecretBackend) rotateRootCredentials(ctx context.Context, storage logical.Storage, cfg azureConfig, expiration time.Time) (cred api.PasswordCredential, err error) { client, err := b.getClient(ctx, storage) if err != nil { - return api.PasswordCredential{}, nil, err + return api.PasswordCredential{}, err } // We need to use List instead of Get here because we don't have the Object ID // (which is different from the Application/Client ID) apps, err := client.provider.ListApplications(ctx, fmt.Sprintf("appId eq '%s'", cfg.ClientID)) if err != nil { - return api.PasswordCredential{}, nil, err + return api.PasswordCredential{}, err } if len(apps) == 0 { - return api.PasswordCredential{}, nil, fmt.Errorf("no application found") + return api.PasswordCredential{}, fmt.Errorf("no application found") } if len(apps) > 1 { - return api.PasswordCredential{}, nil, fmt.Errorf("multiple applications found - double check your client_id") + return api.PasswordCredential{}, fmt.Errorf("multiple applications found - double check your client_id") } app := apps[0] @@ -97,7 +96,7 @@ func (b *azureSecretBackend) rotateRootCredentials(ctx context.Context, storage uniqueID, err := uuid.GenerateUUID() if err != nil { - return api.PasswordCredential{}, nil, fmt.Errorf("failed to generate UUID: %w", err) + return api.PasswordCredential{}, fmt.Errorf("failed to generate UUID: %w", err) } // This could have the same username customization logic put on it if we really wanted it here @@ -105,34 +104,41 @@ func (b *azureSecretBackend) rotateRootCredentials(ctx context.Context, storage newPasswordResp, err := client.provider.AddApplicationPassword(ctx, *app.ID, passwordDisplayName, expiration) if err != nil { - return api.PasswordCredential{}, nil, fmt.Errorf("failed to add new password: %w", err) + return api.PasswordCredential{}, fmt.Errorf("failed to add new password: %w", err) } - cfg.ClientSecret = *newPasswordResp.PasswordCredential.SecretText + // Write a WAL with the new credential (and some other info) to write to the config later + // This is done because the new credential is typically not available to use IMMEDIATELY + // after creating it. This can create errors for callers so instead this will write + // the config and clean up old keys asynchronously after ~10m (backend.WALRollbackMinAge) + // This also will automatically retry because of the standard WAL behavior + wal := rotateCredsWAL{ + AppID: *app.ID, + NewSecret: *newPasswordResp.PasswordCredential.SecretText, + KeyIDsToRemove: credsToDelete, + Expiration: time.Now().Add(walRotateRootCredsExpiration), + } - err = b.saveConfig(ctx, &cfg, storage) - if err != nil { - // Remove the key since we failed to save it to Vault storage. It's reasonable to assume that this call + _, walErr := framework.PutWAL(ctx, storage, walRotateRootCreds, wal) + if walErr != nil { + // Remove the key since we failed to save the WAL to Vault storage. It's reasonable to assume that this call // to Azure will succeed since the AddApplicationPassword call succeeded above. If it doesn't, we aren't going // to do any retries via a WAL here since the current configuration will continue to work. azureErr := client.provider.RemoveApplicationPassword(ctx, *app.ID, *newPasswordResp.PasswordCredential.KeyID) merr := multierror.Append(err, azureErr) - return api.PasswordCredential{}, nil, merr + return api.PasswordCredential{}, merr } - err = removeApplicationPasswords(ctx, storage, client.provider, *app.ID, credsToDelete...) - if err != nil { - warnings = append(warnings, fmt.Sprintf("failed to clean up all other credentials for root user. Will attempt again later.\n%s", err.Error())) - } - - return newPasswordResp.PasswordCredential, warnings, nil + return newPasswordResp.PasswordCredential, nil } -const walRemoveCreds = "removeCreds" -const walRemoveCredsExpiration = 24 * time.Hour +const walRotateRootCreds = "rotateRootCreds" +const walRotateRootCredsExpiration = 24 * time.Hour -type removeCredsWAL struct { +type rotateCredsWAL struct { AppID string + OldSecret string + NewSecret string KeyIDsToRemove []string Expiration time.Time } @@ -141,16 +147,7 @@ type passwordRemover interface { RemoveApplicationPassword(ctx context.Context, applicationObjectID string, keyID string) error } -func removeApplicationPasswords(ctx context.Context, storage logical.Storage, passRemover passwordRemover, appID string, passwordKeyIDs ...string) (err error) { - wal := removeCredsWAL{ - AppID: appID, - KeyIDsToRemove: passwordKeyIDs, - Expiration: time.Now().Add(walRemoveCredsExpiration), - } - walID, walErr := framework.PutWAL(ctx, storage, walRemoveCreds, wal) - - // If writing the WAL failed, continue to try to remove the creds from Azure and report the WAL error later if - // the removal failed +func removeApplicationPasswords(ctx context.Context, passRemover passwordRemover, appID string, passwordKeyIDs ...string) (err error) { merr := new(multierror.Error) var remainingCreds []string for _, keyID := range passwordKeyIDs { @@ -162,22 +159,11 @@ func removeApplicationPasswords(ctx context.Context, storage logical.Storage, pa } } - if len(remainingCreds) == 0 { - // If walErr != nil we failed to write the WAL in the first place, so we don't need to remove it - if walErr == nil { - err := framework.DeleteWAL(ctx, storage, walID) - if err != nil { - return fmt.Errorf("passwords removed, but WAL failed to be removed: %w", err) - } - } - return nil - } - return merr.ErrorOrNil() } -func (b *azureSecretBackend) rollbackCredsWAL(ctx context.Context, req *logical.Request, data interface{}) error { - entry := removeCredsWAL{} +func (b *azureSecretBackend) rotateRootCredsWAL(ctx context.Context, req *logical.Request, data interface{}) error { + entry := rotateCredsWAL{} d, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ DecodeHook: mapstructure.StringToTimeHookFunc(time.RFC3339), @@ -196,6 +182,19 @@ func (b *azureSecretBackend) rollbackCredsWAL(ctx context.Context, req *logical. return nil } + cfg, err := b.getConfig(ctx, req.Storage) + if err != nil { + return fmt.Errorf("failed to load configuration: %w", err) + } + + cfg.ClientSecret = entry.NewSecret + + err = b.saveConfig(ctx, cfg, req.Storage) + if err != nil { + return fmt.Errorf("failed to save new configuration: %w", err) + } + + // b.saveConfig does a reset so this should get a new client with the updated creds client, err := b.getClient(ctx, req.Storage) if err != nil { return err @@ -217,15 +216,12 @@ func (b *azureSecretBackend) rollbackCredsWAL(ctx context.Context, req *logical. b.Logger().Debug("Attempting to remove dangling credentials for root user") - merr := new(multierror.Error) - for _, keyID := range keysToRemove { - // Attempt to remove all of them, don't fail early - err := client.provider.RemoveApplicationPassword(ctx, entry.AppID, keyID) - if err != nil { - merr = multierror.Append(merr, err) - } + err = removeApplicationPasswords(ctx, client.provider, entry.AppID, keysToRemove...) + if err != nil { + return err } - return merr.ErrorOrNil() + b.Logger().Debug("Successfully removed dangling credentials for root user") + return nil } func intersectStrings(a []string, b []string) []string { diff --git a/path_rotate_root_test.go b/path_rotate_root_test.go index f3dc167..7ddc094 100644 --- a/path_rotate_root_test.go +++ b/path_rotate_root_test.go @@ -11,6 +11,7 @@ import ( "github.com/golang/mock/gomock" "github.com/hashicorp/vault-plugin-secrets-azure/api" "github.com/hashicorp/vault/sdk/framework" + "github.com/hashicorp/vault/sdk/logical" ) func TestRotateRootCredentials(t *testing.T) { @@ -97,6 +98,8 @@ func TestRotateRootCredentials(t *testing.T) { mockProvider.EXPECT().RemoveApplicationPassword(gomock.Any(), objID, keyID).Return(nil) + mockProvider.EXPECT().GetApplication(gomock.Any(), objID).Return(apps[0], nil) + b.getProvider = func(_ *clientSettings, _ bool, _ api.Passwords) (api.AzureProvider, error) { return mockProvider, nil } @@ -105,9 +108,8 @@ func TestRotateRootCredentials(t *testing.T) { assertErrorIsNil(t, err) assertNotNil(t, client) - passCred, warnings, err := b.rotateRootCredentials(ctx, storage, *originalCfg, expiration) + passCred, err := b.rotateRootCredentials(ctx, storage, *originalCfg, expiration) assertErrorIsNil(t, err) - assertStrSliceIsEmpty(t, warnings) expectedCred := newPasswordResult.PasswordCredential @@ -115,6 +117,31 @@ func TestRotateRootCredentials(t *testing.T) { t.Fatalf("Expected: %#v\nActual: %#v", expectedCred, passCred) } + // Ensure the WAL exists + wals, err := framework.ListWAL(ctx, storage) + assertErrorIsNil(t, err) + if len(wals) == 0 { + t.Fatalf("Missing WAL for saving config & removing old passwords") + } + if len(wals) > 1 { + t.Fatalf("More than one WAL found") + } + walEntry, err := framework.GetWAL(ctx, storage, wals[0]) + assertErrorIsNil(t, err) + + if walEntry.Kind != walRotateRootCreds { + t.Fatalf("Actual WAL kind: %s Expected WAL kind: %s", walEntry.Kind, walRotateRootCreds) + } + + walReq := &logical.Request{ + Storage: storage, + } + + // Force the WAL to execute + err = b.rotateRootCredsWAL(ctx, walReq, walEntry.Data) + assertErrorIsNil(t, err) + + // Check the config updatedCfg, err := b.getConfig(ctx, storage) assertErrorIsNil(t, err) @@ -125,11 +152,6 @@ func TestRotateRootCredentials(t *testing.T) { if updatedCfg.ClientSecret != *newPasswordResult.PasswordCredential.SecretText { t.Fatalf("Expected client secret: %s Actual client secret: %s", *newPasswordResult.PasswordCredential.SecretText, updatedCfg.ClientSecret) } - - wals, err := framework.ListWAL(ctx, storage) - assertErrorIsNil(t, err) - - assertStrSliceIsEmpty(t, wals) }) } } diff --git a/wal.go b/wal.go index 9d1da8f..5892a4d 100644 --- a/wal.go +++ b/wal.go @@ -24,8 +24,8 @@ func (b *azureSecretBackend) walRollback(ctx context.Context, req *logical.Reque switch kind { case walAppKey: return b.rollbackAppWAL(ctx, req, data) - case walRemoveCreds: - return b.rollbackCredsWAL(ctx, req, data) + case walRotateRootCreds: + return b.rotateRootCredsWAL(ctx, req, data) default: return fmt.Errorf("unknown rollback type %q", kind) } From b46951d6ac9bd5f7cc905bf966df7904706c7b1b Mon Sep 17 00:00:00 2001 From: Michael Golowka <72365+pcman312@users.noreply.github.com> Date: Wed, 13 Oct 2021 16:12:42 -0600 Subject: [PATCH 16/39] Return default_expiration on config get --- path_config.go | 1 + 1 file changed, 1 insertion(+) diff --git a/path_config.go b/path_config.go index a154dc2..569448b 100644 --- a/path_config.go +++ b/path_config.go @@ -187,6 +187,7 @@ func (b *azureSecretBackend) pathConfigRead(ctx context.Context, req *logical.Re "environment": config.Environment, "client_id": config.ClientID, "use_microsoft_graph_api": config.UseMsGraphAPI, + "default_expiration": config.DefaultExpiration.Seconds(), }, } return addAADWarning(resp, config), nil From 23fadb92af450e233d87f69e58ec9c218610a965 Mon Sep 17 00:00:00 2001 From: Michael Golowka <72365+pcman312@users.noreply.github.com> Date: Wed, 13 Oct 2021 16:21:40 -0600 Subject: [PATCH 17/39] Return expiration from GET config --- path_config.go | 14 +++++++++++++- path_config_test.go | 20 ++++++++++++-------- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/path_config.go b/path_config.go index 569448b..6cbd8fe 100644 --- a/path_config.go +++ b/path_config.go @@ -5,6 +5,8 @@ import ( "errors" "time" + "github.com/hashicorp/vault/sdk/helper/parseutil" + "github.com/Azure/go-autorest/autorest/azure" multierror "github.com/hashicorp/go-multierror" "github.com/hashicorp/vault/sdk/framework" @@ -140,6 +142,16 @@ func (b *azureSecretBackend) pathConfigWrite(ctx context.Context, req *logical.R config.PasswordPolicy = data.Get("password_policy").(string) + exp, exists := data.GetOk("default_expiration") + if exists { + expiration, err := parseutil.ParseDurationSecond(exp.(string)) + if err != nil { + merr = multierror.Append(merr, err) + } else { + config.DefaultExpiration = expiration + } + } + if merr.ErrorOrNil() != nil { return logical.ErrorResponse(merr.Error()), nil } @@ -187,7 +199,7 @@ func (b *azureSecretBackend) pathConfigRead(ctx context.Context, req *logical.Re "environment": config.Environment, "client_id": config.ClientID, "use_microsoft_graph_api": config.UseMsGraphAPI, - "default_expiration": config.DefaultExpiration.Seconds(), + "default_expiration": int(config.DefaultExpiration.Seconds()), }, } return addAADWarning(resp, config), nil diff --git a/path_config_test.go b/path_config_test.go index 817488f..906a787 100644 --- a/path_config_test.go +++ b/path_config_test.go @@ -4,6 +4,7 @@ import ( "context" "reflect" "testing" + "time" "github.com/hashicorp/vault/sdk/logical" ) @@ -12,37 +13,38 @@ func TestConfig(t *testing.T) { b, s := getTestBackend(t, false) // Test valid config - config := map[string]interface{}{ + expectedConfig := map[string]interface{}{ "subscription_id": "a228ceec-bf1a-4411-9f95-39678d8cdb34", "tenant_id": "7ac36e27-80fc-4209-a453-e8ad83dc18c2", "client_id": "testClientId", "client_secret": "testClientSecret", "environment": "AZURECHINACLOUD", "use_microsoft_graph_api": false, + "default_expiration": int((24 * time.Hour).Seconds()), } - testConfigCreate(t, b, s, config) + testConfigCreate(t, b, s, expectedConfig) - delete(config, "client_secret") - testConfigRead(t, b, s, config) + delete(expectedConfig, "client_secret") + testConfigRead(t, b, s, expectedConfig) // Test test updating one element retains the others - config["tenant_id"] = "800e371d-ee51-4145-9ac8-5c43e4ceb79b" + expectedConfig["tenant_id"] = "800e371d-ee51-4145-9ac8-5c43e4ceb79b" configSubset := map[string]interface{}{ "tenant_id": "800e371d-ee51-4145-9ac8-5c43e4ceb79b", } testConfigCreate(t, b, s, configSubset) - testConfigUpdate(t, b, s, config) + testConfigUpdate(t, b, s, expectedConfig) // Test bad environment - config = map[string]interface{}{ + expectedConfig = map[string]interface{}{ "environment": "invalidEnv", } resp, _ := b.HandleRequest(context.Background(), &logical.Request{ Operation: logical.UpdateOperation, Path: "config", - Data: config, + Data: expectedConfig, Storage: s, }) @@ -62,6 +64,7 @@ func TestConfigDelete(t *testing.T) { "client_secret": "testClientSecret", "environment": "AZURECHINACLOUD", "use_microsoft_graph_api": false, + "default_expiration": int((24 * time.Hour).Seconds()), } testConfigCreate(t, b, s, config) @@ -87,6 +90,7 @@ func TestConfigDelete(t *testing.T) { "client_id": "", "environment": "", "use_microsoft_graph_api": false, + "default_expiration": 0, } testConfigRead(t, b, s, config) } From 853246451ccc4e95d268ef1f4a994dce8d187c06 Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Mon, 18 Oct 2021 16:21:56 -0400 Subject: [PATCH 18/39] Update path_rotate_root.go Co-authored-by: Jim Kalafut --- path_rotate_root.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/path_rotate_root.go b/path_rotate_root.go index c874264..d5a4b79 100644 --- a/path_rotate_root.go +++ b/path_rotate_root.go @@ -16,9 +16,6 @@ import ( func pathRotateRoot(b *azureSecretBackend) *framework.Path { return &framework.Path{ Pattern: "rotate-root", - Fields: map[string]*framework.FieldSchema{ - // None - }, Operations: map[logical.Operation]framework.OperationHandler{ logical.UpdateOperation: &framework.PathOperation{ Callback: b.pathRotateRoot, From bf9c235f9bf74f666981d7ce109b190f86baa902 Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Wed, 20 Oct 2021 12:59:00 -0400 Subject: [PATCH 19/39] Update per review --- path_config.go | 53 +++++++++++++++++++++------------------------ path_rotate_root.go | 4 ++-- 2 files changed, 27 insertions(+), 30 deletions(-) diff --git a/path_config.go b/path_config.go index 6cbd8fe..03b7002 100644 --- a/path_config.go +++ b/path_config.go @@ -5,8 +5,6 @@ import ( "errors" "time" - "github.com/hashicorp/vault/sdk/helper/parseutil" - "github.com/Azure/go-autorest/autorest/azure" multierror "github.com/hashicorp/go-multierror" "github.com/hashicorp/vault/sdk/framework" @@ -15,20 +13,24 @@ import ( const ( configStoragePath = "config" + // The default password expiration duration is 6 months in + // the Azure UI, so we're setting it to 6 months (in hours) + // as the default. + rootPasswordExpiration = 4380 ) // azureConfig contains values to configure Azure clients and // defaults for roles. The zero value is useful and results in // environments variable and system defaults being used. type azureConfig struct { - SubscriptionID string `json:"subscription_id"` - TenantID string `json:"tenant_id"` - ClientID string `json:"client_id"` - ClientSecret string `json:"client_secret"` - Environment string `json:"environment"` - PasswordPolicy string `json:"password_policy"` - UseMsGraphAPI bool `json:"use_microsoft_graph_api"` - DefaultExpiration time.Duration `json:"default_expiration"` + SubscriptionID string `json:"subscription_id"` + TenantID string `json:"tenant_id"` + ClientID string `json:"client_id"` + ClientSecret string `json:"client_secret"` + Environment string `json:"environment"` + PasswordPolicy string `json:"password_policy"` + UseMsGraphAPI bool `json:"use_microsoft_graph_api"` + RootPasswordExpiration time.Duration `json:"root_password_expiration"` } func pathConfig(b *azureSecretBackend) *framework.Path { @@ -68,10 +70,9 @@ func pathConfig(b *azureSecretBackend) *framework.Path { Type: framework.TypeBool, Description: "Enable usage of the Microsoft Graph API over the deprecated Azure AD Graph API.", }, - "default_expiration": &framework.FieldSchema{ - Type: framework.TypeString, - // 28 weeks (~6 months) -> days -> hours - Default: (28 * 7 * 24 * time.Hour).String(), + "root_password_expiration": &framework.FieldSchema{ + Type: framework.TypeDurationSecond, + Default: rootPasswordExpiration, Description: "The expiration date of the new credentials in Azure. This can be either a number of seconds or a time formatted duration (ex: 24h)", Required: false, }, @@ -142,14 +143,10 @@ func (b *azureSecretBackend) pathConfigWrite(ctx context.Context, req *logical.R config.PasswordPolicy = data.Get("password_policy").(string) - exp, exists := data.GetOk("default_expiration") - if exists { - expiration, err := parseutil.ParseDurationSecond(exp.(string)) - if err != nil { - merr = multierror.Append(merr, err) - } else { - config.DefaultExpiration = expiration - } + config.RootPasswordExpiration = time.Hour * time.Duration(rootPasswordExpiration) + rootExpirationRaw, ok := data.GetOk("root_password_expiration") + if ok { + config.RootPasswordExpiration = time.Second * time.Duration(rootExpirationRaw.(int)) } if merr.ErrorOrNil() != nil { @@ -194,12 +191,12 @@ func (b *azureSecretBackend) pathConfigRead(ctx context.Context, req *logical.Re resp := &logical.Response{ Data: map[string]interface{}{ - "subscription_id": config.SubscriptionID, - "tenant_id": config.TenantID, - "environment": config.Environment, - "client_id": config.ClientID, - "use_microsoft_graph_api": config.UseMsGraphAPI, - "default_expiration": int(config.DefaultExpiration.Seconds()), + "subscription_id": config.SubscriptionID, + "tenant_id": config.TenantID, + "environment": config.Environment, + "client_id": config.ClientID, + "use_microsoft_graph_api": config.UseMsGraphAPI, + "root_password_expiration": int(config.RootPasswordExpiration.Seconds()), }, } return addAADWarning(resp, config), nil diff --git a/path_rotate_root.go b/path_rotate_root.go index d5a4b79..9336cca 100644 --- a/path_rotate_root.go +++ b/path_rotate_root.go @@ -36,9 +36,9 @@ func (b *azureSecretBackend) pathRotateRoot(ctx context.Context, req *logical.Re return nil, err } - expDur := config.DefaultExpiration + expDur := config.RootPasswordExpiration if expDur == 0 { - expDur = 28 * 7 * 24 * time.Hour + expDur = rootPasswordExpiration } expiration := time.Now().Add(expDur) From a69381379a6b0d47ade19599e47459d901db7eb1 Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Wed, 20 Oct 2021 13:09:48 -0400 Subject: [PATCH 20/39] Rebase --- api/application_msgraph.go | 2 +- provider_mock_test.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/api/application_msgraph.go b/api/application_msgraph.go index 53853ae..2a75f79 100644 --- a/api/application_msgraph.go +++ b/api/application_msgraph.go @@ -326,7 +326,7 @@ func (c AppClient) createApplicationResponder(resp *http.Response) (ApplicationR autorest.ByUnmarshallingJSON(&result), autorest.ByClosing()) result.Response = autorest.Response{Response: resp} - return result, nil + return result, err } func (c AppClient) deleteApplicationPreparer(ctx context.Context, applicationObjectID string) (*http.Request, error) { diff --git a/provider_mock_test.go b/provider_mock_test.go index c9ee418..abc5583 100644 --- a/provider_mock_test.go +++ b/provider_mock_test.go @@ -10,7 +10,6 @@ import ( "time" "github.com/Azure/azure-sdk-for-go/profiles/latest/authorization/mgmt/authorization" - "github.com/Azure/azure-sdk-for-go/profiles/latest/compute/mgmt/compute" "github.com/Azure/go-autorest/autorest/date" "github.com/Azure/go-autorest/autorest/to" "github.com/hashicorp/vault-plugin-secrets-azure/api" From 77ae610eb458042839606dbc11fe2dd470a37ed4 Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Wed, 20 Oct 2021 13:12:14 -0400 Subject: [PATCH 21/39] Fix test --- path_config_test.go | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/path_config_test.go b/path_config_test.go index 906a787..e1edd4e 100644 --- a/path_config_test.go +++ b/path_config_test.go @@ -14,13 +14,13 @@ func TestConfig(t *testing.T) { // Test valid config expectedConfig := map[string]interface{}{ - "subscription_id": "a228ceec-bf1a-4411-9f95-39678d8cdb34", - "tenant_id": "7ac36e27-80fc-4209-a453-e8ad83dc18c2", - "client_id": "testClientId", - "client_secret": "testClientSecret", - "environment": "AZURECHINACLOUD", - "use_microsoft_graph_api": false, - "default_expiration": int((24 * time.Hour).Seconds()), + "subscription_id": "a228ceec-bf1a-4411-9f95-39678d8cdb34", + "tenant_id": "7ac36e27-80fc-4209-a453-e8ad83dc18c2", + "client_id": "testClientId", + "client_secret": "testClientSecret", + "environment": "AZURECHINACLOUD", + "use_microsoft_graph_api": false, + "root_password_expiration": int((24 * time.Hour).Seconds()), } testConfigCreate(t, b, s, expectedConfig) @@ -58,13 +58,13 @@ func TestConfigDelete(t *testing.T) { // Test valid config config := map[string]interface{}{ - "subscription_id": "a228ceec-bf1a-4411-9f95-39678d8cdb34", - "tenant_id": "7ac36e27-80fc-4209-a453-e8ad83dc18c2", - "client_id": "testClientId", - "client_secret": "testClientSecret", - "environment": "AZURECHINACLOUD", - "use_microsoft_graph_api": false, - "default_expiration": int((24 * time.Hour).Seconds()), + "subscription_id": "a228ceec-bf1a-4411-9f95-39678d8cdb34", + "tenant_id": "7ac36e27-80fc-4209-a453-e8ad83dc18c2", + "client_id": "testClientId", + "client_secret": "testClientSecret", + "environment": "AZURECHINACLOUD", + "use_microsoft_graph_api": false, + "root_password_expiration": int((24 * time.Hour).Seconds()), } testConfigCreate(t, b, s, config) @@ -85,12 +85,12 @@ func TestConfigDelete(t *testing.T) { } config = map[string]interface{}{ - "subscription_id": "", - "tenant_id": "", - "client_id": "", - "environment": "", - "use_microsoft_graph_api": false, - "default_expiration": 0, + "subscription_id": "", + "tenant_id": "", + "client_id": "", + "environment": "", + "use_microsoft_graph_api": false, + "root_password_expiration": 0, } testConfigRead(t, b, s, config) } From 9d5d3ae0141707895997147ba68fb6a545ad5f38 Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Wed, 20 Oct 2021 13:14:05 -0400 Subject: [PATCH 22/39] Revert "Rebase" This reverts commit a69381379a6b0d47ade19599e47459d901db7eb1. --- api/application_msgraph.go | 2 +- provider_mock_test.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/api/application_msgraph.go b/api/application_msgraph.go index 2a75f79..53853ae 100644 --- a/api/application_msgraph.go +++ b/api/application_msgraph.go @@ -326,7 +326,7 @@ func (c AppClient) createApplicationResponder(resp *http.Response) (ApplicationR autorest.ByUnmarshallingJSON(&result), autorest.ByClosing()) result.Response = autorest.Response{Response: resp} - return result, err + return result, nil } func (c AppClient) deleteApplicationPreparer(ctx context.Context, applicationObjectID string) (*http.Request, error) { diff --git a/provider_mock_test.go b/provider_mock_test.go index abc5583..c9ee418 100644 --- a/provider_mock_test.go +++ b/provider_mock_test.go @@ -10,6 +10,7 @@ import ( "time" "github.com/Azure/azure-sdk-for-go/profiles/latest/authorization/mgmt/authorization" + "github.com/Azure/azure-sdk-for-go/profiles/latest/compute/mgmt/compute" "github.com/Azure/go-autorest/autorest/date" "github.com/Azure/go-autorest/autorest/to" "github.com/hashicorp/vault-plugin-secrets-azure/api" From 699e815f30516fda43b285911a58710f13adb033 Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Wed, 20 Oct 2021 13:19:28 -0400 Subject: [PATCH 23/39] Remove named returns --- api/application_aad.go | 8 ++++---- api/application_msgraph.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/api/application_aad.go b/api/application_aad.go index c1403bd..b8433c7 100644 --- a/api/application_aad.go +++ b/api/application_aad.go @@ -64,7 +64,7 @@ func (a *ActiveDirectoryApplicationClient) ListApplications(ctx context.Context, return results, nil } -func (a *ActiveDirectoryApplicationClient) CreateApplication(ctx context.Context, displayName string) (result ApplicationResult, err error) { +func (a *ActiveDirectoryApplicationClient) CreateApplication(ctx context.Context, displayName string) (ApplicationResult, error) { appURL := fmt.Sprintf("https://%s", displayName) app, err := a.Client.Create(ctx, graphrbac.ApplicationCreateParameters{ @@ -95,7 +95,7 @@ func (a *ActiveDirectoryApplicationClient) DeleteApplication(ctx context.Context return nil } -func (a *ActiveDirectoryApplicationClient) AddApplicationPassword(ctx context.Context, applicationObjectID string, displayName string, endDateTime time.Time) (result PasswordCredentialResult, err error) { +func (a *ActiveDirectoryApplicationClient) AddApplicationPassword(ctx context.Context, applicationObjectID string, displayName string, endDateTime time.Time) (PasswordCredentialResult, error) { keyID, err := uuid.GenerateUUID() if err != nil { return PasswordCredentialResult{}, err @@ -139,7 +139,7 @@ func (a *ActiveDirectoryApplicationClient) AddApplicationPassword(ctx context.Co return PasswordCredentialResult{}, fmt.Errorf("error updating credentials: %w", err) } - result = PasswordCredentialResult{ + result := PasswordCredentialResult{ PasswordCredential: PasswordCredential{ DisplayName: to.StringPtr(displayName), StartDate: &now, @@ -151,7 +151,7 @@ func (a *ActiveDirectoryApplicationClient) AddApplicationPassword(ctx context.Co return result, nil } -func (a *ActiveDirectoryApplicationClient) RemoveApplicationPassword(ctx context.Context, applicationObjectID string, keyID string) (err error) { +func (a *ActiveDirectoryApplicationClient) RemoveApplicationPassword(ctx context.Context, applicationObjectID string, keyID string) error { // Load current credentials resp, err := a.Client.ListPasswordCredentials(ctx, applicationObjectID) if err != nil { diff --git a/api/application_msgraph.go b/api/application_msgraph.go index 53853ae..2a75f79 100644 --- a/api/application_msgraph.go +++ b/api/application_msgraph.go @@ -326,7 +326,7 @@ func (c AppClient) createApplicationResponder(resp *http.Response) (ApplicationR autorest.ByUnmarshallingJSON(&result), autorest.ByClosing()) result.Response = autorest.Response{Response: resp} - return result, nil + return result, err } func (c AppClient) deleteApplicationPreparer(ctx context.Context, applicationObjectID string) (*http.Request, error) { From d5b7a4bd81d5f142419dec6b8317fbab634046fe Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Wed, 20 Oct 2021 16:49:30 -0400 Subject: [PATCH 24/39] Update per review --- api/application_msgraph.go | 5 +++-- provider_mock_test.go | 1 - 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api/application_msgraph.go b/api/application_msgraph.go index 2a75f79..1848483 100644 --- a/api/application_msgraph.go +++ b/api/application_msgraph.go @@ -133,11 +133,12 @@ func (c *AppClient) DeleteApplication(ctx context.Context, applicationObjectID s return autorest.NewErrorWithError(err, "provider", "DeleteApplication", resp, "Failure sending request") } - return autorest.Respond( + err = autorest.Respond( resp, c.client.ByInspecting(), azure.WithErrorUnlessStatusCode(http.StatusNoContent, http.StatusNotFound), autorest.ByClosing()) + if err != nil { return autorest.NewErrorWithError(err, "provider", "DeleteApplication", resp, "Failure responding to request") } @@ -508,7 +509,7 @@ func (c *AppClient) setPasswordForServicePrincipal(ctx context.Context, spID str } reqBody := map[string]interface{}{ "startDateTime": startDate.UTC().Format("2006-01-02T15:04:05Z"), - "endDateTime": startDate.UTC().Format("2006-01-02T15:04:05Z"), + "endDateTime": endDate.UTC().Format("2006-01-02T15:04:05Z"), } preparer := c.GetPreparer( diff --git a/provider_mock_test.go b/provider_mock_test.go index c9ee418..abc5583 100644 --- a/provider_mock_test.go +++ b/provider_mock_test.go @@ -10,7 +10,6 @@ import ( "time" "github.com/Azure/azure-sdk-for-go/profiles/latest/authorization/mgmt/authorization" - "github.com/Azure/azure-sdk-for-go/profiles/latest/compute/mgmt/compute" "github.com/Azure/go-autorest/autorest/date" "github.com/Azure/go-autorest/autorest/to" "github.com/hashicorp/vault-plugin-secrets-azure/api" From 5ba6ffc4d30dd7444363aec647b888809f7adce2 Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Wed, 20 Oct 2021 17:03:43 -0400 Subject: [PATCH 25/39] Update path_config.go Co-authored-by: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> --- path_config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/path_config.go b/path_config.go index 03b7002..a2b20d0 100644 --- a/path_config.go +++ b/path_config.go @@ -143,7 +143,7 @@ func (b *azureSecretBackend) pathConfigWrite(ctx context.Context, req *logical.R config.PasswordPolicy = data.Get("password_policy").(string) - config.RootPasswordExpiration = time.Hour * time.Duration(rootPasswordExpiration) + config.RootPasswordExpiration = defaultRootPasswordExpiration * time.Hour rootExpirationRaw, ok := data.GetOk("root_password_expiration") if ok { config.RootPasswordExpiration = time.Second * time.Duration(rootExpirationRaw.(int)) From 33a8e60a12cb3da28075bacb6f27d26c2761837f Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Wed, 20 Oct 2021 17:08:34 -0400 Subject: [PATCH 26/39] Update per review --- path_config.go | 4 ++-- path_rotate_root.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/path_config.go b/path_config.go index a2b20d0..ece664d 100644 --- a/path_config.go +++ b/path_config.go @@ -16,7 +16,7 @@ const ( // The default password expiration duration is 6 months in // the Azure UI, so we're setting it to 6 months (in hours) // as the default. - rootPasswordExpiration = 4380 + defaultRootPasswordExpiration = 4380 ) // azureConfig contains values to configure Azure clients and @@ -72,7 +72,7 @@ func pathConfig(b *azureSecretBackend) *framework.Path { }, "root_password_expiration": &framework.FieldSchema{ Type: framework.TypeDurationSecond, - Default: rootPasswordExpiration, + Default: defaultRootPasswordExpiration, Description: "The expiration date of the new credentials in Azure. This can be either a number of seconds or a time formatted duration (ex: 24h)", Required: false, }, diff --git a/path_rotate_root.go b/path_rotate_root.go index 9336cca..f1b7b87 100644 --- a/path_rotate_root.go +++ b/path_rotate_root.go @@ -38,7 +38,7 @@ func (b *azureSecretBackend) pathRotateRoot(ctx context.Context, req *logical.Re expDur := config.RootPasswordExpiration if expDur == 0 { - expDur = rootPasswordExpiration + expDur = defaultRootPasswordExpiration } expiration := time.Now().Add(expDur) From 484eb75faa9f090951a58a028c03c2755398bc80 Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Mon, 25 Oct 2021 15:28:00 -0400 Subject: [PATCH 27/39] Use periodicFunc, change wal --- api/application_msgraph.go | 1 + backend.go | 76 +++++++++++++++- go.mod | 2 +- go.sum | 8 +- path_config.go | 57 +++++++----- path_rotate_root.go | 159 ++++++--------------------------- path_rotate_root_test.go | 176 ++++++++++--------------------------- provider_mock_test.go | 10 ++- wal.go | 63 +++++++++++-- 9 files changed, 255 insertions(+), 297 deletions(-) diff --git a/api/application_msgraph.go b/api/application_msgraph.go index 1848483..48d7067 100644 --- a/api/application_msgraph.go +++ b/api/application_msgraph.go @@ -93,6 +93,7 @@ func (c *AppClient) ListApplications(ctx context.Context, filter string) ([]Appl if err != nil { return nil, err } + return listAppResp.Value, nil } diff --git a/backend.go b/backend.go index f886475..5f88904 100644 --- a/backend.go +++ b/backend.go @@ -2,6 +2,7 @@ package azuresecrets import ( "context" + "fmt" "strings" "sync" "time" @@ -22,7 +23,8 @@ type azureSecretBackend struct { // Creating/deleting passwords against a single Application is a PATCH // operation that must be locked per Application Object ID. - appLocks []*locksutil.LockEntry + appLocks []*locksutil.LockEntry + updatePassword bool } func Factory(ctx context.Context, conf *logical.BackendConfig) (logical.Backend, error) { @@ -34,7 +36,9 @@ func Factory(ctx context.Context, conf *logical.BackendConfig) (logical.Backend, } func backend() *azureSecretBackend { - var b = azureSecretBackend{} + var b = azureSecretBackend{ + updatePassword: true, + } b.Backend = &framework.Backend{ Help: strings.TrimSpace(backendHelp), @@ -63,14 +67,80 @@ func backend() *azureSecretBackend { // Role assignment can take up to a few minutes, so ensure we don't try // to roll back during creation. WALRollbackMinAge: 10 * time.Minute, - } + PeriodicFunc: b.periodicFunc, + } b.getProvider = newAzureProvider b.appLocks = locksutil.CreateLocks() return &b } +func (b *azureSecretBackend) periodicFunc(ctx context.Context, sys *logical.Request) error { + if !b.updatePassword { + return nil + } + + config, err := b.getConfig(ctx, sys.Storage) + if err != nil { + return err + } + + if config.NewClientSecret != "" { + if config.NewClientSecretCreated.Add(time.Second * 60).After(time.Now()) { + b.Logger().Debug("periodic func", "new password detected, swapping in storage") + client, err := b.getClient(ctx, sys.Storage) + if err != nil { + return err + } + + b.Logger().Debug("periodic func", "getting object ID for root application") + apps, err := client.provider.ListApplications(ctx, fmt.Sprintf("appId eq '%s'", config.ClientID)) + if err != nil { + return err + } + + if len(apps) == 0 { + return fmt.Errorf("no application found") + } + if len(apps) > 1 { + return fmt.Errorf("multiple applications found - double check your client_id") + } + + app := apps[0] + + credsToDelete := []string{} + for _, cred := range app.PasswordCredentials { + if *cred.KeyID != config.NewClientSecretKeyID { + credsToDelete = append(credsToDelete, *cred.KeyID) + } + } + + b.Logger().Debug("periodic func", "removing old passwords from Azure") + err = removeApplicationPasswords(ctx, client.provider, *app.ID, credsToDelete...) + if err != nil { + return err + } + + b.Logger().Debug("periodic func", "updating config with new password") + config.ClientSecret = config.NewClientSecret + config.ClientSecretKeyID = config.NewClientSecretKeyID + config.NewClientSecret = "" + config.NewClientSecretKeyID = "" + config.NewClientSecretCreated = time.Time{} + } + + err := b.saveConfig(ctx, config, sys.Storage) + if err != nil { + return err + } + + b.updatePassword = false + } + + return nil +} + // reset clears the backend's cached client // This is used when the configuration changes and a new client should be // created with the updated settings. diff --git a/go.mod b/go.mod index a6140a9..b16306f 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,7 @@ require ( github.com/hashicorp/vault/api v1.0.5-0.20200215224050-f6547fa8e820 github.com/hashicorp/vault/sdk v0.1.14-0.20200527182800-ad90e0b39d2f github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d // indirect - github.com/kr/text v0.2.0 // indirect + github.com/kr/pretty v0.3.0 // indirect github.com/mattn/go-colorable v0.1.6 // indirect github.com/mitchellh/mapstructure v1.3.2 github.com/pierrec/lz4 v2.5.2+incompatible // indirect diff --git a/go.sum b/go.sum index 6168d0f..06299b2 100644 --- a/go.sum +++ b/go.sum @@ -130,8 +130,10 @@ github.com/hashicorp/yamux v0.0.0-20180604194846-3520598351bb/go.mod h1:+NfK9FKe github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d h1:kJCB4vdITiW1eC1vq2e6IsrXKrZit1bv/TDYFGMp4BQ= github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d/go.mod h1:+NfK9FKeTrX5uv1uIXGdwYDTeHna2qgaIlx54MXqjAM= github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k= -github.com/kr/pretty v0.2.0 h1:s5hAObm+yFO5uHYt5dYjxi2rXrsnmRpJx4OYvIWUaQs= +github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pretty v0.2.0/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= +github.com/kr/pretty v0.3.0 h1:WgNl7dwNpEZ6jJ9k1snq4pZsg7DOEN8hP9Xw0Tsjwk0= +github.com/kr/pretty v0.3.0/go.mod h1:640gp4NfQd8pI5XOwp5fnNeVWj67G7CFk/SaSQn7NBk= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= @@ -175,6 +177,8 @@ github.com/prometheus/client_model v0.0.0-20180712105110-5c3871d89910/go.mod h1: github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA= github.com/prometheus/common v0.0.0-20181126121408-4724e9255275/go.mod h1:daVV7qP5qjZbuso7PdcryaAu0sAZbrN9i7WWcTMWvro= github.com/prometheus/procfs v0.0.0-20181204211112-1dc9a6cbc91a/go.mod h1:c3At6R/oaqEKCNdg8wHV1ftS6bRYblBhIjjI8uT2IGk= +github.com/rogpeppe/go-internal v1.6.1 h1:/FiVV8dS/e+YqF2JvO3yXRFbBLTIuSDkuC7aBOAvL+k= +github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc= github.com/ryanuber/columnize v2.1.0+incompatible/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts= github.com/ryanuber/go-glob v1.0.0 h1:iQh3xXAumdQ+4Ufa5b25cRpC5TYKlno6hsv6Cb3pkBk= github.com/ryanuber/go-glob v1.0.0/go.mod h1:807d1WSdnB0XRJzKNil9Om6lcp/3a0v4qIHxIXzX/Yc= @@ -265,6 +269,8 @@ google.golang.org/protobuf v1.22.0/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2 google.golang.org/protobuf v1.23.1-0.20200526195155-81db48ad09cc/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU= google.golang.org/protobuf v1.24.0 h1:UhZDfRO8JRQru4/+LlLE0BRKGF8L+PICnvYZmx/fEGA= google.golang.org/protobuf v1.24.0/go.mod h1:r/3tXBNzIEhYS9I1OUVjXDlt8tc493IdKGjtUeSXeh4= +gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= gopkg.in/square/go-jose.v2 v2.3.1/go.mod h1:M9dMgbHiYLoDGQrXy7OpJDJWiKiU//h+vD76mk0e1AI= gopkg.in/square/go-jose.v2 v2.5.1 h1:7odma5RETjNHWJnR32wx8t+Io4djHE1PqxCFx3iiZ2w= gopkg.in/square/go-jose.v2 v2.5.1/go.mod h1:M9dMgbHiYLoDGQrXy7OpJDJWiKiU//h+vD76mk0e1AI= diff --git a/path_config.go b/path_config.go index ece664d..bef9e48 100644 --- a/path_config.go +++ b/path_config.go @@ -16,21 +16,26 @@ const ( // The default password expiration duration is 6 months in // the Azure UI, so we're setting it to 6 months (in hours) // as the default. - defaultRootPasswordExpiration = 4380 + defaultRootPasswordTTL = 4380 ) // azureConfig contains values to configure Azure clients and // defaults for roles. The zero value is useful and results in // environments variable and system defaults being used. type azureConfig struct { - SubscriptionID string `json:"subscription_id"` - TenantID string `json:"tenant_id"` - ClientID string `json:"client_id"` - ClientSecret string `json:"client_secret"` - Environment string `json:"environment"` - PasswordPolicy string `json:"password_policy"` - UseMsGraphAPI bool `json:"use_microsoft_graph_api"` - RootPasswordExpiration time.Duration `json:"root_password_expiration"` + SubscriptionID string `json:"subscription_id"` + TenantID string `json:"tenant_id"` + ClientID string `json:"client_id"` + ClientSecret string `json:"client_secret"` + ClientSecretKeyID string `json:"client_secret_key_id"` + NewClientSecret string `json:"new_client_secret"` + NewClientSecretCreated time.Time `json:"new_client_secret_created"` + NewClientSecretKeyID string `json:"new_client_secret_key_id"` + Environment string `json:"environment"` + PasswordPolicy string `json:"password_policy"` + UseMsGraphAPI bool `json:"use_microsoft_graph_api"` + RootPasswordTTL time.Duration `json:"root_password_ttl"` + RootPasswordExpirationDate string `json:"root_password_expiration_date` } func pathConfig(b *azureSecretBackend) *framework.Path { @@ -70,10 +75,15 @@ func pathConfig(b *azureSecretBackend) *framework.Path { Type: framework.TypeBool, Description: "Enable usage of the Microsoft Graph API over the deprecated Azure AD Graph API.", }, - "root_password_expiration": &framework.FieldSchema{ + "root_password_ttl": &framework.FieldSchema{ Type: framework.TypeDurationSecond, - Default: defaultRootPasswordExpiration, - Description: "The expiration date of the new credentials in Azure. This can be either a number of seconds or a time formatted duration (ex: 24h)", + Default: defaultRootPasswordTTL, + Description: "The TTL of the root password in Azure. This can be either a number of seconds or a time formatted duration (ex: 24h, 48ds)", + Required: false, + }, + "root_password_expiration_date": &framework.FieldSchema{ + Type: framework.TypeString, + Description: "The date when the root password will expire in Azure.", Required: false, }, }, @@ -143,10 +153,10 @@ func (b *azureSecretBackend) pathConfigWrite(ctx context.Context, req *logical.R config.PasswordPolicy = data.Get("password_policy").(string) - config.RootPasswordExpiration = defaultRootPasswordExpiration * time.Hour - rootExpirationRaw, ok := data.GetOk("root_password_expiration") + config.RootPasswordTTL = defaultRootPasswordTTL * time.Hour + rootExpirationRaw, ok := data.GetOk("root_password_ttl") if ok { - config.RootPasswordExpiration = time.Second * time.Duration(rootExpirationRaw.(int)) + config.RootPasswordTTL = time.Second * time.Duration(rootExpirationRaw.(int)) } if merr.ErrorOrNil() != nil { @@ -191,14 +201,19 @@ func (b *azureSecretBackend) pathConfigRead(ctx context.Context, req *logical.Re resp := &logical.Response{ Data: map[string]interface{}{ - "subscription_id": config.SubscriptionID, - "tenant_id": config.TenantID, - "environment": config.Environment, - "client_id": config.ClientID, - "use_microsoft_graph_api": config.UseMsGraphAPI, - "root_password_expiration": int(config.RootPasswordExpiration.Seconds()), + "subscription_id": config.SubscriptionID, + "tenant_id": config.TenantID, + "environment": config.Environment, + "client_id": config.ClientID, + "use_microsoft_graph_api": config.UseMsGraphAPI, + "root_password_ttl": int(config.RootPasswordTTL.Seconds()), }, } + + if config.RootPasswordExpirationDate != "" { + resp.Data["root_password_expiration_date"] = config.RootPasswordExpirationDate + } + return addAADWarning(resp, config), nil } diff --git a/path_rotate_root.go b/path_rotate_root.go index f1b7b87..09f7695 100644 --- a/path_rotate_root.go +++ b/path_rotate_root.go @@ -7,10 +7,8 @@ import ( "github.com/hashicorp/go-multierror" "github.com/hashicorp/go-uuid" - "github.com/hashicorp/vault-plugin-secrets-azure/api" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/logical" - "github.com/mitchellh/mapstructure" ) func pathRotateRoot(b *azureSecretBackend) *framework.Path { @@ -36,108 +34,71 @@ func (b *azureSecretBackend) pathRotateRoot(ctx context.Context, req *logical.Re return nil, err } - expDur := config.RootPasswordExpiration + expDur := config.RootPasswordTTL if expDur == 0 { - expDur = defaultRootPasswordExpiration + expDur = defaultRootPasswordTTL } expiration := time.Now().Add(expDur) - passCred, err := b.rotateRootCredentials(ctx, req.Storage, *config, expiration) + client, err := b.getClient(ctx, req.Storage) if err != nil { return nil, err } - resultData := map[string]interface{}{ - "secret_id": *passCred.KeyID, - "end_date": *passCred.EndDate, - } - if passCred.DisplayName != nil && *passCred.DisplayName != "" { - resultData["display_name"] = *passCred.DisplayName - } - - resp := &logical.Response{ - Data: resultData, - } - - return addAADWarning(resp, config), nil -} - -func (b *azureSecretBackend) rotateRootCredentials(ctx context.Context, storage logical.Storage, cfg azureConfig, expiration time.Time) (cred api.PasswordCredential, err error) { - client, err := b.getClient(ctx, storage) - if err != nil { - return api.PasswordCredential{}, err - } - // We need to use List instead of Get here because we don't have the Object ID // (which is different from the Application/Client ID) - apps, err := client.provider.ListApplications(ctx, fmt.Sprintf("appId eq '%s'", cfg.ClientID)) + apps, err := client.provider.ListApplications(ctx, fmt.Sprintf("appId eq '%s'", config.ClientID)) if err != nil { - return api.PasswordCredential{}, err + return nil, err } if len(apps) == 0 { - return api.PasswordCredential{}, fmt.Errorf("no application found") + return nil, fmt.Errorf("no application found") } if len(apps) > 1 { - return api.PasswordCredential{}, fmt.Errorf("multiple applications found - double check your client_id") + return nil, fmt.Errorf("multiple applications found - double check your client_id") } app := apps[0] - // Get a list of the current passwords to delete if adding a new credential succeeds. This list will not - // include the ID of the new password created via AddApplicationPassword - credsToDelete := []string{} - for _, cred := range app.PasswordCredentials { - credsToDelete = append(credsToDelete, *cred.KeyID) - } - uniqueID, err := uuid.GenerateUUID() if err != nil { - return api.PasswordCredential{}, fmt.Errorf("failed to generate UUID: %w", err) + return nil, fmt.Errorf("failed to generate UUID: %w", err) } // This could have the same username customization logic put on it if we really wanted it here passwordDisplayName := fmt.Sprintf("vault-%s", uniqueID) - newPasswordResp, err := client.provider.AddApplicationPassword(ctx, *app.ID, passwordDisplayName, expiration) if err != nil { - return api.PasswordCredential{}, fmt.Errorf("failed to add new password: %w", err) + return nil, fmt.Errorf("failed to add new password: %w", err) } - // Write a WAL with the new credential (and some other info) to write to the config later - // This is done because the new credential is typically not available to use IMMEDIATELY - // after creating it. This can create errors for callers so instead this will write - // the config and clean up old keys asynchronously after ~10m (backend.WALRollbackMinAge) - // This also will automatically retry because of the standard WAL behavior - wal := rotateCredsWAL{ - AppID: *app.ID, - NewSecret: *newPasswordResp.PasswordCredential.SecretText, - KeyIDsToRemove: credsToDelete, - Expiration: time.Now().Add(walRotateRootCredsExpiration), + wal := walRotateRoot{ + OldPassword: config.ClientSecret, + OldPasswordKeyID: config.ClientSecretKeyID, + OldPasswordExpirationDate: config.RootPasswordExpirationDate, } - _, walErr := framework.PutWAL(ctx, storage, walRotateRootCreds, wal) + walID, walErr := framework.PutWAL(ctx, req.Storage, walRotateRootCreds, wal) if walErr != nil { - // Remove the key since we failed to save the WAL to Vault storage. It's reasonable to assume that this call - // to Azure will succeed since the AddApplicationPassword call succeeded above. If it doesn't, we aren't going - // to do any retries via a WAL here since the current configuration will continue to work. - azureErr := client.provider.RemoveApplicationPassword(ctx, *app.ID, *newPasswordResp.PasswordCredential.KeyID) - merr := multierror.Append(err, azureErr) - return api.PasswordCredential{}, merr + err = client.provider.RemoveApplicationPassword(ctx, *app.ID, *newPasswordResp.PasswordCredential.KeyID) + merr := multierror.Append(err, err) + return &logical.Response{}, merr } - return newPasswordResp.PasswordCredential, nil -} + config.NewClientSecret = *newPasswordResp.SecretText + config.NewClientSecretCreated = time.Now() + config.NewClientSecretKeyID = *newPasswordResp.KeyID + + err = b.saveConfig(ctx, config, req.Storage) + if err != nil { + return nil, fmt.Errorf("failed to save new configuration: %w", err) + } -const walRotateRootCreds = "rotateRootCreds" -const walRotateRootCredsExpiration = 24 * time.Hour + b.updatePassword = true -type rotateCredsWAL struct { - AppID string - OldSecret string - NewSecret string - KeyIDsToRemove []string - Expiration time.Time + err = framework.DeleteWAL(ctx, req.Storage, walID) + return addAADWarning(&logical.Response{}, config), err } type passwordRemover interface { @@ -146,81 +107,17 @@ type passwordRemover interface { func removeApplicationPasswords(ctx context.Context, passRemover passwordRemover, appID string, passwordKeyIDs ...string) (err error) { merr := new(multierror.Error) - var remainingCreds []string for _, keyID := range passwordKeyIDs { // Attempt to remove all of them, don't fail early err := passRemover.RemoveApplicationPassword(ctx, appID, keyID) if err != nil { merr = multierror.Append(merr, err) - remainingCreds = append(remainingCreds, keyID) } } return merr.ErrorOrNil() } -func (b *azureSecretBackend) rotateRootCredsWAL(ctx context.Context, req *logical.Request, data interface{}) error { - entry := rotateCredsWAL{} - - d, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ - DecodeHook: mapstructure.StringToTimeHookFunc(time.RFC3339), - Result: &entry, - }) - if err != nil { - return err - } - err = d.Decode(data) - if err != nil { - return err - } - - if time.Now().After(entry.Expiration) { - b.Logger().Info("WAL for removing dangling credentials for root user has expired") - return nil - } - - cfg, err := b.getConfig(ctx, req.Storage) - if err != nil { - return fmt.Errorf("failed to load configuration: %w", err) - } - - cfg.ClientSecret = entry.NewSecret - - err = b.saveConfig(ctx, cfg, req.Storage) - if err != nil { - return fmt.Errorf("failed to save new configuration: %w", err) - } - - // b.saveConfig does a reset so this should get a new client with the updated creds - client, err := b.getClient(ctx, req.Storage) - if err != nil { - return err - } - - app, err := client.provider.GetApplication(ctx, entry.AppID) - if err != nil { - return fmt.Errorf("failed to retrieve existing application: %w", err) - } - - actualKeys := []string{} - for _, pc := range app.PasswordCredentials { - actualKeys = append(actualKeys, *pc.KeyID) - } - keysToRemove := intersectStrings(entry.KeyIDsToRemove, actualKeys) - if len(keysToRemove) == 0 { - return nil - } - - b.Logger().Debug("Attempting to remove dangling credentials for root user") - - err = removeApplicationPasswords(ctx, client.provider, entry.AppID, keysToRemove...) - if err != nil { - return err - } - b.Logger().Debug("Successfully removed dangling credentials for root user") - return nil -} - func intersectStrings(a []string, b []string) []string { if len(a) == 0 || len(b) == 0 { return []string{} diff --git a/path_rotate_root_test.go b/path_rotate_root_test.go index 7ddc094..401562d 100644 --- a/path_rotate_root_test.go +++ b/path_rotate_root_test.go @@ -5,154 +5,68 @@ import ( "fmt" "reflect" "testing" - "time" - "github.com/Azure/go-autorest/autorest/date" - "github.com/golang/mock/gomock" - "github.com/hashicorp/vault-plugin-secrets-azure/api" - "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/logical" ) -func TestRotateRootCredentials(t *testing.T) { - type testCase struct { - rawConfig map[string]interface{} - } - - tests := map[string]testCase{ - "AAD": { - rawConfig: map[string]interface{}{ - "subscription_id": generateUUID(), - "tenant_id": generateUUID(), - "client_id": testClientID, - "client_secret": testClientSecret, - "environment": "AZURECHINACLOUD", - "ttl": defaultTestTTL, - "max_ttl": defaultTestMaxTTL, - }, - }, - "MS-Graph": { - rawConfig: map[string]interface{}{ - "subscription_id": generateUUID(), - "tenant_id": generateUUID(), - "client_id": testClientID, - "client_secret": testClientSecret, - "environment": "AZURECHINACLOUD", - "ttl": defaultTestTTL, - "max_ttl": defaultTestMaxTTL, - "use_microsoft_graph_api": true, - }, - }, - } - - for name, test := range tests { - t.Run(name, func(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - b, storage := getTestBackend(t, false) - testConfigCreate(t, b, storage, test.rawConfig) - - ctx := context.Background() - - originalCfg, err := b.getConfig(ctx, storage) - assertErrorIsNil(t, err) - - now := time.Now() - - objID := "test-client-uuid" - keyID := "original-credential" - - apps := []api.ApplicationResult{ - { - AppID: &testClientID, - ID: &objID, - PasswordCredentials: []*api.PasswordCredential{ - { - DisplayName: strPtr("test-credential-01"), - StartDate: &date.Time{now.Add(-1 * time.Hour)}, - EndDate: &date.Time{now.Add(1 * time.Hour)}, - KeyID: &keyID, - }, - }, - }, - } - - expiration := now.Add(6 * time.Hour) - - mockProvider := NewMockAzureProvider(ctrl) - mockProvider.EXPECT().ListApplications(gomock.Any(), fmt.Sprintf("appId eq '%s'", testClientID)). - Return(apps, nil) - - newPasswordResult := api.PasswordCredentialResult{ - PasswordCredential: api.PasswordCredential{ - DisplayName: strPtr("vault-plugin-secrets-azure-someuuid"), - StartDate: &date.Time{now}, - EndDate: &date.Time{expiration}, - KeyID: strPtr("new-credential"), - SecretText: strPtr("myreallysecurepassword"), - }, - } - mockProvider.EXPECT().AddApplicationPassword(gomock.Any(), objID, gomock.Any(), expiration). - Return(newPasswordResult, nil) +func TestRotateRoot(t *testing.T) { + b, s := getTestBackend(t, true) - mockProvider.EXPECT().RemoveApplicationPassword(gomock.Any(), objID, keyID).Return(nil) + resp, err := b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.UpdateOperation, + Path: "rotate-root", + Data: map[string]interface{}{}, + Storage: s, + }) - mockProvider.EXPECT().GetApplication(gomock.Any(), objID).Return(apps[0], nil) - - b.getProvider = func(_ *clientSettings, _ bool, _ api.Passwords) (api.AzureProvider, error) { - return mockProvider, nil - } + if err != nil { + t.Fatal(err) + } - client, err := b.getClient(ctx, storage) - assertErrorIsNil(t, err) - assertNotNil(t, client) + if resp != nil && resp.IsError() { + t.Fatal(resp.Error()) + } - passCred, err := b.rotateRootCredentials(ctx, storage, *originalCfg, expiration) - assertErrorIsNil(t, err) + config, err := b.getConfig(context.Background(), s) + if err != nil { + t.Fatal(err) + } - expectedCred := newPasswordResult.PasswordCredential + if config.ClientSecret == "" { + t.Fatal(fmt.Errorf("root password was empty after rotate root, it shouldn't be")) + } - if !reflect.DeepEqual(passCred, expectedCred) { - t.Fatalf("Expected: %#v\nActual: %#v", expectedCred, passCred) - } + if config.NewClientSecret == config.ClientSecret { + t.Fatal("old and new password equal after rotate-root, it shouldn't be") + } - // Ensure the WAL exists - wals, err := framework.ListWAL(ctx, storage) - assertErrorIsNil(t, err) - if len(wals) == 0 { - t.Fatalf("Missing WAL for saving config & removing old passwords") - } - if len(wals) > 1 { - t.Fatalf("More than one WAL found") - } - walEntry, err := framework.GetWAL(ctx, storage, wals[0]) - assertErrorIsNil(t, err) + if config.NewClientSecret == "" { + t.Fatal("new password is empty, it shouldn't be") + } - if walEntry.Kind != walRotateRootCreds { - t.Fatalf("Actual WAL kind: %s Expected WAL kind: %s", walEntry.Kind, walRotateRootCreds) - } + if config.NewClientSecretKeyID == "" { + t.Fatal("new password key id is empty, it shouldn't be") + } - walReq := &logical.Request{ - Storage: storage, - } + if !b.updatePassword { + t.Fatal("update password is false, it shouldn't be") + } - // Force the WAL to execute - err = b.rotateRootCredsWAL(ctx, walReq, walEntry.Data) - assertErrorIsNil(t, err) + err = b.periodicFunc(context.Background(), &logical.Request{ + Storage: s, + }) - // Check the config - updatedCfg, err := b.getConfig(ctx, storage) - assertErrorIsNil(t, err) + if err != nil { + t.Fatal(err) + } - if reflect.DeepEqual(updatedCfg, originalCfg) { - t.Fatalf("New config should not equal the original config") - } + newConfig, err := b.getConfig(context.Background(), s) + if err != nil { + t.Fatal(err) + } - if updatedCfg.ClientSecret != *newPasswordResult.PasswordCredential.SecretText { - t.Fatalf("Expected client secret: %s Actual client secret: %s", *newPasswordResult.PasswordCredential.SecretText, updatedCfg.ClientSecret) - } - }) + if newConfig.ClientSecret != config.NewClientSecret { + t.Fatal(fmt.Errorf("old and new password aren't equal after periodic function, they should be")) } } diff --git a/provider_mock_test.go b/provider_mock_test.go index abc5583..7430f20 100644 --- a/provider_mock_test.go +++ b/provider_mock_test.go @@ -108,13 +108,21 @@ func (m *mockProvider) CreateApplication(_ context.Context, _ string) (api.Appli } func (m *mockProvider) GetApplication(_ context.Context, _ string) (api.ApplicationResult, error) { + appObjID := generateUUID() return api.ApplicationResult{ AppID: to.StringPtr("00000000-0000-0000-0000-000000000000"), + ID: &appObjID, }, nil } func (m *mockProvider) ListApplications(_ context.Context, _ string) ([]api.ApplicationResult, error) { - return nil, fmt.Errorf("not implemented") + appObjID := generateUUID() + return []api.ApplicationResult{ + { + AppID: to.StringPtr("00000000-0000-0000-0000-000000000000"), + ID: &appObjID, + }, + }, nil } func (m *mockProvider) DeleteApplication(_ context.Context, applicationObjectID string) error { diff --git a/wal.go b/wal.go index 5892a4d..1a0e6c7 100644 --- a/wal.go +++ b/wal.go @@ -9,28 +9,31 @@ import ( "github.com/mitchellh/mapstructure" ) -const walAppKey = "appCreate" +const ( + walAppKey = "appCreate" + walRotateRootCreds = "rotateRootCreds" +) // Eventually expire the WAL if for some reason the rollback operation consistently fails var maxWALAge = 24 * time.Hour -type walApp struct { - AppID string - AppObjID string - Expiration time.Time -} - func (b *azureSecretBackend) walRollback(ctx context.Context, req *logical.Request, kind string, data interface{}) error { switch kind { case walAppKey: return b.rollbackAppWAL(ctx, req, data) case walRotateRootCreds: - return b.rotateRootCredsWAL(ctx, req, data) + return b.rollbackRootWAL(ctx, req, data) default: return fmt.Errorf("unknown rollback type %q", kind) } } +type walApp struct { + AppID string + AppObjID string + Expiration time.Time +} + func (b *azureSecretBackend) rollbackAppWAL(ctx context.Context, req *logical.Request, data interface{}) error { // Decode the WAL data var entry walApp @@ -68,3 +71,47 @@ func (b *azureSecretBackend) rollbackAppWAL(ctx context.Context, req *logical.Re return nil } + +type walRotateRoot struct { + OldPassword string + OldPasswordKeyID string + OldPasswordExpirationDate string +} + +func (b *azureSecretBackend) rollbackRootWAL(ctx context.Context, req *logical.Request, data interface{}) error { + // Decode the WAL data + var entry walRotateRoot + d, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ + DecodeHook: mapstructure.StringToTimeHookFunc(time.RFC3339), + Result: &entry, + }) + if err != nil { + return err + } + err = d.Decode(data) + if err != nil { + return err + } + + b.Logger().Debug("rolling back root password in storage") + config, err := b.getConfig(ctx, req.Storage) + if err != nil { + return err + } + + config.ClientID = entry.OldPasswordKeyID + config.ClientSecret = entry.OldPassword + config.RootPasswordExpirationDate = entry.OldPasswordExpirationDate + config.NewClientSecret = "" + config.NewClientSecretCreated = time.Time{} + config.NewClientSecretKeyID = "" + + err = b.saveConfig(ctx, config, req.Storage) + if err != nil { + return err + } + + b.updatePassword = false + + return nil +} From 17e7da49c90318e772981ec934a68874102e0582 Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Mon, 25 Oct 2021 15:30:48 -0400 Subject: [PATCH 28/39] Fix config test --- path_config_test.go | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/path_config_test.go b/path_config_test.go index e1edd4e..f320159 100644 --- a/path_config_test.go +++ b/path_config_test.go @@ -14,13 +14,13 @@ func TestConfig(t *testing.T) { // Test valid config expectedConfig := map[string]interface{}{ - "subscription_id": "a228ceec-bf1a-4411-9f95-39678d8cdb34", - "tenant_id": "7ac36e27-80fc-4209-a453-e8ad83dc18c2", - "client_id": "testClientId", - "client_secret": "testClientSecret", - "environment": "AZURECHINACLOUD", - "use_microsoft_graph_api": false, - "root_password_expiration": int((24 * time.Hour).Seconds()), + "subscription_id": "a228ceec-bf1a-4411-9f95-39678d8cdb34", + "tenant_id": "7ac36e27-80fc-4209-a453-e8ad83dc18c2", + "client_id": "testClientId", + "client_secret": "testClientSecret", + "environment": "AZURECHINACLOUD", + "use_microsoft_graph_api": false, + "root_password_ttl": int((24 * time.Hour).Seconds()), } testConfigCreate(t, b, s, expectedConfig) @@ -58,13 +58,13 @@ func TestConfigDelete(t *testing.T) { // Test valid config config := map[string]interface{}{ - "subscription_id": "a228ceec-bf1a-4411-9f95-39678d8cdb34", - "tenant_id": "7ac36e27-80fc-4209-a453-e8ad83dc18c2", - "client_id": "testClientId", - "client_secret": "testClientSecret", - "environment": "AZURECHINACLOUD", - "use_microsoft_graph_api": false, - "root_password_expiration": int((24 * time.Hour).Seconds()), + "subscription_id": "a228ceec-bf1a-4411-9f95-39678d8cdb34", + "tenant_id": "7ac36e27-80fc-4209-a453-e8ad83dc18c2", + "client_id": "testClientId", + "client_secret": "testClientSecret", + "environment": "AZURECHINACLOUD", + "use_microsoft_graph_api": false, + "root_password_ttl": int((24 * time.Hour).Seconds()), } testConfigCreate(t, b, s, config) @@ -85,12 +85,12 @@ func TestConfigDelete(t *testing.T) { } config = map[string]interface{}{ - "subscription_id": "", - "tenant_id": "", - "client_id": "", - "environment": "", - "use_microsoft_graph_api": false, - "root_password_expiration": 0, + "subscription_id": "", + "tenant_id": "", + "client_id": "", + "environment": "", + "use_microsoft_graph_api": false, + "root_password_ttl": 0, } testConfigRead(t, b, s, config) } From f76c855bf68e44e7bb173c624c870c567cac02c3 Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Mon, 25 Oct 2021 17:05:12 -0400 Subject: [PATCH 29/39] Add expiration date, update logger --- backend.go | 7 +++---- path_config.go | 32 ++++++++++++++------------------ path_rotate_root.go | 1 + wal.go | 1 + 4 files changed, 19 insertions(+), 22 deletions(-) diff --git a/backend.go b/backend.go index 5f88904..d757364 100644 --- a/backend.go +++ b/backend.go @@ -88,13 +88,12 @@ func (b *azureSecretBackend) periodicFunc(ctx context.Context, sys *logical.Requ if config.NewClientSecret != "" { if config.NewClientSecretCreated.Add(time.Second * 60).After(time.Now()) { - b.Logger().Debug("periodic func", "new password detected, swapping in storage") + b.Logger().Debug("periodic func", "new", "new password detected, swapping in storage") client, err := b.getClient(ctx, sys.Storage) if err != nil { return err } - b.Logger().Debug("periodic func", "getting object ID for root application") apps, err := client.provider.ListApplications(ctx, fmt.Sprintf("appId eq '%s'", config.ClientID)) if err != nil { return err @@ -116,13 +115,13 @@ func (b *azureSecretBackend) periodicFunc(ctx context.Context, sys *logical.Requ } } - b.Logger().Debug("periodic func", "removing old passwords from Azure") + b.Logger().Debug("periodic func", "remove", "removing old passwords from Azure") err = removeApplicationPasswords(ctx, client.provider, *app.ID, credsToDelete...) if err != nil { return err } - b.Logger().Debug("periodic func", "updating config with new password") + b.Logger().Debug("periodic func", "updating", "updating config with new password") config.ClientSecret = config.NewClientSecret config.ClientSecretKeyID = config.NewClientSecretKeyID config.NewClientSecret = "" diff --git a/path_config.go b/path_config.go index bef9e48..7e01c27 100644 --- a/path_config.go +++ b/path_config.go @@ -23,19 +23,20 @@ const ( // defaults for roles. The zero value is useful and results in // environments variable and system defaults being used. type azureConfig struct { - SubscriptionID string `json:"subscription_id"` - TenantID string `json:"tenant_id"` - ClientID string `json:"client_id"` - ClientSecret string `json:"client_secret"` - ClientSecretKeyID string `json:"client_secret_key_id"` - NewClientSecret string `json:"new_client_secret"` - NewClientSecretCreated time.Time `json:"new_client_secret_created"` - NewClientSecretKeyID string `json:"new_client_secret_key_id"` - Environment string `json:"environment"` - PasswordPolicy string `json:"password_policy"` - UseMsGraphAPI bool `json:"use_microsoft_graph_api"` - RootPasswordTTL time.Duration `json:"root_password_ttl"` - RootPasswordExpirationDate string `json:"root_password_expiration_date` + SubscriptionID string `json:"subscription_id"` + TenantID string `json:"tenant_id"` + ClientID string `json:"client_id"` + ClientSecret string `json:"client_secret"` + ClientSecretKeyID string `json:"client_secret_key_id"` + NewClientSecret string `json:"new_client_secret"` + NewClientSecretCreated time.Time `json:"new_client_secret_created"` + NewClientSecretExpirationDate time.Time `json:"new_client_secret_expiration_date"` + NewClientSecretKeyID string `json:"new_client_secret_key_id"` + Environment string `json:"environment"` + PasswordPolicy string `json:"password_policy"` + UseMsGraphAPI bool `json:"use_microsoft_graph_api"` + RootPasswordTTL time.Duration `json:"root_password_ttl"` + RootPasswordExpirationDate string `json:"root_password_expiration_date` } func pathConfig(b *azureSecretBackend) *framework.Path { @@ -81,11 +82,6 @@ func pathConfig(b *azureSecretBackend) *framework.Path { Description: "The TTL of the root password in Azure. This can be either a number of seconds or a time formatted duration (ex: 24h, 48ds)", Required: false, }, - "root_password_expiration_date": &framework.FieldSchema{ - Type: framework.TypeString, - Description: "The date when the root password will expire in Azure.", - Required: false, - }, }, Operations: map[logical.Operation]framework.OperationHandler{ logical.ReadOperation: &framework.PathOperation{ diff --git a/path_rotate_root.go b/path_rotate_root.go index 09f7695..18ab001 100644 --- a/path_rotate_root.go +++ b/path_rotate_root.go @@ -88,6 +88,7 @@ func (b *azureSecretBackend) pathRotateRoot(ctx context.Context, req *logical.Re config.NewClientSecret = *newPasswordResp.SecretText config.NewClientSecretCreated = time.Now() + config.NewClientSecretExpirationDate = newPasswordResp.EndDate.Time config.NewClientSecretKeyID = *newPasswordResp.KeyID err = b.saveConfig(ctx, config, req.Storage) diff --git a/wal.go b/wal.go index 1a0e6c7..c28c3a9 100644 --- a/wal.go +++ b/wal.go @@ -104,6 +104,7 @@ func (b *azureSecretBackend) rollbackRootWAL(ctx context.Context, req *logical.R config.RootPasswordExpirationDate = entry.OldPasswordExpirationDate config.NewClientSecret = "" config.NewClientSecretCreated = time.Time{} + config.NewClientSecretExpirationDate = time.Time{} config.NewClientSecretKeyID = "" err = b.saveConfig(ctx, config, req.Storage) From 23e34b4daffd18f444cfa0ac4bc5a9629b6b2280 Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Tue, 26 Oct 2021 11:25:52 -0400 Subject: [PATCH 30/39] Fix timer bug --- backend.go | 97 ++++++++++++++++++++++++++++-------------------------- 1 file changed, 51 insertions(+), 46 deletions(-) diff --git a/backend.go b/backend.go index d757364..cf3bd3e 100644 --- a/backend.go +++ b/backend.go @@ -77,7 +77,10 @@ func backend() *azureSecretBackend { } func (b *azureSecretBackend) periodicFunc(ctx context.Context, sys *logical.Request) error { + b.Logger().Debug("periodic func", "debug", "tick") + if !b.updatePassword { + b.Logger().Debug("periodic func", "debug", "no rotate-root update") return nil } @@ -86,57 +89,59 @@ func (b *azureSecretBackend) periodicFunc(ctx context.Context, sys *logical.Requ return err } - if config.NewClientSecret != "" { - if config.NewClientSecretCreated.Add(time.Second * 60).After(time.Now()) { - b.Logger().Debug("periodic func", "new", "new password detected, swapping in storage") - client, err := b.getClient(ctx, sys.Storage) - if err != nil { - return err - } - - apps, err := client.provider.ListApplications(ctx, fmt.Sprintf("appId eq '%s'", config.ClientID)) - if err != nil { - return err - } - - if len(apps) == 0 { - return fmt.Errorf("no application found") - } - if len(apps) > 1 { - return fmt.Errorf("multiple applications found - double check your client_id") - } - - app := apps[0] - - credsToDelete := []string{} - for _, cred := range app.PasswordCredentials { - if *cred.KeyID != config.NewClientSecretKeyID { - credsToDelete = append(credsToDelete, *cred.KeyID) - } - } - - b.Logger().Debug("periodic func", "remove", "removing old passwords from Azure") - err = removeApplicationPasswords(ctx, client.provider, *app.ID, credsToDelete...) - if err != nil { - return err - } - - b.Logger().Debug("periodic func", "updating", "updating config with new password") - config.ClientSecret = config.NewClientSecret - config.ClientSecretKeyID = config.NewClientSecretKeyID - config.NewClientSecret = "" - config.NewClientSecretKeyID = "" - config.NewClientSecretCreated = time.Time{} - } + // Password should be at least a minute old before we process it + b.Logger().Debug("periodic func", "debug", !(time.Since(config.NewClientSecretCreated) > time.Minute)) + if config.NewClientSecret == "" || (time.Since(config.NewClientSecretCreated) > time.Minute) { + return nil + } - err := b.saveConfig(ctx, config, sys.Storage) - if err != nil { - return err + b.Logger().Debug("periodic func", "msg", "new password detected, swapping in storage") + client, err := b.getClient(ctx, sys.Storage) + if err != nil { + return err + } + + apps, err := client.provider.ListApplications(ctx, fmt.Sprintf("appId eq '%s'", config.ClientID)) + if err != nil { + return err + } + + if len(apps) == 0 { + return fmt.Errorf("no application found") + } + if len(apps) > 1 { + return fmt.Errorf("multiple applications found - double check your client_id") + } + + app := apps[0] + + credsToDelete := []string{} + for _, cred := range app.PasswordCredentials { + if *cred.KeyID != config.NewClientSecretKeyID { + credsToDelete = append(credsToDelete, *cred.KeyID) } + } - b.updatePassword = false + b.Logger().Debug("periodic func", "msg", "removing old passwords from Azure") + err = removeApplicationPasswords(ctx, client.provider, *app.ID, credsToDelete...) + if err != nil { + return err } + b.Logger().Debug("periodic func", "msg", "updating config with new password") + config.ClientSecret = config.NewClientSecret + config.ClientSecretKeyID = config.NewClientSecretKeyID + config.NewClientSecret = "" + config.NewClientSecretKeyID = "" + config.NewClientSecretCreated = time.Time{} + + err = b.saveConfig(ctx, config, sys.Storage) + if err != nil { + return err + } + + b.updatePassword = false + return nil } From a3aae7973bf32ca15d49b90f04bcb37d3af59204 Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Tue, 26 Oct 2021 11:35:59 -0400 Subject: [PATCH 31/39] Change root expiration to timestamp --- backend.go | 13 ++++++------- path_config.go | 4 ++-- wal.go | 2 +- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/backend.go b/backend.go index cf3bd3e..b792ea4 100644 --- a/backend.go +++ b/backend.go @@ -77,10 +77,9 @@ func backend() *azureSecretBackend { } func (b *azureSecretBackend) periodicFunc(ctx context.Context, sys *logical.Request) error { - b.Logger().Debug("periodic func", "debug", "tick") - + b.Logger().Debug("periodic func", "rotate-root", "starting periodic func") if !b.updatePassword { - b.Logger().Debug("periodic func", "debug", "no rotate-root update") + b.Logger().Debug("periodic func", "rotate-root", "no rotate-root update") return nil } @@ -90,12 +89,11 @@ func (b *azureSecretBackend) periodicFunc(ctx context.Context, sys *logical.Requ } // Password should be at least a minute old before we process it - b.Logger().Debug("periodic func", "debug", !(time.Since(config.NewClientSecretCreated) > time.Minute)) if config.NewClientSecret == "" || (time.Since(config.NewClientSecretCreated) > time.Minute) { return nil } - b.Logger().Debug("periodic func", "msg", "new password detected, swapping in storage") + b.Logger().Debug("periodic func", "rotate-root", "new password detected, swapping in storage") client, err := b.getClient(ctx, sys.Storage) if err != nil { return err @@ -122,15 +120,16 @@ func (b *azureSecretBackend) periodicFunc(ctx context.Context, sys *logical.Requ } } - b.Logger().Debug("periodic func", "msg", "removing old passwords from Azure") + b.Logger().Debug("periodic func", "rotate-root", "removing old passwords from Azure") err = removeApplicationPasswords(ctx, client.provider, *app.ID, credsToDelete...) if err != nil { return err } - b.Logger().Debug("periodic func", "msg", "updating config with new password") + b.Logger().Debug("periodic func", "rotate-root", "updating config with new password") config.ClientSecret = config.NewClientSecret config.ClientSecretKeyID = config.NewClientSecretKeyID + config.RootPasswordExpirationDate = config.NewClientSecretExpirationDate config.NewClientSecret = "" config.NewClientSecretKeyID = "" config.NewClientSecretCreated = time.Time{} diff --git a/path_config.go b/path_config.go index 7e01c27..b4aaf0a 100644 --- a/path_config.go +++ b/path_config.go @@ -36,7 +36,7 @@ type azureConfig struct { PasswordPolicy string `json:"password_policy"` UseMsGraphAPI bool `json:"use_microsoft_graph_api"` RootPasswordTTL time.Duration `json:"root_password_ttl"` - RootPasswordExpirationDate string `json:"root_password_expiration_date` + RootPasswordExpirationDate time.Time `json:"root_password_expiration_date` } func pathConfig(b *azureSecretBackend) *framework.Path { @@ -206,7 +206,7 @@ func (b *azureSecretBackend) pathConfigRead(ctx context.Context, req *logical.Re }, } - if config.RootPasswordExpirationDate != "" { + if !config.RootPasswordExpirationDate.IsZero() { resp.Data["root_password_expiration_date"] = config.RootPasswordExpirationDate } diff --git a/wal.go b/wal.go index c28c3a9..a053037 100644 --- a/wal.go +++ b/wal.go @@ -75,7 +75,7 @@ func (b *azureSecretBackend) rollbackAppWAL(ctx context.Context, req *logical.Re type walRotateRoot struct { OldPassword string OldPasswordKeyID string - OldPasswordExpirationDate string + OldPasswordExpirationDate time.Time } func (b *azureSecretBackend) rollbackRootWAL(ctx context.Context, req *logical.Request, data interface{}) error { From 7c9842fb19297ea05e7e53d6be20b232e030f806 Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Tue, 26 Oct 2021 11:42:38 -0400 Subject: [PATCH 32/39] Fix named returns --- api/application_msgraph.go | 8 ++++---- api/groups_aad.go | 4 ++-- api/passwords.go | 2 +- api/service_principals_aad.go | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/api/application_msgraph.go b/api/application_msgraph.go index 48d7067..e078ad6 100644 --- a/api/application_msgraph.go +++ b/api/application_msgraph.go @@ -146,7 +146,7 @@ func (c *AppClient) DeleteApplication(ctx context.Context, applicationObjectID s return nil } -func (c *AppClient) AddApplicationPassword(ctx context.Context, applicationObjectID string, displayName string, endDateTime time.Time) (result PasswordCredentialResult, err error) { +func (c *AppClient) AddApplicationPassword(ctx context.Context, applicationObjectID string, displayName string, endDateTime time.Time) (PasswordCredentialResult, error) { req, err := c.addPasswordPreparer(ctx, applicationObjectID, displayName, date.Time{endDateTime}) if err != nil { return PasswordCredentialResult{}, autorest.NewErrorWithError(err, "provider", "AddApplicationPassword", nil, "Failure preparing request") @@ -154,13 +154,13 @@ func (c *AppClient) AddApplicationPassword(ctx context.Context, applicationObjec resp, err := c.addPasswordSender(req) if err != nil { - result = PasswordCredentialResult{ + result := PasswordCredentialResult{ Response: autorest.Response{Response: resp}, } return result, autorest.NewErrorWithError(err, "provider", "AddApplicationPassword", resp, "Failure sending request") } - result, err = c.addPasswordResponder(resp) + result, err := c.addPasswordResponder(resp) if err != nil { return result, autorest.NewErrorWithError(err, "provider", "AddApplicationPassword", resp, "Failure responding to request") } @@ -168,7 +168,7 @@ func (c *AppClient) AddApplicationPassword(ctx context.Context, applicationObjec return result, nil } -func (c *AppClient) RemoveApplicationPassword(ctx context.Context, applicationObjectID string, keyID string) (err error) { +func (c *AppClient) RemoveApplicationPassword(ctx context.Context, applicationObjectID string, keyID string) error { req, err := c.removePasswordPreparer(ctx, applicationObjectID, keyID) if err != nil { return autorest.NewErrorWithError(err, "provider", "RemoveApplicationPassword", nil, "Failure preparing request") diff --git a/api/groups_aad.go b/api/groups_aad.go index b4d007c..61c0578 100644 --- a/api/groups_aad.go +++ b/api/groups_aad.go @@ -38,7 +38,7 @@ func (a ActiveDirectoryApplicationGroupsClient) RemoveGroupMember(ctx context.Co return err } -func (a ActiveDirectoryApplicationGroupsClient) GetGroup(ctx context.Context, objectID string) (result Group, err error) { +func (a ActiveDirectoryApplicationGroupsClient) GetGroup(ctx context.Context, objectID string) (Group, error) { resp, err := a.Client.Get(ctx, objectID) if err != nil { return Group{}, err @@ -57,7 +57,7 @@ func getGroupFromRBAC(resp graphrbac.ADGroup) Group { return grp } -func (a ActiveDirectoryApplicationGroupsClient) ListGroups(ctx context.Context, filter string) (result []Group, err error) { +func (a ActiveDirectoryApplicationGroupsClient) ListGroups(ctx context.Context, filter string) ([]Group, error) { resp, err := a.Client.List(ctx, filter) if err != nil { return nil, err diff --git a/api/passwords.go b/api/passwords.go index 7ac8b92..685e457 100644 --- a/api/passwords.go +++ b/api/passwords.go @@ -20,7 +20,7 @@ type Passwords struct { PolicyName string } -func (p Passwords) Generate(ctx context.Context) (password string, err error) { +func (p Passwords) Generate(ctx context.Context) (string, error) { if p.PolicyName == "" { return base62.Random(PasswordLength) } diff --git a/api/service_principals_aad.go b/api/service_principals_aad.go index 784f935..3dc3898 100644 --- a/api/service_principals_aad.go +++ b/api/service_principals_aad.go @@ -17,13 +17,13 @@ type AADServicePrincipalsClient struct { Passwords Passwords } -func (c AADServicePrincipalsClient) CreateServicePrincipal(ctx context.Context, appID string, startDate time.Time, endDate time.Time) (id string, password string, err error) { +func (c AADServicePrincipalsClient) CreateServicePrincipal(ctx context.Context, appID string, startDate time.Time, endDate time.Time) (string, string, error) { keyID, err := uuid.GenerateUUID() if err != nil { return "", "", err } - password, err = c.Passwords.Generate(ctx) + password, err := c.Passwords.Generate(ctx) if err != nil { return "", "", err } From 8571e52e7ec880391315808f2d4f8b06802f1ad8 Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Wed, 27 Oct 2021 10:26:03 -0400 Subject: [PATCH 33/39] Update backend.go Co-authored-by: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> --- backend.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend.go b/backend.go index b792ea4..44bb6eb 100644 --- a/backend.go +++ b/backend.go @@ -77,7 +77,7 @@ func backend() *azureSecretBackend { } func (b *azureSecretBackend) periodicFunc(ctx context.Context, sys *logical.Request) error { - b.Logger().Debug("periodic func", "rotate-root", "starting periodic func") + b.Logger().Debug("starting periodic func") if !b.updatePassword { b.Logger().Debug("periodic func", "rotate-root", "no rotate-root update") return nil From 7ffdfd4fa5bd4ad7aa0ad4e46868d6e0f7a5540d Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Wed, 27 Oct 2021 11:16:49 -0400 Subject: [PATCH 34/39] Update per feedback, add more tests --- backend.go | 24 +++++------- path_config.go | 2 +- path_rotate_root.go | 13 +++---- path_rotate_root_test.go | 81 +++++++++++++++++++++++++++++++++++++++- wal.go | 11 +----- 5 files changed, 99 insertions(+), 32 deletions(-) diff --git a/backend.go b/backend.go index 44bb6eb..3bbf24f 100644 --- a/backend.go +++ b/backend.go @@ -59,15 +59,9 @@ func backend() *azureSecretBackend { secretServicePrincipal(&b), secretStaticServicePrincipal(&b), }, - BackendType: logical.TypeLogical, - Invalidate: b.invalidate, - - WALRollback: b.walRollback, - - // Role assignment can take up to a few minutes, so ensure we don't try - // to roll back during creation. - WALRollbackMinAge: 10 * time.Minute, - + BackendType: logical.TypeLogical, + Invalidate: b.invalidate, + WALRollback: b.walRollback, PeriodicFunc: b.periodicFunc, } b.getProvider = newAzureProvider @@ -89,7 +83,7 @@ func (b *azureSecretBackend) periodicFunc(ctx context.Context, sys *logical.Requ } // Password should be at least a minute old before we process it - if config.NewClientSecret == "" || (time.Since(config.NewClientSecretCreated) > time.Minute) { + if config.NewClientSecret == "" || (time.Since(config.NewClientSecretCreated) < time.Minute) { return nil } @@ -120,10 +114,12 @@ func (b *azureSecretBackend) periodicFunc(ctx context.Context, sys *logical.Requ } } - b.Logger().Debug("periodic func", "rotate-root", "removing old passwords from Azure") - err = removeApplicationPasswords(ctx, client.provider, *app.ID, credsToDelete...) - if err != nil { - return err + if len(credsToDelete) != 0 { + b.Logger().Debug("periodic func", "rotate-root", "removing old passwords from Azure") + err = removeApplicationPasswords(ctx, client.provider, *app.ID, credsToDelete...) + if err != nil { + return err + } } b.Logger().Debug("periodic func", "rotate-root", "updating config with new password") diff --git a/path_config.go b/path_config.go index b4aaf0a..de6bd7d 100644 --- a/path_config.go +++ b/path_config.go @@ -36,7 +36,7 @@ type azureConfig struct { PasswordPolicy string `json:"password_policy"` UseMsGraphAPI bool `json:"use_microsoft_graph_api"` RootPasswordTTL time.Duration `json:"root_password_ttl"` - RootPasswordExpirationDate time.Time `json:"root_password_expiration_date` + RootPasswordExpirationDate time.Time `json:"root_password_expiration_date"` } func pathConfig(b *azureSecretBackend) *framework.Path { diff --git a/path_rotate_root.go b/path_rotate_root.go index 18ab001..c00d4a4 100644 --- a/path_rotate_root.go +++ b/path_rotate_root.go @@ -73,12 +73,7 @@ func (b *azureSecretBackend) pathRotateRoot(ctx context.Context, req *logical.Re return nil, fmt.Errorf("failed to add new password: %w", err) } - wal := walRotateRoot{ - OldPassword: config.ClientSecret, - OldPasswordKeyID: config.ClientSecretKeyID, - OldPasswordExpirationDate: config.RootPasswordExpirationDate, - } - + var wal walRotateRoot walID, walErr := framework.PutWAL(ctx, req.Storage, walRotateRootCreds, wal) if walErr != nil { err = client.provider.RemoveApplicationPassword(ctx, *app.ID, *newPasswordResp.PasswordCredential.KeyID) @@ -99,7 +94,11 @@ func (b *azureSecretBackend) pathRotateRoot(ctx context.Context, req *logical.Re b.updatePassword = true err = framework.DeleteWAL(ctx, req.Storage, walID) - return addAADWarning(&logical.Response{}, config), err + if err != nil { + b.Logger().Error("rotate root", "delete wal", err) + } + + return addAADWarning(&logical.Response{}, config), nil } type passwordRemover interface { diff --git a/path_rotate_root_test.go b/path_rotate_root_test.go index 401562d..d0f03d6 100644 --- a/path_rotate_root_test.go +++ b/path_rotate_root_test.go @@ -5,11 +5,12 @@ import ( "fmt" "reflect" "testing" + "time" "github.com/hashicorp/vault/sdk/logical" ) -func TestRotateRoot(t *testing.T) { +func TestRotateRootSuccess(t *testing.T) { b, s := getTestBackend(t, true) resp, err := b.HandleRequest(context.Background(), &logical.Request{ @@ -52,6 +53,12 @@ func TestRotateRoot(t *testing.T) { t.Fatal("update password is false, it shouldn't be") } + config.NewClientSecretCreated = config.NewClientSecretCreated.Add(-(time.Minute * 1)) + err = b.saveConfig(context.Background(), config, s) + if err != nil { + t.Fatal(err) + } + err = b.periodicFunc(context.Background(), &logical.Request{ Storage: s, }) @@ -70,6 +77,78 @@ func TestRotateRoot(t *testing.T) { } } +func TestRotateRootPeroidicFunctionBeforeMinute(t *testing.T) { + b, s := getTestBackend(t, true) + + resp, err := b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.UpdateOperation, + Path: "rotate-root", + Data: map[string]interface{}{}, + Storage: s, + }) + + if err != nil { + t.Fatal(err) + } + + if resp != nil && resp.IsError() { + t.Fatal(resp.Error()) + } + + tests := []struct { + Name string + Created time.Duration + }{ + { + Name: "1 second test:", + Created: time.Second * 1, + }, + { + Name: "5 seconds test:", + Created: time.Second * 5, + }, + { + Name: "30 seconds test:", + Created: time.Second * 30, + }, + { + Name: "50 seconds test:", + Created: time.Second * 50, + }, + } + + for _, test := range tests { + t.Log(test.Name) + config, err := b.getConfig(context.Background(), s) + if err != nil { + t.Fatal(err) + } + + config.NewClientSecretCreated = time.Now().Add(-(test.Created)) + err = b.saveConfig(context.Background(), config, s) + if err != nil { + t.Fatal(test.Name, err) + } + + err = b.periodicFunc(context.Background(), &logical.Request{ + Storage: s, + }) + + if err != nil { + t.Fatal(test.Name, err) + } + + newConfig, err := b.getConfig(context.Background(), s) + if err != nil { + t.Fatal(test.Name, err) + } + + if newConfig.ClientSecret == config.NewClientSecret { + t.Fatal(test.Name, fmt.Errorf("old and new password are equal after periodic function, they shouldn't be")) + } + } +} + func TestIntersectStrings(t *testing.T) { type testCase struct { a []string diff --git a/wal.go b/wal.go index a053037..82fdff6 100644 --- a/wal.go +++ b/wal.go @@ -72,11 +72,7 @@ func (b *azureSecretBackend) rollbackAppWAL(ctx context.Context, req *logical.Re return nil } -type walRotateRoot struct { - OldPassword string - OldPasswordKeyID string - OldPasswordExpirationDate time.Time -} +type walRotateRoot struct{} func (b *azureSecretBackend) rollbackRootWAL(ctx context.Context, req *logical.Request, data interface{}) error { // Decode the WAL data @@ -93,15 +89,12 @@ func (b *azureSecretBackend) rollbackRootWAL(ctx context.Context, req *logical.R return err } - b.Logger().Debug("rolling back root password in storage") + b.Logger().Debug("rolling back config") config, err := b.getConfig(ctx, req.Storage) if err != nil { return err } - config.ClientID = entry.OldPasswordKeyID - config.ClientSecret = entry.OldPassword - config.RootPasswordExpirationDate = entry.OldPasswordExpirationDate config.NewClientSecret = "" config.NewClientSecretCreated = time.Time{} config.NewClientSecretExpirationDate = time.Time{} From de931615fd52329b2242624587d7f2690564138c Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Wed, 27 Oct 2021 11:26:36 -0400 Subject: [PATCH 35/39] Add wal min age --- backend.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/backend.go b/backend.go index 3bbf24f..a95ea57 100644 --- a/backend.go +++ b/backend.go @@ -59,8 +59,13 @@ func backend() *azureSecretBackend { secretServicePrincipal(&b), secretStaticServicePrincipal(&b), }, - BackendType: logical.TypeLogical, - Invalidate: b.invalidate, + BackendType: logical.TypeLogical, + Invalidate: b.invalidate, + + // Role assignment can take up to a few minutes, so ensure we don't try + // to roll back during creation. + WALRollbackMinAge: 10 * time.Minute, + WALRollback: b.walRollback, PeriodicFunc: b.periodicFunc, } From 5ac5c26eedaddc1c0f5b03b4d92aa770d2081e5e Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Wed, 27 Oct 2021 11:32:01 -0400 Subject: [PATCH 36/39] Update mock --- go.mod | 2 +- go.sum | 23 ++++++++++++++++++----- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index 36e1e1e..19415bd 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/Azure/go-autorest/autorest/validation v0.2.0 // indirect github.com/fatih/color v1.9.0 // indirect github.com/go-test/deep v1.0.8 - github.com/golang/mock v1.1.1 + github.com/golang/mock v1.6.0 github.com/hashicorp/go-hclog v1.0.0 github.com/hashicorp/go-multierror v1.1.1 github.com/hashicorp/go-secure-stdlib/base62 v0.1.2 diff --git a/go.sum b/go.sum index 90d2986..426dc73 100644 --- a/go.sum +++ b/go.sum @@ -121,8 +121,9 @@ github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7a github.com/gogo/protobuf v1.2.1/go.mod h1:hp+jE20tsWTFYpLwKvXlhS1hjn+gTNwPg2I6zVXpSg4= github.com/gogo/protobuf v1.3.1/go.mod h1:SlYgWuQ5SjCEi6WLHjHCa1yvBfUnHcTbrrZtXPKa29o= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= -github.com/golang/mock v1.1.1 h1:G5FRp8JnTd7RQH5kemVNlMeyXQAztQ3mOWV95KxsXH8= github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A= +github.com/golang/mock v1.6.0 h1:ErTB+efbowRARo13NNdxyJji2egdxLGQhRaY+DUumQc= +github.com/golang/mock v1.6.0/go.mod h1:p6yTPP+5HYm5mzsMV8JkE6ZKdX+/wYM6Hr+LicevLPs= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= @@ -323,6 +324,7 @@ github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5Cc github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/tv42/httpunix v0.0.0-20150427012821-b75d8614f926/go.mod h1:9ESjWnEqriFuLhtthL60Sar/7RFoluCcXsuvEwTV5KM= github.com/urfave/cli v0.0.0-20171014202726-7bc6a0acffa5/go.mod h1:70zkFmudgCuE/ngEzBv17Jvp/497gISqfk5gWijbERA= +github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= go.opencensus.io v0.22.0/go.mod h1:+kGneAE2xo2IficOXnaByMWTGM9T73dGwxeWcUqIpI8= go.uber.org/atomic v1.6.0 h1:Ezj3JGmsOnG1MoRWQkPBsKLe9DwWD9QeXzTRzzldNVk= go.uber.org/atomic v1.6.0/go.mod h1:sABNBOSYdrvTF6hTgEIbc7YasKWGhgEQZyfxyTvoXHQ= @@ -330,6 +332,7 @@ golang.org/x/crypto v0.0.0-20171113213409-9f005a07e0d3/go.mod h1:6SG95UA2DQfeDnf golang.org/x/crypto v0.0.0-20180904163835-0709b304e793/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20190418165655-df01cb2cc480/go.mod h1:WFFai1msRO1wXaEeE5yQxYXgSfI8pQAWXbQop6sCtWE= +golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200604202706-70a84ac30bf9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20201002170205-7f63de1d35b0/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20210513164829-c07d793c2f9a h1:kr2P4QFmQr29mSLA43kwrOcgcReGTfbE9N577tCTuBc= @@ -340,6 +343,8 @@ golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvx golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= golang.org/x/lint v0.0.0-20190930215403-16217165b5de h1:5hukYrvBGR8/eNkX5mdUezrA6JiaEZDtJb9Ei+1LlBs= golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= +golang.org/x/mod v0.4.2 h1:Gz96sIWK3OalVv/I/qNygP42zyoKp3xptRVCWRFEBvo= +golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -355,8 +360,9 @@ golang.org/x/net v0.0.0-20190813141303-74dc4d7220e7/go.mod h1:z5CRVTTTmAJ677TzLL golang.org/x/net v0.0.0-20191004110552-13f9640d40b9/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20200202094626-16171245cfb2/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20200602114024-627f9648deb9/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A= -golang.org/x/net v0.0.0-20210226172049-e18ecbb05110 h1:qWPm9rbaAMKs8Bq/9LRpbMqxWRVUAQwMI9fVrssnTfw= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= +golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4 h1:4nGaVu0QrbjT/AK2PRLuQfQuh6DJve+pELhqTdAj3x0= +golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -364,6 +370,7 @@ golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20190227155943-e225da77a7e6/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20180823144017-11551d06cbcc/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -385,8 +392,10 @@ golang.org/x/sys v0.0.0-20200122134326-e047566fdf82/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20200223170610-d5e6a3e2c0ae/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200602225109-6fdc65e7d980/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20201119102817-f84b799fce68 h1:nxC68pudNYkKU6jWhgrqdreuFiOQWj1Fs7T3VrH4Pjw= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210510120138-977fb7262007 h1:gG67DSER+11cZvqIMb8S8bt0vZtiN6xWYARwirrOSfE= +golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= @@ -403,11 +412,15 @@ golang.org/x/tools v0.0.0-20190226205152-f727befe758c/go.mod h1:9Yl7xja0Znq3iFh3 golang.org/x/tools v0.0.0-20190311212946-11955173bddd/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs= golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q= golang.org/x/tools v0.0.0-20190624222133-a101b041ded4/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc= -golang.org/x/tools v0.0.0-20191029041327-9cc4af7d6b2c h1:IGkKhmfzcztjm6gYkykvu/NiS8kaqbCWAEWWAyf8J5U= golang.org/x/tools v0.0.0-20191029041327-9cc4af7d6b2c/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= +golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= +golang.org/x/tools v0.1.1 h1:wGiQel/hW0NnEkJUk8lbzkX2gFJU6PFxf1v5OlCfuOs= +golang.org/x/tools v0.1.1/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= +golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE= +golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM= google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4= google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8/go.mod h1:JiN7NxoALGmiZfu7CAH4rXhgtRTLTxftemlI0sWmxmc= From ac58246639a774eeeedbf9e9e2f695f4a8cc5db1 Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Wed, 27 Oct 2021 11:33:26 -0400 Subject: [PATCH 37/39] Update go version --- go.mod | 2 +- go.sum | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 19415bd..d48ca57 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/hashicorp/vault-plugin-secrets-azure -go 1.12 +go 1.16 require ( github.com/Azure/azure-sdk-for-go v58.3.0+incompatible diff --git a/go.sum b/go.sum index 426dc73..0b6c08a 100644 --- a/go.sum +++ b/go.sum @@ -343,7 +343,6 @@ golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvx golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= golang.org/x/lint v0.0.0-20190930215403-16217165b5de h1:5hukYrvBGR8/eNkX5mdUezrA6JiaEZDtJb9Ei+1LlBs= golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= -golang.org/x/mod v0.4.2 h1:Gz96sIWK3OalVv/I/qNygP42zyoKp3xptRVCWRFEBvo= golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -453,7 +452,6 @@ gopkg.in/airbrake/gobrake.v2 v2.0.9/go.mod h1:/h5ZAUhDkGaJfjzjKLSjv6zCL6O0LLBxU4 gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo= gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys= From 495d8d8258282a0c9894fdfbd81be2766647cf42 Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Wed, 27 Oct 2021 11:46:41 -0400 Subject: [PATCH 38/39] Revert "Update go version" This reverts commit ac58246639a774eeeedbf9e9e2f695f4a8cc5db1. --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index d48ca57..19415bd 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/hashicorp/vault-plugin-secrets-azure -go 1.16 +go 1.12 require ( github.com/Azure/azure-sdk-for-go v58.3.0+incompatible diff --git a/go.sum b/go.sum index 0b6c08a..426dc73 100644 --- a/go.sum +++ b/go.sum @@ -343,6 +343,7 @@ golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvx golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= golang.org/x/lint v0.0.0-20190930215403-16217165b5de h1:5hukYrvBGR8/eNkX5mdUezrA6JiaEZDtJb9Ei+1LlBs= golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= +golang.org/x/mod v0.4.2 h1:Gz96sIWK3OalVv/I/qNygP42zyoKp3xptRVCWRFEBvo= golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -452,6 +453,7 @@ gopkg.in/airbrake/gobrake.v2 v2.0.9/go.mod h1:/h5ZAUhDkGaJfjzjKLSjv6zCL6O0LLBxU4 gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo= gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys= From 723696969260d4a896267e6b695d9d5614495de2 Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Thu, 28 Oct 2021 10:15:06 -0400 Subject: [PATCH 39/39] Remove unused wal code --- wal.go | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/wal.go b/wal.go index 82fdff6..6cdeeff 100644 --- a/wal.go +++ b/wal.go @@ -75,20 +75,6 @@ func (b *azureSecretBackend) rollbackAppWAL(ctx context.Context, req *logical.Re type walRotateRoot struct{} func (b *azureSecretBackend) rollbackRootWAL(ctx context.Context, req *logical.Request, data interface{}) error { - // Decode the WAL data - var entry walRotateRoot - d, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ - DecodeHook: mapstructure.StringToTimeHookFunc(time.RFC3339), - Result: &entry, - }) - if err != nil { - return err - } - err = d.Decode(data) - if err != nil { - return err - } - b.Logger().Debug("rolling back config") config, err := b.getConfig(ctx, req.Storage) if err != nil {