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
67 changes: 63 additions & 4 deletions controller/internal/service/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,44 @@ package auth

import (
"context"
"net"

jumpstarterdevv1alpha1 "github.com/jumpstarter-dev/jumpstarter-controller/api/v1alpha1"
"github.com/jumpstarter-dev/jumpstarter-controller/internal/authentication"
"github.com/jumpstarter-dev/jumpstarter-controller/internal/authorization"
"github.com/jumpstarter-dev/jumpstarter-controller/internal/oidc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/peer"
"google.golang.org/grpc/status"
"k8s.io/apiserver/pkg/authorization/authorizer"
kclient "sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log"
)

// PeerAddr returns the remote IP address from the gRPC peer info stored in ctx,
// or "unknown" if unavailable. Port number and transport paths (e.g. Unix socket
// paths) are intentionally stripped to avoid leaking internal details.
func PeerAddr(ctx context.Context) string {
p, ok := peer.FromContext(ctx)
if !ok || p.Addr == nil {
return "unknown"
}
host, _, err := net.SplitHostPort(p.Addr.String())
if err != nil {
return "unknown"
}
Comment thread
mangelajo marked this conversation as resolved.
return host
}

// LogContext returns ctx with its logger enriched with the peer address under
// the "peer" key ("unknown" when no usable address is available, so all log
// lines share a uniform shape). The gRPC interceptors apply this to every RPC;
// it owns the peer enrichment, so loggers derived from ctx (including the
// auth-failure logs below) must not add "peer" again.
func LogContext(ctx context.Context) context.Context {
return log.IntoContext(ctx, log.FromContext(ctx, "peer", PeerAddr(ctx)))
}
Comment on lines +22 to +41

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[MEDIUM] PeerAddr and LogContext are transport-level infrastructure used by ALL incoming RPCs (not just auth), but they live in the auth package. Any future non-auth service or interceptor needing peer-enriched logging would have to import the auth package, conflating transport concerns with authentication logic.

Suggested fix: extract PeerAddr and LogContext into a dedicated package (e.g., internal/grpcutil or internal/peerlog) that both the auth package and the gRPC interceptors import.

AI-generated, human reviewed


type Auth struct {
client kclient.Client
authn authentication.ContextAuthenticator
Expand All @@ -34,7 +61,11 @@ func NewAuth(
}
}

func (s *Auth) AuthClient(ctx context.Context, namespace string) (*jumpstarterdevv1alpha1.Client, error) {
// VerifyClient authenticates the client token in ctx and returns the matching
// Client object without enforcing a namespace. Authentication failures are
// logged via the context logger, which carries the peer address when the
// caller applied LogContext (as the gRPC interceptors do).
func (s *Auth) VerifyClient(ctx context.Context) (*jumpstarterdevv1alpha1.Client, error) {
jclient, err := oidc.VerifyClientObjectToken(
ctx,
s.authn,
Expand All @@ -43,18 +74,34 @@ func (s *Auth) AuthClient(ctx context.Context, namespace string) (*jumpstarterde
s.client,
)

if err != nil {
log.FromContext(ctx).Info("client authentication failed", "error", err.Error())
return nil, err
}

return jclient, nil
}

func (s *Auth) AuthClient(ctx context.Context, namespace string) (*jumpstarterdevv1alpha1.Client, error) {
jclient, err := s.VerifyClient(ctx)
if err != nil {
return nil, err
}

if namespace != jclient.Namespace {
return nil, status.Error(codes.PermissionDenied, "namespace mismatch")
err := status.Error(codes.PermissionDenied, "namespace mismatch")
log.FromContext(ctx).Info("client authentication failed", "client", jclient.Name, "error", err.Error())
return nil, err
}

return jclient, nil
}

func (s *Auth) AuthExporter(ctx context.Context, namespace string) (*jumpstarterdevv1alpha1.Exporter, error) {
// VerifyExporter authenticates the exporter token in ctx and returns the
// matching Exporter object without enforcing a namespace. Authentication
// failures are logged via the context logger, which carries the peer address
// when the caller applied LogContext (as the gRPC interceptors do).
func (s *Auth) VerifyExporter(ctx context.Context) (*jumpstarterdevv1alpha1.Exporter, error) {
jexporter, err := oidc.VerifyExporterObjectToken(
ctx,
s.authn,
Expand All @@ -63,12 +110,24 @@ func (s *Auth) AuthExporter(ctx context.Context, namespace string) (*jumpstarter
s.client,
)

if err != nil {
log.FromContext(ctx).Info("exporter authentication failed", "error", err.Error())
return nil, err
}

return jexporter, nil
}

func (s *Auth) AuthExporter(ctx context.Context, namespace string) (*jumpstarterdevv1alpha1.Exporter, error) {
jexporter, err := s.VerifyExporter(ctx)
if err != nil {
return nil, err
}

if namespace != jexporter.Namespace {
return nil, status.Error(codes.PermissionDenied, "namespace mismatch")
err := status.Error(codes.PermissionDenied, "namespace mismatch")
log.FromContext(ctx).Info("exporter authentication failed", "exporter", jexporter.Name, "error", err.Error())
return nil, err
}

return jexporter, nil
Expand Down
Loading
Loading