Skip to content

Commit

Permalink
CBG-4116 skip successful auth on silent handlers (#7017)
Browse files Browse the repository at this point in the history
  • Loading branch information
@torcolvin
torcolvin authored Jul 29, 2024
1 parent 9026ed8 commit dedf99f
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 18 deletions.
130 changes: 115 additions & 15 deletions rest/audit_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@ import (
)

func TestAuditLoggingFields(t *testing.T) {
if !base.UnitTestUrlIsWalrus() {
t.Skip("This test can panic with gocb logging CBG-4076")
}

if !base.IsEnterpriseEdition() {
t.Skip("Audit logging only works in EE")
}
Expand All @@ -56,9 +52,10 @@ func TestAuditLoggingFields(t *testing.T) {
)

rt := NewRestTester(t, &RestTesterConfig{
GuestEnabled: true,
AdminInterfaceAuthentication: !base.UnitTestUrlIsWalrus(), // disable admin auth for walrus so we can get coverage of both subtests
PersistentConfig: true,
GuestEnabled: true,
AdminInterfaceAuthentication: !base.UnitTestUrlIsWalrus(), // disable admin auth for walrus so we can get coverage of both subtests
metricsInterfaceAuthentication: true,
PersistentConfig: true,
MutateStartupConfig: func(config *StartupConfig) {
config.Unsupported.AuditInfoProvider = &AuditInfoProviderConfig{
RequestInfoHeaderName: base.StringPtr(requestInfoHeaderName),
Expand Down Expand Up @@ -149,6 +146,45 @@ func TestAuditLoggingFields(t *testing.T) {
RequireStatus(t, rt.SendRequest(http.MethodGet, "/_ping", ""), http.StatusOK)
},
},
{
name: "admin silent request, admin authentication enabled",
auditableAction: func(t testing.TB) {
if !rt.AdminInterfaceAuthentication {
t.Skip("Skipping subtest that requires admin auth to be enabled")
}
RequireStatus(t, rt.SendAdminRequest(http.MethodGet, "/_expvar", ""), http.StatusUnauthorized)
},
expectedAuditEventFields: map[base.AuditID]base.AuditFields{
base.AuditIDAdminUserAuthenticationFailed: {
base.AuditFieldUserName: "",
},
},
},
{
name: "admin silent request authenticated",
auditableAction: func(t testing.TB) {
RequireStatus(t, rt.SendAdminRequestWithAuth(http.MethodGet, "/_expvar", "", base.TestClusterUsername(), base.TestClusterPassword()), http.StatusOK)
},
expectedAuditEventFields: map[base.AuditID]base.AuditFields{
base.AuditIDSyncGatewayStats: {
base.AuditFieldStatsFormat: "expvar",
},
},
},
{
name: "admin silent request bad creds",
auditableAction: func(t testing.TB) {
if !rt.AdminInterfaceAuthentication {
t.Skip("Skipping subtest that requires admin auth to be enabled")
}
RequireStatus(t, rt.SendAdminRequestWithAuth(http.MethodGet, "/_expvar", "", "not a real user", base.TestClusterPassword()), http.StatusUnauthorized)
},
expectedAuditEventFields: map[base.AuditID]base.AuditFields{
base.AuditIDAdminUserAuthenticationFailed: {
base.AuditFieldUserName: "not a real user",
},
},
},
{
name: "public request",
auditableAction: func(t testing.TB) {
Expand Down Expand Up @@ -247,7 +283,7 @@ func TestAuditLoggingFields(t *testing.T) {
},
},
{
name: "anon admin request",
name: "anon admin request, admin authentication disabled",
auditableAction: func(t testing.TB) {
if rt.AdminInterfaceAuthentication {
t.Skip("Skipping subtest that requires admin auth to be disabled")
Expand All @@ -264,6 +300,24 @@ func TestAuditLoggingFields(t *testing.T) {
},
},
},
{
name: "anon admin request, rejected",
auditableAction: func(t testing.TB) {
if !rt.AdminInterfaceAuthentication {
t.Skip("Skipping subtest that requires admin auth to be enabled")
}
RequireStatus(t, rt.SendAdminRequest(http.MethodGet, "/db/", ""), http.StatusUnauthorized)
},
expectedAuditEventFields: map[base.AuditID]base.AuditFields{
base.AuditIDAdminHTTPAPIRequest: {
base.AuditFieldHTTPMethod: http.MethodGet,
base.AuditFieldHTTPPath: "/db/",
},
base.AuditIDAdminUserAuthenticationFailed: {
base.AuditFieldUserName: "",
},
},
},
{
name: "authed admin request",
auditableAction: func(t testing.TB) {
Expand All @@ -275,7 +329,7 @@ func TestAuditLoggingFields(t *testing.T) {
expectedAuditEventFields: map[base.AuditID]base.AuditFields{
base.AuditIDAdminUserAuthenticated: {
base.AuditFieldCorrelationID: auditFieldValueIgnored,
// base.AuditFieldRealUserID: map[string]any{"domain": "cbs", "user": base.TestClusterUsername()},
base.AuditFieldRealUserID: map[string]any{"domain": "cbs", "user": base.TestClusterUsername()},
},
base.AuditIDReadDatabase: {
base.AuditFieldCorrelationID: auditFieldValueIgnored,
Expand Down Expand Up @@ -371,7 +425,7 @@ func TestAuditLoggingFields(t *testing.T) {
expectedAuditEventFields: map[base.AuditID]base.AuditFields{
base.AuditIDAdminUserAuthenticated: {
base.AuditFieldCorrelationID: auditFieldValueIgnored,
// base.AuditFieldRealUserID: map[string]any{"domain": "cbs", "user": unfilteredAdminRoleUsername},
base.AuditFieldRealUserID: map[string]any{"domain": "cbs", "user": unfilteredAdminRoleUsername},
},
base.AuditIDReadDatabase: {
base.AuditFieldCorrelationID: auditFieldValueIgnored,
Expand All @@ -383,6 +437,51 @@ func TestAuditLoggingFields(t *testing.T) {
},
},
},
{
name: "metrics request authenticated",
auditableAction: func(t testing.TB) {
headers := map[string]string{
"Authorization": getBasicAuthHeader(base.TestClusterUsername(), base.TestClusterPassword()),
}
RequireStatus(t, rt.SendMetricsRequestWithHeaders(http.MethodGet, "/_metrics", "", headers), http.StatusOK)
},
expectedAuditEventFields: map[base.AuditID]base.AuditFields{
base.AuditIDSyncGatewayStats: {
base.AuditFieldStatsFormat: "prometheus",
},
},
},
{
name: "metrics request no authentication",
auditableAction: func(t testing.TB) {
if !rt.AdminInterfaceAuthentication {
t.Skip("Skipping subtest that requires admin auth to be enabled")
}
RequireStatus(t, rt.SendMetricsRequestWithHeaders(http.MethodGet, "/_metrics", "", nil), http.StatusUnauthorized)
},
expectedAuditEventFields: map[base.AuditID]base.AuditFields{
base.AuditIDAdminUserAuthenticationFailed: {
base.AuditFieldUserName: "",
},
},
},
{
name: "metrics request bad credentials",
auditableAction: func(t testing.TB) {
if !rt.AdminInterfaceAuthentication {
t.Skip("Skipping subtest that requires admin auth to be enabled")
}
headers := map[string]string{
"Authorization": getBasicAuthHeader("notauser", base.TestClusterPassword()),
}
RequireStatus(t, rt.SendMetricsRequestWithHeaders(http.MethodGet, "/_metrics", "", headers), http.StatusUnauthorized)
},
expectedAuditEventFields: map[base.AuditID]base.AuditFields{
base.AuditIDAdminUserAuthenticationFailed: {
base.AuditFieldUserName: "notauser",
},
},
},
}
for _, testCase := range testCases {
rt.Run(testCase.name, func(t *testing.T) {
Expand Down Expand Up @@ -413,8 +512,9 @@ func TestAuditLoggingFields(t *testing.T) {
}
}
}

}
assert.Truef(t, eventFound, "expected event %v not found in set of events", expectedAuditID)
assert.Truef(t, eventFound, "expected event %s:%v not found in set of events", base.AuditEvents[expectedAuditID].Name, expectedAuditID)
}

})
Expand Down Expand Up @@ -858,7 +958,7 @@ func TestAuditAttachmentEvents(t *testing.T) {
}{
{
name: "add attachment",
setupCode: func(t testing.TB, docID string) DocVersion {
setupCode: func(_ testing.TB, docID string) DocVersion {
return rt.CreateTestDoc(docID)
},
auditableCode: func(t testing.TB, docID string, docVersion DocVersion) {
Expand All @@ -868,7 +968,7 @@ func TestAuditAttachmentEvents(t *testing.T) {
},
{
name: "add inline attachment",
setupCode: func(t testing.TB, docID string) DocVersion {
setupCode: func(_ testing.TB, docID string) DocVersion {
return rt.CreateTestDoc(docID)
},
auditableCode: func(t testing.TB, docID string, docVersion DocVersion) {
Expand Down Expand Up @@ -910,7 +1010,7 @@ func TestAuditAttachmentEvents(t *testing.T) {
},
{
name: "all_docs attachment with rev",
setupCode: func(t testing.TB, docID string) DocVersion {
setupCode: func(_ testing.TB, docID string) DocVersion {
return rt.CreateTestDoc(docID)
},
auditableCode: func(t testing.TB, docID string, docVersion DocVersion) {
Expand Down Expand Up @@ -1012,7 +1112,7 @@ func TestAuditDocumentCreateUpdateEvents(t *testing.T) {
},
{
name: "update doc",
setupCode: func(t testing.TB, docID string) DocVersion {
setupCode: func(_ testing.TB, docID string) DocVersion {
return rt.CreateTestDoc(docID)
},
auditableCode: func(t testing.TB, docID string, docVersion DocVersion) {
Expand Down
13 changes: 10 additions & 3 deletions rest/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,7 @@ func (h *handler) validateAndWriteHeaders(method handlerMethod, accessPermission
if dbContext == nil || dbContext.Options.SendWWWAuthenticateHeader == nil || *dbContext.Options.SendWWWAuthenticateHeader {
h.response.Header().Set("WWW-Authenticate", wwwAuthenticateHeader)
}
base.Audit(h.ctx(), base.AuditIDAdminUserAuthenticationFailed, base.AuditFields{base.AuditFieldUserName: username})
return ErrLoginRequired
}

Expand Down Expand Up @@ -576,16 +577,17 @@ func (h *handler) validateAndWriteHeaders(method handlerMethod, accessPermission
base.Audit(h.ctx(), base.AuditIDAdminUserAuthorizationFailed, base.AuditFields{base.AuditFieldUserName: username})
return base.HTTPErrorf(statusCode, "")
}

// even though `checkAdminAuth` _can_ issue whoami to find user's roles, it doesn't always...
// to reduce code complexity, we'll potentially be making this whoami request twice if we need it for audit filtering
auditRoleNames := getCBUserRolesForAudit(dbContext, authScope, h.ctx(), httpClient, managementEndpoints, username, password)

h.authorizedAdminUser = username
h.permissionsResults = permissions
h.rqCtx = base.UserLogCtx(h.ctx(), username, base.UserDomainCBServer, auditRoleNames)
base.Audit(h.ctx(), base.AuditIDAdminUserAuthenticated, base.AuditFields{})

// query auditRoleNames above even if this is a silent request can need "real_userid" on a ctx. Example: /_expvar should not log AuditIDAdminUserAuthenticated but it should log AuditIDSyncGatewayStats
if !h.isSilentRequest() {
base.Audit(h.ctx(), base.AuditIDAdminUserAuthenticated, base.AuditFields{})
}
base.DebugfCtx(h.ctx(), base.KeyAuth, "%s: User %s was successfully authorized as an admin", h.formatSerialNumber(), base.UD(username))
} else {
// If admin auth is not enabled we should set any responsePermissions to true so that any handlers checking for
Expand Down Expand Up @@ -711,6 +713,11 @@ func (h *handler) removeCorruptConfigIfExists(ctx context.Context, bucket, confi
return nil
}

// isSilentRequest returns true if the handler represents a request we should suppress http logging on.
func (h *handler) isSilentRequest() bool {
return h.httpLogLevel != nil && *h.httpLogLevel == base.LevelDebug
}

func (h *handler) logRequestLine() {

logLevel := base.LevelInfo
Expand Down

0 comments on commit dedf99f

Please sign in to comment.