Skip to content

Commit

Permalink
feat: use secret management caveat wrapper
Browse files Browse the repository at this point in the history
This is preparation for using WithLease in the secrets service. Instead
of doing a point in time check and return, we return a function for use
to execute logic requiring secret management.

At present the caveat is always permissive if we have passed checks, but
we will use WithLease to ensure the checks hold for the duration of
execution.
  • Loading branch information
manadart committed Nov 29, 2024
1 parent 4ae0453 commit 73f2262
Show file tree
Hide file tree
Showing 5 changed files with 211 additions and 153 deletions.
49 changes: 35 additions & 14 deletions domain/secret/service/access.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,14 @@ func (s *SecretService) getSecretAccess(ctx context.Context, uri *secrets.URI, a
// satisfying [secreterrors.InvalidSecretPermissionChange] is returned.
// It returns [secreterrors.PermissionDenied] if the secret cannot be managed by the accessor.
func (s *SecretService) GrantSecretAccess(ctx context.Context, uri *secrets.URI, params SecretAccessParams) error {
if err := s.canManage(ctx, uri, params.Accessor, params.LeaderToken); err != nil {
withCaveat, err := s.getManagementCaveat(ctx, uri, params.Accessor, params.LeaderToken)
if err != nil {
return errors.Trace(err)
}
return s.secretState.GrantAccess(ctx, uri, grantParams(params))

return withCaveat(ctx, func(innerCtx context.Context) error {
return s.secretState.GrantAccess(innerCtx, uri, grantParams(params))
})
}

func grantParams(in SecretAccessParams) domainsecret.GrantParams {
Expand Down Expand Up @@ -168,7 +172,8 @@ func grantParams(in SecretAccessParams) domainsecret.GrantParams {
// RevokeSecretAccess revokes access to the secret for the specified subject.
// It returns an error satisfying [secreterrors.SecretNotFound] if the secret is not found.
func (s *SecretService) RevokeSecretAccess(ctx context.Context, uri *secrets.URI, params SecretAccessParams) error {
if err := s.canManage(ctx, uri, params.Accessor, params.LeaderToken); err != nil {
withCaveat, err := s.getManagementCaveat(ctx, uri, params.Accessor, params.LeaderToken)
if err != nil {
return errors.Trace(err)
}

Expand All @@ -186,29 +191,42 @@ func (s *SecretService) RevokeSecretAccess(ctx context.Context, uri *secrets.URI
p.SubjectTypeID = domainsecret.SubjectModel
}

return s.secretState.RevokeAccess(ctx, uri, p)
return withCaveat(ctx, func(innerCtx context.Context) error {
return s.secretState.RevokeAccess(innerCtx, uri, p)
})
}

// canManage checks that the accessor can manage the secret.
// If the request is for a secret owned by an application, the unit must be the leader.
func (s *SecretService) canManage(
// getManagementCaveat returns a function within which an operation can be
// executed if the caveat remains satisfied.
// If the secret is unit-owned and the unit can manage it, the caveat is always
// permissive.
// If the secret is application-owned, the unit must be, and remain the leader
// of that application.
// If the caveat can never be satisfied, an error is returned - the input
// accessor can never manage the input secret.
func (s *SecretService) getManagementCaveat(
ctx context.Context,
uri *secrets.URI, assessor SecretAccessor,
leaderToken leadership.Token,
) error {
) (func(context.Context, func(context.Context) error) error, error) {
hasRole, err := s.getSecretAccess(ctx, uri, assessor)
if err != nil {
// Typically not found error.
return errors.Trace(err)
return nil, errors.Trace(err)
}
if hasRole.Allowed(secrets.RoleManage) {
return nil
return func(ctx context.Context, fn func(context.Context) error) error {
return fn(ctx)
}, nil
}
// Units can manage app owned secrets if they are the leader.
if assessor.Kind == UnitAccessor {
if leaderToken == nil {
return secreterrors.PermissionDenied
return nil, secreterrors.PermissionDenied
}

// TODO (manadart 2024-11-29): This section will be updated to
// return a WithLease wrapper in a patch to follow.
if err := leaderToken.Check(); err == nil {
appName, _ := names.UnitApplication(assessor.ID)
hasRole, err = s.getSecretAccess(ctx, uri, SecretAccessor{
Expand All @@ -217,14 +235,17 @@ func (s *SecretService) canManage(
})
if err != nil {
// Typically not found error.
return errors.Trace(err)
return nil, errors.Trace(err)
}
if hasRole.Allowed(secrets.RoleManage) {
return nil
return func(ctx context.Context, fn func(context.Context) error) error {
return fn(ctx)
}, nil
}
}
}
return fmt.Errorf("%q is not allowed to manage this secret%w", assessor.ID, errors.Hide(secreterrors.PermissionDenied))
return nil, fmt.Errorf(
"%q is not allowed to manage this secret %w", assessor.ID, errors.Hide(secreterrors.PermissionDenied))
}

// canRead checks that the accessor can read the secret.
Expand Down
12 changes: 6 additions & 6 deletions domain/secret/service/access_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
domainsecret "github.com/juju/juju/domain/secret"
)

func (s *serviceSuite) TestCanManageOwnerUnit(c *gc.C) {
func (s *serviceSuite) TestGetManagementCaveatOwnerUnit(c *gc.C) {
ctrl := s.setupMocks(c)
defer ctrl.Finish()

Expand All @@ -27,14 +27,14 @@ func (s *serviceSuite) TestCanManageOwnerUnit(c *gc.C) {

token := NewMockToken(ctrl)

err := s.service.canManage(context.Background(), uri, SecretAccessor{
_, err := s.service.getManagementCaveat(context.Background(), uri, SecretAccessor{
Kind: UnitAccessor,
ID: "mariadb/0",
}, token)
c.Assert(err, jc.ErrorIsNil)
}

func (s *serviceSuite) TestCanManageLeaderUnitAppSecret(c *gc.C) {
func (s *serviceSuite) TestGetManagementCaveatLeaderUnitAppSecret(c *gc.C) {
ctrl := s.setupMocks(c)
defer ctrl.Finish()

Expand All @@ -52,14 +52,14 @@ func (s *serviceSuite) TestCanManageLeaderUnitAppSecret(c *gc.C) {
token := NewMockToken(ctrl)
token.EXPECT().Check().Return(nil)

err := s.service.canManage(context.Background(), uri, SecretAccessor{
_, err := s.service.getManagementCaveat(context.Background(), uri, SecretAccessor{
Kind: UnitAccessor,
ID: "mariadb/0",
}, token)
c.Assert(err, jc.ErrorIsNil)
}

func (s *serviceSuite) TestCanManageUserSecrets(c *gc.C) {
func (s *serviceSuite) TestGetManagementCaveatUserSecrets(c *gc.C) {
ctrl := s.setupMocks(c)
defer ctrl.Finish()

Expand All @@ -72,7 +72,7 @@ func (s *serviceSuite) TestCanManageUserSecrets(c *gc.C) {

token := NewMockToken(ctrl)

err := s.service.canManage(context.Background(), uri, SecretAccessor{
_, err := s.service.getManagementCaveat(context.Background(), uri, SecretAccessor{
Kind: ModelAccessor,
ID: "model-uuid",
}, token)
Expand Down
27 changes: 16 additions & 11 deletions domain/secret/service/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,16 @@ package service
import (
"context"

"github.com/juju/errors"

"github.com/juju/juju/core/secrets"
"github.com/juju/juju/domain"
"github.com/juju/juju/internal/errors"
)

// DeleteObsoleteUserSecretRevisions deletes any obsolete user secret revisions that are marked as auto-prune.
func (s *SecretService) DeleteObsoleteUserSecretRevisions(ctx context.Context) error {
deletedRevisionIDs, err := s.secretState.DeleteObsoleteUserSecretRevisions(ctx)
if err != nil {
return errors.Trace(err)
return errors.Capture(err)
}
if err = s.secretBackendState.RemoveSecretBackendReference(ctx, deletedRevisionIDs...); err != nil {
// We don't want to error out if we can't remove the backend reference.
Expand All @@ -29,13 +28,19 @@ func (s *SecretService) DeleteObsoleteUserSecretRevisions(ctx context.Context) e
// If revisions is nil or the last remaining revisions are removed.
// It returns [secreterrors.PermissionDenied] if the secret cannot be managed by the accessor.
func (s *SecretService) DeleteSecret(ctx context.Context, uri *secrets.URI, params DeleteSecretParams) error {
if err := s.canManage(ctx, uri, params.Accessor, params.LeaderToken); err != nil {
return errors.Trace(err)
}
if err := s.secretState.RunAtomic(ctx, func(ctx domain.AtomicContext) error {
return s.secretState.DeleteSecret(ctx, uri, params.Revisions)
}); err != nil {
return errors.Annotatef(err, "deleting secret %q", uri.ID)
withCaveat, err := s.getManagementCaveat(ctx, uri, params.Accessor, params.LeaderToken)
if err != nil {
return errors.Capture(err)
}
return nil

return withCaveat(ctx, func(innerCtx context.Context) error {
// TODO (manadart 2024-11-29): This context naming is nasty,
// but will be removed with RunAtomic.
if err := s.secretState.RunAtomic(innerCtx, func(innerInnerCtx domain.AtomicContext) error {
return s.secretState.DeleteSecret(innerInnerCtx, uri, params.Revisions)
}); err != nil {
return errors.Errorf("deleting secret %q: %w", uri.ID, err)
}
return nil
})
}
Loading

0 comments on commit 73f2262

Please sign in to comment.