From 946dab487efe86b1b9efc32d100e4f7a7521f73c Mon Sep 17 00:00:00 2001 From: Irena Rindos Date: Fri, 28 Oct 2022 11:17:33 -0400 Subject: [PATCH] enable recovery user to list targets and sessions (#2576) * enable recovery user to list targets and sessions --- internal/daemon/controller/auth/auth.go | 6 +- internal/daemon/controller/auth/testing.go | 3 +- .../handlers/groups/group_service.go | 4 +- .../controller/handlers/roles/role_service.go | 4 +- .../handlers/sessions/session_service.go | 2 +- .../handlers/sessions/session_service_test.go | 108 ++++++++++++------ .../handlers/targets/target_service.go | 2 +- internal/iam/repository_scope.go | 9 +- internal/perms/acl.go | 18 ++- internal/perms/acl_test.go | 57 ++++++++- 10 files changed, 161 insertions(+), 52 deletions(-) diff --git a/internal/daemon/controller/auth/auth.go b/internal/daemon/controller/auth/auth.go index 8714d2e32d..7fc598d608 100644 --- a/internal/daemon/controller/auth/auth.go +++ b/internal/daemon/controller/auth/auth.go @@ -849,13 +849,13 @@ func (r *VerifyResults) ScopesAuthorizedForList(ctx context.Context, rootScopeId // We only expect the action set to be nothing, or list. In case // this is not the case, we bail out. - switch len(aSet) { - case 0: + switch { + case len(aSet) == 0: // Defer until we've read all scopes. We do this because if the // ordering coming back isn't in parent-first ordering our map // lookup might fail. deferredScopes = append(deferredScopes, scp) - case 1: + case len(aSet) == 1 || r.UserId == perms.RecoveryUserId: if aSet[0] != action.List { return nil, errors.New(ctx, errors.Internal, op, "unexpected action in set") } diff --git a/internal/daemon/controller/auth/testing.go b/internal/daemon/controller/auth/testing.go index f3b7d6bd68..101a618c91 100644 --- a/internal/daemon/controller/auth/testing.go +++ b/internal/daemon/controller/auth/testing.go @@ -5,6 +5,7 @@ import ( "github.com/hashicorp/boundary/internal/daemon/controller/common" authpb "github.com/hashicorp/boundary/internal/gen/controller/auth" + "github.com/hashicorp/boundary/internal/perms" "github.com/hashicorp/boundary/internal/requests" ) @@ -20,7 +21,7 @@ func DisabledAuthTestContext(iamRepoFn common.IamRepoFactory, scopeId string, op } reqInfo.UserIdOverride = opts.withUserId if reqInfo.UserIdOverride == "" { - reqInfo.UserIdOverride = "u_auth" + reqInfo.UserIdOverride = perms.AnyAuthenticatedUserId } requestContext := context.WithValue(context.Background(), requests.ContextRequestInformationKey, &requests.RequestContext{}) return NewVerifierContext(requestContext, iamRepoFn, nil, nil, opts.withKms, &reqInfo) diff --git a/internal/daemon/controller/handlers/groups/group_service.go b/internal/daemon/controller/handlers/groups/group_service.go index d51a7c3f8d..50f2655c64 100644 --- a/internal/daemon/controller/handlers/groups/group_service.go +++ b/internal/daemon/controller/handlers/groups/group_service.go @@ -710,7 +710,7 @@ func validateAddGroupMembersRequest(req *pbs.AddGroupMembersRequest) error { badFields["member_ids"] = fmt.Sprintf("Must only contain valid user ids but found %q.", id) break } - if id == "u_recovery" { + if id == perms.RecoveryUserId { badFields["member_ids"] = "u_recovery cannot be assigned to a group." break } @@ -734,7 +734,7 @@ func validateSetGroupMembersRequest(req *pbs.SetGroupMembersRequest) error { badFields["member_ids"] = fmt.Sprintf("Must only contain valid user ids but found %q.", id) break } - if id == "u_recovery" { + if id == perms.RecoveryUserId { badFields["member_ids"] = "u_recovery cannot be assigned to a group." break } diff --git a/internal/daemon/controller/handlers/roles/role_service.go b/internal/daemon/controller/handlers/roles/role_service.go index 3d7253603e..da3a890f0f 100644 --- a/internal/daemon/controller/handlers/roles/role_service.go +++ b/internal/daemon/controller/handlers/roles/role_service.go @@ -975,7 +975,7 @@ func validateAddRolePrincipalsRequest(req *pbs.AddRolePrincipalsRequest) error { badFields["principal_ids"] = "Must only have valid user, group, and/or managed group ids." break } - if id == "u_recovery" { + if id == perms.RecoveryUserId { badFields["principal_ids"] = "u_recovery cannot be assigned to a role" break } @@ -1001,7 +1001,7 @@ func validateSetRolePrincipalsRequest(req *pbs.SetRolePrincipalsRequest) error { badFields["principal_ids"] = "Must only have valid user, group, and/or managed group ids." break } - if id == "u_recovery" { + if id == perms.RecoveryUserId { badFields["principal_ids"] = "u_recovery cannot be assigned to a role" break } diff --git a/internal/daemon/controller/handlers/sessions/session_service.go b/internal/daemon/controller/handlers/sessions/session_service.go index 9a73750584..4352a60778 100644 --- a/internal/daemon/controller/handlers/sessions/session_service.go +++ b/internal/daemon/controller/handlers/sessions/session_service.go @@ -148,7 +148,7 @@ func (s Service) ListSessions(ctx context.Context, req *pbs.ListSessionsRequest) scopeIds, err = authResults.ScopesAuthorizedForList(ctx, req.GetScopeId(), resource.Session) } - listPerms := authResults.ACL().ListPermissions(scopeIds, resource.Session, IdActions) + listPerms := authResults.ACL().ListPermissions(scopeIds, resource.Session, IdActions, authResults.UserId) repo, err := s.repoFn(session.WithPermissions(&perms.UserPermissions{ UserId: authResults.UserId, diff --git a/internal/daemon/controller/handlers/sessions/session_service_test.go b/internal/daemon/controller/handlers/sessions/session_service_test.go index a65234ce31..a5e953d5fc 100644 --- a/internal/daemon/controller/handlers/sessions/session_service_test.go +++ b/internal/daemon/controller/handlers/sessions/session_service_test.go @@ -314,6 +314,7 @@ func TestList(t *testing.T) { var wantSession []*pb.Session var wantOtherSession []*pb.Session + var wantAllSessions []*pb.Session var wantIncludeTerminatedSessions []*pb.Session for i := 0; i < 10; i++ { sess := session.TestSession(t, conn, wrap, session.ComposedOf{ @@ -330,7 +331,7 @@ func TestList(t *testing.T) { status, states := convertStates(sess.States) - wantSession = append(wantSession, &pb.Session{ + firstOrgSession := &pb.Session{ Id: sess.GetPublicId(), ScopeId: pWithSessions.GetPublicId(), AuthTokenId: at.GetPublicId(), @@ -350,7 +351,9 @@ func TestList(t *testing.T) { Type: tcp.Subtype.String(), AuthorizedActions: testAuthorizedActions, Connections: []*pb.Connection{}, // connections should not be returned for list - }) + } + wantSession = append(wantSession, firstOrgSession) + wantAllSessions = append(wantAllSessions, firstOrgSession) wantIncludeTerminatedSessions = append(wantIncludeTerminatedSessions, wantSession[i]) @@ -368,7 +371,7 @@ func TestList(t *testing.T) { status, states = convertStates(sess.States) - wantOtherSession = append(wantOtherSession, &pb.Session{ + otherOrgSession := &pb.Session{ Id: sess.GetPublicId(), ScopeId: pWithSessions.GetPublicId(), AuthTokenId: at.GetPublicId(), @@ -388,7 +391,10 @@ func TestList(t *testing.T) { Type: tcp.Subtype.String(), AuthorizedActions: testAuthorizedActions, Connections: []*pb.Connection{}, // connections should not be returned for list - }) + } + wantOtherSession = append(wantOtherSession, otherOrgSession) + + wantAllSessions = append(wantAllSessions, otherOrgSession) } { @@ -439,41 +445,47 @@ func TestList(t *testing.T) { } cases := []struct { - name string - req *pbs.ListSessionsRequest - res *pbs.ListSessionsResponse - otherRes *pbs.ListSessionsResponse - err error + name string + req *pbs.ListSessionsRequest + res *pbs.ListSessionsResponse + otherRes *pbs.ListSessionsResponse + allSessionRes *pbs.ListSessionsResponse + err error }{ { - name: "List Many Sessions", - req: &pbs.ListSessionsRequest{ScopeId: pWithSessions.GetPublicId()}, - res: &pbs.ListSessionsResponse{Items: wantSession}, - otherRes: &pbs.ListSessionsResponse{Items: []*pb.Session{}}, + name: "List Many Sessions", + req: &pbs.ListSessionsRequest{ScopeId: pWithSessions.GetPublicId()}, + res: &pbs.ListSessionsResponse{Items: wantSession}, + otherRes: &pbs.ListSessionsResponse{Items: []*pb.Session{}}, + allSessionRes: &pbs.ListSessionsResponse{Items: wantSession}, }, { - name: "List Many Include Terminated", - req: &pbs.ListSessionsRequest{ScopeId: pWithSessions.GetPublicId(), IncludeTerminated: true}, - res: &pbs.ListSessionsResponse{Items: wantIncludeTerminatedSessions}, - otherRes: &pbs.ListSessionsResponse{Items: []*pb.Session{}}, + name: "List Many Include Terminated", + req: &pbs.ListSessionsRequest{ScopeId: pWithSessions.GetPublicId(), IncludeTerminated: true}, + res: &pbs.ListSessionsResponse{Items: wantIncludeTerminatedSessions}, + otherRes: &pbs.ListSessionsResponse{Items: []*pb.Session{}}, + allSessionRes: &pbs.ListSessionsResponse{Items: wantIncludeTerminatedSessions}, }, { - name: "List No Sessions", - req: &pbs.ListSessionsRequest{ScopeId: pNoSessions.GetPublicId()}, - res: &pbs.ListSessionsResponse{}, - otherRes: &pbs.ListSessionsResponse{Items: []*pb.Session{}}, + name: "List No Sessions", + req: &pbs.ListSessionsRequest{ScopeId: pNoSessions.GetPublicId()}, + res: &pbs.ListSessionsResponse{}, + otherRes: &pbs.ListSessionsResponse{Items: []*pb.Session{}}, + allSessionRes: &pbs.ListSessionsResponse{}, }, { - name: "List Sessions Recursively", - req: &pbs.ListSessionsRequest{ScopeId: scope.Global.String(), Recursive: true}, - res: &pbs.ListSessionsResponse{Items: wantSession}, - otherRes: &pbs.ListSessionsResponse{Items: wantOtherSession}, + name: "List Sessions Recursively", + req: &pbs.ListSessionsRequest{ScopeId: scope.Global.String(), Recursive: true}, + res: &pbs.ListSessionsResponse{Items: wantSession}, + otherRes: &pbs.ListSessionsResponse{Items: wantOtherSession}, + allSessionRes: &pbs.ListSessionsResponse{Items: wantAllSessions}, }, { - name: "Filter To Single Sessions", - req: &pbs.ListSessionsRequest{ScopeId: pWithSessions.GetPublicId(), Filter: fmt.Sprintf(`"/item/id"==%q`, wantSession[4].Id)}, - res: &pbs.ListSessionsResponse{Items: wantSession[4:5]}, - otherRes: &pbs.ListSessionsResponse{Items: []*pb.Session{}}, + name: "Filter To Single Sessions", + req: &pbs.ListSessionsRequest{ScopeId: pWithSessions.GetPublicId(), Filter: fmt.Sprintf(`"/item/id"==%q`, wantSession[4].Id)}, + res: &pbs.ListSessionsResponse{Items: wantSession[4:5]}, + otherRes: &pbs.ListSessionsResponse{Items: []*pb.Session{}}, + allSessionRes: &pbs.ListSessionsResponse{Items: wantSession[4:5]}, }, { name: "Filter To Many Sessions", @@ -481,14 +493,16 @@ func TestList(t *testing.T) { ScopeId: scope.Global.String(), Recursive: true, Filter: fmt.Sprintf(`"/item/scope/id" matches "^%s"`, pWithSessions.GetPublicId()[:8]), }, - res: &pbs.ListSessionsResponse{Items: wantSession}, - otherRes: &pbs.ListSessionsResponse{Items: []*pb.Session{}}, + res: &pbs.ListSessionsResponse{Items: wantSession}, + otherRes: &pbs.ListSessionsResponse{Items: []*pb.Session{}}, + allSessionRes: &pbs.ListSessionsResponse{Items: wantSession}, }, { - name: "Filter To Nothing", - req: &pbs.ListSessionsRequest{ScopeId: pWithSessions.GetPublicId(), Filter: `"/item/id" == ""`}, - res: &pbs.ListSessionsResponse{}, - otherRes: &pbs.ListSessionsResponse{Items: []*pb.Session{}}, + name: "Filter To Nothing", + req: &pbs.ListSessionsRequest{ScopeId: pWithSessions.GetPublicId(), Filter: `"/item/id" == ""`}, + res: &pbs.ListSessionsResponse{}, + otherRes: &pbs.ListSessionsResponse{Items: []*pb.Session{}}, + allSessionRes: &pbs.ListSessionsResponse{}, }, { name: "Filter Bad Format", @@ -544,6 +558,30 @@ func TestList(t *testing.T) { assert.Equal(0, len(wantSess.GetConnections())) // no connections on list wantSess.ExpirationTime = got.GetItems()[i].GetExpirationTime() } + + // Test with recovery user + recoveryRequestInfo := authpb.RequestInfo{ + TokenFormat: uint32(auth.AuthTokenTypeRecoveryKms), + PublicId: at.GetPublicId(), + Token: at.GetToken(), + } + recoveryRequestContext := context.WithValue(context.Background(), requests.ContextRequestInformationKey, &requests.RequestContext{}) + recoveryCtx := auth.NewVerifierContext(recoveryRequestContext, iamRepoFn, tokenRepoFn, serversRepoFn, kms, &recoveryRequestInfo) + recoveryGot, gErr := s.ListSessions(recoveryCtx, tc.req) + if tc.err != nil { + require.Error(gErr) + assert.True(errors.Is(gErr, tc.err), "ListSessions(%+v) got error %v, wanted %v", tc.req, gErr, tc.err) + return + } + require.NoError(gErr) + if tc.allSessionRes != nil { + require.Equal(len(tc.allSessionRes.GetItems()), len(recoveryGot.GetItems()), "Didn't get expected number of sessions: %v", recoveryGot.GetItems()) + for i, wantSess := range tc.allSessionRes.GetItems() { + assert.True(recoveryGot.GetItems()[i].GetExpirationTime().AsTime().Sub(wantSess.GetExpirationTime().AsTime()) < 10*time.Millisecond) + assert.Equal(0, len(wantSess.GetConnections())) // no connections on list + wantSess.ExpirationTime = recoveryGot.GetItems()[i].GetExpirationTime() + } + } }) } } diff --git a/internal/daemon/controller/handlers/targets/target_service.go b/internal/daemon/controller/handlers/targets/target_service.go index 0a03d03d5b..d46fb985c6 100644 --- a/internal/daemon/controller/handlers/targets/target_service.go +++ b/internal/daemon/controller/handlers/targets/target_service.go @@ -192,7 +192,7 @@ func (s Service) ListTargets(ctx context.Context, req *pbs.ListTargetsRequest) ( } // Get all user permissions for the requested scope(s). - userPerms := authResults.ACL().ListPermissions(authzScopes, resource.Target, IdActions) + userPerms := authResults.ACL().ListPermissions(authzScopes, resource.Target, IdActions, authResults.UserId) if len(userPerms) == 0 { return &pbs.ListTargetsResponse{}, nil } diff --git a/internal/iam/repository_scope.go b/internal/iam/repository_scope.go index 95555b2c81..a6b2d734b9 100644 --- a/internal/iam/repository_scope.go +++ b/internal/iam/repository_scope.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/oplog" + "github.com/hashicorp/boundary/internal/perms" "github.com/hashicorp/boundary/internal/types/resource" "github.com/hashicorp/boundary/internal/types/scope" "github.com/hashicorp/go-dbw" @@ -85,9 +86,9 @@ func (r *Repository) CreateScope(ctx context.Context, s *Scope, userId string, o var adminRoleRaw any switch { case userId == "", - userId == "u_anon", - userId == "u_auth", - userId == "u_recovery", + userId == perms.AnonymousUserId, + userId == perms.AnyAuthenticatedUserId, + userId == perms.RecoveryUserId, opts.withSkipAdminRoleCreation: // TODO: Cause a log entry. The repo doesn't have a logger right now, // and ideally we will be using context to pass around log info scoped @@ -337,7 +338,7 @@ func (r *Repository) CreateScope(ctx context.Context, s *Scope, userId string, o principals := []any{} userId := "u_anon" if s.Type == scope.Project.String() { - userId = "u_auth" + userId = perms.AnyAuthenticatedUserId } rolePrincipal, err := NewUserRole(defaultRolePublicId, userId) if err != nil { diff --git a/internal/perms/acl.go b/internal/perms/acl.go index 5141a201ce..4e8d1b63f9 100644 --- a/internal/perms/acl.go +++ b/internal/perms/acl.go @@ -8,7 +8,11 @@ import ( "github.com/hashicorp/boundary/sdk/pbs/controller/api/resources/scopes" ) -const AnonymousUserId = "u_anon" +const ( + AnyAuthenticatedUserId = "u_auth" + AnonymousUserId = "u_anon" + RecoveryUserId = "u_recovery" +) // ACL provides an entry point into the permissions engine for determining if an // action is allowed on a resource based on a principal's (user or group) grants. @@ -234,7 +238,11 @@ func (a ACL) Allowed(r Resource, aType action.Type, userId string, opt ...Option // or for action.All in order for a Permission to be created for the scope. // The set of "id actions" is resource dependant, but will generally include all // actions that can be taken on an individual resource. -func (a ACL) ListPermissions(requestedScopes map[string]*scopes.ScopeInfo, requestedType resource.Type, idActions action.ActionSet) []Permission { +func (a ACL) ListPermissions(requestedScopes map[string]*scopes.ScopeInfo, + requestedType resource.Type, + idActions action.ActionSet, + userId string, +) []Permission { perms := make([]Permission, 0, len(requestedScopes)) for scopeId := range requestedScopes { p := Permission{ @@ -243,6 +251,12 @@ func (a ACL) ListPermissions(requestedScopes map[string]*scopes.ScopeInfo, reque Action: action.List, OnlySelf: true, // default to only self to be restrictive } + if userId == RecoveryUserId { + p.All = true + p.OnlySelf = false + perms = append(perms, p) + continue + } // Get grants for a specific scope id from the source of truth. grants := a.scopeMap[scopeId] diff --git a/internal/perms/acl_test.go b/internal/perms/acl_test.go index 21cbbaf066..d3e54612f6 100644 --- a/internal/perms/acl_test.go +++ b/internal/perms/acl_test.go @@ -377,6 +377,7 @@ func Test_ACLAllowed(t *testing.T) { func TestACL_ListPermissions(t *testing.T) { tests := []struct { name string + userId string aclGrants []scopeGrant scopes map[string]*scopes.ScopeInfo // *scopes.ScopeInfo isn't used at the moment. resourceType resource.Type @@ -720,10 +721,64 @@ func TestACL_ListPermissions(t *testing.T) { }, }, }, + { + name: "Allow recovery user full access to sessions", + userId: RecoveryUserId, + scopes: map[string]*scopes.ScopeInfo{"o_1": nil, "o_2": nil}, + resourceType: resource.Session, + actionSet: action.ActionSet{action.List, action.Read, action.Create, action.Delete}, + expPermissions: []Permission{ + { + ScopeId: "o_1", + Resource: resource.Session, + Action: action.List, + ResourceIds: nil, + OnlySelf: false, + All: true, + }, + { + ScopeId: "o_2", + Resource: resource.Session, + Action: action.List, + ResourceIds: nil, + OnlySelf: false, + All: true, + }, + }, + }, + { + name: "Allow recovery user full access to targets", + userId: RecoveryUserId, + scopes: map[string]*scopes.ScopeInfo{"o_1": nil, "o_2": nil}, + resourceType: resource.Target, + actionSet: action.ActionSet{action.List, action.Read, action.Create, action.Delete}, + expPermissions: []Permission{ + { + ScopeId: "o_1", + Resource: resource.Target, + Action: action.List, + ResourceIds: nil, + OnlySelf: false, + All: true, + }, + { + ScopeId: "o_2", + Resource: resource.Target, + Action: action.List, + ResourceIds: nil, + OnlySelf: false, + All: true, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + userId := tt.userId + if userId == "" { + userId = "u_1234567890" + } var grants []Grant for _, sg := range tt.aclGrants { for _, g := range sg.grants { @@ -734,7 +789,7 @@ func TestACL_ListPermissions(t *testing.T) { } acl := NewACL(grants...) - perms := acl.ListPermissions(tt.scopes, tt.resourceType, tt.actionSet) + perms := acl.ListPermissions(tt.scopes, tt.resourceType, tt.actionSet, userId) require.ElementsMatch(t, tt.expPermissions, perms) }) }