diff --git a/internal/api/handlers/addons.go b/internal/api/handlers/addons.go index 484ce69..a6e5853 100644 --- a/internal/api/handlers/addons.go +++ b/internal/api/handlers/addons.go @@ -1151,7 +1151,7 @@ func (h *AddonsHandler) checkOperatePermission(user *auth.UserSession, cluster * return nil } - if user.IsAdmin() { + if user.PlatformRole == auth.RoleAdmin { return nil } diff --git a/internal/api/handlers/certificates.go b/internal/api/handlers/certificates.go index 31984a8..7be46ae 100644 --- a/internal/api/handlers/certificates.go +++ b/internal/api/handlers/certificates.go @@ -300,13 +300,8 @@ func (h *CertificateHandler) GetCertificatesByCategory(w http.ResponseWriter, r // checkClusterAccess verifies the user has access to a cluster based on its teamRef. func (h *CertificateHandler) checkClusterAccess(user *auth.UserSession, cluster *unstructured.Unstructured) error { - // Platform admins can access all clusters - if user.IsPlatformAdmin { - return nil - } - - // Admins can access all clusters - if user.IsAdmin() { + // Platform roles can read certificates of all clusters + if user.IsPlatformViewerOrAbove() { return nil } @@ -329,12 +324,7 @@ func (h *CertificateHandler) checkClusterAccess(user *auth.UserSession, cluster // canRotateCertificates checks if the user has operator role for the cluster's team. func (h *CertificateHandler) canRotateCertificates(user *auth.UserSession, cluster *unstructured.Unstructured) bool { // Platform admins can rotate any certificates - if user.IsPlatformAdmin { - return true - } - - // Admins can rotate any certificates - if user.IsAdmin() { + if user.PlatformRole == auth.RoleAdmin { return true } @@ -350,7 +340,7 @@ func (h *CertificateHandler) canRotateCertificates(user *auth.UserSession, clust // canRotateCA checks if the user has admin role for CA rotation. func (h *CertificateHandler) canRotateCA(user *auth.UserSession, cluster *unstructured.Unstructured) bool { // Platform admins can rotate CA - if user.IsPlatformAdmin { + if user.PlatformRole == auth.RoleAdmin { return true } diff --git a/internal/api/handlers/clusters.go b/internal/api/handlers/clusters.go index 7a5a9c7..c7a9491 100644 --- a/internal/api/handlers/clusters.go +++ b/internal/api/handlers/clusters.go @@ -75,14 +75,15 @@ func (h *ClusterHandler) checkClusterAccess(user *auth.UserSession, cluster *uns return nil } - // No team context - use legacy behavior - // Admins can access all clusters - if user.IsAdmin() { + // No team context: platform roles see every cluster; everyone else + // sees the clusters of teams they belong to. Being an admin of some + // other team grants nothing here. + if user.IsPlatformViewerOrAbove() { return nil } // If cluster has no teamRef, it's a platform-level cluster - // Only admins should see these (already checked above) + // Only platform roles should see these (already checked above) if !found || clusterTeam == "" { return fmt.Errorf("forbidden: cluster is not associated with any team") } @@ -111,8 +112,9 @@ func (h *ClusterHandler) checkOperatePermission(user *auth.UserSession, teamRef return nil } - // No team context - use legacy behavior - if user.IsAdmin() { + // No team context: platform admins may operate on any cluster; + // everyone else needs an operate-capable role on the cluster's team. + if user.PlatformRole == auth.RoleAdmin { return nil } @@ -163,8 +165,8 @@ func (h *ClusterHandler) List(w http.ResponseWriter, r *http.Request) { if clusterTeam != user.SelectedTeam { continue } - } else if !user.IsAdmin() { - // No team context and not admin - filter by team membership + } else if !user.IsPlatformViewerOrAbove() { + // No team context and no platform role - filter by team membership if clusterTeam == "" { continue } diff --git a/internal/api/handlers/clusters_operate_test.go b/internal/api/handlers/clusters_operate_test.go new file mode 100644 index 0000000..6fb89af --- /dev/null +++ b/internal/api/handlers/clusters_operate_test.go @@ -0,0 +1,101 @@ +/* +Copyright 2026 The Butler Authors. + +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 handlers + +import ( + "testing" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + "github.com/butlerdotdev/butler-server/internal/auth" +) + +func teamCluster(team string) *unstructured.Unstructured { + spec := map[string]interface{}{} + if team != "" { + spec["teamRef"] = map[string]interface{}{"name": team} + } + return &unstructured.Unstructured{Object: map[string]interface{}{"spec": spec}} +} + +// An admin of team alpha with no platform role. Before this change +// IsAdmin() made this session pass every no-header check. +func alphaAdmin() *auth.UserSession { + return &auth.UserSession{Teams: []auth.TeamMembership{{Name: "alpha", Role: auth.RoleAdmin}}} +} + +func TestNoHeaderChecks_TeamAdminIsNotPlatformAdmin(t *testing.T) { + ch := &ClusterHandler{} + ah := &AddonsHandler{} + gh := &GitOpsHandler{} + certs := &CertificateHandler{} + user := alphaAdmin() + + if err := ch.checkClusterAccess(user, teamCluster("alpha")); err != nil { + t.Fatalf("own-team visibility: %v", err) + } + if err := ch.checkClusterAccess(user, teamCluster("beta")); err == nil { + t.Fatal("team admin of alpha could see beta's cluster") + } + if err := ch.checkOperatePermission(user, "beta", "delete"); err == nil { + t.Fatal("team admin of alpha could operate on beta") + } + if err := ch.checkOperatePermission(user, "alpha", "delete"); err != nil { + t.Fatalf("own-team operate: %v", err) + } + if err := ah.checkOperatePermission(user, teamCluster("beta"), "install"); err == nil { + t.Fatal("team admin of alpha could install addons on beta") + } + if err := gh.checkOperatePermission(user, teamCluster("beta"), "enable"); err == nil { + t.Fatal("team admin of alpha could enable gitops on beta") + } + if err := certs.checkClusterAccess(user, teamCluster("beta")); err == nil { + t.Fatal("team admin of alpha could read beta's certificates") + } + if certs.canRotateCertificates(user, teamCluster("beta")) { + t.Fatal("team admin of alpha could rotate beta's certificates") + } + if !certs.canRotateCertificates(user, teamCluster("alpha")) { + t.Fatal("team admin of alpha should rotate alpha's certificates") + } +} + +func TestNoHeaderChecks_PlatformRoles(t *testing.T) { + ch := &ClusterHandler{} + certs := &CertificateHandler{} + admin := &auth.UserSession{PlatformRole: auth.RoleAdmin} + viewer := &auth.UserSession{PlatformRole: auth.RoleViewer} + + if err := ch.checkClusterAccess(viewer, teamCluster("beta")); err != nil { + t.Fatalf("platform viewer visibility: %v", err) + } + if err := ch.checkOperatePermission(viewer, "beta", "scale"); err == nil { + t.Fatal("platform viewer could operate") + } + if err := ch.checkOperatePermission(admin, "", "scale"); err != nil { + t.Fatalf("platform admin operate on team-less cluster: %v", err) + } + if err := certs.checkClusterAccess(viewer, teamCluster("beta")); err != nil { + t.Fatalf("platform viewer certificate read: %v", err) + } + if certs.canRotateCertificates(viewer, teamCluster("beta")) { + t.Fatal("platform viewer could rotate certificates") + } + if !certs.canRotateCA(admin, teamCluster("")) { + t.Fatal("platform admin should rotate CA of team-less cluster") + } +} diff --git a/internal/api/handlers/gitops.go b/internal/api/handlers/gitops.go index a4cc673..8e3e96b 100644 --- a/internal/api/handlers/gitops.go +++ b/internal/api/handlers/gitops.go @@ -2396,7 +2396,7 @@ func (h *GitOpsHandler) checkOperatePermission(user *auth.UserSession, cluster * return nil } - if user.IsAdmin() { + if user.PlatformRole == auth.RoleAdmin { return nil } diff --git a/internal/api/handlers/providers.go b/internal/api/handlers/providers.go index 4a2b7ef..35145a0 100644 --- a/internal/api/handlers/providers.go +++ b/internal/api/handlers/providers.go @@ -185,6 +185,24 @@ func (h *ProvidersHandler) Get(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, provider.Object) } +// TestTeamConnection tests provider credentials on behalf of a team. +// Same body as TestConnection, but the caller must be able to manage +// the named team's providers; the platform-scoped route is gated by +// middleware instead. +func (h *ProvidersHandler) TestTeamConnection(w http.ResponseWriter, r *http.Request) { + teamName := chi.URLParam(r, "name") + user := auth.UserFromContext(r.Context()) + if user == nil { + writeError(w, http.StatusUnauthorized, "unauthorized") + return + } + if !user.CanOperateTeam(teamName) { + writeError(w, http.StatusForbidden, fmt.Sprintf("you don't have permission to manage providers for team '%s'", teamName)) + return + } + h.TestConnection(w, r) +} + // TestConnection tests provider credentials without creating anything. func (h *ProvidersHandler) TestConnection(w http.ResponseWriter, r *http.Request) { var req CreateProviderRequest diff --git a/internal/api/router.go b/internal/api/router.go index 9405a06..55ea033 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -504,7 +504,7 @@ func NewRouter(cfg RouterConfig) (http.Handler, error) { // Team provider management (team members can list, team admins can create/delete) r.Get("/teams/{name}/providers", providerHandler.ListTeamProviders) r.Post("/teams/{name}/providers", providerHandler.CreateTeamProvider) - r.Post("/teams/{name}/providers/test", providerHandler.TestConnection) + r.Post("/teams/{name}/providers/test", providerHandler.TestTeamConnection) r.Delete("/teams/{name}/providers/{namespace}/{providerName}", providerHandler.DeleteTeamProvider) // Team environment management (ADR-009). The Team admission