From c9b780659f9aaeacf517679b9826ebb1c249306b Mon Sep 17 00:00:00 2001 From: kyy Date: Tue, 3 Mar 2026 14:09:43 +0900 Subject: [PATCH] =?UTF-8?q?Dev=20=EA=B6=8C=ED=95=9C/=ED=85=8C=EB=84=8C?= =?UTF-8?q?=ED=8A=B8=20=EA=B2=A9=EB=A6=AC=20=ED=9A=8C=EA=B7=80=20=ED=85=8C?= =?UTF-8?q?=EC=8A=A4=ED=8A=B8=20=EB=B3=B4=EA=B0=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../handler/dev_handler_isolation_test.go | 193 ++++++++++++++++++ backend/internal/handler/dev_handler_test.go | 22 ++ 2 files changed, 215 insertions(+) create mode 100644 backend/internal/handler/dev_handler_isolation_test.go diff --git a/backend/internal/handler/dev_handler_isolation_test.go b/backend/internal/handler/dev_handler_isolation_test.go new file mode 100644 index 00000000..ff334f46 --- /dev/null +++ b/backend/internal/handler/dev_handler_isolation_test.go @@ -0,0 +1,193 @@ +package handler + +import ( + "baron-sso-backend/internal/domain" + "baron-sso-backend/internal/service" + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/gofiber/fiber/v2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" +) + +func TestDevHandler_Isolation(t *testing.T) { + mockKeto := new(MockKetoService) + + h := &DevHandler{ + Hydra: &service.HydraAdminService{ + AdminURL: "http://hydra.test", + HTTPClient: &http.Client{ + Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + if r.Method == http.MethodGet && r.URL.Path == "/clients" { + return httpJSONAny(r, http.StatusOK, []map[string]interface{}{ + { + "client_id": "client-tenant-a", + "client_name": "App Tenant A", + "token_endpoint_auth_method": "none", // PKCE + "metadata": map[string]interface{}{"tenant_id": "tenant-a"}, + }, + { + "client_id": "client-tenant-b", + "client_name": "App Tenant B", + "token_endpoint_auth_method": "none", // PKCE + "metadata": map[string]interface{}{"tenant_id": "tenant-b"}, + }, + }), nil + } + if (r.Method == http.MethodGet || r.Method == http.MethodPut) && strings.HasPrefix(r.URL.Path, "/clients/") { + id := strings.TrimPrefix(r.URL.Path, "/clients/") + tenantID := "tenant-a" + if id == "client-tenant-b" { + tenantID = "tenant-b" + } + return httpJSONAny(r, http.StatusOK, map[string]interface{}{ + "client_id": id, + "client_name": "App " + id, + "token_endpoint_auth_method": "none", + "metadata": map[string]interface{}{"tenant_id": tenantID}, + }), nil + } + if r.Method == http.MethodPost && r.URL.Path == "/clients" { + var body map[string]interface{} + json.NewDecoder(r.Body).Decode(&body) + return httpJSONAny(r, http.StatusCreated, body), nil + } + return httpJSONAny(r, http.StatusNotFound, nil), nil + }), + }, + }, + Keto: mockKeto, + } + + t.Run("Local bypass should be removed", func(t *testing.T) { + app := fiber.New() + app.Get("/api/v1/dev/clients", h.ListClients) + + req := httptest.NewRequest(http.MethodGet, "/api/v1/dev/clients", nil) + req.Header.Set("Origin", "http://localhost:5174") + + resp, _ := app.Test(req, -1) + // We expect 401 now because ListClients enforces authentication. + assert.Equal(t, http.StatusUnauthorized, resp.StatusCode) + }) + + t.Run("ListClients should filter by tenant_id for non-SuperAdmin", func(t *testing.T) { + app := fiber.New() + tenantA := "tenant-a" + app.Use(func(c *fiber.Ctx) error { + c.Locals("user_profile", &domain.UserProfileResponse{ + ID: "user-a", + Role: domain.RoleUser, + TenantID: &tenantA, + }) + return c.Next() + }) + app.Get("/api/v1/dev/clients", h.ListClients) + + mockKeto.On("CheckPermission", mock.Anything, "user-a", "System", "AppManager", "member").Return(false, nil) + + req := httptest.NewRequest(http.MethodGet, "/api/v1/dev/clients", nil) + resp, _ := app.Test(req, -1) + + assert.Equal(t, http.StatusOK, resp.StatusCode) + var res struct { + Items []clientSummary `json:"items"` + } + json.NewDecoder(resp.Body).Decode(&res) + + // Should only see client-tenant-a + assert.Equal(t, 1, len(res.Items)) + assert.Equal(t, "client-tenant-a", res.Items[0].ID) + }) + + t.Run("GetClient should enforce tenant isolation", func(t *testing.T) { + app := fiber.New() + tenantA := "tenant-a" + app.Use(func(c *fiber.Ctx) error { + c.Locals("user_profile", &domain.UserProfileResponse{ + ID: "user-a", + Role: domain.RoleUser, + TenantID: &tenantA, + }) + return c.Next() + }) + app.Get("/api/v1/dev/clients/:id", h.GetClient) + + // Case 1: Same tenant + req := httptest.NewRequest(http.MethodGet, "/api/v1/dev/clients/client-tenant-a", nil) + resp, _ := app.Test(req, -1) + assert.Equal(t, http.StatusOK, resp.StatusCode) + + // Case 2: Different tenant + req = httptest.NewRequest(http.MethodGet, "/api/v1/dev/clients/client-tenant-b", nil) + resp, _ = app.Test(req, -1) + assert.Equal(t, http.StatusForbidden, resp.StatusCode) + }) + + t.Run("UpdateClient should enforce tenant isolation", func(t *testing.T) { + app := fiber.New() + tenantA := "tenant-a" + app.Use(func(c *fiber.Ctx) error { + c.Locals("user_profile", &domain.UserProfileResponse{ + ID: "user-a", + Role: domain.RoleUser, + TenantID: &tenantA, + }) + return c.Next() + }) + app.Put("/api/v1/dev/clients/:id", h.UpdateClient) + + body, _ := json.Marshal(map[string]interface{}{ + "client_name": "Updated Name", + }) + + // Case 1: Same tenant + req := httptest.NewRequest(http.MethodPut, "/api/v1/dev/clients/client-tenant-a", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + resp, _ := app.Test(req, -1) + assert.Equal(t, http.StatusOK, resp.StatusCode) + + // Case 2: Different tenant + req = httptest.NewRequest(http.MethodPut, "/api/v1/dev/clients/client-tenant-b", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + resp, _ = app.Test(req, -1) + assert.Equal(t, http.StatusForbidden, resp.StatusCode) + }) + + t.Run("CreateClient should record user_id and tenant_id", func(t *testing.T) { + app := fiber.New() + tenantA := "tenant-a" + app.Use(func(c *fiber.Ctx) error { + c.Locals("user_profile", &domain.UserProfileResponse{ + ID: "user-a", + Role: domain.RoleSuperAdmin, // Bypass for creation permission + TenantID: &tenantA, + }) + return c.Next() + }) + app.Post("/api/v1/dev/clients", h.CreateClient) + + body, _ := json.Marshal(map[string]interface{}{ + "client_name": "New App", + "type": "pkce", + "redirectUris": []string{"http://localhost/cb"}, + }) + req := httptest.NewRequest(http.MethodPost, "/api/v1/dev/clients", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-Tenant-ID", "tenant-a") + + resp, _ := app.Test(req, -1) + assert.Equal(t, http.StatusCreated, resp.StatusCode) + + var res clientDetailResponse + json.NewDecoder(resp.Body).Decode(&res) + + assert.Equal(t, "tenant-a", res.Client.Metadata["tenant_id"]) + assert.Equal(t, "user-a", res.Client.Metadata["user_id"]) + }) +} diff --git a/backend/internal/handler/dev_handler_test.go b/backend/internal/handler/dev_handler_test.go index acbfcd1c..58ea45b1 100644 --- a/backend/internal/handler/dev_handler_test.go +++ b/backend/internal/handler/dev_handler_test.go @@ -227,3 +227,25 @@ func TestListAuditLogs_FilterByActionAndClientID(t *testing.T) { assert.Equal(t, "evt-1", res.Items[0].EventID) assert.Equal(t, "success", res.Items[0].Status) } + +func TestListAuditLogs_NonAdminKetoErrorReturnsForbidden(t *testing.T) { + mockKeto := new(MockKetoService) + mockKeto.On("CheckPermission", mock.Anything, "user-1", "System", "AppManager", "member").Return(false, assert.AnError) + + h := &DevHandler{ + AuditRepo: &mockAuditRepo{}, + Keto: mockKeto, + } + app := fiber.New() + app.Use(func(c *fiber.Ctx) error { + c.Locals("user_profile", &domain.UserProfileResponse{ID: "user-1", Role: domain.RoleUser}) + return c.Next() + }) + app.Get("/api/v1/dev/audit-logs", h.ListAuditLogs) + + req := httptest.NewRequest(http.MethodGet, "/api/v1/dev/audit-logs?limit=50", nil) + resp, _ := app.Test(req, -1) + + assert.Equal(t, http.StatusForbidden, resp.StatusCode) + mockKeto.AssertExpectations(t) +}