Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions pkg/service/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"github.com/kuadrant/authorino/pkg/auth"
"github.com/kuadrant/authorino/pkg/context"
"github.com/kuadrant/authorino/pkg/evaluators"
"github.com/kuadrant/authorino/pkg/index"
"github.com/kuadrant/authorino/pkg/log"
"github.com/kuadrant/authorino/pkg/metrics"
Expand All @@ -23,7 +24,9 @@ import (
envoy_type "github.com/envoyproxy/go-control-plane/envoy/type/v3"
"github.com/gogo/googleapis/google/rpc"
"github.com/google/uuid"
otel_attr "go.opentelemetry.io/otel/attribute"
otel_codes "go.opentelemetry.io/otel/codes"
otel_trace "go.opentelemetry.io/otel/trace"
rpcstatus "google.golang.org/genproto/googleapis/rpc/status"
"google.golang.org/protobuf/types/known/structpb"
v1 "k8s.io/api/admission/v1"
Expand Down Expand Up @@ -251,6 +254,7 @@ func (a *AuthService) Check(parentContext gocontext.Context, req *envoy_auth.Che
span.RecordError(err)
span.SetStatus(otel_codes.Error, err.Error())
result := auth.AuthResult{Code: rpc.INVALID_ARGUMENT, Message: RESPONSE_MESSAGE_INVALID_REQUEST}
setAuthResultSpanAttrs(span, result)
return a.deniedResponse(result), nil
}

Expand Down Expand Up @@ -284,12 +288,19 @@ func (a *AuthService) Check(parentContext gocontext.Context, req *envoy_auth.Che
// If we couldn't find the AuthConfig in the config, we return and deny.
if authConfig == nil {
result := auth.AuthResult{Code: rpc.NOT_FOUND, Message: RESPONSE_MESSAGE_SERVICE_NOT_FOUND}
setAuthResultSpanAttrs(span, result)
a.logAuthResult(result, ctx)
return a.deniedResponse(result), nil
}

span.SetAttributes(
otel_attr.String(trace.AuthConfigNameAttr, authConfig.Labels["authconfig"]),
otel_attr.String(trace.AuthConfigNamespaceAttr, authConfig.Labels["namespace"]),
)
Comment on lines +296 to +299

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should iterate over the AuthConfig labels and set them dynamically into the span, similarly to what we do with the metrics.

I understand the keys wouldn't be the same as proposed, but perhaps this is also an opportunity to standardise them across the different types of observability sources?

E.g.:

Suggested change
span.SetAttributes(
otel_attr.String(trace.AuthConfigNameAttr, authConfig.Labels["authconfig"]),
otel_attr.String(trace.AuthConfigNamespaceAttr, authConfig.Labels["namespace"]),
)
for k, v := range authConfig.Labels {
span.SetAttributes(otel_attr.String(fmt.Sprintf("authorino.authconfig.%s", k), v))
}


if err := context.CheckContext(ctx); err != nil {
result := auth.AuthResult{Code: rpc.UNAVAILABLE}
setAuthResultSpanAttrs(span, result)
a.logAuthResult(result, ctx)
context.Cancel(ctx)
span.RecordError(err)
Expand All @@ -308,6 +319,17 @@ func (a *AuthService) Check(parentContext gocontext.Context, req *envoy_auth.Che
span.SetStatus(otel_codes.Error, err.Error())
}

setAuthResultSpanAttrs(span, result)

if idConfig, _ := pipeline.GetResolvedIdentity(); idConfig != nil {
if ic, ok := idConfig.(*evaluators.IdentityConfig); ok {
span.SetAttributes(
otel_attr.String(trace.IdentitySourceAttr, ic.GetName()),
otel_attr.String(trace.IdentityTypeAttr, ic.GetType()),
)
}
}

a.logAuthResult(result, ctx)

if result.Success() {
Expand Down Expand Up @@ -499,6 +521,18 @@ func closeWithStatus(respStatusCode envoy_type.StatusCode, response http.Respons
context.Cancel(ctx)
}

func setAuthResultSpanAttrs(span otel_trace.Span, result auth.AuthResult) {
if result.Success() {
span.SetAttributes(otel_attr.String(trace.AuthResultAttr, "ALLOW"))
} else {
span.SetAttributes(otel_attr.String(trace.AuthResultAttr, "DENY"))
if result.Message != "" {
span.SetAttributes(otel_attr.String(trace.AuthDenialReasonAttr, result.Message))
}
}
span.SetAttributes(otel_attr.String(trace.AuthResponseCodeAttr, result.Code.String()))
}
Comment on lines +524 to +534

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs documentation at https://github.com/Kuadrant/authorino/blob/main/docs/user-guides/observability.md#data-plane-tracing.

Other than the new attributes, a couple of things that seem important to get covered IMO:

  • DENY is also used along with other GRPC response codes such as UNAVAILABLE and NOT_FOUND.
    • Note: either ALLOW or DENY, the PEP can still behave otherwise. E.g.: a 50x that falls back to access granted, flipping the default failure_mode_allow
  • result.Code is the ext_authz GRPC response code, not the HTTP status code one may expect


func ensureRequestId(requestIdCandidates ...string) string {
for _, requestId := range requestIdCandidates {
if requestId != "" {
Expand Down
190 changes: 190 additions & 0 deletions pkg/service/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,14 @@ import (
envoy_type "github.com/envoyproxy/go-control-plane/envoy/type/v3"
"github.com/gogo/googleapis/google/rpc"
opaParser "github.com/open-policy-agent/opa/v1/ast"
"go.opentelemetry.io/otel"
otel_attr "go.opentelemetry.io/otel/attribute"
sdktrace "go.opentelemetry.io/otel/sdk/trace"
"go.opentelemetry.io/otel/sdk/trace/tracetest"
"go.uber.org/mock/gomock"
"k8s.io/apimachinery/pkg/runtime"

"github.com/kuadrant/authorino/pkg/trace"
)

const (
Expand Down Expand Up @@ -375,6 +381,190 @@ func TestCheckFailsClosedOnContextTimeout(t *testing.T) {
assert.Check(t, denied != nil, "Expected denied response")
}

func setupTestTracer(t *testing.T) *tracetest.InMemoryExporter {
t.Helper()
exporter := tracetest.NewInMemoryExporter()
tp := sdktrace.NewTracerProvider(sdktrace.WithSyncer(exporter))
prev := otel.GetTracerProvider()
otel.SetTracerProvider(tp)
t.Cleanup(func() {
otel.SetTracerProvider(prev)
if err := tp.Shutdown(context.Background()); err != nil {
t.Errorf("failed to shutdown TracerProvider: %v", err)
}
})
Comment thread
coderabbitai[bot] marked this conversation as resolved.
return exporter
}

func findSpanAttr(spans tracetest.SpanStubs, attrKey string) (otel_attr.Value, bool) {
for _, s := range spans {
for _, a := range s.Attributes {
if string(a.Key) == attrKey {
return a.Value, true
}
}
}
return otel_attr.Value{}, false
}

func TestCheckSpanAttributes_AllowedRequest(t *testing.T) {
exporter := setupTestTracer(t)
mockController := gomock.NewController(t)
defer mockController.Finish()

authConfig := mockAnonymousAccessAuthConfig()
authConfig.Labels = map[string]string{"authconfig": "my-config", "namespace": "my-ns"}

indexMock := mock_index.NewMockIndex(mockController)
indexMock.EXPECT().Get("myapp.io").Return(authConfig)

service := &AuthService{Index: indexMock}
_, err := service.Check(context.Background(), &envoy_auth.CheckRequest{
Attributes: &envoy_auth.AttributeContext{
Request: &envoy_auth.AttributeContext_Request{
Http: &envoy_auth.AttributeContext_HttpRequest{Host: "myapp.io", Method: "GET", Path: "/"},
},
},
})
assert.NilError(t, err)

spans := exporter.GetSpans()
val, ok := findSpanAttr(spans, trace.AuthResultAttr)
assert.Assert(t, ok, "expected authorino.auth.result attribute")
assert.Equal(t, val.AsString(), "ALLOW")

val, ok = findSpanAttr(spans, trace.AuthResponseCodeAttr)
assert.Assert(t, ok)
assert.Equal(t, val.AsString(), "OK")

val, ok = findSpanAttr(spans, trace.AuthConfigNameAttr)
assert.Assert(t, ok)
assert.Equal(t, val.AsString(), "my-config")

val, ok = findSpanAttr(spans, trace.AuthConfigNamespaceAttr)
assert.Assert(t, ok)
assert.Equal(t, val.AsString(), "my-ns")

val, ok = findSpanAttr(spans, trace.IdentitySourceAttr)
assert.Assert(t, ok)
assert.Equal(t, val.AsString(), "anonymous")

val, ok = findSpanAttr(spans, trace.IdentityTypeAttr)
assert.Assert(t, ok)
assert.Equal(t, val.AsString(), "IDENTITY_NOOP")

_, ok = findSpanAttr(spans, trace.AuthDenialReasonAttr)
assert.Assert(t, !ok, "denial reason should not be set on allowed request")
}

func TestCheckSpanAttributes_DeniedRequest(t *testing.T) {
exporter := setupTestTracer(t)
mockController := gomock.NewController(t)
defer mockController.Finish()

authCred := auth.NewAuthCredential("", "")
identityConfig := &evaluators.IdentityConfig{Name: "anonymous", Noop: &identity.Noop{AuthCredentials: authCred}}
authorizationPolicy, _ := authorization.NewOPAAuthorization("deny-policy", `allow := false`, nil, false, opaParser.RegoV1, 0, context.TODO())
authorizationConfig := &evaluators.AuthorizationConfig{Name: "always-deny", OPA: authorizationPolicy}
authConfig := &evaluators.AuthConfig{
Labels: map[string]string{"authconfig": "protected-api", "namespace": "prod"},
IdentityConfigs: []auth.AuthConfigEvaluator{identityConfig},
AuthorizationConfigs: []auth.AuthConfigEvaluator{authorizationConfig},
}

indexMock := mock_index.NewMockIndex(mockController)
indexMock.EXPECT().Get("myapp.io").Return(authConfig)

service := &AuthService{Index: indexMock}
resp, err := service.Check(context.Background(), &envoy_auth.CheckRequest{
Attributes: &envoy_auth.AttributeContext{
Request: &envoy_auth.AttributeContext_Request{
Http: &envoy_auth.AttributeContext_HttpRequest{Host: "myapp.io", Method: "GET", Path: "/"},
},
},
})
assert.NilError(t, err)
assert.Equal(t, resp.Status.Code, int32(rpc.PERMISSION_DENIED))

spans := exporter.GetSpans()
val, ok := findSpanAttr(spans, trace.AuthResultAttr)
assert.Assert(t, ok)
assert.Equal(t, val.AsString(), "DENY")

val, ok = findSpanAttr(spans, trace.AuthResponseCodeAttr)
assert.Assert(t, ok)
assert.Equal(t, val.AsString(), "PERMISSION_DENIED")

_, ok = findSpanAttr(spans, trace.AuthDenialReasonAttr)
assert.Assert(t, ok, "denial reason should be set on denied request")

val, ok = findSpanAttr(spans, trace.AuthConfigNameAttr)
assert.Assert(t, ok)
assert.Equal(t, val.AsString(), "protected-api")

val, ok = findSpanAttr(spans, trace.AuthConfigNamespaceAttr)
assert.Assert(t, ok)
assert.Equal(t, val.AsString(), "prod")
}

func TestCheckSpanAttributes_ServiceNotFound(t *testing.T) {
exporter := setupTestTracer(t)
mockController := gomock.NewController(t)
defer mockController.Finish()

indexMock := mock_index.NewMockIndex(mockController)
indexMock.EXPECT().Get("unknown.io").Return(nil)

service := &AuthService{Index: indexMock}
resp, err := service.Check(context.Background(), &envoy_auth.CheckRequest{
Attributes: &envoy_auth.AttributeContext{
Request: &envoy_auth.AttributeContext_Request{
Http: &envoy_auth.AttributeContext_HttpRequest{Host: "unknown.io", Method: "GET", Path: "/"},
},
},
})
assert.NilError(t, err)
assert.Equal(t, resp.Status.Code, int32(rpc.NOT_FOUND))

spans := exporter.GetSpans()
val, ok := findSpanAttr(spans, trace.AuthResultAttr)
assert.Assert(t, ok)
assert.Equal(t, val.AsString(), "DENY")

val, ok = findSpanAttr(spans, trace.AuthResponseCodeAttr)
assert.Assert(t, ok)
assert.Equal(t, val.AsString(), "NOT_FOUND")

val, ok = findSpanAttr(spans, trace.AuthDenialReasonAttr)
assert.Assert(t, ok)
assert.Equal(t, val.AsString(), "Service not found")

_, ok = findSpanAttr(spans, trace.AuthConfigNameAttr)
assert.Assert(t, !ok, "auth_config.name should not be set when service not found")
}

func TestCheckSpanAttributes_InvalidRequest(t *testing.T) {
exporter := setupTestTracer(t)

service := &AuthService{Index: index.NewIndex()}
resp, err := service.Check(context.Background(), &envoy_auth.CheckRequest{})
assert.NilError(t, err)
assert.Equal(t, resp.Status.Code, int32(rpc.INVALID_ARGUMENT))

spans := exporter.GetSpans()
val, ok := findSpanAttr(spans, trace.AuthResultAttr)
assert.Assert(t, ok)
assert.Equal(t, val.AsString(), "DENY")

val, ok = findSpanAttr(spans, trace.AuthResponseCodeAttr)
assert.Assert(t, ok)
assert.Equal(t, val.AsString(), "INVALID_ARGUMENT")

val, ok = findSpanAttr(spans, trace.AuthDenialReasonAttr)
assert.Assert(t, ok)
assert.Equal(t, val.AsString(), "Invalid request")
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

func mockAnonymousAccessAuthConfig() *evaluators.AuthConfig {
authCred := auth.NewAuthCredential("", "")
identityConfig := &evaluators.IdentityConfig{Name: "anonymous", Noop: &identity.Noop{AuthCredentials: authCred}}
Expand Down
8 changes: 8 additions & 0 deletions pkg/trace/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@ import (
const (
AuthorinoRequestIdAttr = "authorino.request_id"
PropagationRequestIdAttr = "guid:x-request-id"

AuthResultAttr = "authorino.auth.result"
AuthResponseCodeAttr = "authorino.auth.response_code"
AuthDenialReasonAttr = "authorino.auth.denial_reason"
AuthConfigNameAttr = "authorino.auth_config.name"
AuthConfigNamespaceAttr = "authorino.auth_config.namespace"
IdentitySourceAttr = "authorino.identity.source"
IdentityTypeAttr = "authorino.identity.type"
)

func NewSpan(parentContext context.Context, tracerName, spanName string, options ...otel_trace.SpanStartOption) (context.Context, otel_trace.Span) {
Expand Down