From 572ac39e609819e713c3a81ed952f5aa5c852863 Mon Sep 17 00:00:00 2001 From: kyy Date: Wed, 29 Apr 2026 14:12:37 +0900 Subject: [PATCH] =?UTF-8?q?RP=20=EC=83=9D=EC=84=B1=20admin=20=EA=B4=80?= =?UTF-8?q?=EA=B3=84=20=EC=A4=91=EB=B3=B5=20=EB=B6=80=EC=97=AC=20=EC=88=98?= =?UTF-8?q?=EC=A0=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- backend/internal/handler/dev_handler.go | 68 ++++++++--- backend/internal/handler/dev_handler_test.go | 112 +++++++++++++++++-- 2 files changed, 157 insertions(+), 23 deletions(-) diff --git a/backend/internal/handler/dev_handler.go b/backend/internal/handler/dev_handler.go index 507dacf9..5c0c96b2 100644 --- a/backend/internal/handler/dev_handler.go +++ b/backend/internal/handler/dev_handler.go @@ -752,6 +752,24 @@ func mapRelationTupleSummary(tuple service.RelationTuple, identity *service.Krat return summary } +func dedupeRelationTuples(tuples []service.RelationTuple) []service.RelationTuple { + if len(tuples) <= 1 { + return tuples + } + + seen := make(map[string]struct{}, len(tuples)) + deduped := make([]service.RelationTuple, 0, len(tuples)) + for _, tuple := range tuples { + key := strings.TrimSpace(tuple.Relation) + "\x00" + strings.TrimSpace(tuple.SubjectID) + if _, exists := seen[key]; exists { + continue + } + seen[key] = struct{}{} + deduped = append(deduped, tuple) + } + return deduped +} + func (h *DevHandler) loadClientSummary(ctx context.Context, clientID string) (clientSummary, error) { clientID = strings.TrimSpace(clientID) if clientID == "" { @@ -1210,6 +1228,7 @@ func (h *DevHandler) ListClientRelations(c *fiber.Ctx) error { if err != nil { return errorJSON(c, fiber.StatusInternalServerError, err.Error()) } + tuples = dedupeRelationTuples(tuples) for _, tuple := range tuples { var identity *service.KratosIdentity if tuple.SubjectID != "" && h.KratosAdmin != nil { @@ -1722,23 +1741,11 @@ func (h *DevHandler) CreateClient(c *fiber.Ctx) error { // [New] Automatically grant admin permission to the creator in Keto if h.KetoOutbox != nil && profile != nil { subject := "User:" + profile.ID - err := h.KetoOutbox.Create(c.Context(), &domain.KetoOutbox{ - Namespace: "RelyingParty", - Object: created.ClientID, - Relation: "admins", - Subject: subject, - Action: domain.KetoOutboxActionCreate, - }) - if err != nil { + if err := h.grantCreatorAdminRelation(c, created.ClientID, subject); err != nil { slog.Warn("failed to grant automatic admin permission to creator", "clientID", created.ClientID, "userID", profile.ID, "error", err) } else { slog.Info("granted automatic admin permission to creator", "clientID", created.ClientID, "userID", profile.ID) } - if h.Keto != nil { - if err := h.Keto.CreateRelation(c.Context(), "RelyingParty", created.ClientID, "admins", subject); err != nil { - slog.Warn("failed to grant immediate admin permission to creator", "clientID", created.ClientID, "userID", profile.ID, "error", err) - } - } } // Store secret in metadata for later retrieval @@ -3314,6 +3321,41 @@ func (h *DevHandler) ensureDeveloperGrantRelation(c *fiber.Ctx, userID, tenantID } } +func (h *DevHandler) grantCreatorAdminRelation(c *fiber.Ctx, clientID string, subject string) error { + clientID = strings.TrimSpace(clientID) + subject = strings.TrimSpace(subject) + if clientID == "" || subject == "" { + return nil + } + + if h.Keto != nil { + existing, err := h.Keto.ListRelations(c.Context(), "RelyingParty", clientID, "admins", subject) + if err == nil && len(existing) > 0 { + return nil + } + if err == nil { + if createErr := h.Keto.CreateRelation(c.Context(), "RelyingParty", clientID, "admins", subject); createErr == nil { + return nil + } else { + slog.Warn("failed to grant immediate admin permission to creator; falling back to outbox", "clientID", clientID, "subject", subject, "error", createErr) + } + } else { + slog.Warn("failed to check existing admin relation for creator; falling back to outbox", "clientID", clientID, "subject", subject, "error", err) + } + } + + if h.KetoOutbox == nil { + return nil + } + return h.KetoOutbox.Create(c.Context(), &domain.KetoOutbox{ + Namespace: "RelyingParty", + Object: clientID, + Relation: "admins", + Subject: subject, + Action: domain.KetoOutboxActionCreate, + }) +} + func (h *DevHandler) revokeDeveloperGrantRelation(c *fiber.Ctx, userID, tenantID string) { if h.KetoOutbox == nil || strings.TrimSpace(userID) == "" || strings.TrimSpace(tenantID) == "" { return diff --git a/backend/internal/handler/dev_handler_test.go b/backend/internal/handler/dev_handler_test.go index 4d630316..f04f05f4 100644 --- a/backend/internal/handler/dev_handler_test.go +++ b/backend/internal/handler/dev_handler_test.go @@ -1053,17 +1053,9 @@ func TestCreateClient_ApprovedDeveloperCanCreatePrivateClient(t *testing.T) { mockKeto := new(devMockKetoService) mockKeto.On("CheckPermission", mock.Anything, "User:user-1", "Tenant", "tenant-a", "grant_dev_permissions").Return(true, nil) + mockKeto.On("ListRelations", mock.Anything, "RelyingParty", "client-1", "admins", "User:user-1").Return([]service.RelationTuple{}, nil) mockKeto.On("CreateRelation", mock.Anything, "RelyingParty", "client-1", "admins", "User:user-1").Return(nil) - mockOutbox := new(devMockKetoOutboxRepository) - mockOutbox.On("Create", mock.Anything, mock.MatchedBy(func(entry *domain.KetoOutbox) bool { - return entry.Namespace == "RelyingParty" && - entry.Object == "client-1" && - entry.Relation == "admins" && - entry.Subject == "User:user-1" && - entry.Action == domain.KetoOutboxActionCreate - })).Return(nil) - h := &DevHandler{ Hydra: &service.HydraAdminService{ AdminURL: "http://hydra.test", @@ -1072,7 +1064,7 @@ func TestCreateClient_ApprovedDeveloperCanCreatePrivateClient(t *testing.T) { SecretRepo: &mockSecretRepo{secrets: make(map[string]string)}, Redis: &devMockRedisRepo{data: make(map[string]string)}, Keto: mockKeto, - KetoOutbox: mockOutbox, + KetoOutbox: new(devMockKetoOutboxRepository), } app := fiber.New() @@ -1099,6 +1091,37 @@ func TestCreateClient_ApprovedDeveloperCanCreatePrivateClient(t *testing.T) { resp, _ := app.Test(req, -1) assert.Equal(t, http.StatusCreated, resp.StatusCode) mockKeto.AssertExpectations(t) +} + +func TestGrantCreatorAdminRelation_FallsBackToOutboxOnImmediateFailure(t *testing.T) { + mockKeto := new(devMockKetoService) + mockKeto.On("ListRelations", mock.Anything, "RelyingParty", "client-1", "admins", "User:user-1").Return([]service.RelationTuple{}, nil).Once() + mockKeto.On("CreateRelation", mock.Anything, "RelyingParty", "client-1", "admins", "User:user-1").Return(assert.AnError).Once() + + mockOutbox := new(devMockKetoOutboxRepository) + mockOutbox.On("Create", mock.Anything, mock.MatchedBy(func(entry *domain.KetoOutbox) bool { + return entry.Namespace == "RelyingParty" && + entry.Object == "client-1" && + entry.Relation == "admins" && + entry.Subject == "User:user-1" && + entry.Action == domain.KetoOutboxActionCreate + })).Return(nil).Once() + + h := &DevHandler{ + Keto: mockKeto, + KetoOutbox: mockOutbox, + } + + app := fiber.New() + app.Get("/test", func(c *fiber.Ctx) error { + assert.NoError(t, h.grantCreatorAdminRelation(c, "client-1", "User:user-1")) + return c.SendStatus(fiber.StatusOK) + }) + + req := httptest.NewRequest(http.MethodGet, "/test", nil) + resp, _ := app.Test(req, -1) + assert.Equal(t, http.StatusOK, resp.StatusCode) + mockKeto.AssertExpectations(t) mockOutbox.AssertExpectations(t) } @@ -2487,6 +2510,75 @@ func TestListClientRelations_RPAdminAllowedByViewRelationshipsPermission(t *test assert.Equal(t, "kyy01", result.Items[0].UserLoginID) } +func TestListClientRelations_DedupesDuplicateRelations(t *testing.T) { + transport := roundTripFunc(func(r *http.Request) (*http.Response, error) { + if r.Method == http.MethodGet && r.URL.Path == "/clients/client-1" { + return httpJSONAny(r, http.StatusOK, map[string]any{ + "client_id": "client-1", + "client_name": "App One", + "metadata": map[string]any{ + "tenant_id": "tenant-1", + "status": "active", + }, + }), nil + } + return httpJSONAny(r, http.StatusNotFound, nil), nil + }) + + mockKeto := new(devMockKetoService) + mockKeto.On("CheckPermission", mock.Anything, "User:user-1", "RelyingParty", "client-1", "view_relationships").Return(true, nil) + mockKeto.On("ListRelations", mock.Anything, "RelyingParty", "client-1", "admins", "").Return([]service.RelationTuple{ + {Namespace: "RelyingParty", Object: "client-1", Relation: "admins", SubjectID: "User:user-1"}, + {Namespace: "RelyingParty", Object: "client-1", Relation: "admins", SubjectID: "User:user-1"}, + }, nil) + for _, relation := range []string{"creator", "config_editor", "secret_viewer", "secret_rotator", "jwks_viewer", "jwks_operator", "consent_viewer", "consent_revoker", "relationship_viewer", "audit_viewer", "status_operator"} { + mockKeto.On("ListRelations", mock.Anything, "RelyingParty", "client-1", relation, "").Return([]service.RelationTuple{}, nil) + } + + mockKratos := new(devMockKratosAdmin) + mockKratos.On("GetIdentity", mock.Anything, "user-1").Return(&service.KratosIdentity{ + ID: "user-1", + Traits: map[string]interface{}{ + "name": "Tester", + "email": "tester@example.com", + }, + }, nil).Once() + + h := &DevHandler{ + Hydra: &service.HydraAdminService{ + AdminURL: "http://hydra.test", + HTTPClient: &http.Client{Transport: transport}, + }, + Keto: mockKeto, + KratosAdmin: mockKratos, + } + + app := fiber.New() + app.Use(func(c *fiber.Ctx) error { + tenantID := "tenant-1" + c.Locals("user_profile", &domain.UserProfileResponse{ + ID: "user-1", + Role: domain.RoleRPAdmin, + TenantID: &tenantID, + }) + return c.Next() + }) + app.Get("/api/v1/dev/clients/:id/relations", h.ListClientRelations) + + req := httptest.NewRequest(http.MethodGet, "/api/v1/dev/clients/client-1/relations", nil) + resp, _ := app.Test(req, -1) + + assert.Equal(t, http.StatusOK, resp.StatusCode) + var result clientRelationListResponse + _ = json.NewDecoder(resp.Body).Decode(&result) + if assert.Len(t, result.Items, 1) { + assert.Equal(t, "admins", result.Items[0].Relation) + assert.Equal(t, "User:user-1", result.Items[0].Subject) + } + mockKeto.AssertExpectations(t) + mockKratos.AssertExpectations(t) +} + func TestAddClientRelation_RPAdminAllowedByTenantGrantPermission(t *testing.T) { transport := roundTripFunc(func(r *http.Request) (*http.Response, error) { if r.Method == http.MethodGet && r.URL.Path == "/clients/client-1" {