From 8ae6e9892fe340b4552c76878a5e4ec5dcfa45fc Mon Sep 17 00:00:00 2001 From: Jim Date: Thu, 28 Oct 2021 21:14:39 -0400 Subject: [PATCH] feature (events/audit): Add auth info to audit events (#1644) --- go.mod | 2 +- go.sum | 5 +- .../auth_method_service_taggable_test.go | 7 +- internal/observability/event/context_test.go | 2 +- internal/observability/event/event.go | 16 ++- internal/servers/controller/auth/auth.go | 66 +++++++-- internal/servers/controller/auth/auth_test.go | 128 ++++++++++++++++++ sdk/go.mod | 2 +- sdk/go.sum | 4 +- .../authmethods/auth_method_taggable_test.go | 3 +- 10 files changed, 205 insertions(+), 30 deletions(-) diff --git a/go.mod b/go.mod index d4889798c1..2620e050d4 100644 --- a/go.mod +++ b/go.mod @@ -26,7 +26,7 @@ require ( github.com/hashicorp/dawdle v0.4.0 github.com/hashicorp/dbassert v0.0.0-20210708202608-ecf920cf1ed8 github.com/hashicorp/eventlogger v0.1.0 - github.com/hashicorp/eventlogger/filters/encrypt v0.1.5 + github.com/hashicorp/eventlogger/filters/encrypt v0.1.6-0.20211027211326-5db60a48f239 github.com/hashicorp/go-bexpr v0.1.10 github.com/hashicorp/go-cleanhttp v0.5.2 github.com/hashicorp/go-hclog v0.16.2 diff --git a/go.sum b/go.sum index 7b149e68c4..783ca44919 100644 --- a/go.sum +++ b/go.sum @@ -458,9 +458,8 @@ github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= github.com/hashicorp/eventlogger v0.1.0 h1:S6xc4gZVzewuDUP4R4Ngko419h/CGDuV/b4ADL3XLik= github.com/hashicorp/eventlogger v0.1.0/go.mod h1:a3IXf1aEJfpCPzseTOrwKj4fVW/Qn3oEmpQeaIznzH0= -github.com/hashicorp/eventlogger/filters/encrypt v0.1.5-0.20211025115820-78e1ded4aea1/go.mod h1:8rcez7Kw1zanB0/074qnOuGu7zxmNh9Xr2ZI+K4xVIA= -github.com/hashicorp/eventlogger/filters/encrypt v0.1.5 h1:kNkH4G6zzWlZSoI1I+B/ud4chVKTPL516C6jB7dRdlE= -github.com/hashicorp/eventlogger/filters/encrypt v0.1.5/go.mod h1:8rcez7Kw1zanB0/074qnOuGu7zxmNh9Xr2ZI+K4xVIA= +github.com/hashicorp/eventlogger/filters/encrypt v0.1.6-0.20211027211326-5db60a48f239 h1:Yh9tY0lige+y0trmjQeT9NRDo6+YvtNAzbmUNOsIUzI= +github.com/hashicorp/eventlogger/filters/encrypt v0.1.6-0.20211027211326-5db60a48f239/go.mod h1:8rcez7Kw1zanB0/074qnOuGu7zxmNh9Xr2ZI+K4xVIA= github.com/hashicorp/go-bexpr v0.1.10 h1:9kuI5PFotCboP3dkDYFr/wi0gg0QVbSNz5oFRpxn4uE= github.com/hashicorp/go-bexpr v0.1.10/go.mod h1:oxlubA2vC/gFVfX1A6JGp7ls7uCDlfJn732ehYYg+g0= github.com/hashicorp/go-cleanhttp v0.5.0/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80= diff --git a/internal/gen/controller/api/services/auth_method_service_taggable_test.go b/internal/gen/controller/api/services/auth_method_service_taggable_test.go index 0dc6fb6b60..ad9176bcde 100644 --- a/internal/gen/controller/api/services/auth_method_service_taggable_test.go +++ b/internal/gen/controller/api/services/auth_method_service_taggable_test.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/boundary/sdk/pbs/controller/api" "github.com/hashicorp/boundary/sdk/wrapper" "github.com/hashicorp/eventlogger" + "github.com/hashicorp/eventlogger/filters/encrypt" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" structpb "google.golang.org/protobuf/types/known/structpb" @@ -81,7 +82,7 @@ func TestAuthenticate_Tags(t *testing.T) { "auth_url": structpb.NewStringValue("public-auth_url"), "token_id": structpb.NewStringValue("public-token_id"), "final_redirect_url": structpb.NewStringValue("public-final_redirect_url"), - "token": structpb.NewStringValue(""), + "token": structpb.NewStringValue(encrypt.RedactedData), }, }, }, @@ -121,8 +122,8 @@ func TestAuthenticate_Tags(t *testing.T) { "auth_url": structpb.NewStringValue("public-auth_url"), "token_id": structpb.NewStringValue("public-token_id"), "state": structpb.NewStringValue("public-state"), - "password": structpb.NewStringValue(""), - "code": structpb.NewStringValue(""), + "password": structpb.NewStringValue(encrypt.RedactedData), + "code": structpb.NewStringValue(encrypt.RedactedData), }, }, }, diff --git a/internal/observability/event/context_test.go b/internal/observability/event/context_test.go index f8a1c15559..67c402e38e 100644 --- a/internal/observability/event/context_test.go +++ b/internal/observability/event/context_test.go @@ -662,7 +662,7 @@ func Test_WriteAudit(t *testing.T) { AuthAccountId: "test_auth_account_id", }, GrantsInfo: &event.GrantsInfo{ - Grants: []event.GrantsPair{ + Grants: []event.Grant{ { Grant: "test_grant", ScopeId: "test_grant_scope_id", diff --git a/internal/observability/event/event.go b/internal/observability/event/event.go index 53fb9ba8d4..2de944ac88 100644 --- a/internal/observability/event/event.go +++ b/internal/observability/event/event.go @@ -25,20 +25,22 @@ type UserInfo struct { } type GrantsInfo struct { - Grants []GrantsPair `json:"grants_pair,omitempty"` + Grants []Grant `json:"grants,omitempty"` } -type GrantsPair struct { +type Grant struct { Grant string `json:"grant,omitempty" class:"public"` ScopeId string `json:"scope_id,omitempty" class:"public"` + RoleId string `json:"role_id,omitempty" class:"public"` } type Auth struct { - AuthTokenId string `json:"auth_token_id" class:"public"` - UserInfo *UserInfo `json:"user_info,omitempty"` // boundary field - GrantsInfo *GrantsInfo `json:"grants_info,omitempty"` - UserEmail string `json:"email,omitempty" class:"sensitive"` - UserName string `json:"name,omitempty" class:"sensitive"` + DisabledAuthEntirely *bool `json:"disabled_auth_entirely,omitempty" class:"public"` + AuthTokenId string `json:"auth_token_id" class:"public"` + UserInfo *UserInfo `json:"user_info,omitempty"` // boundary field + GrantsInfo *GrantsInfo `json:"grants_info,omitempty"` + UserEmail string `json:"email,omitempty" class:"sensitive"` + UserName string `json:"name,omitempty" class:"sensitive"` } type Request struct { diff --git a/internal/servers/controller/auth/auth.go b/internal/servers/controller/auth/auth.go index f39a765055..ebc6d23bb9 100644 --- a/internal/servers/controller/auth/auth.go +++ b/internal/servers/controller/auth/auth.go @@ -140,6 +140,9 @@ func Verify(ctx context.Context, opt ...Option) (ret VerifyResults) { v.ctx = ctx + ea := &event.Auth{} + defer event.WriteAudit(ctx, op, event.WithAuth(ea)) + opts := getOpts(opt...) ret.Scope = new(scopes.ScopeInfo) @@ -160,6 +163,8 @@ func Verify(ctx context.Context, opt ...Option) (ret VerifyResults) { // In tests we often simply disable auth so we can test the service handlers // without fuss if v.requestInfo.DisableAuthEntirely { + yes := true + ea.DisabledAuthEntirely = &yes const op = "auth.(disabled).lookupScope" ret.Scope.Id = v.requestInfo.ScopeIdOverride if ret.Scope.Id == "" { @@ -205,6 +210,7 @@ func Verify(ctx context.Context, opt ...Option) (ret VerifyResults) { if reqInfo != nil { reqInfo.UserId = ret.UserId } + ea.UserInfo = &event.UserInfo{UserId: ret.UserId} ret.Error = nil return } @@ -226,8 +232,10 @@ func Verify(ctx context.Context, opt ...Option) (ret VerifyResults) { } var authResults perms.ACLResults + var grantTuples []perms.GrantTuple + var accountId, userName, userEmail string var err error - authResults, ret.UserId, ret.Scope, v.acl, err = v.performAuthCheck(ctx) + authResults, ret.UserId, accountId, userName, userEmail, ret.Scope, v.acl, grantTuples, err = v.performAuthCheck(ctx) if err != nil { event.WriteError(ctx, op, err, event.WithInfoMsg("error performing authn/authz check")) return @@ -262,10 +270,31 @@ func Verify(ctx context.Context, opt ...Option) (ret VerifyResults) { if ret.UserId == AnonymousUserId { ret.Error = handlers.UnauthenticatedError() } + ea.UserInfo = &event.UserInfo{ + UserId: ret.UserId, + } return } } + grants := make([]event.Grant, 0, len(grantTuples)) + for _, g := range grantTuples { + grants = append(grants, event.Grant{ + Grant: g.Grant, + RoleId: g.RoleId, + ScopeId: g.ScopeId, + }) + } + ea.UserInfo = &event.UserInfo{ + UserId: ret.UserId, + AuthAccountId: accountId, + } + ea.GrantsInfo = &event.GrantsInfo{ + Grants: grants, + } + ea.UserName = userName + ea.UserEmail = userEmail + if reqInfo != nil { reqInfo.UserId = ret.UserId reqInfo.OutputFields = authResults.OutputFields @@ -401,15 +430,23 @@ func (v *verifier) decryptToken(ctx context.Context) { } } -func (v verifier) performAuthCheck(ctx context.Context) (aclResults perms.ACLResults, userId string, scopeInfo *scopes.ScopeInfo, retAcl perms.ACL, retErr error) { +func (v verifier) performAuthCheck(ctx context.Context) ( + aclResults perms.ACLResults, + userId string, + accountId string, + userName string, + userEmail string, + scopeInfo *scopes.ScopeInfo, + retAcl perms.ACL, + grantTuples []perms.GrantTuple, + retErr error) { const op = "auth.(verifier).performAuthCheck" // Ensure we return an error by default if we forget to set this somewhere - retErr = errors.NewDeprecated(errors.Unknown, op, "default auth error", errors.WithoutEvent()) + retErr = errors.New(ctx, errors.Unknown, op, "default auth error", errors.WithoutEvent()) // Make the linter happy _ = retErr scopeInfo = new(scopes.ScopeInfo) userId = AnonymousUserId - var accountId string // Validate the token and fetch the corresponding user ID switch v.requestInfo.TokenFormat { @@ -428,7 +465,7 @@ func (v verifier) performAuthCheck(ctx context.Context) (aclResults perms.ACLRes } tokenRepo, err := v.authTokenRepoFn() if err != nil { - retErr = errors.WrapDeprecated(err, op) + retErr = errors.Wrap(ctx, err, op) return } at, err := tokenRepo.ValidateToken(v.ctx, v.requestInfo.PublicId, v.requestInfo.Token) @@ -451,9 +488,17 @@ func (v verifier) performAuthCheck(ctx context.Context) (aclResults perms.ACLRes iamRepo, err := v.iamRepoFn() if err != nil { - retErr = errors.WrapDeprecated(err, op, errors.WithMsg("failed to get iam repo")) + retErr = errors.Wrap(ctx, err, op, errors.WithMsg("failed to get iam repo")) + return + } + + u, _, err := iamRepo.LookupUser(ctx, userId) + if err != nil { + retErr = errors.Wrap(ctx, err, op, errors.WithMsg("failed to lookup user")) return } + userEmail = u.Email + userName = u.FullName // Look up scope details to return. We can skip a lookup when using the // global scope @@ -470,11 +515,11 @@ func (v verifier) performAuthCheck(ctx context.Context) (aclResults perms.ACLRes default: scp, err := iamRepo.LookupScope(v.ctx, v.res.ScopeId) if err != nil { - retErr = errors.WrapDeprecated(err, op) + retErr = errors.Wrap(ctx, err, op) return } if scp == nil { - retErr = errors.NewDeprecated(errors.InvalidParameter, op, fmt.Sprint("non-existent scope $q", v.res.ScopeId)) + retErr = errors.New(ctx, errors.InvalidParameter, op, fmt.Sprint("non-existent scope $q", v.res.ScopeId)) return } scopeInfo = &scopes.ScopeInfo{ @@ -495,13 +540,12 @@ func (v verifier) performAuthCheck(ctx context.Context) (aclResults perms.ACLRes } var parsedGrants []perms.Grant - var grantTuples []perms.GrantTuple // Fetch and parse grants for this user ID (which may include grants for // u_anon and u_auth) grantTuples, err = iamRepo.GrantsForUser(v.ctx, userId) if err != nil { - retErr = errors.WrapDeprecated(err, op) + retErr = errors.Wrap(ctx, err, op) return } parsedGrants = make([]perms.Grant, 0, len(grantTuples)) @@ -516,7 +560,7 @@ func (v verifier) performAuthCheck(ctx context.Context) (aclResults perms.ACLRes perms.WithAccountId(accountId), perms.WithSkipFinalValidation(true)) if err != nil { - retErr = errors.WrapDeprecated(err, op, errors.WithMsg(fmt.Sprintf("failed to parse grant %#v", pair.Grant))) + retErr = errors.Wrap(ctx, err, op, errors.WithMsg(fmt.Sprintf("failed to parse grant %#v", pair.Grant))) return } parsedGrants = append(parsedGrants, parsed) diff --git a/internal/servers/controller/auth/auth_test.go b/internal/servers/controller/auth/auth_test.go index e90b523f2d..0eda12218a 100644 --- a/internal/servers/controller/auth/auth_test.go +++ b/internal/servers/controller/auth/auth_test.go @@ -5,6 +5,8 @@ import ( "fmt" "net/http" "net/http/httptest" + "os" + "sync" "testing" "github.com/hashicorp/boundary/internal/authtoken" @@ -12,8 +14,12 @@ import ( authpb "github.com/hashicorp/boundary/internal/gen/controller/auth" "github.com/hashicorp/boundary/internal/iam" "github.com/hashicorp/boundary/internal/kms" + "github.com/hashicorp/boundary/internal/observability/event" "github.com/hashicorp/boundary/internal/servers" "github.com/hashicorp/boundary/internal/servers/controller/handlers" + "github.com/hashicorp/boundary/internal/tests/api" + "github.com/hashicorp/eventlogger/filters/encrypt" + "github.com/hashicorp/go-hclog" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -132,3 +138,125 @@ func TestAuthTokenAuthenticator(t *testing.T) { }) } } + +func TestVerify_AuditEvent(t *testing.T) { + // this cannot run in parallel because it relies on envvar + // globals.BOUNDARY_DEVELOPER_ENABLE_EVENTS + event.TestEnableEventing(t, true) + eventConfig := event.TestEventerConfig(t, "Test_Verify", event.TestWithAuditSink(t)) + testLock := &sync.Mutex{} + testLogger := hclog.New(&hclog.LoggerOptions{ + Mutex: testLock, + Name: "test", + }) + require.NoError(t, event.InitSysEventer(testLogger, testLock, "Test_Verify", event.WithEventerConfig(&eventConfig.EventerConfig))) + + conn, _ := db.TestSetup(t, "postgres") + rw := db.New(conn) + wrapper := db.TestWrapper(t) + testKms := kms.TestKms(t, conn, wrapper) + tokenRepo, err := authtoken.NewRepository(rw, rw, testKms) + require.NoError(t, err) + iamRepo := iam.TestRepo(t, conn, wrapper) + tokenRepoFn := func() (*authtoken.Repository, error) { + return tokenRepo, nil + } + iamRepoFn := func() (*iam.Repository, error) { + return iamRepo, nil + } + serversRepoFn := func() (*servers.Repository, error) { + return servers.NewRepository(rw, rw, testKms) + } + + o, _ := iam.TestScopes(t, iamRepo) + at := authtoken.TestAuthToken(t, conn, testKms, o.GetPublicId()) + encToken, err := authtoken.EncryptToken(context.Background(), testKms, o.GetPublicId(), at.GetPublicId(), at.GetToken()) + require.NoError(t, err) + + tokValue := at.GetPublicId() + "_" + encToken + jsCookieVal, httpCookieVal := tokValue[:len(tokValue)/2], tokValue[len(tokValue)/2:] + + tests := []struct { + name string + headers map[string]string + cookies []http.Cookie + opt []Option + disableAuth bool + wantResults VerifyResults + wantAuthAuditData bool + wantUserId string + }{ + { + name: "bearer-token", + headers: map[string]string{"Authorization": fmt.Sprintf("Bearer %s", tokValue)}, + opt: []Option{WithScopeId(o.PublicId)}, + wantAuthAuditData: true, + wantUserId: at.IamUserId, + }, + { + name: "split-cookie-token", + cookies: []http.Cookie{ + {Name: handlers.HttpOnlyCookieName, Value: httpCookieVal}, + {Name: handlers.JsVisibleCookieName, Value: jsCookieVal}, + }, + opt: []Option{WithScopeId(o.PublicId)}, + wantAuthAuditData: true, + wantUserId: at.IamUserId, + }, + { + name: "no-auth-data", + opt: []Option{WithScopeId(o.PublicId)}, + wantAuthAuditData: true, + wantUserId: "u_anon", + }, + { + name: "disable-auth", + opt: []Option{WithScopeId(o.PublicId)}, + disableAuth: true, + wantAuthAuditData: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + req := httptest.NewRequest("GET", "http://127.0.0.1/v1/scopes/o_1", nil) + for k, v := range tt.headers { + req.Header.Set(k, v) + } + for _, c := range tt.cookies { + req.AddCookie(&c) + } + + // Add values for authn/authz checking + requestInfo := authpb.RequestInfo{ + Path: req.URL.Path, + Method: req.Method, + DisableAuthEntirely: tt.disableAuth, + } + requestInfo.PublicId, requestInfo.EncryptedToken, requestInfo.TokenFormat = GetTokenFromRequest(context.TODO(), testKms, req) + + ctx := NewVerifierContext(context.Background(), iamRepoFn, tokenRepoFn, serversRepoFn, testKms, &requestInfo) + + _ = os.WriteFile(eventConfig.AuditEvents.Name(), nil, 0o666) // clean out audit events from previous calls + _ = Verify(ctx, tt.opt...) + got := api.CloudEventFromFile(t, eventConfig.AuditEvents.Name()) + + if tt.wantAuthAuditData { + auth, ok := got.Data.(map[string]interface{})["auth"].(map[string]interface{}) + require.True(ok) + assert.Equal(encrypt.RedactedData, auth["email"]) + assert.Equal(encrypt.RedactedData, auth["name"]) + + if tt.disableAuth { + assert.Equal(true, auth["disabled_auth_entirely"]) + } + if tt.wantUserId != "" { + userInfo, ok := auth["user_info"].(map[string]interface{}) + require.True(ok) + assert.Equal(tt.wantUserId, userInfo["id"]) + } + } + }) + } +} diff --git a/sdk/go.mod b/sdk/go.mod index e4ec45cf97..90c8087873 100644 --- a/sdk/go.mod +++ b/sdk/go.mod @@ -7,7 +7,7 @@ require ( github.com/golang/protobuf v1.5.2 // indirect github.com/google/go-cmp v0.5.6 // indirect github.com/hashicorp/eventlogger v0.1.0 - github.com/hashicorp/eventlogger/filters/encrypt v0.1.5 + github.com/hashicorp/eventlogger/filters/encrypt v0.1.6-0.20211027211326-5db60a48f239 github.com/hashicorp/go-cleanhttp v0.5.2 // indirect github.com/hashicorp/go-kms-wrapping v0.6.6 github.com/hashicorp/go-retryablehttp v0.7.0 // indirect diff --git a/sdk/go.sum b/sdk/go.sum index ce2eba2626..1460170102 100644 --- a/sdk/go.sum +++ b/sdk/go.sum @@ -237,8 +237,8 @@ github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= github.com/hashicorp/eventlogger v0.1.0 h1:S6xc4gZVzewuDUP4R4Ngko419h/CGDuV/b4ADL3XLik= github.com/hashicorp/eventlogger v0.1.0/go.mod h1:a3IXf1aEJfpCPzseTOrwKj4fVW/Qn3oEmpQeaIznzH0= -github.com/hashicorp/eventlogger/filters/encrypt v0.1.5 h1:kNkH4G6zzWlZSoI1I+B/ud4chVKTPL516C6jB7dRdlE= -github.com/hashicorp/eventlogger/filters/encrypt v0.1.5/go.mod h1:8rcez7Kw1zanB0/074qnOuGu7zxmNh9Xr2ZI+K4xVIA= +github.com/hashicorp/eventlogger/filters/encrypt v0.1.6-0.20211027211326-5db60a48f239 h1:Yh9tY0lige+y0trmjQeT9NRDo6+YvtNAzbmUNOsIUzI= +github.com/hashicorp/eventlogger/filters/encrypt v0.1.6-0.20211027211326-5db60a48f239/go.mod h1:8rcez7Kw1zanB0/074qnOuGu7zxmNh9Xr2ZI+K4xVIA= github.com/hashicorp/go-cleanhttp v0.5.0/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80= github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80= github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ= diff --git a/sdk/pbs/controller/api/resources/authmethods/auth_method_taggable_test.go b/sdk/pbs/controller/api/resources/authmethods/auth_method_taggable_test.go index a4018792aa..aab9f930d0 100644 --- a/sdk/pbs/controller/api/resources/authmethods/auth_method_taggable_test.go +++ b/sdk/pbs/controller/api/resources/authmethods/auth_method_taggable_test.go @@ -9,6 +9,7 @@ import ( "github.com/hashicorp/boundary/sdk/pbs/controller/api" "github.com/hashicorp/boundary/sdk/wrapper" "github.com/hashicorp/eventlogger" + "github.com/hashicorp/eventlogger/filters/encrypt" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/protobuf/types/known/structpb" @@ -92,7 +93,7 @@ func TestAuthMethod_Tags(t *testing.T) { "account_claim_maps": structpb.NewStringValue("public-account_claim_maps"), "disable_discovered_config_validation": structpb.NewStringValue("public-disable_discovered_config_validation"), "dry_run": structpb.NewStringValue("public-dry_run"), - "client_secret": structpb.NewStringValue(""), + "client_secret": structpb.NewStringValue(encrypt.RedactedData), }, }, AuthorizedActions: []string{"action-1", "action-2"},