diff --git a/.circleci/config.yml b/.circleci/config.yml index 5dc4ac822..d962d1ddf 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -128,6 +128,10 @@ jobs: echo "$ go version" go version name: Install go + - run: + command: | + make tools + name: Install tools - run: command: | which pg_isready || sudo apt-get update && sudo apt-get install -y postgresql-client @@ -349,22 +353,12 @@ workflows: # resource_class: medium # working_directory: ~/boundary # jobs: -# algolia-index: -# docker: -# - image: docker.mirror.hashicorp.services/node:12 -# steps: -# - checkout -# - run: -# command: |- -# cd website/ -# npm install -# node scripts/index_search_content.js -# name: Push content to Algolia Index # build: # executor: go-machine # steps: # - checkout # - install-go +# - install-tools # - run: # command: | # which pg_isready || sudo apt-get update && sudo apt-get install -y postgresql-client @@ -455,11 +449,6 @@ workflows: # - 11-alpine # - 12-alpine # - 13-alpine -# - algolia-index: -# filters: -# branches: -# only: -# - stable-website # - make-gen-deltas # trigger-merge-to-ent: # jobs: @@ -467,4 +456,4 @@ workflows: # filters: # branches: # only: -# - main +# - main \ No newline at end of file diff --git a/.circleci/config/jobs/build.yml b/.circleci/config/jobs/build.yml index ad75eb5ba..331c9440c 100644 --- a/.circleci/config/jobs/build.yml +++ b/.circleci/config/jobs/build.yml @@ -2,6 +2,7 @@ executor: go-machine steps: - checkout - install-go +- install-tools - run: name: "Initialize Test Database" command: | @@ -21,4 +22,4 @@ steps: - slack/status: fail_only: true only_for_branches: master - webhook: webhook \ No newline at end of file + webhook: webhook diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 1d1e7bf32..5233786e1 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -20,7 +20,7 @@ jobs: - name: Setup go uses: actions/setup-go@v3 with: - go-version: "1.18.1" + go-version: "1.18.3" - name: get product version id: get-product-version run: | @@ -70,7 +70,7 @@ jobs: - name: Setup Go uses: actions/setup-go@v3 with: - go-version: "1.18.1" + go-version: "1.18.3" - name: Get product edition id: get-product-edition run: | @@ -88,7 +88,7 @@ jobs: matrix: goos: [ freebsd, windows, netbsd, openbsd, solaris ] goarch: [ "386", "amd64", "arm" ] - go: [ "1.18.1" ] + go: [ "1.18.3" ] exclude: - goos: solaris goarch: 386 @@ -149,7 +149,7 @@ jobs: matrix: goos: [linux] goarch: ["arm", "arm64", "386", "amd64"] - go: [ "1.18.1" ] + go: [ "1.18.3" ] fail-fast: true name: Go ${{ matrix.go }} ${{ matrix.goos }} ${{ matrix.goarch }} build @@ -235,7 +235,7 @@ jobs: matrix: goos: [ darwin ] goarch: [ "amd64", "arm64" ] - go: [ "1.18.1" ] + go: [ "1.18.3" ] fail-fast: true name: Go ${{ matrix.go }} ${{ matrix.goos }} ${{ matrix.goarch }} build diff --git a/CHANGELOG.md b/CHANGELOG.md index 24a59ef26..bd660f6bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,30 @@ Canonical reference for changes, improvements, and bugfixes for Boundary. ## Next +### New and Improved + +* controller: Improve response time for listing sessions. + This also creates a new periodic job that will delete terminated + sessions after 1 hour. + See Deprecations/Changes for some additional details. + [PR](https://github.com/hashicorp/boundary/pull/2160). + +### Deprecations/Changes + +* controller: Change the default behavior of the session list endpoint + to no longer include sessions in a terminated state and introduces + a new query parameter/cli flag to include the terminated sessions. + This also removes the connection information from the list response. + [PR](https://github.com/hashicorp/boundary/pull/2160). +* Anonymous user permissions: In order to reduce the risk of accidental and + unintended granting of permissions to anonymous users, the permissions system + now only allows certain actions on certain resources to be assigned to the + anonymous user; currently these are the same permissions as assigned in + Boundary's default role permissions. If other use-cases arise this list can be + expanded. See [the + documentation](https://www.boundaryproject.io/docs/concepts/security/permissions/assignable-permissions) + for more details. + ### Bug Fixes ## 0.8.1 (2022/05/13) @@ -13,7 +37,8 @@ Canonical reference for changes, improvements, and bugfixes for Boundary. * controller: Do not shut down cluster listener when it receives an invalid packet ([Issue](https://github.com/hashicorp/boundary/issues/2072), [PR](https://github.com/hashicorp/boundary/pull/2073)) -* session: update cancel_session() function to check for terminated state ([Issue](https://github.com/hashicorp/boundary/issues/2064), +* session: update cancel_session() function to check for terminated state + ([Issue](https://github.com/hashicorp/boundary/issues/2064), [PR](https://github.com/hashicorp/boundary/pull/2065)) ## 0.8.0 (2022/05/03) @@ -25,26 +50,30 @@ Canonical reference for changes, improvements, and bugfixes for Boundary. [PR](https://github.com/hashicorp/boundary/pull/2049) * ui: Add support for worker filters in targets * ui: Add manual refresh button in sessions list +* Audit events are no longer a WIP ([PR](https://github.com/hashicorp/boundary/pull/2031). ### Bug Fixes -* worker: create new error to prevent `event.newError: missing error: invalid parameter` and handle session cancel - with no TOFU token ([Issue](https://github.com/hashicorp/boundary/issues/1902), +* worker: create new error to prevent `event.newError: missing error: invalid + parameter` and handle session cancel with no TOFU token + ([Issue](https://github.com/hashicorp/boundary/issues/1902), [PR](https://github.com/hashicorp/boundary/pull/1929)) -* controller: Reconcile DEKs with existing scopes ([Issue](https://github.com/hashicorp/boundary/issues/1856), +* controller: Reconcile DEKs with existing scopes + ([Issue](https://github.com/hashicorp/boundary/issues/1856), [PR](https://github.com/hashicorp/boundary/pull/1976)) -* controller: Fix for retrieving sessions that could result in incomplete results when - there is a large number (10k+) of sessions. +* controller: Fix for retrieving sessions that could result in incomplete + results when there is a large number (10k+) of sessions. [PR](https://github.com/hashicorp/boundary/pull/2049) -* session: update session state trigger to prevent transitions to invalid states ([Issue](https://github.com/hashicorp/boundary/issues/2040), +* session: update session state trigger to prevent transitions to invalid states + ([Issue](https://github.com/hashicorp/boundary/issues/2040), [PR](https://github.com/hashicorp/boundary/pull/2046)) ## 0.7.6 (2022/03/15) ### Bug Fixes -* sessions: Sessions and session connections have been refactored -to better isolate transactions and prevent resource contention that caused deadlocks. +* sessions: Sessions and session connections have been refactored to better +isolate transactions and prevent resource contention that caused deadlocks. ([Issue](https://github.com/hashicorp/boundary/issues/1812), [PR](https://github.com/hashicorp/boundary/pull/1919)) * scheduler: Fix bug that causes erroneous logs when racing controllers diff --git a/CODEOWNERS b/CODEOWNERS new file mode 100644 index 000000000..85b7c48a2 --- /dev/null +++ b/CODEOWNERS @@ -0,0 +1,4 @@ +# release configuration + +/.release/ @hashicorp/release-engineering @hashicorp/github-secure-boundary +/.github/workflows/build.yml @hashicorp/release-engineering @hashicorp/github-secure-boundary diff --git a/Makefile b/Makefile index 83c16cbc9..cbab664db 100644 --- a/Makefile +++ b/Makefile @@ -22,6 +22,7 @@ cli: tools: go generate -tags tools tools/tools.go go install github.com/bufbuild/buf/cmd/buf@v1.3.1 + go install github.com/mfridman/tparse@v0.10.3 .PHONY: cleangen cleangen: @@ -141,10 +142,10 @@ protobuild: @protoc-go-inject-tag -input=./internal/auth/store/account.pb.go @protoc-go-inject-tag -input=./internal/auth/password/store/password.pb.go @protoc-go-inject-tag -input=./internal/auth/password/store/argon2.pb.go - @protoc-go-inject-tag -input=./internal/kms/store/root_key.pb.go - @protoc-go-inject-tag -input=./internal/kms/store/database_key.pb.go - @protoc-go-inject-tag -input=./internal/kms/store/oplog_key.pb.go - @protoc-go-inject-tag -input=./internal/kms/store/token_key.pb.go + @protoc-go-inject-tag -input=./internal/kms/store/root_key.pb.go + @protoc-go-inject-tag -input=./internal/kms/store/database_key.pb.go + @protoc-go-inject-tag -input=./internal/kms/store/oplog_key.pb.go + @protoc-go-inject-tag -input=./internal/kms/store/token_key.pb.go @protoc-go-inject-tag -input=./internal/kms/store/session_key.pb.go @protoc-go-inject-tag -input=./internal/kms/store/oidc_key.pb.go @protoc-go-inject-tag -input=./internal/servers/store/controller.pb.go @@ -235,7 +236,7 @@ generate-database-dumps: test-ci: export CI_BUILD=1 test-ci: CGO_ENABLED=$(CGO_ENABLED) BUILD_TAGS='$(BUILD_TAGS)' sh -c "'$(CURDIR)/scripts/build.sh'" - ~/.go/bin/go test ./... -v $(TESTARGS) -timeout 120m + ~/.go/bin/go test ./... -v $(TESTARGS) -json -cover -timeout 120m | tparse .PHONY: test-sql test-sql: @@ -243,7 +244,7 @@ test-sql: .PHONY: test test: - go test ./... -timeout 30m + go test ./... -timeout 30m -json -cover | tparse -follow .PHONY: test-sdk test-sdk: diff --git a/api/sessions/option.gen.go b/api/sessions/option.gen.go index 6367b922b..d84fa64b6 100644 --- a/api/sessions/option.gen.go +++ b/api/sessions/option.gen.go @@ -1,6 +1,7 @@ package sessions import ( + "fmt" "strconv" "strings" @@ -86,3 +87,15 @@ func WithRecursive(recurse bool) Option { o.withRecursive = true } } + +func WithIncludeTerminated(inIncludeTerminated bool) Option { + return func(o *options) { + o.queryMap["include_terminated"] = fmt.Sprintf("%v", inIncludeTerminated) + } +} + +func DefaultIncludeTerminated() Option { + return func(o *options) { + o.postMap["include_terminated"] = nil + } +} diff --git a/ci/goinstall.sh b/ci/goinstall.sh index 955a834a3..9a9b56e79 100755 --- a/ci/goinstall.sh +++ b/ci/goinstall.sh @@ -1,7 +1,7 @@ #!/bin/bash set -e -VERSION="1.18.1" +VERSION="1.18.3" [ -z "$GOROOT" ] && GOROOT="$HOME/.go" [ -z "$GOPATH" ] && GOPATH="$HOME/go" diff --git a/internal/api/genapi/input.go b/internal/api/genapi/input.go index 52032262d..bd2179a61 100644 --- a/internal/api/genapi/input.go +++ b/internal/api/genapi/input.go @@ -733,6 +733,14 @@ var inputStructs = []*structInfo{ readTemplate, listTemplate, }, + extraFields: []fieldInfo{ + { + Name: "IncludeTerminated", + ProtoName: "include_terminated", + FieldType: "bool", + Query: true, + }, + }, pluralResourceName: "sessions", createResponseTypes: true, fieldFilter: []string{"private_key"}, diff --git a/internal/cmd/commands/sessionscmd/funcs.go b/internal/cmd/commands/sessionscmd/funcs.go index 117940fcf..4cc7bdf2d 100644 --- a/internal/cmd/commands/sessionscmd/funcs.go +++ b/internal/cmd/commands/sessionscmd/funcs.go @@ -10,17 +10,48 @@ import ( "github.com/hashicorp/boundary/internal/cmd/base" ) +const ( + flagIncludeTerminated = "include-terminated" +) + func init() { extraActionsFlagsMapFunc = extraActionsFlagsMapFuncImpl + extraFlagsFunc = extraFlagsFuncImpl + extraFlagsHandlingFunc = extraFlagsHandlingFuncImpl executeExtraActions = executeExtraActionsImpl } func extraActionsFlagsMapFuncImpl() map[string][]string { return map[string][]string{ "cancel": {"id"}, + "list": {flagIncludeTerminated}, } } +type extraCmdVars struct { + flagIncludeTerminated bool +} + +func extraFlagsFuncImpl(c *Command, set *base.FlagSets, f *base.FlagSet) { + for _, name := range flagsMap[c.Func] { + switch name { + case flagIncludeTerminated: + f.BoolVar(&base.BoolVar{ + Name: flagIncludeTerminated, + Target: &c.flagIncludeTerminated, + Usage: "If set, terminated sessions will be included in the results.", + }) + } + } +} + +func extraFlagsHandlingFuncImpl(c *Command, _ *base.FlagSets, opts *[]sessions.Option) bool { + if c.flagIncludeTerminated { + *opts = append(*opts, sessions.WithIncludeTerminated(c.flagIncludeTerminated)) + } + return true +} + func (c *Command) extraHelpFunc(helpMap map[string]func() string) string { var helpStr string switch c.Func { diff --git a/internal/cmd/commands/sessionscmd/sessions.gen.go b/internal/cmd/commands/sessionscmd/sessions.gen.go index 550b4246b..5308fd5da 100644 --- a/internal/cmd/commands/sessionscmd/sessions.gen.go +++ b/internal/cmd/commands/sessionscmd/sessions.gen.go @@ -35,6 +35,8 @@ type Command struct { Func string plural string + + extraCmdVars } func (c *Command) AutocompleteArgs() complete.Predictor { diff --git a/internal/cmd/commands/targetscmd/funcs.go b/internal/cmd/commands/targetscmd/funcs.go index cf1d267bf..385c2c174 100644 --- a/internal/cmd/commands/targetscmd/funcs.go +++ b/internal/cmd/commands/targetscmd/funcs.go @@ -702,23 +702,6 @@ func printItemTable(result api.GenericResult) string { } var credentialSourceMaps map[credential.Purpose][]map[string]interface{} - if len(item.ApplicationCredentialSources) > 0 { - if credentialSourceMaps == nil { - credentialSourceMaps = make(map[credential.Purpose][]map[string]interface{}) - } - var applicationCredentialSourceMaps []map[string]interface{} - for _, lib := range item.ApplicationCredentialSources { - m := map[string]interface{}{ - "ID": lib.Id, - "Credential Store ID": lib.CredentialStoreId, - } - applicationCredentialSourceMaps = append(applicationCredentialSourceMaps, m) - } - credentialSourceMaps[credential.ApplicationPurpose] = applicationCredentialSourceMaps - if l := len("Credential Store ID"); l > maxLength { - maxLength = l - } - } if len(item.ApplicationCredentialLibraries) > 0 { if credentialSourceMaps == nil { credentialSourceMaps = make(map[credential.Purpose][]map[string]interface{}) @@ -736,6 +719,40 @@ func printItemTable(result api.GenericResult) string { maxLength = l } } + if len(item.ApplicationCredentialSources) > 0 { + if credentialSourceMaps == nil { + credentialSourceMaps = make(map[credential.Purpose][]map[string]interface{}) + } + var applicationCredentialSourceMaps []map[string]interface{} + for _, source := range item.ApplicationCredentialSources { + m := map[string]interface{}{ + "ID": source.Id, + "Credential Store ID": source.CredentialStoreId, + } + applicationCredentialSourceMaps = append(applicationCredentialSourceMaps, m) + } + credentialSourceMaps[credential.ApplicationPurpose] = applicationCredentialSourceMaps + if l := len("Credential Store ID"); l > maxLength { + maxLength = l + } + } + if len(item.EgressCredentialSources) > 0 { + if credentialSourceMaps == nil { + credentialSourceMaps = make(map[credential.Purpose][]map[string]interface{}) + } + var egressCredentialSourceMaps []map[string]interface{} + for _, source := range item.EgressCredentialSources { + m := map[string]interface{}{ + "ID": source.Id, + "Credential Store ID": source.CredentialStoreId, + } + egressCredentialSourceMaps = append(egressCredentialSourceMaps, m) + } + credentialSourceMaps[credential.EgressPurpose] = egressCredentialSourceMaps + if l := len("Credential Store ID"); l > maxLength { + maxLength = l + } + } ret := []string{ "", @@ -775,17 +792,22 @@ func printItemTable(result api.GenericResult) string { } } - if len(credentialSourceMaps) > 0 { - if appMap := credentialSourceMaps[credential.ApplicationPurpose]; len(appMap) > 0 { + for purpose, sources := range credentialSourceMaps { + switch purpose { + case credential.ApplicationPurpose: ret = append(ret, " Application Credential Sources:", ) - for _, m := range appMap { - ret = append(ret, - base.WrapMap(4, maxLength, m), - "", - ) - } + case credential.EgressPurpose: + ret = append(ret, + " Egress Credential Sources:", + ) + } + for _, m := range sources { + ret = append(ret, + base.WrapMap(4, maxLength, m), + "", + ) } } @@ -951,8 +973,8 @@ func exampleOutput() string { HostCatalogId: "hcst_1234567890", }, }, - ApplicationCredentialLibraryIds: []string{"clvlt_1234567890", "clvlt_0987654321"}, - ApplicationCredentialLibraries: []*targets.CredentialLibrary{ + ApplicationCredentialSourceIds: []string{"clvlt_1234567890", "clvlt_0987654321"}, + ApplicationCredentialSources: []*targets.CredentialSource{ { Id: "clvlt_1234567890", CredentialStoreId: "csvlt_1234567890", diff --git a/internal/cmd/gencli/input.go b/internal/cmd/gencli/input.go index cc64b3d85..062a16178 100644 --- a/internal/cmd/gencli/input.go +++ b/internal/cmd/gencli/input.go @@ -400,13 +400,14 @@ var inputStructs = map[string][]*cmdInfo{ }, "sessions": { { - ResourceType: resource.Session.String(), - Pkg: "sessions", - StdActions: []string{"read", "list"}, - Container: "Scope", - HasExtraHelpFunc: true, - HasId: true, - VersionedActions: []string{"cancel"}, + ResourceType: resource.Session.String(), + Pkg: "sessions", + StdActions: []string{"read", "list"}, + Container: "Scope", + HasExtraCommandVars: true, + HasExtraHelpFunc: true, + HasId: true, + VersionedActions: []string{"cancel"}, }, }, "targets": { diff --git a/internal/daemon/controller/auth/auth.go b/internal/daemon/controller/auth/auth.go index 91c122bd1..add85456f 100644 --- a/internal/daemon/controller/auth/auth.go +++ b/internal/daemon/controller/auth/auth.go @@ -569,7 +569,7 @@ func (v verifier) performAuthCheck(ctx context.Context) ( } retAcl = perms.NewACL(parsedGrants...) - aclResults = retAcl.Allowed(*v.res, v.act) + aclResults = retAcl.Allowed(*v.res, v.act, userId) // We don't set authenticated above because setting this but not authorized // is used for further permissions checks, such as during recursive listing. // So we want to make sure any code relying on that has the full set of @@ -619,7 +619,7 @@ func (r *VerifyResults) fetchActions(id string, typ resource.Type, availableActi ret := make(action.ActionSet, 0, len(availableActions)) for _, act := range availableActions { - if r.v.acl.Allowed(*res, act).Authorized { + if r.v.acl.Allowed(*res, act, r.UserId).Authorized { ret = append(ret, act) } } @@ -637,7 +637,7 @@ func (r *VerifyResults) FetchOutputFields(res perms.Resource, act action.Type) p return nil } - return r.v.acl.Allowed(res, act).OutputFields + return r.v.acl.Allowed(res, act, r.UserId).OutputFields } // GetTokenFromRequest pulls the token from either the Authorization header or @@ -675,7 +675,7 @@ func GetTokenFromRequest(ctx context.Context, kmsCache *kms.Kms, req *http.Reque if receivedTokenType == AuthTokenTypeUnknown || fullToken == "" { // We didn't find auth info or a client screwed up and put in a blank // header instead of nothing at all, so return blank which will indicate - // the anonymouse user + // the anonymous user return "", "", uint32(AuthTokenTypeUnknown) } diff --git a/internal/daemon/controller/controller.go b/internal/daemon/controller/controller.go index 1d590ee04..47d7931e1 100644 --- a/internal/daemon/controller/controller.go +++ b/internal/daemon/controller/controller.go @@ -361,7 +361,7 @@ func (c *Controller) registerJobs() error { if err := pluginhost.RegisterJobs(c.baseContext, c.scheduler, rw, rw, c.kms, c.conf.HostPlugins); err != nil { return err } - if err := session.RegisterJobs(c.baseContext, c.scheduler, rw, c.conf.StatusGracePeriodDuration); err != nil { + if err := session.RegisterJobs(c.baseContext, c.scheduler, rw, rw, c.kms, c.conf.StatusGracePeriodDuration); err != nil { return err } if err := serversjob.RegisterJobs(c.baseContext, c.scheduler, rw, rw, c.kms); err != nil { diff --git a/internal/daemon/controller/handlers/sessions/session_service.go b/internal/daemon/controller/handlers/sessions/session_service.go index 4fa4d01c6..3a0388544 100644 --- a/internal/daemon/controller/handlers/sessions/session_service.go +++ b/internal/daemon/controller/handlers/sessions/session_service.go @@ -152,7 +152,7 @@ func (s Service) ListSessions(ctx context.Context, req *pbs.ListSessionsRequest) RootScopeId: req.GetScopeId(), Type: resource.Session, Recursive: req.GetRecursive(), - AuthzProtectedEntityProvider: repo, + AuthzProtectedEntityProvider: session.ListForAuthzCheck(repo, session.WithTerminated(req.IncludeTerminated)), ActionSet: IdActions, }, ) diff --git a/internal/daemon/controller/handlers/sessions/session_service_test.go b/internal/daemon/controller/handlers/sessions/session_service_test.go index 2b3d7b7a6..cbd864502 100644 --- a/internal/daemon/controller/handlers/sessions/session_service_test.go +++ b/internal/daemon/controller/handlers/sessions/session_service_test.go @@ -232,6 +232,7 @@ func TestList(t *testing.T) { conn, _ := db.TestSetup(t, "postgres") wrap := db.TestWrapper(t) kms := kms.TestKms(t, conn, wrap) + ctx := context.Background() iamRepo := iam.TestRepo(t, conn, wrap) @@ -260,16 +261,17 @@ func TestList(t *testing.T) { hs := static.TestSets(t, conn, hc.GetPublicId(), 1)[0] h := static.TestHosts(t, conn, hc.GetPublicId(), 1)[0] static.TestSetMembers(t, conn, hs.GetPublicId(), []*static.Host{h}) - tar := tcp.TestTarget(context.Background(), t, conn, pWithSessions.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) + tar := tcp.TestTarget(ctx, t, conn, pWithSessions.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) hcOther := static.TestCatalogs(t, conn, pWithOtherSessions.GetPublicId(), 1)[0] hsOther := static.TestSets(t, conn, hcOther.GetPublicId(), 1)[0] hOther := static.TestHosts(t, conn, hcOther.GetPublicId(), 1)[0] static.TestSetMembers(t, conn, hsOther.GetPublicId(), []*static.Host{hOther}) - tarOther := tcp.TestTarget(context.Background(), t, conn, pWithOtherSessions.GetPublicId(), "test", target.WithHostSources([]string{hsOther.GetPublicId()})) + tarOther := tcp.TestTarget(ctx, t, conn, pWithOtherSessions.GetPublicId(), "test", target.WithHostSources([]string{hsOther.GetPublicId()})) var wantSession []*pb.Session var totalSession []*pb.Session + var wantIncludeTerminatedSessions []*pb.Session for i := 0; i < 10; i++ { sess := session.TestSession(t, conn, wrap, session.ComposedOf{ UserId: uId, @@ -281,7 +283,7 @@ func TestList(t *testing.T) { Endpoint: "tcp://127.0.0.1:22", }) - c := session.TestConnection(t, conn, sess.PublicId, "127.0.0.1", 22, "127.0.0.2", 23, "127.0.0.1") + session.TestConnection(t, conn, sess.PublicId, "127.0.0.1", 22, "127.0.0.2", 23, "127.0.0.1") status, states := convertStates(sess.States) @@ -304,17 +306,11 @@ func TestList(t *testing.T) { Certificate: sess.Certificate, Type: tcp.Subtype.String(), AuthorizedActions: testAuthorizedActions, - Connections: []*pb.Connection{ - { - ClientTcpAddress: c.ClientTcpAddress, - ClientTcpPort: c.ClientTcpPort, - EndpointTcpAddress: c.EndpointTcpAddress, - EndpointTcpPort: c.EndpointTcpPort, - }, - }, + Connections: []*pb.Connection{}, // connections should not be returned for list }) totalSession = append(totalSession, wantSession[i]) + wantIncludeTerminatedSessions = append(wantIncludeTerminatedSessions, wantSession[i]) sess = session.TestSession(t, conn, wrap, session.ComposedOf{ UserId: uIdOther, @@ -326,7 +322,7 @@ func TestList(t *testing.T) { Endpoint: "tcp://127.0.0.1:22", }) - c = session.TestConnection(t, conn, sess.PublicId, "127.0.0.1", 22, "127.0.0.2", 23, "127.0.0.1") + session.TestConnection(t, conn, sess.PublicId, "127.0.0.1", 22, "127.0.0.2", 23, "127.0.0.1") status, states = convertStates(sess.States) @@ -349,17 +345,57 @@ func TestList(t *testing.T) { Certificate: sess.Certificate, Type: tcp.Subtype.String(), AuthorizedActions: testAuthorizedActions, - Connections: []*pb.Connection{ - { - ClientTcpAddress: c.ClientTcpAddress, - ClientTcpPort: c.ClientTcpPort, - EndpointTcpAddress: c.EndpointTcpAddress, - EndpointTcpPort: c.EndpointTcpPort, - }, - }, + Connections: []*pb.Connection{}, // connections should not be returned for list }) } + { + sess := session.TestSession(t, conn, wrap, session.ComposedOf{ + UserId: uId, + HostId: h.GetPublicId(), + TargetId: tar.GetPublicId(), + HostSetId: hs.GetPublicId(), + AuthTokenId: at.GetPublicId(), + ScopeId: pWithSessions.GetPublicId(), + Endpoint: "tcp://127.0.0.1:22", + }) + + sess, err := sessRepo.CancelSession(ctx, sess.PublicId, sess.Version) + require.NoError(t, err) + terminated, err := sessRepo.TerminateCompletedSessions(ctx) + require.NoError(t, err) + require.Equal(t, 1, terminated) + + sess, _, err = sessRepo.LookupSession(ctx, sess.PublicId) + require.NoError(t, err) + status, states := convertStates(sess.States) + + expected := &pb.Session{ + Id: sess.GetPublicId(), + ScopeId: pWithSessions.GetPublicId(), + AuthTokenId: at.GetPublicId(), + UserId: at.GetIamUserId(), + TargetId: sess.TargetId, + Endpoint: sess.Endpoint, + HostSetId: sess.HostSetId, + HostId: sess.HostId, + Version: sess.Version, + UpdatedTime: sess.UpdateTime.GetTimestamp(), + CreatedTime: sess.CreateTime.GetTimestamp(), + ExpirationTime: sess.ExpirationTime.GetTimestamp(), + Scope: &scopes.ScopeInfo{Id: pWithSessions.GetPublicId(), Type: scope.Project.String(), ParentScopeId: o.GetPublicId()}, + Status: status, + States: states, + Certificate: sess.Certificate, + TerminationReason: sess.TerminationReason, + Type: tcp.Subtype.String(), + AuthorizedActions: testAuthorizedActions, + Connections: []*pb.Connection{}, // connections should not be returned for list + } + + wantIncludeTerminatedSessions = append(wantIncludeTerminatedSessions, expected) + } + cases := []struct { name string req *pbs.ListSessionsRequest @@ -371,6 +407,11 @@ func TestList(t *testing.T) { req: &pbs.ListSessionsRequest{ScopeId: pWithSessions.GetPublicId()}, res: &pbs.ListSessionsResponse{Items: wantSession}, }, + { + name: "List Many Include Terminated", + req: &pbs.ListSessionsRequest{ScopeId: pWithSessions.GetPublicId(), IncludeTerminated: true}, + res: &pbs.ListSessionsResponse{Items: wantIncludeTerminatedSessions}, + }, { name: "List No Sessions", req: &pbs.ListSessionsRequest{ScopeId: pNoSessions.GetPublicId()}, @@ -423,14 +464,8 @@ func TestList(t *testing.T) { require.Equal(len(tc.res.GetItems()), len(got.GetItems()), "Didn't get expected number of sessions: %v", got.GetItems()) for i, wantSess := range tc.res.GetItems() { assert.True(got.GetItems()[i].GetExpirationTime().AsTime().Sub(wantSess.GetExpirationTime().AsTime()) < 10*time.Millisecond) - assert.Equal(1, len(wantSess.GetConnections())) + assert.Equal(0, len(wantSess.GetConnections())) // no connections on list wantSess.ExpirationTime = got.GetItems()[i].GetExpirationTime() - for _, c := range got.GetItems()[i].GetConnections() { - assert.Equal("127.0.0.1", c.ClientTcpAddress) - assert.Equal(uint32(22), c.ClientTcpPort) - assert.Equal("127.0.0.2", c.EndpointTcpAddress) - assert.Equal(uint32(23), c.EndpointTcpPort) - } } } assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "ListSessions(%q) got response %q, wanted %q", tc.req, got, tc.res) diff --git a/internal/daemon/controller/handlers/targets/target_service.go b/internal/daemon/controller/handlers/targets/target_service.go index efa23f89e..ad2c33956 100644 --- a/internal/daemon/controller/handlers/targets/target_service.go +++ b/internal/daemon/controller/handlers/targets/target_service.go @@ -1581,45 +1581,56 @@ func toProto(ctx context.Context, in target.Target, hostSources []target.HostSou }) } } - if outputFields.Has(globals.ApplicationCredentialLibraryIdsField) { - for _, cs := range credSources { - out.ApplicationCredentialLibraryIds = append(out.ApplicationCredentialLibraryIds, cs.Id()) + + var appCredSources, egressCredSources []*pb.CredentialSource + var appCredSourceIds, egressCredSourceIds []string + var appCredLibraries []*pb.CredentialLibrary + + for _, cs := range credSources { + switch cs.CredentialPurpose() { + case credential.ApplicationPurpose: + appCredSourceIds = append(appCredSourceIds, cs.Id()) + appCredSources = append(appCredSources, &pb.CredentialSource{ + Id: cs.Id(), + CredentialStoreId: cs.CredentialStoreId(), + }) + + // ApplicationCredentialLibrariesField is deprecated and should only be populated + // for application purpose + appCredLibraries = append(appCredLibraries, &pb.CredentialLibrary{ + Id: cs.Id(), + CredentialStoreId: cs.CredentialStoreId(), + }) + + case credential.EgressPurpose: + egressCredSources = append(egressCredSources, &pb.CredentialSource{ + Id: cs.Id(), + CredentialStoreId: cs.CredentialStoreId(), + }) + egressCredSourceIds = append(egressCredSourceIds, cs.Id()) + + default: + return nil, errors.New(ctx, errors.Internal, op, fmt.Sprintf("unrecognized purpose %q for credential source on target", cs.CredentialPurpose())) } } + + if outputFields.Has(globals.ApplicationCredentialLibraryIdsField) { + out.ApplicationCredentialLibraryIds = appCredSourceIds + } if outputFields.Has(globals.ApplicationCredentialSourceIdsField) { - for _, cs := range credSources { - out.ApplicationCredentialSourceIds = append(out.ApplicationCredentialSourceIds, cs.Id()) - } + out.ApplicationCredentialSourceIds = appCredSourceIds } if outputFields.Has(globals.ApplicationCredentialLibrariesField) { - for _, cs := range credSources { - switch credential.Purpose(cs.CredentialPurpose()) { - case credential.ApplicationPurpose: - out.ApplicationCredentialLibraries = append(out.ApplicationCredentialLibraries, &pb.CredentialLibrary{ - Id: cs.Id(), - CredentialStoreId: cs.CredentialStoreId(), - }) - case credential.IngressPurpose, credential.EgressPurpose: - // TODO: When we support other purposes add them to different fields here. - default: - return nil, errors.New(ctx, errors.Internal, op, fmt.Sprintf("unrecognized purpose %q for credential source on target", cs.CredentialPurpose())) - } - } + out.ApplicationCredentialLibraries = appCredLibraries } if outputFields.Has(globals.ApplicationCredentialSourcesField) { - for _, cs := range credSources { - switch credential.Purpose(cs.CredentialPurpose()) { - case credential.ApplicationPurpose: - out.ApplicationCredentialSources = append(out.ApplicationCredentialSources, &pb.CredentialSource{ - Id: cs.Id(), - CredentialStoreId: cs.CredentialStoreId(), - }) - case credential.IngressPurpose, credential.EgressPurpose: - // TODO: When we support other purposes add them to different fields here. - default: - return nil, errors.New(ctx, errors.Internal, op, fmt.Sprintf("unrecognized purpose %q for credential source on target", cs.CredentialPurpose())) - } - } + out.ApplicationCredentialSources = appCredSources + } + if outputFields.Has(globals.EgressCredentialSourceIdsField) { + out.EgressCredentialSourceIds = egressCredSourceIds + } + if outputFields.Has(globals.EgressCredentialSourcesField) { + out.EgressCredentialSources = egressCredSources } if outputFields.Has(globals.AttributesField) { if err := subtypeRegistry.setAttributes(in.GetType(), in, &out); err != nil { diff --git a/internal/daemon/controller/testing.go b/internal/daemon/controller/testing.go index ecbf36466..2111407d8 100644 --- a/internal/daemon/controller/testing.go +++ b/internal/daemon/controller/testing.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "io" "net" "strconv" "strings" @@ -373,6 +374,11 @@ type TestControllerOpts struct { // If true, the controller will not be started DisableAutoStart bool + // DisableEventing, if true the test controller will not create events + // You must not run the test in parallel (no calls to t.Parallel) since the + // this option relies on modifying the system wide default eventer. + DisableEventing bool + // DisableAuthorizationFailures will still cause authz checks to be // performed but they won't cause 403 Forbidden. Useful for API-level // testing to avoid a lot of faff. @@ -543,8 +549,32 @@ func TestControllerConfig(t testing.TB, ctx context.Context, tc *TestController, } opts.Config.Controller.SchedulerRunJobInterval = opts.SchedulerRunJobInterval - if err := tc.b.SetupEventing(tc.b.Logger, tc.b.StderrLock, opts.Config.Controller.Name, base.WithEventerConfig(opts.Config.Eventing)); err != nil { - t.Fatal(err) + switch { + case opts.DisableEventing: + opts.Config.Eventing = &event.EventerConfig{ + AuditEnabled: false, + ObservationsEnabled: false, + SysEventsEnabled: false, + } + testLogger := hclog.New(&hclog.LoggerOptions{ + Mutex: tc.b.StderrLock, + Output: io.Discard, + }) + e, err := event.NewEventer( + testLogger, + tc.b.StderrLock, + opts.Config.Controller.Name, + *opts.Config.Eventing, + ) + if err != nil { + t.Fatal(err) + } + tc.b.Eventer = e + event.TestWithoutEventing(t) // this ensures the sys eventer will also stop eventing + default: + if err := tc.b.SetupEventing(tc.b.Logger, tc.b.StderrLock, opts.Config.Controller.Name, base.WithEventerConfig(opts.Config.Eventing)); err != nil { + t.Fatal(err) + } } // Initialize status grace period diff --git a/internal/daemon/controller/testing_test.go b/internal/daemon/controller/testing_test.go index e0f0104ed..b699ac88e 100644 --- a/internal/daemon/controller/testing_test.go +++ b/internal/daemon/controller/testing_test.go @@ -1,7 +1,12 @@ package controller import ( + "bytes" + "io" + "os" "testing" + + "github.com/stretchr/testify/assert" ) func Test_TestController(t *testing.T) { @@ -17,4 +22,44 @@ func Test_TestController(t *testing.T) { defer tc1.Shutdown() defer tc2.Shutdown() }) + t.Run("controller-without-eventing", func(t *testing.T) { + const op = "Test_TestWithoutEventing" + assert := assert.New(t) + + // this isn't the best solution for capturing stdout but it works for now... + captureFn := func(fn func()) string { + old := os.Stdout + defer func() { + os.Stderr = old + }() + + r, w, _ := os.Pipe() + os.Stderr = w + + { + fn() + } + + outC := make(chan string) + // copy the output in a separate goroutine so writing to stderr can't block indefinitely + go func() { + var buf bytes.Buffer + io.Copy(&buf, r) + outC <- buf.String() + }() + + // back to normal state + w.Close() + return <-outC + } + + assert.Empty(captureFn(func() { + tc := NewTestController(t, &TestControllerOpts{DisableEventing: true}) + defer tc.Shutdown() + })) + assert.NotEmpty(captureFn(func() { + tc := NewTestController(t, nil) + defer tc.Shutdown() + })) + }) } diff --git a/internal/db/db.go b/internal/db/db.go index 7add89bc6..bb387807f 100644 --- a/internal/db/db.go +++ b/internal/db/db.go @@ -95,6 +95,9 @@ func Open(dbType DbType, connectionUrl string, opt ...Option) (*DB, error) { wrappedOpts = append(wrappedOpts, dbw.WithLogger(opts.withGormFormatter)) } if opts.withMaxOpenConnections > 0 { + if opts.withMaxOpenConnections < 5 { + return nil, fmt.Errorf("max_open_connections cannot be below 5") + } wrappedOpts = append(wrappedOpts, dbw.WithMaxOpenConnections(opts.withMaxOpenConnections)) } diff --git a/internal/db/db_test.go b/internal/db/db_test.go index d0730c790..8af72e482 100644 --- a/internal/db/db_test.go +++ b/internal/db/db_test.go @@ -22,6 +22,7 @@ func TestOpen(t *testing.T) { type args struct { dbType DbType connectionUrl string + opt []Option } tests := []struct { name string @@ -44,6 +45,15 @@ func TestOpen(t *testing.T) { }, wantErr: true, }, + { + name: "invalid - max_open_connections set to 3", + args: args{ + dbType: Postgres, + connectionUrl: "", + opt: []Option{WithMaxOpenConnections(3)}, + }, + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/internal/db/schema/migrations/oss/postgres/20/09_session_list_view.up.sql b/internal/db/schema/migrations/oss/postgres/20/09_session_list_view.up.sql index 2feb2a68c..8d9b9f30f 100644 --- a/internal/db/schema/migrations/oss/postgres/20/09_session_list_view.up.sql +++ b/internal/db/schema/migrations/oss/postgres/20/09_session_list_view.up.sql @@ -1,6 +1,7 @@ begin; -- Replaces the view created in 1/01 to include connections +-- Replaced in 31/0_session_list_no_connections drop view session_with_state; create view session_list as select diff --git a/internal/db/schema/migrations/oss/postgres/31/01_delete_termianted_sessions.up.sql b/internal/db/schema/migrations/oss/postgres/31/01_delete_termianted_sessions.up.sql new file mode 100644 index 000000000..9b0a49102 --- /dev/null +++ b/internal/db/schema/migrations/oss/postgres/31/01_delete_termianted_sessions.up.sql @@ -0,0 +1,12 @@ +begin; + delete from session + using session_state + where + session.public_id = session_state.session_id + and + session_state.state = 'terminated' + and + session_state.start_time < wt_sub_seconds_from_now(3600); + + analyze; +commit; diff --git a/internal/db/schema/migrations/oss/postgres/31/02_session_list_no_connections.up.sql b/internal/db/schema/migrations/oss/postgres/31/02_session_list_no_connections.up.sql new file mode 100644 index 000000000..e012f2d77 --- /dev/null +++ b/internal/db/schema/migrations/oss/postgres/31/02_session_list_no_connections.up.sql @@ -0,0 +1,38 @@ +begin; + +-- Replaces the view created in 2/09_session_list_view +drop view session_list; +create view session_list as + select + s.public_id, + s.user_id, + s.host_id, + s.server_id, + s.server_type, + s.target_id, + s.host_set_id, + s.auth_token_id, + s.scope_id, + s.certificate, + s.expiration_time, + s.connection_limit, + s.tofu_token, + s.key_id, + s.termination_reason, + s.version, + s.create_time, + s.update_time, + s.endpoint, + s.worker_filter, + ss.state, + ss.previous_end_time, + ss.start_time, + ss.end_time + from + session s + join + session_state ss + on + s.public_id = ss.session_id; + +commit; diff --git a/internal/gen/controller.swagger.json b/internal/gen/controller.swagger.json index 98dffffe8..48efff41a 100644 --- a/internal/gen/controller.swagger.json +++ b/internal/gen/controller.swagger.json @@ -2526,6 +2526,13 @@ "in": "query", "required": false, "type": "string" + }, + { + "name": "include_terminated", + "description": "Experimental. By default only non-terminated (i.e. pending, active, canceling) are returned.\nSet this option to include terminated sessions as well.", + "in": "query", + "required": false, + "type": "boolean" } ], "tags": [ diff --git a/internal/gen/controller/api/services/session_service.pb.go b/internal/gen/controller/api/services/session_service.pb.go index fa7698e56..28a2aaf64 100644 --- a/internal/gen/controller/api/services/session_service.pb.go +++ b/internal/gen/controller/api/services/session_service.pb.go @@ -125,6 +125,9 @@ type ListSessionsRequest struct { ScopeId string `protobuf:"bytes,1,opt,name=scope_id,json=scopeId,proto3" json:"scope_id,omitempty" class:"public"` // @gotags: `class:"public"` Recursive bool `protobuf:"varint,20,opt,name=recursive,proto3" json:"recursive,omitempty" class:"public"` // @gotags: `class:"public"` Filter string `protobuf:"bytes,30,opt,name=filter,proto3" json:"filter,omitempty" class:"public"` // @gotags: `class:"public"` + // Experimental. By default only non-terminated (i.e. pending, active, canceling) are returned. + // Set this option to include terminated sessions as well. + IncludeTerminated bool `protobuf:"varint,40,opt,name=include_terminated,proto3" json:"include_terminated,omitempty" class:"public"` // @gotags: `class:"public"` } func (x *ListSessionsRequest) Reset() { @@ -180,6 +183,13 @@ func (x *ListSessionsRequest) GetFilter() string { return "" } +func (x *ListSessionsRequest) GetIncludeTerminated() bool { + if x != nil { + return x.IncludeTerminated + } + return false +} + type ListSessionsResponse struct { state protoimpl.MessageState sizeCache protoimpl.SizeCache @@ -352,68 +362,72 @@ var file_controller_api_services_v1_session_service_proto_rawDesc = []byte{ 0x74, 0x65, 0x6d, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x2d, 0x2e, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, 0x2e, 0x61, 0x70, 0x69, 0x2e, 0x72, 0x65, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x73, 0x2e, 0x73, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x73, 0x2e, 0x76, 0x31, - 0x2e, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x52, 0x04, 0x69, 0x74, 0x65, 0x6d, 0x22, 0x66, - 0x0a, 0x13, 0x4c, 0x69, 0x73, 0x74, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x73, 0x52, 0x65, - 0x71, 0x75, 0x65, 0x73, 0x74, 0x12, 0x19, 0x0a, 0x08, 0x73, 0x63, 0x6f, 0x70, 0x65, 0x5f, 0x69, - 0x64, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x07, 0x73, 0x63, 0x6f, 0x70, 0x65, 0x49, 0x64, - 0x12, 0x1c, 0x0a, 0x09, 0x72, 0x65, 0x63, 0x75, 0x72, 0x73, 0x69, 0x76, 0x65, 0x18, 0x14, 0x20, - 0x01, 0x28, 0x08, 0x52, 0x09, 0x72, 0x65, 0x63, 0x75, 0x72, 0x73, 0x69, 0x76, 0x65, 0x12, 0x16, - 0x0a, 0x06, 0x66, 0x69, 0x6c, 0x74, 0x65, 0x72, 0x18, 0x1e, 0x20, 0x01, 0x28, 0x09, 0x52, 0x06, - 0x66, 0x69, 0x6c, 0x74, 0x65, 0x72, 0x22, 0x5b, 0x0a, 0x14, 0x4c, 0x69, 0x73, 0x74, 0x53, 0x65, - 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x73, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x12, 0x43, - 0x0a, 0x05, 0x69, 0x74, 0x65, 0x6d, 0x73, 0x18, 0x01, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x2d, 0x2e, + 0x2e, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x52, 0x04, 0x69, 0x74, 0x65, 0x6d, 0x22, 0x96, + 0x01, 0x0a, 0x13, 0x4c, 0x69, 0x73, 0x74, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x73, 0x52, + 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x12, 0x19, 0x0a, 0x08, 0x73, 0x63, 0x6f, 0x70, 0x65, 0x5f, + 0x69, 0x64, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x07, 0x73, 0x63, 0x6f, 0x70, 0x65, 0x49, + 0x64, 0x12, 0x1c, 0x0a, 0x09, 0x72, 0x65, 0x63, 0x75, 0x72, 0x73, 0x69, 0x76, 0x65, 0x18, 0x14, + 0x20, 0x01, 0x28, 0x08, 0x52, 0x09, 0x72, 0x65, 0x63, 0x75, 0x72, 0x73, 0x69, 0x76, 0x65, 0x12, + 0x16, 0x0a, 0x06, 0x66, 0x69, 0x6c, 0x74, 0x65, 0x72, 0x18, 0x1e, 0x20, 0x01, 0x28, 0x09, 0x52, + 0x06, 0x66, 0x69, 0x6c, 0x74, 0x65, 0x72, 0x12, 0x2e, 0x0a, 0x12, 0x69, 0x6e, 0x63, 0x6c, 0x75, + 0x64, 0x65, 0x5f, 0x74, 0x65, 0x72, 0x6d, 0x69, 0x6e, 0x61, 0x74, 0x65, 0x64, 0x18, 0x28, 0x20, + 0x01, 0x28, 0x08, 0x52, 0x12, 0x69, 0x6e, 0x63, 0x6c, 0x75, 0x64, 0x65, 0x5f, 0x74, 0x65, 0x72, + 0x6d, 0x69, 0x6e, 0x61, 0x74, 0x65, 0x64, 0x22, 0x5b, 0x0a, 0x14, 0x4c, 0x69, 0x73, 0x74, 0x53, + 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x73, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x12, + 0x43, 0x0a, 0x05, 0x69, 0x74, 0x65, 0x6d, 0x73, 0x18, 0x01, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x2d, + 0x2e, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, 0x2e, 0x61, 0x70, 0x69, 0x2e, + 0x72, 0x65, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x73, 0x2e, 0x73, 0x65, 0x73, 0x73, 0x69, 0x6f, + 0x6e, 0x73, 0x2e, 0x76, 0x31, 0x2e, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x52, 0x05, 0x69, + 0x74, 0x65, 0x6d, 0x73, 0x22, 0x40, 0x0a, 0x14, 0x43, 0x61, 0x6e, 0x63, 0x65, 0x6c, 0x53, 0x65, + 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x12, 0x0e, 0x0a, 0x02, + 0x69, 0x64, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x02, 0x69, 0x64, 0x12, 0x18, 0x0a, 0x07, + 0x76, 0x65, 0x72, 0x73, 0x69, 0x6f, 0x6e, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0d, 0x52, 0x07, 0x76, + 0x65, 0x72, 0x73, 0x69, 0x6f, 0x6e, 0x22, 0x5a, 0x0a, 0x15, 0x43, 0x61, 0x6e, 0x63, 0x65, 0x6c, + 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x12, + 0x41, 0x0a, 0x04, 0x69, 0x74, 0x65, 0x6d, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x2d, 0x2e, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, 0x2e, 0x61, 0x70, 0x69, 0x2e, 0x72, 0x65, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x73, 0x2e, 0x73, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, - 0x73, 0x2e, 0x76, 0x31, 0x2e, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x52, 0x05, 0x69, 0x74, - 0x65, 0x6d, 0x73, 0x22, 0x40, 0x0a, 0x14, 0x43, 0x61, 0x6e, 0x63, 0x65, 0x6c, 0x53, 0x65, 0x73, - 0x73, 0x69, 0x6f, 0x6e, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x12, 0x0e, 0x0a, 0x02, 0x69, - 0x64, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x02, 0x69, 0x64, 0x12, 0x18, 0x0a, 0x07, 0x76, - 0x65, 0x72, 0x73, 0x69, 0x6f, 0x6e, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0d, 0x52, 0x07, 0x76, 0x65, - 0x72, 0x73, 0x69, 0x6f, 0x6e, 0x22, 0x5a, 0x0a, 0x15, 0x43, 0x61, 0x6e, 0x63, 0x65, 0x6c, 0x53, - 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x12, 0x41, - 0x0a, 0x04, 0x69, 0x74, 0x65, 0x6d, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x2d, 0x2e, 0x63, - 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, 0x2e, 0x61, 0x70, 0x69, 0x2e, 0x72, 0x65, - 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x73, 0x2e, 0x73, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x73, - 0x2e, 0x76, 0x31, 0x2e, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x52, 0x04, 0x69, 0x74, 0x65, - 0x6d, 0x32, 0x95, 0x04, 0x0a, 0x0e, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x53, 0x65, 0x72, - 0x76, 0x69, 0x63, 0x65, 0x12, 0xa7, 0x01, 0x0a, 0x0a, 0x47, 0x65, 0x74, 0x53, 0x65, 0x73, 0x73, - 0x69, 0x6f, 0x6e, 0x12, 0x2d, 0x2e, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, - 0x2e, 0x61, 0x70, 0x69, 0x2e, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x73, 0x2e, 0x76, 0x31, - 0x2e, 0x47, 0x65, 0x74, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x52, 0x65, 0x71, 0x75, 0x65, - 0x73, 0x74, 0x1a, 0x2e, 0x2e, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, 0x2e, - 0x61, 0x70, 0x69, 0x2e, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x73, 0x2e, 0x76, 0x31, 0x2e, - 0x47, 0x65, 0x74, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, - 0x73, 0x65, 0x22, 0x3a, 0x92, 0x41, 0x18, 0x12, 0x16, 0x47, 0x65, 0x74, 0x73, 0x20, 0x61, 0x20, - 0x73, 0x69, 0x6e, 0x67, 0x6c, 0x65, 0x20, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x2e, 0x82, - 0xd3, 0xe4, 0x93, 0x02, 0x19, 0x12, 0x11, 0x2f, 0x76, 0x31, 0x2f, 0x73, 0x65, 0x73, 0x73, 0x69, - 0x6f, 0x6e, 0x73, 0x2f, 0x7b, 0x69, 0x64, 0x7d, 0x62, 0x04, 0x69, 0x74, 0x65, 0x6d, 0x12, 0x9f, - 0x01, 0x0a, 0x0c, 0x4c, 0x69, 0x73, 0x74, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x73, 0x12, - 0x2f, 0x2e, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, 0x2e, 0x61, 0x70, 0x69, - 0x2e, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x73, 0x2e, 0x76, 0x31, 0x2e, 0x4c, 0x69, 0x73, - 0x74, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x73, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, - 0x1a, 0x30, 0x2e, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, 0x2e, 0x61, 0x70, - 0x69, 0x2e, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x73, 0x2e, 0x76, 0x31, 0x2e, 0x4c, 0x69, - 0x73, 0x74, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x73, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, - 0x73, 0x65, 0x22, 0x2c, 0x92, 0x41, 0x15, 0x12, 0x13, 0x4c, 0x69, 0x73, 0x74, 0x73, 0x20, 0x61, - 0x6c, 0x6c, 0x20, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x73, 0x2e, 0x82, 0xd3, 0xe4, 0x93, - 0x02, 0x0e, 0x12, 0x0c, 0x2f, 0x76, 0x31, 0x2f, 0x73, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x73, - 0x12, 0xb6, 0x01, 0x0a, 0x0d, 0x43, 0x61, 0x6e, 0x63, 0x65, 0x6c, 0x53, 0x65, 0x73, 0x73, 0x69, - 0x6f, 0x6e, 0x12, 0x30, 0x2e, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, 0x2e, - 0x61, 0x70, 0x69, 0x2e, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x73, 0x2e, 0x76, 0x31, 0x2e, - 0x43, 0x61, 0x6e, 0x63, 0x65, 0x6c, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x52, 0x65, 0x71, - 0x75, 0x65, 0x73, 0x74, 0x1a, 0x31, 0x2e, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, + 0x73, 0x2e, 0x76, 0x31, 0x2e, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x52, 0x04, 0x69, 0x74, + 0x65, 0x6d, 0x32, 0x95, 0x04, 0x0a, 0x0e, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x53, 0x65, + 0x72, 0x76, 0x69, 0x63, 0x65, 0x12, 0xa7, 0x01, 0x0a, 0x0a, 0x47, 0x65, 0x74, 0x53, 0x65, 0x73, + 0x73, 0x69, 0x6f, 0x6e, 0x12, 0x2d, 0x2e, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, 0x2e, 0x61, 0x70, 0x69, 0x2e, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x73, 0x2e, 0x76, - 0x31, 0x2e, 0x43, 0x61, 0x6e, 0x63, 0x65, 0x6c, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x52, - 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x22, 0x40, 0x92, 0x41, 0x14, 0x12, 0x12, 0x43, 0x61, - 0x6e, 0x63, 0x65, 0x6c, 0x73, 0x20, 0x61, 0x20, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x2e, - 0x82, 0xd3, 0xe4, 0x93, 0x02, 0x23, 0x22, 0x18, 0x2f, 0x76, 0x31, 0x2f, 0x73, 0x65, 0x73, 0x73, - 0x69, 0x6f, 0x6e, 0x73, 0x2f, 0x7b, 0x69, 0x64, 0x7d, 0x3a, 0x63, 0x61, 0x6e, 0x63, 0x65, 0x6c, - 0x3a, 0x01, 0x2a, 0x62, 0x04, 0x69, 0x74, 0x65, 0x6d, 0x42, 0x4d, 0x5a, 0x4b, 0x67, 0x69, 0x74, - 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x68, 0x61, 0x73, 0x68, 0x69, 0x63, 0x6f, 0x72, - 0x70, 0x2f, 0x62, 0x6f, 0x75, 0x6e, 0x64, 0x61, 0x72, 0x79, 0x2f, 0x69, 0x6e, 0x74, 0x65, 0x72, - 0x6e, 0x61, 0x6c, 0x2f, 0x67, 0x65, 0x6e, 0x2f, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, - 0x65, 0x72, 0x2f, 0x61, 0x70, 0x69, 0x2f, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x73, 0x3b, - 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x73, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, + 0x31, 0x2e, 0x47, 0x65, 0x74, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x52, 0x65, 0x71, 0x75, + 0x65, 0x73, 0x74, 0x1a, 0x2e, 0x2e, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, + 0x2e, 0x61, 0x70, 0x69, 0x2e, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x73, 0x2e, 0x76, 0x31, + 0x2e, 0x47, 0x65, 0x74, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x52, 0x65, 0x73, 0x70, 0x6f, + 0x6e, 0x73, 0x65, 0x22, 0x3a, 0x92, 0x41, 0x18, 0x12, 0x16, 0x47, 0x65, 0x74, 0x73, 0x20, 0x61, + 0x20, 0x73, 0x69, 0x6e, 0x67, 0x6c, 0x65, 0x20, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x2e, + 0x82, 0xd3, 0xe4, 0x93, 0x02, 0x19, 0x12, 0x11, 0x2f, 0x76, 0x31, 0x2f, 0x73, 0x65, 0x73, 0x73, + 0x69, 0x6f, 0x6e, 0x73, 0x2f, 0x7b, 0x69, 0x64, 0x7d, 0x62, 0x04, 0x69, 0x74, 0x65, 0x6d, 0x12, + 0x9f, 0x01, 0x0a, 0x0c, 0x4c, 0x69, 0x73, 0x74, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x73, + 0x12, 0x2f, 0x2e, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, 0x2e, 0x61, 0x70, + 0x69, 0x2e, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x73, 0x2e, 0x76, 0x31, 0x2e, 0x4c, 0x69, + 0x73, 0x74, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x73, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, + 0x74, 0x1a, 0x30, 0x2e, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, 0x2e, 0x61, + 0x70, 0x69, 0x2e, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x73, 0x2e, 0x76, 0x31, 0x2e, 0x4c, + 0x69, 0x73, 0x74, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x73, 0x52, 0x65, 0x73, 0x70, 0x6f, + 0x6e, 0x73, 0x65, 0x22, 0x2c, 0x92, 0x41, 0x15, 0x12, 0x13, 0x4c, 0x69, 0x73, 0x74, 0x73, 0x20, + 0x61, 0x6c, 0x6c, 0x20, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x73, 0x2e, 0x82, 0xd3, 0xe4, + 0x93, 0x02, 0x0e, 0x12, 0x0c, 0x2f, 0x76, 0x31, 0x2f, 0x73, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, + 0x73, 0x12, 0xb6, 0x01, 0x0a, 0x0d, 0x43, 0x61, 0x6e, 0x63, 0x65, 0x6c, 0x53, 0x65, 0x73, 0x73, + 0x69, 0x6f, 0x6e, 0x12, 0x30, 0x2e, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, + 0x2e, 0x61, 0x70, 0x69, 0x2e, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x73, 0x2e, 0x76, 0x31, + 0x2e, 0x43, 0x61, 0x6e, 0x63, 0x65, 0x6c, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x52, 0x65, + 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x31, 0x2e, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, + 0x65, 0x72, 0x2e, 0x61, 0x70, 0x69, 0x2e, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x73, 0x2e, + 0x76, 0x31, 0x2e, 0x43, 0x61, 0x6e, 0x63, 0x65, 0x6c, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, + 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x22, 0x40, 0x92, 0x41, 0x14, 0x12, 0x12, 0x43, + 0x61, 0x6e, 0x63, 0x65, 0x6c, 0x73, 0x20, 0x61, 0x20, 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, + 0x2e, 0x82, 0xd3, 0xe4, 0x93, 0x02, 0x23, 0x22, 0x18, 0x2f, 0x76, 0x31, 0x2f, 0x73, 0x65, 0x73, + 0x73, 0x69, 0x6f, 0x6e, 0x73, 0x2f, 0x7b, 0x69, 0x64, 0x7d, 0x3a, 0x63, 0x61, 0x6e, 0x63, 0x65, + 0x6c, 0x3a, 0x01, 0x2a, 0x62, 0x04, 0x69, 0x74, 0x65, 0x6d, 0x42, 0x4d, 0x5a, 0x4b, 0x67, 0x69, + 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x68, 0x61, 0x73, 0x68, 0x69, 0x63, 0x6f, + 0x72, 0x70, 0x2f, 0x62, 0x6f, 0x75, 0x6e, 0x64, 0x61, 0x72, 0x79, 0x2f, 0x69, 0x6e, 0x74, 0x65, + 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x67, 0x65, 0x6e, 0x2f, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, + 0x6c, 0x65, 0x72, 0x2f, 0x61, 0x70, 0x69, 0x2f, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x73, + 0x3b, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x73, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, + 0x33, } var ( diff --git a/internal/observability/event/eventer.go b/internal/observability/event/eventer.go index f12cd9d36..0cc6962f8 100644 --- a/internal/observability/event/eventer.go +++ b/internal/observability/event/eventer.go @@ -170,7 +170,8 @@ func NewAuditEncryptFilter(opt ...Option) (*encrypt.Filter, error) { } // NewEventer creates a new Eventer using the config. Supports options: -// WithNow, WithSerializationLock, WithBroker, WithAuditWrapper +// WithNow, WithSerializationLock, WithBroker, WithAuditWrapper, +// WithNoDefaultSink func NewEventer(log hclog.Logger, serializationLock *sync.Mutex, serverName string, c EventerConfig, opt ...Option) (*Eventer, error) { const op = "event.NewEventer" if log == nil { @@ -183,9 +184,11 @@ func NewEventer(log hclog.Logger, serializationLock *sync.Mutex, serverName stri return nil, fmt.Errorf("%s: missing server name: %w", op, ErrInvalidParameter) } + opts := getOpts(opt...) + // if there are no sinks in config, then we'll default to just one stderr // sink. - if len(c.Sinks) == 0 { + if len(c.Sinks) == 0 && !opts.withNoDefaultSink { c.Sinks = append(c.Sinks, DefaultSink()) } @@ -195,7 +198,6 @@ func NewEventer(log hclog.Logger, serializationLock *sync.Mutex, serverName stri var auditPipelines, observationPipelines, errPipelines, sysPipelines []pipeline - opts := getOpts(opt...) var b broker switch { case opts.withBroker != nil: diff --git a/internal/observability/event/options.go b/internal/observability/event/options.go index 44740c458..6a8cd4ff7 100644 --- a/internal/observability/event/options.go +++ b/internal/observability/event/options.go @@ -54,6 +54,7 @@ type options struct { withObservationSink bool // test only option withSysSink bool // test only option withSinkFormat SinkFormat // test only option + withNoDefaultSink bool // test only option } func getDefaultOptions() options { diff --git a/internal/observability/event/testing.go b/internal/observability/event/testing.go index e4220dd67..a0922ddb0 100644 --- a/internal/observability/event/testing.go +++ b/internal/observability/event/testing.go @@ -4,16 +4,41 @@ import ( "context" "io/ioutil" "os" + "sync" "testing" "time" pbs "github.com/hashicorp/boundary/internal/gen/controller/api/services" "github.com/hashicorp/boundary/sdk/pbs/controller/api/resources/groups" "github.com/hashicorp/eventlogger" + "github.com/hashicorp/go-hclog" "github.com/stretchr/testify/require" "google.golang.org/protobuf/types/known/wrapperspb" ) +// TestWithoutEventing allows the caller to "disable" all eventing for a test. +// You must not run the test in parallel (no calls to t.Parallel) since the +// function relies on modifying the system wide default eventer. +func TestWithoutEventing(t testing.TB) *Eventer { + t.Helper() + require := require.New(t) + testConfig := EventerConfig{ + AuditEnabled: false, + ObservationsEnabled: false, + SysEventsEnabled: false, + } + testLock := &sync.Mutex{} + testLogger := hclog.New(&hclog.LoggerOptions{ + Mutex: testLock, + Output: ioutil.Discard, + }) + testEventer, err := NewEventer(testLogger, testLock, "TestWithoutEventing", testConfig, withNoDefaultSink(t)) + require.NoError(err) + + require.NoError(InitSysEventer(testLogger, testLock, "TestWithoutEventing", WithEventer(testEventer))) + return testEventer +} + // TestGetEventerConfig is a test accessor for the eventer's config func TestGetEventerConfig(t testing.TB, e *Eventer) EventerConfig { t.Helper() @@ -231,6 +256,14 @@ func TestWithSysSink(t testing.TB) Option { } } +// withNoDefaultSink is an unexported test option +func withNoDefaultSink(t testing.TB) Option { + t.Helper() + return func(o *options) { + o.withNoDefaultSink = true + } +} + // testWithSinkFormat is an unexported and a test option func testWithSinkFormat(t testing.TB, fmt SinkFormat) Option { t.Helper() diff --git a/internal/observability/event/testing_test.go b/internal/observability/event/testing_test.go new file mode 100644 index 000000000..cdbed4c8c --- /dev/null +++ b/internal/observability/event/testing_test.go @@ -0,0 +1,55 @@ +package event_test + +import ( + "bytes" + "context" + "fmt" + "io" + "os" + "testing" + + "github.com/hashicorp/boundary/internal/observability/event" + "github.com/stretchr/testify/assert" +) + +func Test_TestWithoutEventing(t *testing.T) { + const op = "Test_TestWithoutEventing" + assert := assert.New(t) + + // this isn't the best solution for capturing stdout but it works for now... + captureFn := func(fn func()) string { + old := os.Stdout + defer func() { + os.Stderr = old + }() + + r, w, _ := os.Pipe() + os.Stderr = w + + { + fn() + } + + outC := make(chan string) + // copy the output in a separate goroutine so writing to stderr can't block indefinitely + go func() { + var buf bytes.Buffer + io.Copy(&buf, r) + outC <- buf.String() + }() + + // back to normal state + w.Close() + return <-outC + } + + assert.NotEmpty(captureFn(func() { + fmt.Fprintln(os.Stderr, "not-empty") + })) + + assert.Empty(captureFn(func() { + testCtx := context.Background() + event.TestWithoutEventing(t) + event.WriteSysEvent(testCtx, op, "test-event") + })) +} diff --git a/internal/perms/acl.go b/internal/perms/acl.go index d31b1838a..b8e2f3c0d 100644 --- a/internal/perms/acl.go +++ b/internal/perms/acl.go @@ -28,7 +28,7 @@ type ACLResults struct { } // Resource defines something within boundary that requires authorization -// capabilities. Resources must have a ScopeId. +// capabilities. Resources must have a ScopeId. type Resource struct { // ScopeId is the scope that contains the Resource. ScopeId string `json:"scope_id,omitempty"` @@ -58,7 +58,9 @@ func NewACL(grants ...Grant) ACL { } // Allowed determines if the grants for an ACL allow an action for a resource. -func (a ACL) Allowed(r Resource, aType action.Type) (results ACLResults) { +func (a ACL) Allowed(r Resource, aType action.Type, userId string, opt ...Option) (results ACLResults) { + opts := getOpts(opt...) + // First, get the grants within the specified scope grants := a.scopeMap[r.ScopeId] results.scopeMap = a.scopeMap @@ -110,6 +112,30 @@ func (a ACL) Allowed(r Resource, aType action.Type) (results ACLResults) { // is granted to the user already happened above. var found bool switch { + // Case 1: We only allow specific actions on specific types for the + // anonymous user. ID being supplied or not doesn't matter in this case, + // it must be an explicit type and action(s); adding this as an explicit + // case here prevents duplicating logic in two of the other more + // general-purpose cases below (3 and 4). See notes there about ID being + // present or not. + case !opts.withSkipAnonymousUserRestrictions && + (userId == AnonymousUserId || userId == ""): + switch { + // Allow discovery of scopes, so that auth methods within can be + // discovered + case grant.typ == r.Type && + grant.typ == resource.Scope && + (aType == action.List || aType == action.NoOp): + found = true + + // Allow discovery of and authenticating to auth methods + case grant.typ == r.Type && + grant.typ == resource.AuthMethod && + (aType == action.List || aType == action.NoOp || aType == action.Authenticate): + found = true + } + + // Case 2: // id=;actions= where ID cannot be a wildcard; or // id=;output_fields= where fields cannot be a // wildcard. @@ -122,10 +148,21 @@ func (a ACL) Allowed(r Resource, aType action.Type) (results ACLResults) { found = true - // type=;actions= when action is list or create. - // Must be a top level collection, otherwise must be one of the two - // formats specified below. Or, - // type=resource.type;output_fields= and no action. + // Case 3: type=;actions= when action is list or + // create. Must be a top level collection, otherwise must be one of the + // two formats specified in cases 4 or 5. Or, + // type=resource.type;output_fields= and no action. This is more + // of a semantic difference compared to 4 more than a security + // difference; this type is for clarity as it ties more closely to the + // concept of create and list as actions on a collection, operating on a + // collection directly. The format in case 4 will still work for + // create/list on collections but that's more of a shortcut to allow + // things like id=*;type=*;actions=* for admin flows so that you don't + // need to separate out explicit collection actions into separate typed + // grants for each collection within a role. This does mean there are + // "two ways of doing things" but it's a reasonable UX tradeoff given + // that "all IDs" can reasonably be construed to include "and the one + // I'm making" and "all of them for listing". case grant.id == "" && r.Id == "" && grant.typ == r.Type && @@ -136,6 +173,7 @@ func (a ACL) Allowed(r Resource, aType action.Type) (results ACLResults) { found = true + // Case 4: // id=*;type=;actions= where type cannot be // unknown but can be a wildcard to allow any resource at all; or // id=*;type=;output_fields= with no action. @@ -146,6 +184,7 @@ func (a ACL) Allowed(r Resource, aType action.Type) (results ACLResults) { found = true + // Case 5: // id=;type=;actions= where type can be a // wildcard and this this is operating on a non-top-level type. Same for // output fields only. diff --git a/internal/perms/acl_test.go b/internal/perms/acl_test.go index 194bb5904..14088b423 100644 --- a/internal/perms/acl_test.go +++ b/internal/perms/acl_test.go @@ -360,8 +360,12 @@ func Test_ACLAllowed(t *testing.T) { } acl := NewACL(grants...) for _, aa := range test.actionsAuthorized { - result := acl.Allowed(test.resource, aa.action) - assert.True(t, result.Authorized == aa.authorized, "action: %s, acl authorized: %t, test action authorized: %t", aa.action, acl.Allowed(test.resource, aa.action).Authorized, aa.authorized) + userId := test.userId + if userId == "" { + userId = "u_1234567890" + } + result := acl.Allowed(test.resource, aa.action, userId) + assert.True(t, result.Authorized == aa.authorized, "action: %s, acl authorized: %t, test action authorized: %t", aa.action, result.Authorized, aa.authorized) assert.ElementsMatch(t, result.OutputFields.Fields(), aa.outputFields) } }) @@ -380,3 +384,85 @@ func TestJsonMarshal(t *testing.T) { require.NoError(t, err) assert.Equal(t, `{"scope_id":"scope","id":"id","type":"controller"}`, string(out)) } + +// Test_AnonRestrictions loops through every resource and action and ensures +// that it always fails for the anonymous user, regardless of what is granted, +// except for the specific things allowed. +func Test_AnonRestrictions(t *testing.T) { + t.Parallel() + + type input struct { + name string + grant string + templatedType bool + shouldHaveSuccess bool + } + tests := []input{ + { + name: "id-specific", + grant: "id=foobar;actions=%s", + }, + { + name: "wildcard-id", + grant: "id=*;type=%s;actions=%s", + templatedType: true, + shouldHaveSuccess: true, + }, + { + name: "wildcard-id-and-type", + grant: "id=*;type=*;actions=%s", + }, + { + name: "no-id", + grant: "type=%s;actions=%s", + templatedType: true, + shouldHaveSuccess: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require, assert := require.New(t), assert.New(t) + for i := resource.Type(1); i <= resource.CredentialLibrary; i++ { + if i == resource.Controller || i == resource.Worker { + continue + } + for j := action.Type(1); j <= action.RemoveHostSources; j++ { + res := Resource{ + ScopeId: scope.Global.String(), + Id: "foobar", + Type: resource.Type(i), + } + grant := test.grant + if test.templatedType { + grant = fmt.Sprintf(grant, resource.Type(i).String(), action.Type(j).String()) + } else { + grant = fmt.Sprintf(grant, action.Type(j).String()) + } + + parsedGrant, err := Parse(scope.Global.String(), grant, WithSkipFinalValidation(true)) + require.NoError(err) + + acl := NewACL(parsedGrant) + results := acl.Allowed(res, action.Type(j), AnonymousUserId) + + switch test.shouldHaveSuccess { + case true: + // Ensure it's one of the specific cases and fail otherwise + switch { + case i == resource.Scope && (j == action.List || j == action.NoOp): + assert.True(results.Authorized, fmt.Sprintf("i: %v, j: %v", i, j)) + case i == resource.AuthMethod && (j == action.List || j == action.NoOp || j == action.Authenticate): + assert.True(results.Authorized, fmt.Sprintf("i: %v, j: %v", i, j)) + default: + assert.False(results.Authorized, fmt.Sprintf("i: %v, j: %v", i, j)) + } + default: + // Should always fail + assert.False(results.Authorized) + } + } + } + }) + } +} diff --git a/internal/perms/grants.go b/internal/perms/grants.go index 279573f38..f4817e495 100644 --- a/internal/perms/grants.go +++ b/internal/perms/grants.go @@ -422,7 +422,7 @@ func Parse(scopeId, grantString string, opt ...Option) (Grant, error) { } var allowed bool for k := range grant.actions { - results := acl.Allowed(r, k) + results := acl.Allowed(r, k, AnonymousUserId, WithSkipAnonymousUserRestrictions(true)) if results.Authorized { allowed = true break diff --git a/internal/perms/option.go b/internal/perms/option.go index ab779ddee..4aa29574b 100644 --- a/internal/perms/option.go +++ b/internal/perms/option.go @@ -4,7 +4,9 @@ package perms func getOpts(opt ...Option) options { opts := getDefaultOptions() for _, o := range opt { - o(&opts) + if o != nil { + o(&opts) + } } return opts } @@ -14,9 +16,10 @@ type Option func(*options) // options = how options are represented type options struct { - withUserId string - withAccountId string - withSkipFinalValidation bool + withUserId string + withAccountId string + withSkipFinalValidation bool + withSkipAnonymousUserRestrictions bool } func getDefaultOptions() options { @@ -45,3 +48,12 @@ func WithSkipFinalValidation(skipFinalValidation bool) Option { o.withSkipFinalValidation = skipFinalValidation } } + +// WithSkipAnonymousUserRestrictions allows skipping the restrictions on +// anonymous users, useful when e.g. validating parsed grants where we may not +// have a user ID yet. +func WithSkipAnonymousUserRestrictions(with bool) Option { + return func(o *options) { + o.withSkipAnonymousUserRestrictions = with + } +} diff --git a/internal/perms/option_test.go b/internal/perms/option_test.go new file mode 100644 index 000000000..51c02071c --- /dev/null +++ b/internal/perms/option_test.go @@ -0,0 +1,43 @@ +package perms + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test_GetOpts(t *testing.T) { + t.Parallel() + t.Run("nil", func(t *testing.T) { + opts := getOpts(nil) + assert.NotNil(t, opts) + }) + t.Run("with-user-id", func(t *testing.T) { + assert := assert.New(t) + opts := getOpts() + assert.Empty(opts.withUserId) + opts = getOpts(WithUserId("foo")) + assert.Equal("foo", opts.withUserId) + }) + t.Run("with-account-id", func(t *testing.T) { + assert := assert.New(t) + opts := getOpts() + assert.Empty(opts.withAccountId) + opts = getOpts(WithAccountId("foo")) + assert.Equal("foo", opts.withAccountId) + }) + t.Run("with-skip-final-validation", func(t *testing.T) { + assert := assert.New(t) + opts := getOpts() + assert.False(opts.withSkipFinalValidation) + opts = getOpts(WithSkipFinalValidation(true)) + assert.True(opts.withSkipFinalValidation) + }) + t.Run("with-skip-anonymous-user-restrictions", func(t *testing.T) { + assert := assert.New(t) + opts := getOpts() + assert.False(opts.withSkipAnonymousUserRestrictions) + opts = getOpts(WithSkipAnonymousUserRestrictions(true)) + assert.True(opts.withSkipAnonymousUserRestrictions) + }) +} diff --git a/internal/perms/output_fields_test.go b/internal/perms/output_fields_test.go index 7a7080559..3ef0532f2 100644 --- a/internal/perms/output_fields_test.go +++ b/internal/perms/output_fields_test.go @@ -221,7 +221,7 @@ func Test_ACLOutputFields(t *testing.T) { grants = append(grants, grant) } acl := NewACL(grants...) - results := acl.Allowed(test.resource, test.action) + results := acl.Allowed(test.resource, test.action, AnonymousUserId, WithSkipAnonymousUserRestrictions(true)) assert.ElementsMatch(t, results.OutputFields.Fields(), test.fields) assert.True(t, test.authorized == results.Authorized) }) diff --git a/internal/proto/controller/api/services/v1/session_service.proto b/internal/proto/controller/api/services/v1/session_service.proto index 6d379450d..e0eeed08b 100644 --- a/internal/proto/controller/api/services/v1/session_service.proto +++ b/internal/proto/controller/api/services/v1/session_service.proto @@ -63,6 +63,9 @@ message ListSessionsRequest { string scope_id = 1; // @gotags: `class:"public"` bool recursive = 20 [json_name = "recursive"]; // @gotags: `class:"public"` string filter = 30 [json_name = "filter"]; // @gotags: `class:"public"` + // Experimental. By default only non-terminated (i.e. pending, active, canceling) are returned. + // Set this option to include terminated sessions as well. + bool include_terminated = 40 [json_name = "include_terminated"]; // @gotags: `class:"public"` } message ListSessionsResponse { diff --git a/internal/session/job_delete_terminated_sessions.go b/internal/session/job_delete_terminated_sessions.go new file mode 100644 index 000000000..4318a9e12 --- /dev/null +++ b/internal/session/job_delete_terminated_sessions.go @@ -0,0 +1,75 @@ +package session + +import ( + "context" + "time" + + "github.com/hashicorp/boundary/internal/errors" + "github.com/hashicorp/boundary/internal/scheduler" +) + +type deleteTerminatedJob struct { + repo *Repository + + // the amount of time that a session must be in the terminated + // state for it to be deleted. + threshold time.Duration + + // the number of sessions deleted in the most recent run + deletedInRun int +} + +func newDeleteTerminatedJob(ctx context.Context, repo *Repository, threshold time.Duration) (*deleteTerminatedJob, error) { + const op = "session.newDeleteTerminatedJob" + switch { + case repo == nil: + return nil, errors.New(ctx, errors.InvalidParameter, op, "missing repository") + } + + return &deleteTerminatedJob{ + repo: repo, + threshold: threshold, + }, nil +} + +// Status reports the job’s current status. The status is periodically persisted by +// the scheduler when a job is running, and will be used to verify a job is making progress. +func (d *deleteTerminatedJob) Status() scheduler.JobStatus { + return scheduler.JobStatus{ + Completed: d.deletedInRun, + Total: d.deletedInRun, + } +} + +// Run performs the required work depending on the implementation. +// The context is used to notify the job that it should exit early. +func (d *deleteTerminatedJob) Run(ctx context.Context) error { + const op = "session.(deleteTerminatedJob).Run" + d.deletedInRun = 0 + var err error + + d.deletedInRun, err = d.repo.deleteSessionsTerminatedBefore(ctx, d.threshold) + if err != nil { + return errors.Wrap(ctx, err, op) + } + return nil +} + +// NextRunIn returns the duration until the next job run should be scheduled. This +// method is invoked after a run has successfully completed and the next run time +// is being persisted by the scheduler. If an error is returned, the error will be logged +// but the duration returned will still be used in scheduling. If a zero duration is returned +// the job will be scheduled to run again immediately. +func (d *deleteTerminatedJob) NextRunIn(_ context.Context) (time.Duration, error) { + return time.Minute * 30, nil +} + +// Name is the unique name of the job. +func (d *deleteTerminatedJob) Name() string { + return "delete_terminated_sessions" +} + +// Description is the human readable description of the job. +func (d *deleteTerminatedJob) Description() string { + return "Delete terminated sessions that were terminated " +} diff --git a/internal/session/job_delete_terminated_sessions_test.go b/internal/session/job_delete_terminated_sessions_test.go new file mode 100644 index 000000000..40fc58d92 --- /dev/null +++ b/internal/session/job_delete_terminated_sessions_test.go @@ -0,0 +1,106 @@ +package session + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/hashicorp/boundary/internal/db" + "github.com/hashicorp/boundary/internal/iam" + "github.com/hashicorp/boundary/internal/kms" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDeleteTermiantedSessionsJob(t *testing.T) { + ctx := context.Background() + conn, _ := db.TestSetup(t, "postgres") + rw := db.New(conn) + wrapper := db.TestWrapper(t) + iamRepo := iam.TestRepo(t, conn, wrapper) + kms := kms.TestKms(t, conn, wrapper) + repo, err := NewRepository(rw, rw, kms) + require.NoError(t, err) + composedOf := TestSessionParams(t, conn, wrapper, iamRepo) + require.NoError(t, err) + + cases := []struct { + sessionCount int + terminateCount int + threshold time.Duration + expected int + }{ + { + 0, + 0, + time.Nanosecond, + 0, + }, + { + 1, + 1, + time.Nanosecond, + 1, + }, + { + 1, + 1, + time.Hour, + 0, + }, + { + 10, + 10, + time.Nanosecond, + 10, + }, + { + 10, + 4, + time.Nanosecond, + 4, + }, + { + 10, + 0, + time.Nanosecond, + 0, + }, + { + 10, + 10, + time.Hour, + 0, + }, + } + + for _, tc := range cases { + t.Run(fmt.Sprintf("%d_%d_%s", tc.sessionCount, tc.terminateCount, tc.threshold), func(t *testing.T) { + t.Cleanup(func() { + sdb, err := conn.SqlDB(ctx) + require.NoError(t, err) + _, err = sdb.Exec(`delete from session;`) + require.NoError(t, err) + }) + + for i := 0; i < tc.sessionCount; i++ { + s := TestSession(t, conn, wrapper, composedOf) + if i < tc.terminateCount { + _, err = repo.CancelSession(ctx, s.PublicId, s.Version) + require.NoError(t, err) + } + + } + c, err := repo.TerminateCompletedSessions(ctx) + require.NoError(t, err) + assert.Equal(t, tc.terminateCount, c) + + job, err := newDeleteTerminatedJob(ctx, repo, tc.threshold) + require.NoError(t, err) + err = job.Run(ctx) + require.NoError(t, err) + assert.Equal(t, tc.expected, job.deletedInRun) + }) + } +} diff --git a/internal/session/session_cleanup_job.go b/internal/session/job_session_cleanup.go similarity index 100% rename from internal/session/session_cleanup_job.go rename to internal/session/job_session_cleanup.go diff --git a/internal/session/session_cleanup_job_test.go b/internal/session/job_session_cleanup_test.go similarity index 100% rename from internal/session/session_cleanup_job_test.go rename to internal/session/job_session_cleanup_test.go diff --git a/internal/session/jobs.go b/internal/session/jobs.go index f99d7ed06..70f1e113b 100644 --- a/internal/session/jobs.go +++ b/internal/session/jobs.go @@ -6,12 +6,16 @@ import ( "time" "github.com/hashicorp/boundary/internal/db" + "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/scheduler" ) -// RegisterJobs registers plugin host related jobs with the provided scheduler. -func RegisterJobs(ctx context.Context, scheduler *scheduler.Scheduler, w db.Writer, gracePeriod time.Duration) error { +const deleteTerminatedThreshold = time.Hour + +// RegisterJobs registers session related jobs with the provided scheduler. +func RegisterJobs(ctx context.Context, scheduler *scheduler.Scheduler, w db.Writer, r db.Reader, k *kms.Kms, gracePeriod time.Duration) error { const op = "session.RegisterJobs" + sessionConnectionCleanupJob, err := newSessionConnectionCleanupJob(w, gracePeriod) if err != nil { return fmt.Errorf("error creating session cleanup job: %w", err) @@ -20,5 +24,17 @@ func RegisterJobs(ctx context.Context, scheduler *scheduler.Scheduler, w db.Writ return fmt.Errorf("error registering session cleanup job: %w", err) } + repo, err := NewRepository(r, w, k) + if err != nil { + return fmt.Errorf("error creating repository: %w", err) + } + deleteTerminatedJob, err := newDeleteTerminatedJob(ctx, repo, deleteTerminatedThreshold) + if err != nil { + return fmt.Errorf("error creating delete terminated session job: %w", err) + } + if err = scheduler.RegisterJob(ctx, deleteTerminatedJob); err != nil { + return fmt.Errorf("error registering delete terminated session job: %w", err) + } + return nil } diff --git a/internal/session/options.go b/internal/session/options.go index 1fdaaeb06..a46fe134d 100644 --- a/internal/session/options.go +++ b/internal/session/options.go @@ -31,6 +31,7 @@ type options struct { withSessionIds []string withDbOpts []db.Option withWorkerStateDelay time.Duration + withTerminated bool } func getDefaultOptions() options { @@ -112,3 +113,10 @@ func WithWorkerStateDelay(d time.Duration) Option { o.withWorkerStateDelay = d } } + +// WithTerminated is used to include terminated sessions in a list request. +func WithTerminated(withTerminated bool) Option { + return func(o *options) { + o.withTerminated = withTerminated + } +} diff --git a/internal/session/query.go b/internal/session/query.go index 3e7a3e3b1..4e086c37f 100644 --- a/internal/session/query.go +++ b/internal/session/query.go @@ -117,11 +117,20 @@ session_connection_limit(expiration_time, connection_limit) as ( select expiration_time, connection_limit, current_connection_count from session_connection_limit, session_connection_count; +` + nonTerminatedSessionPublicIdList = ` +select public_id, scope_id, user_id from session +where + session.termination_reason is null +and + scope_id = any(@scope_ids) +; ` sessionPublicIdList = ` select public_id, scope_id, user_id from session -%s +where + scope_id = any(@scope_ids) ; ` @@ -402,6 +411,17 @@ where and ss.end_time is null %s ; +` + deleteTerminated = ` +delete from session +using session_state +where + session.public_id = session_state.session_id +and + session_state.state = 'terminated' +and + session_state.start_time < wt_sub_seconds_from_now(@threshold_seconds) +; ` ) diff --git a/internal/session/repository.go b/internal/session/repository.go index 7ded1414e..d35abfdb0 100644 --- a/internal/session/repository.go +++ b/internal/session/repository.go @@ -83,8 +83,6 @@ func (r *Repository) convertToSessions(ctx context.Context, sessionList []*sessi return nil, nil } sessions := []*Session{} - // deduplication map of connections by connection_id - connections := map[string]*Connection{} // deduplication map of states by end_time states := map[*timestamp.Timestamp]*State{} var prevSessionId string @@ -92,10 +90,6 @@ func (r *Repository) convertToSessions(ctx context.Context, sessionList []*sessi for _, sv := range sessionList { if sv.PublicId != prevSessionId { if prevSessionId != "" { - for _, c := range connections { - workingSession.Connections = append(workingSession.Connections, c) - } - connections = map[string]*Connection{} for _, s := range states { workingSession.States = append(workingSession.States, s) } @@ -155,22 +149,6 @@ func (r *Repository) convertToSessions(ctx context.Context, sessionList []*sessi } } - if sv.ConnectionId != "" { - if _, ok := connections[sv.ConnectionId]; !ok { - connections[sv.ConnectionId] = &Connection{ - PublicId: sv.ConnectionId, - SessionId: sv.PublicId, - ClientTcpAddress: sv.ClientTcpAddress, - ClientTcpPort: sv.ClientTcpPort, - EndpointTcpAddress: sv.EndpointTcpAddress, - EndpointTcpPort: sv.EndpointTcpPort, - BytesUp: sv.BytesUp, - BytesDown: sv.BytesDown, - ClosedReason: sv.ClosedReason, - } - } - } - } for _, s := range states { workingSession.States = append(workingSession.States, s) @@ -178,9 +156,6 @@ func (r *Repository) convertToSessions(ctx context.Context, sessionList []*sessi sort.Slice(workingSession.States, func(i, j int) bool { return workingSession.States[i].StartTime.GetTimestamp().AsTime().After(workingSession.States[j].StartTime.GetTimestamp().AsTime()) }) - for _, c := range connections { - workingSession.Connections = append(workingSession.Connections, c) - } sessions = append(sessions, workingSession) return sessions, nil } diff --git a/internal/session/repository_session.go b/internal/session/repository_session.go index bc89c7183..7ba29ac74 100644 --- a/internal/session/repository_session.go +++ b/internal/session/repository_session.go @@ -7,6 +7,7 @@ import ( "database/sql" "fmt" "strings" + "time" "github.com/hashicorp/boundary/internal/boundary" "github.com/hashicorp/boundary/internal/db" @@ -200,32 +201,31 @@ func (r *Repository) LookupSession(ctx context.Context, sessionId string, _ ...O return &session, authzSummary, nil } -// FetchAuthzProtectedEntitiesByScope implements boundary.AuthzProtectedEntityProvider -func (r *Repository) FetchAuthzProtectedEntitiesByScope(ctx context.Context, scopeIds []string) (map[string][]boundary.AuthzProtectedEntity, error) { - const op = "session.(Repository).FetchAuthzProtectedEntityInfo" +// fetchAuthzProtectedSessionsByScope fetches sessions for the given scopes. +// Note that the sessions are not fully populated, and only contain the +// necessary information to implement the boundary.AuthzProtectedEntity +// interface. Supports the WithTerminated option. +func (r *Repository) fetchAuthzProtectedSessionsByScope( + ctx context.Context, scopeIds []string, opt ...Option, +) (map[string][]boundary.AuthzProtectedEntity, error) { + const op = "session.(Repository).fetchAuthzProtectedSessionsByScope" - var where string - var args []interface{} + opts := getOpts(opt...) - inClauseCnt := 0 - - switch len(scopeIds) { - case 0: + if len(scopeIds) == 0 { return nil, errors.New(ctx, errors.InvalidParameter, op, "no scopes given") - case 1: - inClauseCnt += 1 - where, args = fmt.Sprintf("where scope_id = @%d", inClauseCnt), append(args, sql.Named(fmt.Sprintf("%d", inClauseCnt), scopeIds[0])) - default: - idsInClause := make([]string, 0, len(scopeIds)) - for _, id := range scopeIds { - inClauseCnt += 1 - idsInClause, args = append(idsInClause, fmt.Sprintf("@%d", inClauseCnt)), append(args, sql.Named(fmt.Sprintf("%d", inClauseCnt), id)) - } - where = fmt.Sprintf("where scope_id in (%s)", strings.Join(idsInClause, ",")) } - q := sessionPublicIdList - query := fmt.Sprintf(q, where) + args := []interface{}{ + sql.Named("scope_ids", "{"+strings.Join(scopeIds, ",")+"}"), + } + + var query string + if opts.withTerminated { + query = sessionPublicIdList + } else { + query = nonTerminatedSessionPublicIdList + } rows, err := r.reader.Query(ctx, query, args) if err != nil { @@ -681,6 +681,20 @@ func (r *Repository) checkIfNoLongerActive(ctx context.Context, reportedSessions return notActive, nil } +func (r *Repository) deleteSessionsTerminatedBefore(ctx context.Context, threshold time.Duration) (int, error) { + const op = "session.(Repository).deleteTerminated" + + args := []any{ + sql.Named("threshold_seconds", threshold.Seconds()), + } + + c, err := r.writer.Exec(ctx, deleteTerminated, args) + if err != nil { + return 0, errors.Wrap(ctx, err, op, errors.WithMsg("error deleting terminated sessions")) + } + return c, nil +} + func fetchStates(ctx context.Context, r db.Reader, sessionId string, opt ...db.Option) ([]*State, error) { const op = "session.fetchStates" var states []*State diff --git a/internal/session/repository_session_test.go b/internal/session/repository_session_test.go index 0dd8d440c..345440260 100644 --- a/internal/session/repository_session_test.go +++ b/internal/session/repository_session_test.go @@ -125,7 +125,8 @@ func TestRepository_ListSession(t *testing.T) { require.NoError(err) assert.Equal(tt.wantCnt, len(got)) for i := 0; i < len(got); i++ { - assert.Equal(tt.withConnections, len(got[i].Connections)) + // connections should not be returned for list requests + assert.Equal(0, len(got[i].Connections)) for _, c := range got[i].Connections { assert.Equal("127.0.0.1", c.ClientTcpAddress) assert.Equal(uint32(22), c.ClientTcpPort) @@ -1473,3 +1474,320 @@ func TestRepository_deleteTargetFKey(t *testing.T) { }) } } + +func TestRepository_deleteTerminated(t *testing.T) { + ctx := context.Background() + conn, _ := db.TestSetup(t, "postgres") + rw := db.New(conn) + wrapper := db.TestWrapper(t) + iamRepo := iam.TestRepo(t, conn, wrapper) + kms := kms.TestKms(t, conn, wrapper) + repo, err := NewRepository(rw, rw, kms) + composedOf := TestSessionParams(t, conn, wrapper, iamRepo) + + cases := []struct { + sessionCount int + terminateCount int + threshold time.Duration + expected int + }{ + { + 0, + 0, + time.Nanosecond, + 0, + }, + { + 1, + 1, + time.Nanosecond, + 1, + }, + { + 1, + 1, + time.Hour, + 0, + }, + { + 10, + 10, + time.Nanosecond, + 10, + }, + { + 10, + 4, + time.Nanosecond, + 4, + }, + { + 10, + 0, + time.Nanosecond, + 0, + }, + { + 10, + 10, + time.Hour, + 0, + }, + } + + for _, tc := range cases { + t.Run(fmt.Sprintf("%d_%d_%s", tc.sessionCount, tc.terminateCount, tc.threshold), func(t *testing.T) { + t.Cleanup(func() { + sdb, err := conn.SqlDB(ctx) + require.NoError(t, err) + _, err = sdb.Exec(`delete from session;`) + require.NoError(t, err) + }) + + for i := 0; i < tc.sessionCount; i++ { + s := TestSession(t, conn, wrapper, composedOf) + if i < tc.terminateCount { + _, err = repo.CancelSession(ctx, s.PublicId, s.Version) + require.NoError(t, err) + } + + } + c, err := repo.TerminateCompletedSessions(ctx) + require.NoError(t, err) + assert.Equal(t, tc.terminateCount, c) + + c, err = repo.deleteSessionsTerminatedBefore(ctx, tc.threshold) + require.NoError(t, err) + assert.Equal(t, tc.expected, c) + }) + } +} + +func TestFetchAuthzProtectedSessionsByScopes(t *testing.T) { + conn, _ := db.TestSetup(t, "postgres") + ctx := context.Background() + wrapper := db.TestWrapper(t) + iamRepo := iam.TestRepo(t, conn, wrapper) + rw := db.New(conn) + kms := kms.TestKms(t, conn, wrapper) + repo, err := NewRepository(rw, rw, kms) + require.NoError(t, err) + composedOf := TestSessionParams(t, conn, wrapper, iamRepo) + + _, pWithOtherSessions := iam.TestScopes(t, iamRepo) + + hcOther := static.TestCatalogs(t, conn, pWithOtherSessions.GetPublicId(), 1)[0] + hsOther := static.TestSets(t, conn, hcOther.GetPublicId(), 1)[0] + hOther := static.TestHosts(t, conn, hcOther.GetPublicId(), 1)[0] + static.TestSetMembers(t, conn, hsOther.GetPublicId(), []*static.Host{hOther}) + tarOther := tcp.TestTarget(ctx, t, conn, pWithOtherSessions.GetPublicId(), "test", target.WithHostSources([]string{hsOther.GetPublicId()})) + + composedOfOther := ComposedOf{ + UserId: composedOf.UserId, + HostId: hOther.GetPublicId(), + TargetId: tarOther.GetPublicId(), + HostSetId: hsOther.GetPublicId(), + AuthTokenId: composedOf.AuthTokenId, + ScopeId: pWithOtherSessions.GetPublicId(), + Endpoint: "tcp://127.0.0.1:22", + } + + type testCase struct { + name string + createCnt int + terminateCnt int + otherCnt int + otherTerminateCnt int + opts []Option + reqScopes []string + wantCnt int + wantOtherCnt int + wantErr bool + } + + cases := []testCase{ + { + name: "NonTerminated/none", + createCnt: 0, + reqScopes: []string{composedOf.ScopeId}, + wantCnt: 0, + wantErr: false, + }, + { + name: "NonTerminated/one", + createCnt: 1, + reqScopes: []string{composedOf.ScopeId}, + wantCnt: 1, + wantErr: false, + }, + { + name: "NonTerminated/many", + createCnt: 5, + reqScopes: []string{composedOf.ScopeId}, + wantCnt: 5, + wantErr: false, + }, + { + name: "NonTerminated/many one terminated", + createCnt: 5, + terminateCnt: 1, + reqScopes: []string{composedOf.ScopeId}, + wantCnt: 4, + wantErr: false, + }, + { + name: "NonTerminated/many terminated", + createCnt: 5, + terminateCnt: 3, + reqScopes: []string{composedOf.ScopeId}, + wantCnt: 2, + wantErr: false, + }, + { + name: "NonTerminated/many multiple scopes", + createCnt: 5, + terminateCnt: 3, + reqScopes: []string{composedOf.ScopeId, composedOfOther.ScopeId}, + wantCnt: 2, + wantErr: false, + }, + { + name: "NonTerminated/many multiple scopes", + createCnt: 5, + terminateCnt: 3, + otherCnt: 3, + otherTerminateCnt: 1, + reqScopes: []string{composedOf.ScopeId, composedOfOther.ScopeId}, + wantCnt: 2, + wantOtherCnt: 2, + wantErr: false, + }, + { + name: "NonTerminated/no scopes", + createCnt: 2, + terminateCnt: 1, + otherCnt: 2, + otherTerminateCnt: 1, + reqScopes: []string{}, + wantErr: true, + }, + { + name: "none", + opts: []Option{WithTerminated(true)}, + createCnt: 0, + reqScopes: []string{composedOf.ScopeId}, + wantCnt: 0, + wantErr: false, + }, + { + name: "one", + opts: []Option{WithTerminated(true)}, + createCnt: 1, + reqScopes: []string{composedOf.ScopeId}, + wantCnt: 1, + wantErr: false, + }, + { + name: "many", + opts: []Option{WithTerminated(true)}, + createCnt: 5, + reqScopes: []string{composedOf.ScopeId}, + wantCnt: 5, + wantErr: false, + }, + { + name: "many one terminated", + opts: []Option{WithTerminated(true)}, + createCnt: 5, + terminateCnt: 1, + reqScopes: []string{composedOf.ScopeId}, + wantCnt: 5, + wantErr: false, + }, + { + name: "many terminated", + opts: []Option{WithTerminated(true)}, + createCnt: 5, + terminateCnt: 3, + reqScopes: []string{composedOf.ScopeId}, + wantCnt: 5, + wantErr: false, + }, + { + name: "many multiple scopes", + opts: []Option{WithTerminated(true)}, + createCnt: 5, + terminateCnt: 3, + reqScopes: []string{composedOf.ScopeId, composedOfOther.ScopeId}, + wantCnt: 5, + wantErr: false, + }, + { + name: "many multiple scopes", + opts: []Option{WithTerminated(true)}, + createCnt: 5, + terminateCnt: 3, + otherCnt: 3, + otherTerminateCnt: 1, + reqScopes: []string{composedOf.ScopeId, composedOfOther.ScopeId}, + wantCnt: 5, + wantOtherCnt: 3, + wantErr: false, + }, + { + name: "no scopes", + opts: []Option{WithTerminated(true)}, + createCnt: 2, + terminateCnt: 1, + otherCnt: 2, + otherTerminateCnt: 1, + reqScopes: []string{}, + wantErr: true, + }, + } + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + db.TestDeleteWhere(t, conn, func() interface{} { i := AllocSession(); return &i }(), "1=1") + + testSessions := []*Session{} + for i := 0; i < tt.createCnt; i++ { + s := TestSession(t, conn, wrapper, composedOf) + _ = TestState(t, conn, s.PublicId, StatusActive) + testSessions = append(testSessions, s) + } + for i := 0; i < tt.terminateCnt; i++ { + _, err := repo.CancelSession(ctx, testSessions[i].PublicId, testSessions[i].Version) + require.NoError(err) + } + terminated, err := repo.TerminateCompletedSessions(ctx) + require.NoError(err) + require.Equal(tt.terminateCnt, terminated) + + otherTestSessions := []*Session{} + for i := 0; i < tt.otherCnt; i++ { + s := TestSession(t, conn, wrapper, composedOfOther) + _ = TestState(t, conn, s.PublicId, StatusActive) + otherTestSessions = append(otherTestSessions, s) + } + for i := 0; i < tt.otherTerminateCnt; i++ { + _, err := repo.CancelSession(ctx, otherTestSessions[i].PublicId, otherTestSessions[i].Version) + require.NoError(err) + } + terminated, err = repo.TerminateCompletedSessions(ctx) + require.NoError(err) + require.Equal(tt.otherTerminateCnt, terminated) + + assert.Equal(tt.otherCnt, len(otherTestSessions)) + + got, err := repo.fetchAuthzProtectedSessionsByScope(ctx, tt.reqScopes, tt.opts...) + if tt.wantErr { + require.Error(err) + return + } + require.NoError(err) + assert.Equal(tt.wantCnt, len(got[composedOf.ScopeId])) + assert.Equal(tt.wantOtherCnt, len(got[composedOfOther.ScopeId])) + }) + } +} diff --git a/internal/session/service_list_for_authz_check.go b/internal/session/service_list_for_authz_check.go new file mode 100644 index 000000000..c1c2f0508 --- /dev/null +++ b/internal/session/service_list_for_authz_check.go @@ -0,0 +1,23 @@ +package session + +import ( + "context" + + "github.com/hashicorp/boundary/internal/boundary" +) + +type listForAuthzCheckFunc func(ctx context.Context, scopeIds []string) (map[string][]boundary.AuthzProtectedEntity, error) + +func (a listForAuthzCheckFunc) FetchAuthzProtectedEntitiesByScope(ctx context.Context, scopeIds []string) (map[string][]boundary.AuthzProtectedEntity, error) { + return a(ctx, scopeIds) +} + +// ListForAuthzCheck returns a functions that fetches sessions for the given +// scopes. Note that the sessions are not fully populated, and only contain the +// necessary information to implement the boundary.AuthzProtectedEntity +// interface. Supports the WithTerminated option. +func ListForAuthzCheck(repo *Repository, opt ...Option) listForAuthzCheckFunc { + return func(ctx context.Context, scopeIds []string) (map[string][]boundary.AuthzProtectedEntity, error) { + return repo.fetchAuthzProtectedSessionsByScope(ctx, scopeIds, opt...) + } +} diff --git a/internal/session/session.go b/internal/session/session.go index 2c9e51579..f4a479654 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -458,16 +458,6 @@ type sessionListView struct { PreviousEndTime *timestamp.Timestamp `json:"previous_end_time,omitempty" gorm:"default:current_timestamp"` StartTime *timestamp.Timestamp `json:"start_time,omitempty" gorm:"default:current_timestamp;primary_key"` EndTime *timestamp.Timestamp `json:"end_time,omitempty" gorm:"default:current_timestamp"` - - // Connection fields - ConnectionId string `json:"connection_id,omitempty" gorm:"default:null"` - ClientTcpAddress string `json:"client_tcp_address,omitempty" gorm:"default:null"` - ClientTcpPort uint32 `json:"client_tcp_port,omitempty" gorm:"default:null"` - EndpointTcpAddress string `json:"endpoint_tcp_address,omitempty" gorm:"default:null"` - EndpointTcpPort uint32 `json:"endpoint_tcp_port,omitempty" gorm:"default:null"` - BytesUp uint64 `json:"bytes_up,omitempty" gorm:"default:null"` - BytesDown uint64 `json:"bytes_down,omitempty" gorm:"default:null"` - ClosedReason string `json:"closed_reason,omitempty" gorm:"default:null"` } // TableName returns the tablename to override the default gorm table name diff --git a/internal/types/action/action.go b/internal/types/action/action.go index 010f3a699..77e2b6264 100644 --- a/internal/types/action/action.go +++ b/internal/types/action/action.go @@ -54,6 +54,10 @@ const ( SetHostSources Type = 43 RemoveHostSources Type = 44 CreateWorkerLed Type = 45 + + // When adding new actions, be sure to update: + // + // * The Test_AnonRestrictions test ) var Map = map[string]Type{ diff --git a/internal/types/resource/resource.go b/internal/types/resource/resource.go index c8f40e9cc..fd833e6a9 100644 --- a/internal/types/resource/resource.go +++ b/internal/types/resource/resource.go @@ -30,6 +30,7 @@ const ( // * The Grant.validateType function and test // * The perms.topLevelType function // * The scopes service collection actions for appropriate scopes + // * The Test_AnonRestrictions test ) func (r Type) MarshalJSON() ([]byte, error) { diff --git a/internal/ui/VERSION b/internal/ui/VERSION index c5193b209..cf707da16 100644 --- a/internal/ui/VERSION +++ b/internal/ui/VERSION @@ -1,4 +1,4 @@ -698160d423e8fee640d3286c3a3f17434f56a12b +dc78fe90286d8b24e70dca36b19ce77a5a032dcb # This file determines the version of the UI to embed in the boundary binary. # Update this file by running 'make update-ui-version' from the root of this repo. # Set UI_COMMITISH when running the above target to update to a specific version. diff --git a/website/content/docs/concepts/security/permissions/assignable-permissions.mdx b/website/content/docs/concepts/security/permissions/assignable-permissions.mdx index 9e9886a2e..b1f65df4d 100644 --- a/website/content/docs/concepts/security/permissions/assignable-permissions.mdx +++ b/website/content/docs/concepts/security/permissions/assignable-permissions.mdx @@ -24,33 +24,51 @@ know. #### Subactions -Starting in Boundary 0.1.6, some subactions are supported. These actions have a -format `top_level_action:subaction`, such as `read:self`. Being granted the top -level action infers being granted all subactions. Thus, if a grant conveys -`read`, it also matches the API actions `read` and `read:self`. However, if a -grant conveys `read:self`, it will match the API action `read:self` but will not -match `read`. +Some subactions are supported. These actions have a format +`top_level_action:subaction`, such as `read:self`. Being granted the top level +action infers being granted all subactions. Thus, if a grant conveys `read`, it +also matches the API actions `read` and `read:self`. However, if a grant conveys +`read:self`, it will match the API action `read:self` but will not match `read`. -#### The `no-op` Action +#### The `no-op` Action and Listing Visibility -Starting in Boundary 0.2.1 there is an action that can be granted called -`no-op`. As might be apparent, `no-op` is not used for any real action in -Boundary; the purpose of this action is for listing visibility. Boundary only -shows resources in the output of a `list` action for which a user has at least -one granted action. Thus, without `no-op`, in order for a resource to be visible -in a `list` action, a different action such as `read` would have to be granted -to a user. This could result in exposing more information than desired, -especially in the case of listing scopes and authentication methods so that -users can perform initial authentication to Boundary (that is, granting access -to `u_anon`). +There is an action that can be granted called `no-op`. As might be apparent, +`no-op` is not used for any real action in Boundary; the purpose of this action +is for listing visibility. Boundary only shows resources in the output of a +`list` action for which a user has at least one granted action. Thus, without +`no-op`, in order for a resource to be visible in a `list` action, a different +action such as `read` would have to be granted to a user. This could result in +exposing more information than desired, especially in the case of listing scopes +and authentication methods so that users can perform initial authentication to +Boundary (that is, granting access to `u_anon`). By granting the `no-op` action to users, they can see the resources in the output of a list command without needing other capability grants as well. +### Anonymous User Restrictions + +Starting in Boundary 0.9.0, there are severe limits placed on the actions +allowed to be assigned to the anonymous user `u_anon`. This is meant to avoid +situations where an operator accidentally or mistakenly adds the anonymous user +to a role that provides more privileges than might be intended for +unauthenticated users. + +The set of actions is currently restricted to listing scopes and auth methods +and authentication to auth methods (plus `no-op` actions for listing +visibility). If further use-cases arise from user feedback this list can be +expanded. + +Note: there is no special error message returned when access is denied due to it +being disallowed to the anonymous user. It is still possible to assign grants to +a role on which the anonymous user is a principal that can never match the +anonymous user. In the future this may change, but trying to limit this +introduces other restrictions on reasonable administrative workflows so an +acceptable set of tradeoffs would need to be figured out. + ### Output Fields -Starting in Boundary 0.2.1, grant strings can contain an `output_fields` field. -This allows fine-grained control over visibility of data returned to users. +Grant strings can contain an `output_fields` field. This allows fine-grained +control over visibility of data returned to users. In many cases, `output_fields` will not need to be set directly. However, an example in the form of some history helps provide some context as to when this @@ -116,20 +134,4 @@ composition, is equivalent to using Boundary's default; however the moment that grants start specifying output fields, it is composed from an empty set and thus nothing is contained unless explicitly specified. (An actual empty set is not currently supported, as we don't perform validation on the values given. -However, this means setting `output_fields=none` is functionally equivalent!) - -#### Output Fields and Monitoring - -As another example, it's possible to get creative and use `output_fields` to -allow services to keep tabs on various aspects of the system for monitoring -purposes without needing to provision Boundary tokens to those services. For -instance, by allowing access to list users to `u_anon` but restrict the set of -fields to `id` only, a monitoring system could watch and alert if more users -than expected are showing up in the system, while the IDs themselves are not -really meaningful to any other caller that accesses the same endpoint. - -This will be especially useful once we introduce the ability to have CIDR -restrictions against roles and/or grants, which are planned but not currently -scheduled, as you can only have these grants apply to specific internal -services, along with restricting the data that is returned for those services -that do match. \ No newline at end of file +However, this means setting `output_fields=none` is functionally equivalent!) \ No newline at end of file diff --git a/website/content/docs/configuration/events/file.mdx b/website/content/docs/configuration/events/file.mdx index bc98443a4..420f9adf2 100644 --- a/website/content/docs/configuration/events/file.mdx +++ b/website/content/docs/configuration/events/file.mdx @@ -16,7 +16,8 @@ sink { event_types = ["observation"] format = "cloudevents-json" file { - file_name = "file-name" + path = "/var/log/boundary" + file_name = "events.ndjson" } } ``` @@ -33,14 +34,14 @@ These parameters are shared across all sink types: [common sink parameters](/doc These parameters are only valid for a `file` sink. -- `path` - Specifies the file path for the sink. - - `file_name` - Specifies the file name for the sink. -- `rotate_bytes` - Specifies the number of bytes that should trigger rotation of +- `path` - Optionally specifies the file path for the sink. + +- `rotate_bytes` - Optionally specifies the number of bytes that should trigger rotation of a file sink. -- `rotate_duration` - Specifies how often a file sink should be rotated. +- `rotate_duration` - Optionally specifies how often a file sink should be rotated. -- `rotate_max_files` - Specifies how many historical rotated files should be kept +- `rotate_max_files` - Optionally specifies how many historical rotated files should be kept for a file sink. diff --git a/website/content/docs/configuration/events/overview.mdx b/website/content/docs/configuration/events/overview.mdx index fb7182217..2f3c43f3f 100644 --- a/website/content/docs/configuration/events/overview.mdx +++ b/website/content/docs/configuration/events/overview.mdx @@ -27,7 +27,8 @@ events { event_types = ["observation"] format = "cloudevents-json" file { - file_name = "file-name" + path = "/var/log/boundary" + file_name = "events.ndjson" } } }