Skip to content

Commit

Permalink
fix wrong ID usage in rotate root
Browse files Browse the repository at this point in the history
  • Loading branch information
vinay-gopalan committed Nov 14, 2023
1 parent 9a88c68 commit 8f3888d
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 41 deletions.
36 changes: 16 additions & 20 deletions api/applications.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,18 +187,21 @@ func getPasswordCredentialsForApplication(app models.Applicationable) []Password
return appCredentials
}

func ptrToString(s *string) string {
if s != nil {
return *s
}
return ""
}

func getApplicationResponse(app models.Applicationable) Application {
if app != nil {
appID := app.GetAppId()
appObjectID := app.GetId()

if appID != nil && appObjectID != nil {
return Application{
AppID: *appID,
AppObjectID: *appObjectID,
PasswordCredentials: getPasswordCredentialsForApplication(app),
}
return Application{
AppID: ptrToString(app.GetAppId()),
AppObjectID: ptrToString(app.GetId()),
PasswordCredentials: getPasswordCredentialsForApplication(app),
}

}

// return zero-value result if app in nil
Expand All @@ -212,18 +215,11 @@ func getApplicationResponse(app models.Applicationable) Application {

func getPasswordCredentialResponse(cred models.PasswordCredentialable) PasswordCredential {
if cred != nil {
secretText := cred.GetSecretText()
endDate := cred.GetEndDateTime()
keyID := cred.GetKeyId()

if secretText != nil && endDate != nil && keyID != nil {
return PasswordCredential{
SecretText: *secretText,
EndDate: *endDate,
KeyID: keyID.String(),
}
return PasswordCredential{
SecretText: ptrToString(cred.GetSecretText()),
EndDate: *cred.GetEndDateTime(),
KeyID: cred.GetKeyId().String(),
}

}
return PasswordCredential{
SecretText: "",
Expand Down
11 changes: 3 additions & 8 deletions api/groups.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,9 @@ func (c *MSGraphClient) ListGroups(ctx context.Context, filter string) ([]Group,

func getGroupResponse(group models.Groupable) Group {
if group != nil {
groupID := group.GetId()
name := group.GetDisplayName()

if groupID != nil && name != nil {
return Group{
ID: *groupID,
DisplayName: *name,
}
return Group{
ID: ptrToString(group.GetId()),
DisplayName: ptrToString(group.GetDisplayName()),
}
}

Expand Down
11 changes: 3 additions & 8 deletions api/service_principals.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,15 +97,10 @@ func (c *MSGraphClient) GetServicePrincipalByID(ctx context.Context, spObjectID

func getServicePrincipalResponse(sp models.ServicePrincipalable) ServicePrincipal {
if sp != nil {
spID := sp.GetId()
spAppID := sp.GetAppId()
if spID != nil && spAppID != nil {
return ServicePrincipal{
ID: *spID,
AppID: *spAppID,
}
return ServicePrincipal{
ID: ptrToString(sp.GetId()),
AppID: ptrToString(sp.GetAppId()),
}

}
return ServicePrincipal{
ID: "",
Expand Down
2 changes: 1 addition & 1 deletion backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ func (b *azureSecretBackend) periodicFunc(ctx context.Context, sys *logical.Requ

if len(credsToDelete) != 0 {
b.Logger().Debug("periodic func", "rotate-root", "removing old passwords from Azure")
err = removeApplicationPasswords(ctx, client.provider, app.AppID, credsToDelete...)
err = removeApplicationPasswords(ctx, client.provider, app.AppObjectID, credsToDelete...)
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions path_rotate_root.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,15 @@ func (b *azureSecretBackend) pathRotateRoot(ctx context.Context, req *logical.Re

// 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.AppID, passwordDisplayName, expiration)
newPasswordResp, err := client.provider.AddApplicationPassword(ctx, app.AppObjectID, passwordDisplayName, expiration)
if err != nil {
return nil, fmt.Errorf("failed to add new password: %w", err)
}

var wal walRotateRoot
walID, walErr := framework.PutWAL(ctx, req.Storage, walRotateRootCreds, wal)
if walErr != nil {
err = client.provider.RemoveApplicationPassword(ctx, app.AppID, newPasswordResp.KeyID)
err = client.provider.RemoveApplicationPassword(ctx, app.AppObjectID, newPasswordResp.KeyID)
merr := multierror.Append(err, err)
return &logical.Response{}, merr
}
Expand Down
4 changes: 2 additions & 2 deletions path_service_principal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -777,13 +777,13 @@ func TestRoleAssignmentWALRollback(t *testing.T) {
ra, err := provider.raClient.GetByID(context.Background(), raIDs[0], nil)
assertErrorIsNil(t, err)

roleDefs, err := provider.ListRoleDefinitions(nil, fmt.Sprintf("subscriptions/%s", subscriptionID), "")
roleDefs, err := provider.ListRoleDefinitions(context.Background(), fmt.Sprintf("subscriptions/%s", subscriptionID), "")
assertErrorIsNil(t, err)

defID := *ra.Properties.RoleDefinitionID
found := false
for _, def := range roleDefs {
if *def.ID == defID && *def.Name == "Storage Blob Data Owner" {
if *def.ID == defID && *def.Properties.RoleName == "Storage Blob Data Owner" {
found = true
break
}
Expand Down

0 comments on commit 8f3888d

Please sign in to comment.