From 871b1fb8f42b72360d267b340d7b81c3196e21e7 Mon Sep 17 00:00:00 2001 From: Kyle Isom Date: Sat, 28 Mar 2026 15:52:43 -0700 Subject: [PATCH] Add record-level authorization for system accounts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Record mutations (create, update, delete) no longer require admin role. Authorization rules: - admin: full access (unchanged) - system mcp-agent: create/delete any record - system account α: create/delete records named α only - human users: read-only (unchanged) Zone mutations remain admin-only. Both REST and gRPC paths enforce the same rules. Update checks authorization against both old and new names. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/grpcserver/handlers_test.go | 6 ++-- internal/grpcserver/interceptors.go | 16 ++++----- internal/grpcserver/records.go | 50 ++++++++++++++++++++++++++-- internal/server/handlers_test.go | 12 +++++-- internal/server/middleware.go | 23 +++++++++++++ internal/server/records.go | 24 +++++++++++++ internal/server/routes.go | 10 +++--- 7 files changed, 120 insertions(+), 21 deletions(-) diff --git a/internal/grpcserver/handlers_test.go b/internal/grpcserver/handlers_test.go index c367086..fc375c6 100644 --- a/internal/grpcserver/handlers_test.go +++ b/internal/grpcserver/handlers_test.go @@ -769,6 +769,9 @@ func TestMethodMapCompleteness(t *testing.T) { "/mcns.v1.ZoneService/GetZone", "/mcns.v1.RecordService/ListRecords", "/mcns.v1.RecordService/GetRecord", + "/mcns.v1.RecordService/CreateRecord", + "/mcns.v1.RecordService/UpdateRecord", + "/mcns.v1.RecordService/DeleteRecord", } for _, method := range expectedAuth { if !mm.AuthRequired[method] { @@ -783,9 +786,6 @@ func TestMethodMapCompleteness(t *testing.T) { "/mcns.v1.ZoneService/CreateZone", "/mcns.v1.ZoneService/UpdateZone", "/mcns.v1.ZoneService/DeleteZone", - "/mcns.v1.RecordService/CreateRecord", - "/mcns.v1.RecordService/UpdateRecord", - "/mcns.v1.RecordService/DeleteRecord", } for _, method := range expectedAdmin { if !mm.AdminRequired[method] { diff --git a/internal/grpcserver/interceptors.go b/internal/grpcserver/interceptors.go index 86172d2..e9fa0d8 100644 --- a/internal/grpcserver/interceptors.go +++ b/internal/grpcserver/interceptors.go @@ -25,11 +25,14 @@ func publicMethods() map[string]bool { func authRequiredMethods() map[string]bool { return map[string]bool{ - "/mcns.v1.AuthService/Logout": true, - "/mcns.v1.ZoneService/ListZones": true, - "/mcns.v1.ZoneService/GetZone": true, - "/mcns.v1.RecordService/ListRecords": true, - "/mcns.v1.RecordService/GetRecord": true, + "/mcns.v1.AuthService/Logout": true, + "/mcns.v1.ZoneService/ListZones": true, + "/mcns.v1.ZoneService/GetZone": true, + "/mcns.v1.RecordService/ListRecords": true, + "/mcns.v1.RecordService/GetRecord": true, + "/mcns.v1.RecordService/CreateRecord": true, + "/mcns.v1.RecordService/UpdateRecord": true, + "/mcns.v1.RecordService/DeleteRecord": true, } } @@ -38,8 +41,5 @@ func adminRequiredMethods() map[string]bool { "/mcns.v1.ZoneService/CreateZone": true, "/mcns.v1.ZoneService/UpdateZone": true, "/mcns.v1.ZoneService/DeleteZone": true, - "/mcns.v1.RecordService/CreateRecord": true, - "/mcns.v1.RecordService/UpdateRecord": true, - "/mcns.v1.RecordService/DeleteRecord": true, } } diff --git a/internal/grpcserver/records.go b/internal/grpcserver/records.go index d903299..5c8f510 100644 --- a/internal/grpcserver/records.go +++ b/internal/grpcserver/records.go @@ -10,10 +10,36 @@ import ( "google.golang.org/grpc/status" "google.golang.org/protobuf/types/known/timestamppb" + mcdslauth "git.wntrmute.dev/mc/mcdsl/auth" + mcdslgrpc "git.wntrmute.dev/mc/mcdsl/grpcserver" + pb "git.wntrmute.dev/mc/mcns/gen/mcns/v1" "git.wntrmute.dev/mc/mcns/internal/db" ) +// authorizeRecordMutation checks whether the caller may create, update, +// or delete a DNS record with the given name. The rules are: +// +// - admin role: always allowed +// - system account "mcp-agent": allowed for any record name +// - system account α: allowed only when recordName == α +// - all others: denied +func authorizeRecordMutation(info *mcdslauth.TokenInfo, recordName string) bool { + if info == nil { + return false + } + if info.IsAdmin { + return true + } + if info.AccountType != "system" { + return false + } + if info.Username == "mcp-agent" { + return true + } + return recordName == info.Username +} + type recordService struct { pb.UnimplementedRecordServiceServer db *db.DB @@ -55,7 +81,7 @@ func (s *recordService) GetRecord(_ context.Context, req *pb.GetRecordRequest) ( return s.recordToProto(*record), nil } -func (s *recordService) CreateRecord(_ context.Context, req *pb.CreateRecordRequest) (*pb.Record, error) { +func (s *recordService) CreateRecord(ctx context.Context, req *pb.CreateRecordRequest) (*pb.Record, error) { if req.Zone == "" { return nil, status.Error(codes.InvalidArgument, "zone is required") } @@ -69,6 +95,10 @@ func (s *recordService) CreateRecord(_ context.Context, req *pb.CreateRecordRequ return nil, status.Error(codes.InvalidArgument, "value is required") } + if !authorizeRecordMutation(mcdslgrpc.TokenInfoFromContext(ctx), req.Name) { + return nil, status.Error(codes.PermissionDenied, "not authorized for record name") + } + record, err := s.db.CreateRecord(req.Zone, req.Name, req.Type, req.Value, int(req.Ttl)) if errors.Is(err, db.ErrNotFound) { return nil, status.Error(codes.NotFound, "zone not found") @@ -82,7 +112,7 @@ func (s *recordService) CreateRecord(_ context.Context, req *pb.CreateRecordRequ return s.recordToProto(*record), nil } -func (s *recordService) UpdateRecord(_ context.Context, req *pb.UpdateRecordRequest) (*pb.Record, error) { +func (s *recordService) UpdateRecord(ctx context.Context, req *pb.UpdateRecordRequest) (*pb.Record, error) { if req.Id <= 0 { return nil, status.Error(codes.InvalidArgument, "id must be positive") } @@ -96,6 +126,15 @@ func (s *recordService) UpdateRecord(_ context.Context, req *pb.UpdateRecordRequ return nil, status.Error(codes.InvalidArgument, "value is required") } + info := mcdslgrpc.TokenInfoFromContext(ctx) + existing, lookupErr := s.db.GetRecord(req.Id) + if lookupErr == nil && !authorizeRecordMutation(info, existing.Name) { + return nil, status.Error(codes.PermissionDenied, "not authorized for record name") + } + if !authorizeRecordMutation(info, req.Name) { + return nil, status.Error(codes.PermissionDenied, "not authorized for record name") + } + record, err := s.db.UpdateRecord(req.Id, req.Name, req.Type, req.Value, int(req.Ttl)) if errors.Is(err, db.ErrNotFound) { return nil, status.Error(codes.NotFound, "record not found") @@ -109,11 +148,16 @@ func (s *recordService) UpdateRecord(_ context.Context, req *pb.UpdateRecordRequ return s.recordToProto(*record), nil } -func (s *recordService) DeleteRecord(_ context.Context, req *pb.DeleteRecordRequest) (*pb.DeleteRecordResponse, error) { +func (s *recordService) DeleteRecord(ctx context.Context, req *pb.DeleteRecordRequest) (*pb.DeleteRecordResponse, error) { if req.Id <= 0 { return nil, status.Error(codes.InvalidArgument, "id must be positive") } + existing, lookupErr := s.db.GetRecord(req.Id) + if lookupErr == nil && !authorizeRecordMutation(mcdslgrpc.TokenInfoFromContext(ctx), existing.Name) { + return nil, status.Error(codes.PermissionDenied, "not authorized for record name") + } + err := s.db.DeleteRecord(req.Id) if errors.Is(err, db.ErrNotFound) { return nil, status.Error(codes.NotFound, "record not found") diff --git a/internal/server/handlers_test.go b/internal/server/handlers_test.go index ed649a4..ddaed9b 100644 --- a/internal/server/handlers_test.go +++ b/internal/server/handlers_test.go @@ -42,6 +42,7 @@ func createTestZone(t *testing.T, database *db.DB) *db.Zone { } // newChiRequest builds a request with chi URL params injected into the context. +// An admin TokenInfo is added so that handler-level authorization passes. func newChiRequest(method, target string, body string, params map[string]string) *http.Request { var r *http.Request if body != "" { @@ -51,14 +52,21 @@ func newChiRequest(method, target string, body string, params map[string]string) } r.Header.Set("Content-Type", "application/json") + ctx := r.Context() if len(params) > 0 { rctx := chi.NewRouteContext() for k, v := range params { rctx.URLParams.Add(k, v) } - r = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, rctx)) + ctx = context.WithValue(ctx, chi.RouteCtxKey, rctx) } - return r + + // Inject admin TokenInfo for handler-level authorization. + ctx = context.WithValue(ctx, tokenInfoKey, &mcdslauth.TokenInfo{ + Username: "testadmin", + IsAdmin: true, + }) + return r.WithContext(ctx) } // decodeJSON decodes the response body into v. diff --git a/internal/server/middleware.go b/internal/server/middleware.go index b31a920..61d0e34 100644 --- a/internal/server/middleware.go +++ b/internal/server/middleware.go @@ -48,6 +48,29 @@ func requireAdmin(next http.Handler) http.Handler { }) } +// authorizeRecordMutation checks whether the caller may create, update, +// or delete a DNS record with the given name. The rules are: +// +// - admin role: always allowed +// - system account "mcp-agent": allowed for any record name +// - system account α: allowed only when recordName == α +// - all others: denied +func authorizeRecordMutation(info *mcdslauth.TokenInfo, recordName string) bool { + if info == nil { + return false + } + if info.IsAdmin { + return true + } + if info.AccountType != "system" { + return false + } + if info.Username == "mcp-agent" { + return true + } + return recordName == info.Username +} + // tokenInfoFromContext extracts the TokenInfo from the request context. func tokenInfoFromContext(ctx context.Context) *mcdslauth.TokenInfo { info, _ := ctx.Value(tokenInfoKey).(*mcdslauth.TokenInfo) diff --git a/internal/server/records.go b/internal/server/records.go index 7a4b39d..9bfa670 100644 --- a/internal/server/records.go +++ b/internal/server/records.go @@ -86,6 +86,11 @@ func createRecordHandler(database *db.DB) http.HandlerFunc { return } + if !authorizeRecordMutation(tokenInfoFromContext(r.Context()), req.Name) { + writeError(w, http.StatusForbidden, "not authorized for record name") + return + } + record, err := database.CreateRecord(zoneName, req.Name, req.Type, req.Value, req.TTL) if errors.Is(err, db.ErrNotFound) { writeError(w, http.StatusNotFound, "zone not found") @@ -132,6 +137,18 @@ func updateRecordHandler(database *db.DB) http.HandlerFunc { return } + // Authorize against both old and new record names. + info := tokenInfoFromContext(r.Context()) + existing, lookupErr := database.GetRecord(id) + if lookupErr == nil && !authorizeRecordMutation(info, existing.Name) { + writeError(w, http.StatusForbidden, "not authorized for record name") + return + } + if !authorizeRecordMutation(info, req.Name) { + writeError(w, http.StatusForbidden, "not authorized for record name") + return + } + record, err := database.UpdateRecord(id, req.Name, req.Type, req.Value, req.TTL) if errors.Is(err, db.ErrNotFound) { writeError(w, http.StatusNotFound, "record not found") @@ -159,6 +176,13 @@ func deleteRecordHandler(database *db.DB) http.HandlerFunc { return } + // Look up the record to authorize by name. + existing, lookupErr := database.GetRecord(id) + if lookupErr == nil && !authorizeRecordMutation(tokenInfoFromContext(r.Context()), existing.Name) { + writeError(w, http.StatusForbidden, "not authorized for record name") + return + } + err = database.DeleteRecord(id) if errors.Is(err, db.ErrNotFound) { writeError(w, http.StatusNotFound, "record not found") diff --git a/internal/server/routes.go b/internal/server/routes.go index d8484d0..78009b9 100644 --- a/internal/server/routes.go +++ b/internal/server/routes.go @@ -44,14 +44,14 @@ func NewRouter(deps Deps) *chi.Mux { r.With(requireAdmin).Put("/v1/zones/{zone}", updateZoneHandler(deps.DB)) r.With(requireAdmin).Delete("/v1/zones/{zone}", deleteZoneHandler(deps.DB)) - // Record endpoints — reads for all authenticated users, writes for admin. + // Record endpoints — reads for all authenticated users. r.Get("/v1/zones/{zone}/records", listRecordsHandler(deps.DB)) r.Get("/v1/zones/{zone}/records/{id}", getRecordHandler(deps.DB)) - // Admin-only record mutations. - r.With(requireAdmin).Post("/v1/zones/{zone}/records", createRecordHandler(deps.DB)) - r.With(requireAdmin).Put("/v1/zones/{zone}/records/{id}", updateRecordHandler(deps.DB)) - r.With(requireAdmin).Delete("/v1/zones/{zone}/records/{id}", deleteRecordHandler(deps.DB)) + // Record mutations — admin, mcp-agent (any name), or system account (own name). + r.Post("/v1/zones/{zone}/records", createRecordHandler(deps.DB)) + r.Put("/v1/zones/{zone}/records/{id}", updateRecordHandler(deps.DB)) + r.Delete("/v1/zones/{zone}/records/{id}", deleteRecordHandler(deps.DB)) }) return r