goclaw/internal/gateway/methods/api_keys_tenant_guard_test.go

package methods

import (
	"context"
	"encoding/json"
	"errors"
	"slices"
	"sync"
	"testing"

	"github.com/google/uuid"

	"github.com/nextlevelbuilder/goclaw/internal/gateway"
	"github.com/nextlevelbuilder/goclaw/internal/permissions"
	"github.com/nextlevelbuilder/goclaw/internal/store"
	"github.com/nextlevelbuilder/goclaw/pkg/protocol"
)

// Phase 0b review regression coverage: WS api_keys.revoke.
//
// The HTTP fix in internal/http/api_keys.go has its own test file, but the
// original Phase 0b WS fix (internal/gateway/methods/api_keys.go) had zero
// test coverage — and a Major bug slipped past: store.IsOwnerRole(ctx)
// always returns false in WS handlers because the router does not inject
// role into ctx. The bug was caught by code review, not tests. These tests
// lock the fix in place and cover the four critical paths.

// ---- minimal stub store ----

type stubAPIKeyStore struct {
	mu           sync.Mutex
	byID         map[uuid.UUID]*store.APIKeyData
	revokedCalls []uuid.UUID
}

func newStubAPIKeyStore() *stubAPIKeyStore {
	return &stubAPIKeyStore{byID: make(map[uuid.UUID]*store.APIKeyData)}
}

func (s *stubAPIKeyStore) Create(_ context.Context, k *store.APIKeyData) error {
	s.mu.Lock()
	defer s.mu.Unlock()
	s.byID[k.ID] = k
	return nil
}

func (s *stubAPIKeyStore) Get(_ context.Context, id uuid.UUID) (*store.APIKeyData, error) {
	s.mu.Lock()
	defer s.mu.Unlock()
	if k, ok := s.byID[id]; ok {
		return k, nil
	}
	return nil, errors.New("not found")
}

func (s *stubAPIKeyStore) GetByHash(_ context.Context, _ string) (*store.APIKeyData, error) {
	return nil, nil
}

func (s *stubAPIKeyStore) List(_ context.Context, _ string) ([]store.APIKeyData, error) {
	return nil, nil
}

func (s *stubAPIKeyStore) Revoke(_ context.Context, id uuid.UUID, _ string) error {
	s.mu.Lock()
	defer s.mu.Unlock()
	s.revokedCalls = append(s.revokedCalls, id)
	if k, ok := s.byID[id]; ok {
		k.Revoked = true
	}
	return nil
}

func (s *stubAPIKeyStore) TouchLastUsed(_ context.Context, _ uuid.UUID) error { return nil }

func (s *stubAPIKeyStore) wasRevoked(id uuid.UUID) bool {
	s.mu.Lock()
	defer s.mu.Unlock()
	return slices.Contains(s.revokedCalls, id)
}

// wsCallCtx mirrors what router.handleRequest injects into ctx for method
// handlers: locale, tenantID (from client.TenantID()), and role (added
// by Phase 0b hardening). Tests bypass the router, so we reproduce the
// minimum set the handler reads.
func wsCallCtx(client *gateway.Client) context.Context {
	ctx := context.Background()
	if tid := client.TenantID(); tid != uuid.Nil {
		ctx = store.WithTenantID(ctx, tid)
	}
	if role := client.Role(); role != "" {
		ctx = store.WithRole(ctx, string(role))
	}
	return ctx
}

// ---- harness ----

func buildRevokeRequest(t *testing.T, keyID uuid.UUID) *protocol.RequestFrame {
	t.Helper()
	raw, err := json.Marshal(map[string]string{"id": keyID.String()})
	if err != nil {
		t.Fatalf("marshal: %v", err)
	}
	return &protocol.RequestFrame{
		Type:   protocol.FrameTypeRequest,
		ID:     "revoke-req-1",
		Method: protocol.MethodAPIKeysRevoke,
		Params: raw,
	}
}

// ---- tests ----

// TestWSRevoke_TenantAdmin_CrossTenantKey_Denied is the core HIGH finding:
// a tenant admin attempting to revoke a key owned by another tenant must
// be rejected, and Revoke must not be called on the store.
func TestWSRevoke_TenantAdmin_CrossTenantKey_Denied(t *testing.T) {
	callerTID := uuid.MustParse("11111111-1111-1111-1111-111111111111")
	keyOwnerTID := uuid.MustParse("22222222-2222-2222-2222-222222222222")
	keyID := uuid.New()

	stub := newStubAPIKeyStore()
	_ = stub.Create(context.Background(), &store.APIKeyData{
		ID:       keyID,
		Name:     "other-tenant-key",
		TenantID: keyOwnerTID,
	})

	m := &APIKeysMethods{apiKeys: stub}
	client := gateway.NewTestClient(permissions.RoleAdmin, callerTID, "user-1")

	m.handleRevoke(wsCallCtx(client), client, buildRevokeRequest(t, keyID))

	if stub.wasRevoked(keyID) {
		t.Fatalf("cross-tenant key should NOT have been revoked")
	}
}

// TestWSRevoke_TenantAdmin_SystemKey_Denied is the second HIGH path: the
// NULL-tenant (system) key must not be revocable by a tenant admin.
func TestWSRevoke_TenantAdmin_SystemKey_Denied(t *testing.T) {
	callerTID := uuid.MustParse("33333333-3333-3333-3333-333333333333")
	keyID := uuid.New()

	stub := newStubAPIKeyStore()
	_ = stub.Create(context.Background(), &store.APIKeyData{
		ID:       keyID,
		Name:     "system-ci-key",
		TenantID: uuid.Nil, // NULL-tenant system key
	})

	m := &APIKeysMethods{apiKeys: stub}
	client := gateway.NewTestClient(permissions.RoleAdmin, callerTID, "user-1")

	m.handleRevoke(wsCallCtx(client), client, buildRevokeRequest(t, keyID))

	if stub.wasRevoked(keyID) {
		t.Fatalf("system (NULL-tenant) key should NOT have been revoked by tenant admin")
	}
}

// TestWSRevoke_SystemOwner_SystemKey_Allowed is the regression caught by
// code review: the original Phase 0b fix used store.IsOwnerRole(ctx),
// which returns false in WS because the router does not inject role into
// ctx. This test exercises the now-correct client.IsOwner() path.
func TestWSRevoke_SystemOwner_SystemKey_Allowed(t *testing.T) {
	keyID := uuid.New()

	stub := newStubAPIKeyStore()
	_ = stub.Create(context.Background(), &store.APIKeyData{
		ID:       keyID,
		Name:     "system-ci-key",
		TenantID: uuid.Nil,
	})

	m := &APIKeysMethods{apiKeys: stub}
	// Owner client — narrowed-to-master tenant scope is the default for
	// owner logins per router.applyTenantScope.
	client := gateway.NewTestClient(permissions.RoleOwner, store.MasterTenantID, "owner-1")

	m.handleRevoke(wsCallCtx(client), client, buildRevokeRequest(t, keyID))

	if !stub.wasRevoked(keyID) {
		t.Fatalf("system owner must be allowed to revoke NULL-tenant system key")
	}
}

// TestWSRevoke_TenantAdmin_OwnTenantKey_Allowed is the happy path: admin
// revoking a key inside their own tenant — no regression for legitimate
// scope-matching revocations.
func TestWSRevoke_TenantAdmin_OwnTenantKey_Allowed(t *testing.T) {
	tid := uuid.MustParse("44444444-4444-4444-4444-444444444444")
	keyID := uuid.New()

	stub := newStubAPIKeyStore()
	_ = stub.Create(context.Background(), &store.APIKeyData{
		ID:       keyID,
		Name:     "own-tenant-key",
		TenantID: tid,
	})

	m := &APIKeysMethods{apiKeys: stub}
	client := gateway.NewTestClient(permissions.RoleAdmin, tid, "user-1")

	m.handleRevoke(wsCallCtx(client), client, buildRevokeRequest(t, keyID))

	if !stub.wasRevoked(keyID) {
		t.Fatalf("own-tenant key must be revocable by same-tenant admin")
	}
}

// TestWSRevoke_SystemOwner_CrossTenantKey_Allowed documents the intentional
// behaviour: a system owner narrowed to the master scope can still revoke
// keys belonging to other tenants (operator-level housekeeping). If this
// is ever tightened, update both the test and the handler comment.
func TestWSRevoke_SystemOwner_CrossTenantKey_Allowed(t *testing.T) {
	otherTID := uuid.MustParse("55555555-5555-5555-5555-555555555555")
	keyID := uuid.New()

	stub := newStubAPIKeyStore()
	_ = stub.Create(context.Background(), &store.APIKeyData{
		ID:       keyID,
		Name:     "tenant-specific-key",
		TenantID: otherTID,
	})

	m := &APIKeysMethods{apiKeys: stub}
	client := gateway.NewTestClient(permissions.RoleOwner, store.MasterTenantID, "owner-1")

	m.handleRevoke(wsCallCtx(client), client, buildRevokeRequest(t, keyID))

	if !stub.wasRevoked(keyID) {
		t.Fatalf("system owner must be allowed to revoke cross-tenant keys")
	}
}