Skip to content

Commit

Permalink
refactor: add required field checks, fix: missing method_name check
Browse files Browse the repository at this point in the history
  • Loading branch information
sqin2019 committed Aug 2, 2023
1 parent 3e5ca59 commit 8f1d11d
Show file tree
Hide file tree
Showing 8 changed files with 263 additions and 109 deletions.
7 changes: 5 additions & 2 deletions clients/go/pkg/audit/interceptor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1070,15 +1070,18 @@ func TestStreamInterceptor(t *testing.T) {
},
},
handler: func(srv interface{}, ss grpc.ServerStream) error {
logReq, _ := LogReqFromCtx(ss.Context())
logReq.Payload.ResourceName = "ExampleResourceName"
return grpcstatus.Error(codes.Internal, "something is wrong")
},
wantErrSubstr: "something is wrong",
wantLogReqs: []*api.AuditLogRequest{
{
Type: api.AuditLogRequest_DATA_ACCESS,
Payload: &capi.AuditLog{
ServiceName: "ExampleService",
MethodName: "/ExampleService/ExampleMethod",
ServiceName: "ExampleService",
MethodName: "/ExampleService/ExampleMethod",
ResourceName: "ExampleResourceName",
AuthenticationInfo: &capi.AuthenticationInfo{
PrincipalEmail: "user@example.com",
},
Expand Down
34 changes: 3 additions & 31 deletions clients/go/pkg/audit/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ package audit
import (
"context"
"fmt"
"strings"

api "github.com/abcxyz/lumberjack/clients/go/apis/v1alpha1"
"github.com/abcxyz/lumberjack/clients/go/pkg/auditerrors"
"github.com/abcxyz/lumberjack/pkg/util"
)

// RequestValidator validates log request fields.
Expand All @@ -46,36 +46,8 @@ func (p *RequestValidator) process(ctx context.Context, logReq *api.AuditLogRequ
return fmt.Errorf("AuditLogRequest cannot be nil")
}

if logReq.Payload == nil {
return fmt.Errorf("AuditLogRequest.Payload cannot be nil")
}

if logReq.Payload.ServiceName == "" {
return fmt.Errorf("ServiceName cannot be empty")
}

if logReq.Payload.AuthenticationInfo == nil {
return fmt.Errorf("AuthenticationInfo cannot be nil")
}

email := logReq.Payload.AuthenticationInfo.PrincipalEmail
if err := p.validateEmail(email); err != nil {
return err
}

return nil
}

// This method is intended to validate that the email associated with the
// authentication request has the correct format and in a valid domain.
func (p *RequestValidator) validateEmail(email string) error {
if email == "" {
return fmt.Errorf("PrincipalEmail cannot be empty")
}

parts := strings.Split(email, "@")
if len(parts) != 2 || parts[1] == "" {
return fmt.Errorf("PrincipalEmail %q is malformed", email)
if err := util.Validate(logReq.Payload); err != nil {
return fmt.Errorf("AuditLogRequest does not have a valid payload: %w", err)
}
return nil
}
71 changes: 0 additions & 71 deletions clients/go/pkg/audit/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"testing"

"github.com/google/go-cmp/cmp"
"google.golang.org/genproto/googleapis/cloud/audit"
"google.golang.org/protobuf/testing/protocmp"

api "github.com/abcxyz/lumberjack/clients/go/apis/v1alpha1"
Expand Down Expand Up @@ -53,76 +52,6 @@ func TestRequestValidation_Process(t *testing.T) {
name: "should_error_when_logReq_is_nil",
wantErr: auditerrors.ErrInvalidRequest,
},
{
name: "should_error_when_authInfo_is_nil",
logReq: &api.AuditLogRequest{
Payload: &audit.AuditLog{
ServiceName: "test-service",
},
},
wantLogReq: &api.AuditLogRequest{
Payload: &audit.AuditLog{
ServiceName: "test-service",
},
},
wantErr: auditerrors.ErrInvalidRequest,
},
{
name: "should_error_when_auth_email_is_nil",
logReq: &api.AuditLogRequest{
Payload: &audit.AuditLog{
ServiceName: "test-service",
AuthenticationInfo: &audit.AuthenticationInfo{},
},
},
wantLogReq: &api.AuditLogRequest{
Payload: &audit.AuditLog{
ServiceName: "test-service",
AuthenticationInfo: &audit.AuthenticationInfo{},
},
},
wantErr: auditerrors.ErrInvalidRequest,
},
{
name: "should_error_when_auth_email_has_no_domain",
logReq: &api.AuditLogRequest{
Payload: &audit.AuditLog{
ServiceName: "test-service",
AuthenticationInfo: &audit.AuthenticationInfo{
PrincipalEmail: "user",
},
},
},
wantLogReq: &api.AuditLogRequest{
Payload: &audit.AuditLog{
ServiceName: "test-service",
AuthenticationInfo: &audit.AuthenticationInfo{
PrincipalEmail: "user",
},
},
},
wantErr: auditerrors.ErrInvalidRequest,
},
{
name: "should_error_when_serviceName_is_empty",
logReq: &api.AuditLogRequest{
Payload: &audit.AuditLog{
ServiceName: "",
AuthenticationInfo: &audit.AuthenticationInfo{
PrincipalEmail: "user@test.com",
},
},
},
wantLogReq: &api.AuditLogRequest{
Payload: &audit.AuditLog{
ServiceName: "",
AuthenticationInfo: &audit.AuthenticationInfo{
PrincipalEmail: "user@test.com",
},
},
},
wantErr: auditerrors.ErrInvalidRequest,
},
}

for _, tc := range tests {
Expand Down
1 change: 1 addition & 0 deletions clients/go/pkg/testutil/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ func NewRequest(opts ...RequestOptions) *api.AuditLogRequest {
request := &api.AuditLogRequest{
Type: api.AuditLogRequest_DATA_ACCESS,
Payload: &audit.AuditLog{
MethodName: "test-method",
ServiceName: "test-service",
ResourceName: "test-resource",
AuthenticationInfo: &audit.AuthenticationInfo{
Expand Down
67 changes: 67 additions & 0 deletions pkg/util/payload.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// Copyright 2023 The Authors (see AUTHORS file)
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

// Package util provides untils for audit log payload validation.
package util

import (
"fmt"
"strings"

"google.golang.org/genproto/googleapis/cloud/audit"
)

// Validate validates the audit log payload for lumberjack.
func Validate(payload *audit.AuditLog) error {
if payload == nil {
return fmt.Errorf("audit log payload cannot be nil")
}

if payload.MethodName == "" {
return fmt.Errorf("MethodName cannot be empty")
}

if payload.ServiceName == "" {
return fmt.Errorf("ServiceName cannot be empty")
}

if payload.ResourceName == "" {
return fmt.Errorf("ResourceName cannot be empty")
}

if payload.AuthenticationInfo == nil {
return fmt.Errorf("AuthenticationInfo cannot be nil")
}

email := payload.AuthenticationInfo.PrincipalEmail
if err := validateEmail(email); err != nil {
return err
}

return nil
}

// This method is intended to validate that the email associated with the
// authentication request has the correct format and in a valid domain.
func validateEmail(email string) error {
if email == "" {
return fmt.Errorf("PrincipalEmail cannot be empty")
}

parts := strings.Split(email, "@")
if len(parts) != 2 || parts[1] == "" {
return fmt.Errorf("PrincipalEmail %q is malformed", email)
}
return nil
}
126 changes: 126 additions & 0 deletions pkg/util/payload_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
// Copyright 2023 The Authors (see AUTHORS file)
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package util

import (
"testing"

"google.golang.org/genproto/googleapis/cloud/audit"

pkgtestutil "github.com/abcxyz/pkg/testutil"
)

func TestValidate(t *testing.T) {
t.Parallel()

tests := []struct {
name string
payload *audit.AuditLog
wantErrSubstr string
}{
{
name: "success",
payload: &audit.AuditLog{
MethodName: "test-method",
ServiceName: "test-service",
ResourceName: "test-resource",
AuthenticationInfo: &audit.AuthenticationInfo{
PrincipalEmail: "user@example.com",
},
},
},
{
name: "payload_is_nil",
wantErrSubstr: "audit log payload cannot be nil",
},
{
name: "authInfo_is_nil",
payload: &audit.AuditLog{
MethodName: "test-method",
ServiceName: "test-service",
ResourceName: "test-resource",
},
wantErrSubstr: "AuthenticationInfo cannot be nil",
},
{
name: "auth_email_is_nil",
payload: &audit.AuditLog{
MethodName: "test-method",
ServiceName: "test-service",
ResourceName: "test-resource",
AuthenticationInfo: &audit.AuthenticationInfo{},
},
wantErrSubstr: "PrincipalEmail cannot be empty",
},
{
name: "auth_email_has_no_domain",
payload: &audit.AuditLog{
MethodName: "test-method",
ServiceName: "test-service",
ResourceName: "test-resource",
AuthenticationInfo: &audit.AuthenticationInfo{
PrincipalEmail: "user",
},
},
wantErrSubstr: `PrincipalEmail "user" is malformed`,
},
{
name: "serviceName_is_empty",
payload: &audit.AuditLog{
MethodName: "test-method",
ResourceName: "test-resource",
AuthenticationInfo: &audit.AuthenticationInfo{
PrincipalEmail: "user@example.com",
},
},
wantErrSubstr: "ServiceName cannot be empty",
},
{
name: "resourceName_is_empty",
payload: &audit.AuditLog{
MethodName: "test-method",
ServiceName: "test-service",
AuthenticationInfo: &audit.AuthenticationInfo{
PrincipalEmail: "user@example.com",
},
},
wantErrSubstr: "ResourceName cannot be empty",
},
{
name: "methodName_is_empty",
payload: &audit.AuditLog{
ServiceName: "test-service",
ResourceName: "test-resource",
AuthenticationInfo: &audit.AuthenticationInfo{
PrincipalEmail: "user@example.com",
},
},
wantErrSubstr: "MethodName cannot be empty",
},
}

for _, tc := range tests {
tc := tc

t.Run(tc.name, func(t *testing.T) {
t.Parallel()

err := Validate(tc.payload)
if diff := pkgtestutil.DiffErrString(err, tc.wantErrSubstr); diff != "" {
t.Errorf("Process(%+v) got unexpected error substring: %v", tc.name, diff)
}
})
}
}
Loading

0 comments on commit 8f1d11d

Please sign in to comment.