fix(bitrix24): register newly-created portal in live router [B24:2794] (#1404)

bitrix.portals.create only persisted the DB row — it never called
Router.RegisterPortal, so a portal created while the gateway was
already running stayed invisible to PortalByDomain/PortalByKey until
the next full restart. The only other RegisterPortal call site is
BootstrapPortals, which runs once at boot. Result: the OAuth install
callback 404'd with "unknown portal" for any portal created through
the self-service UI mid-uptime — hit live on web1trang.bitrix24.com.

Fix: inject registerPortal/unregisterPortal func fields into
BitrixPortalsMethods (mirrors the existing gatewayPublicURL func()
pattern), defaulting to bitrix24.WebhookRouter() + NewPortal/
RegisterPortal (same construction path as BootstrapPortals) and
Router.UnregisterPortal respectively. handleCreate calls
registerPortal after a successful Create; handleDelete calls
unregisterPortal after a successful Delete for symmetry. Both are
best-effort — a registration failure is logged, not returned as an
RPC error, since the DB row is already valid either way.

Injected via func fields (not calling bitrix24.WebhookRouter()
inline) so tests can stub live-router registration without touching
that process-wide singleton, whose test-reset helper isn't exported
outside the bitrix24 package.

Tests: 4 new cases covering register-on-create, best-effort failure
handling, unregister-on-delete, and no-unregister-when-delete-blocked.
All 20 tests in bitrix_portals_test.go pass; go build (both PG and
sqliteonly tags) and go vet clean.

Co-authored-by: DangTinh311 <dangtinh31193@gmail.com>
This commit is contained in:
SYNITYandDangTinh311 authored and GitHub committed 2026-07-09 10:54:56 +07:00
1 parent 47068ad563
commit 51ed7845e2
3 files changed
+206 -17

No files matched your search

+1
View File
@@ -788,6 +788,7 @@ func runGateway() {
pgStores.BitrixPortals,
pgStores.ChannelInstances,
server.PublicURLSnapshot().Get,
bitrixEncKey,
).Register(server.Router())
}
+53 -2
View File
@@ -13,6 +13,7 @@ import (
"github.com/google/uuid"
"github.com/nextlevelbuilder/goclaw/internal/channels/bitrix24"
"github.com/nextlevelbuilder/goclaw/internal/gateway"
"github.com/nextlevelbuilder/goclaw/internal/i18n"
"github.com/nextlevelbuilder/goclaw/internal/permissions"
@@ -35,22 +36,57 @@ type BitrixPortalsMethods struct {
portalStore store.BitrixPortalStore
channelStore store.ChannelInstanceStore // for "portal_in_use" check on delete
gatewayPublicURL func() string
encKey string
// registerPortal/unregisterPortal keep the process-wide bitrix24.Router
// (webhook install lookups) in sync with rows created/deleted through
// this RPC, without which a newly created portal is invisible to
// PortalByDomain/PortalByKey until the next full gateway restart — the
// only other place anything calls Router.RegisterPortal is
// bitrix24.BootstrapPortals, which only runs once at boot. Injected
// (rather than calling bitrix24.WebhookRouter() inline) so tests can
// stub live-router registration without touching that process-wide
// singleton. Defaults set in NewBitrixPortalsMethods.
registerPortal func(ctx context.Context, tenantID uuid.UUID, name string) error
unregisterPortal func(tenantID uuid.UUID, name string)
}
// NewBitrixPortalsMethods constructs the handler. gatewayPublicURL may return
// empty string when the gateway hasn't observed any public-URL request yet;
// callers see an INVALID_REQUEST error with a hint to open the UI via the
// public URL first.
// public URL first. encKey mirrors the one used by pg.NewPGBitrixPortalStore /
// bitrix24.BootstrapPortals — needed to construct a *bitrix24.Portal for live
// router registration.
func NewBitrixPortalsMethods(
portalStore store.BitrixPortalStore,
channelStore store.ChannelInstanceStore,
gatewayPublicURL func() string,
encKey string,
) *BitrixPortalsMethods {
return &BitrixPortalsMethods{
m := &BitrixPortalsMethods{
portalStore: portalStore,
channelStore: channelStore,
gatewayPublicURL: gatewayPublicURL,
encKey: encKey,
}
m.registerPortal = func(ctx context.Context, tenantID uuid.UUID, name string) error {
router := bitrix24.WebhookRouter()
if router == nil {
return errors.New("bitrix24 webhook router not initialized")
}
p, err := bitrix24.NewPortal(ctx, tenantID, name, m.portalStore, m.encKey)
if err != nil {
return err
}
router.RegisterPortal(p)
return nil
}
m.unregisterPortal = func(tenantID uuid.UUID, name string) {
if router := bitrix24.WebhookRouter(); router != nil {
router.UnregisterPortal(tenantID, name)
}
}
return m
}
func (m *BitrixPortalsMethods) Register(router *gateway.MethodRouter) {
@@ -207,6 +243,17 @@ func (m *BitrixPortalsMethods) handleCreate(ctx context.Context, client *gateway
}
slog.Info("bitrix.portals.create", "tenant", tid, "name", name, "domain", domain)
// Best-effort: make the portal immediately installable. Failure here
// (router not initialized yet, decode error) is non-fatal — the row is
// already persisted and BootstrapPortals picks it up on the next
// restart — but it means the admin can't complete OAuth until then, so
// log loudly rather than silently.
if err := m.registerPortal(ctx, tid, name); err != nil {
slog.Warn("bitrix.portals.create: live router registration failed — portal unreachable by webhook install until gateway restart",
"tenant", tid, "name", name, "error", err)
}
client.SendResponse(protocol.NewOKResponse(req.ID, map[string]any{
"name": row.Name,
"domain": row.Domain,
@@ -300,6 +347,10 @@ func (m *BitrixPortalsMethods) handleDelete(ctx context.Context, client *gateway
client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInternal, i18n.T(locale, i18n.MsgFailedToDelete, "portal", err.Error())))
return
}
// Symmetric with registerPortal in handleCreate — otherwise a portal
// registered into the live router (at boot, or by a prior create) stays
// routable in-memory after its DB row is gone until the next restart.
m.unregisterPortal(tid, name)
slog.Info("bitrix.portals.delete", "tenant", tid, "name", name)
client.SendResponse(protocol.NewOKResponse(req.ID, map[string]any{"status": "deleted"}))
}
+152 -15
View File
@@ -217,7 +217,7 @@ func TestBitrixPortals_List_TenantIsolation(t *testing.T) {
TenantID: tidB, Name: "beta", Domain: "beta.bitrix24.com",
})
m := NewBitrixPortalsMethods(pStore, newStubChannelInstanceStore(), gatewayURLFn("https://gw.example.com"))
m := NewBitrixPortalsMethods(pStore, newStubChannelInstanceStore(), gatewayURLFn("https://gw.example.com"), "test-enc-key")
// Tenant A list should NOT see tenant B's portal.
client, ch := gateway.NewCapturingTestClient(permissions.RoleOperator, tidA, "user-A", 4)
@@ -254,7 +254,7 @@ func TestBitrixPortals_List_MasksCredentials(t *testing.T) {
Credentials: credsJSON,
})
m := NewBitrixPortalsMethods(pStore, newStubChannelInstanceStore(), gatewayURLFn("https://gw.example.com"))
m := NewBitrixPortalsMethods(pStore, newStubChannelInstanceStore(), gatewayURLFn("https://gw.example.com"), "test-enc-key")
client, ch := gateway.NewCapturingTestClient(permissions.RoleAdmin, tid, "u", 4)
m.handleList(store.WithTenantID(context.Background(), tid), client, buildBitrixReq(t, protocol.MethodBitrixPortalsList, nil))
@@ -267,7 +267,7 @@ func TestBitrixPortals_List_MasksCredentials(t *testing.T) {
}
func TestBitrixPortals_List_RejectsMissingTenant(t *testing.T) {
m := NewBitrixPortalsMethods(newStubBitrixPortalStore(), newStubChannelInstanceStore(), gatewayURLFn(""))
m := NewBitrixPortalsMethods(newStubBitrixPortalStore(), newStubChannelInstanceStore(), gatewayURLFn(""), "test-enc-key")
client, ch := gateway.NewCapturingTestClient(permissions.RoleAdmin, uuid.Nil, "u", 4)
m.handleList(context.Background(), client, buildBitrixReq(t, protocol.MethodBitrixPortalsList, nil))
@@ -288,7 +288,7 @@ func TestBitrixPortals_List_SurfacesInstalledFromState(t *testing.T) {
TenantID: tid, Name: "p", Domain: "p.bitrix24.com", State: state,
})
m := NewBitrixPortalsMethods(pStore, newStubChannelInstanceStore(), gatewayURLFn("https://gw.example.com"))
m := NewBitrixPortalsMethods(pStore, newStubChannelInstanceStore(), gatewayURLFn("https://gw.example.com"), "test-enc-key")
client, ch := gateway.NewCapturingTestClient(permissions.RoleAdmin, tid, "u", 4)
m.handleList(store.WithTenantID(context.Background(), tid), client, buildBitrixReq(t, protocol.MethodBitrixPortalsList, nil))
@@ -310,7 +310,7 @@ func TestBitrixPortals_List_SurfacesInstalledFromState(t *testing.T) {
func TestBitrixPortals_Create_RBAC_OperatorDenied(t *testing.T) {
tid := uuid.New()
pStore := newStubBitrixPortalStore()
m := NewBitrixPortalsMethods(pStore, newStubChannelInstanceStore(), gatewayURLFn("https://gw.example.com"))
m := NewBitrixPortalsMethods(pStore, newStubChannelInstanceStore(), gatewayURLFn("https://gw.example.com"), "test-enc-key")
client, ch := gateway.NewCapturingTestClient(permissions.RoleOperator, tid, "u", 4)
m.handleCreate(store.WithTenantID(context.Background(), tid), client, buildBitrixReq(t, protocol.MethodBitrixPortalsCreate, map[string]string{
@@ -329,7 +329,7 @@ func TestBitrixPortals_Create_RBAC_OperatorDenied(t *testing.T) {
func TestBitrixPortals_Create_HappyPath_ReturnsInstallURL(t *testing.T) {
tid := uuid.MustParse("11111111-1111-1111-1111-111111111111")
pStore := newStubBitrixPortalStore()
m := NewBitrixPortalsMethods(pStore, newStubChannelInstanceStore(), gatewayURLFn("https://goclaw.tamgiac.com"))
m := NewBitrixPortalsMethods(pStore, newStubChannelInstanceStore(), gatewayURLFn("https://goclaw.tamgiac.com"), "test-enc-key")
client, ch := gateway.NewCapturingTestClient(permissions.RoleAdmin, tid, "admin", 4)
m.handleCreate(store.WithTenantID(context.Background(), tid), client, buildBitrixReq(t, protocol.MethodBitrixPortalsCreate, map[string]string{
@@ -364,9 +364,81 @@ func TestBitrixPortals_Create_HappyPath_ReturnsInstallURL(t *testing.T) {
}
}
// TestBitrixPortals_Create_RegistersInLiveRouter guards the fix for a real
// production bug: handleCreate used to only persist the DB row, leaving the
// process-wide bitrix24.Router unaware of the new portal until the next
// gateway restart (the only other RegisterPortal call site is
// bitrix24.BootstrapPortals, which runs once at boot) — so a freshly created
// portal's install callback always 404'd with "unknown portal" until an
// operator restarted the gateway.
func TestBitrixPortals_Create_RegistersInLiveRouter(t *testing.T) {
tid := uuid.New()
pStore := newStubBitrixPortalStore()
m := NewBitrixPortalsMethods(pStore, newStubChannelInstanceStore(), gatewayURLFn("https://gw.example.com"), "test-enc-key")
var gotTenant uuid.UUID
var gotName string
calls := 0
m.registerPortal = func(_ context.Context, tenantID uuid.UUID, name string) error {
calls++
gotTenant = tenantID
gotName = name
return nil
}
client, ch := gateway.NewCapturingTestClient(permissions.RoleAdmin, tid, "admin", 4)
m.handleCreate(store.WithTenantID(context.Background(), tid), client, buildBitrixReq(t, protocol.MethodBitrixPortalsCreate, map[string]string{
"name": "myportal",
"domain": "myportal.bitrix24.com",
"client_id": "local.abc",
"client_secret": "secret123",
}))
resp := readResponse(t, ch)
if resp.Error != nil {
t.Fatalf("create failed: %+v", resp.Error)
}
if calls != 1 {
t.Fatalf("registerPortal called %d times, want 1", calls)
}
if gotTenant != tid || gotName != "myportal" {
t.Errorf("registerPortal called with (%s, %q), want (%s, %q)", gotTenant, gotName, tid, "myportal")
}
}
// TestBitrixPortals_Create_RegisterFailure_StillReturnsInstallURL confirms the
// registerPortal call is best-effort: a failure (e.g. router not initialized)
// must not turn a successfully persisted create into an error response — the
// row is real, the admin just needs a gateway restart to install it (logged
// as a warning, not returned to the caller).
func TestBitrixPortals_Create_RegisterFailure_StillReturnsInstallURL(t *testing.T) {
tid := uuid.New()
pStore := newStubBitrixPortalStore()
m := NewBitrixPortalsMethods(pStore, newStubChannelInstanceStore(), gatewayURLFn("https://gw.example.com"), "test-enc-key")
m.registerPortal = func(_ context.Context, _ uuid.UUID, _ string) error {
return errors.New("router not initialized")
}
client, ch := gateway.NewCapturingTestClient(permissions.RoleAdmin, tid, "admin", 4)
m.handleCreate(store.WithTenantID(context.Background(), tid), client, buildBitrixReq(t, protocol.MethodBitrixPortalsCreate, map[string]string{
"name": "myportal",
"domain": "myportal.bitrix24.com",
"client_id": "local.abc",
"client_secret": "secret123",
}))
resp := readResponse(t, ch)
if resp.Error != nil {
t.Fatalf("create should still succeed when registerPortal fails, got: %+v", resp.Error)
}
if _, err := pStore.GetByName(context.Background(), tid, "myportal"); err != nil {
t.Fatalf("row should still be persisted: %v", err)
}
}
func TestBitrixPortals_Create_InvalidDomain(t *testing.T) {
tid := uuid.New()
m := NewBitrixPortalsMethods(newStubBitrixPortalStore(), newStubChannelInstanceStore(), gatewayURLFn("https://gw.example.com"))
m := NewBitrixPortalsMethods(newStubBitrixPortalStore(), newStubChannelInstanceStore(), gatewayURLFn("https://gw.example.com"), "test-enc-key")
cases := []struct {
name string
@@ -403,7 +475,7 @@ func TestBitrixPortals_Create_InvalidDomain(t *testing.T) {
func TestBitrixPortals_Create_SelfHostedDomain(t *testing.T) {
tid := uuid.MustParse("22222222-2222-2222-2222-222222222222")
pStore := newStubBitrixPortalStore()
m := NewBitrixPortalsMethods(pStore, newStubChannelInstanceStore(), gatewayURLFn("https://goclaw.tamgiac.com"))
m := NewBitrixPortalsMethods(pStore, newStubChannelInstanceStore(), gatewayURLFn("https://goclaw.tamgiac.com"), "test-enc-key")
// Use a cloud domain (bitrixCloudDomainRegex) for the happy-path test
// since it bypasses SSRF DNS validation. Self-hosted SSRF validation
@@ -428,7 +500,7 @@ func TestBitrixPortals_Create_SelfHostedDomain(t *testing.T) {
func TestBitrixPortals_Create_InvalidName(t *testing.T) {
tid := uuid.New()
m := NewBitrixPortalsMethods(newStubBitrixPortalStore(), newStubChannelInstanceStore(), gatewayURLFn("https://gw.example.com"))
m := NewBitrixPortalsMethods(newStubBitrixPortalStore(), newStubChannelInstanceStore(), gatewayURLFn("https://gw.example.com"), "test-enc-key")
client, ch := gateway.NewCapturingTestClient(permissions.RoleAdmin, tid, "u", 4)
// Name with uppercase + special char → rejected.
m.handleCreate(store.WithTenantID(context.Background(), tid), client, buildBitrixReq(t, protocol.MethodBitrixPortalsCreate, map[string]string{
@@ -450,7 +522,7 @@ func TestBitrixPortals_Create_DuplicateReturnsAlreadyExists(t *testing.T) {
TenantID: tid, Name: "dup", Domain: "dup.bitrix24.com",
})
m := NewBitrixPortalsMethods(pStore, newStubChannelInstanceStore(), gatewayURLFn("https://gw.example.com"))
m := NewBitrixPortalsMethods(pStore, newStubChannelInstanceStore(), gatewayURLFn("https://gw.example.com"), "test-enc-key")
client, ch := gateway.NewCapturingTestClient(permissions.RoleAdmin, tid, "u", 4)
m.handleCreate(store.WithTenantID(context.Background(), tid), client, buildBitrixReq(t, protocol.MethodBitrixPortalsCreate, map[string]string{
"name": "dup",
@@ -470,7 +542,7 @@ func TestBitrixPortals_Create_DuplicateReturnsAlreadyExists(t *testing.T) {
func TestBitrixPortals_Create_GatewayURLUnknown_RejectsBeforePersist(t *testing.T) {
tid := uuid.New()
pStore := newStubBitrixPortalStore()
m := NewBitrixPortalsMethods(pStore, newStubChannelInstanceStore(), gatewayURLFn("")) // empty
m := NewBitrixPortalsMethods(pStore, newStubChannelInstanceStore(), gatewayURLFn(""), "test-enc-key") // empty
client, ch := gateway.NewCapturingTestClient(permissions.RoleAdmin, tid, "admin", 4)
m.handleCreate(store.WithTenantID(context.Background(), tid), client, buildBitrixReq(t, protocol.MethodBitrixPortalsCreate, map[string]string{
@@ -499,7 +571,7 @@ func TestBitrixPortals_GetInstallURL_TenantIsolation(t *testing.T) {
TenantID: tidB, Name: "secret", Domain: "secret.bitrix24.com",
})
m := NewBitrixPortalsMethods(pStore, newStubChannelInstanceStore(), gatewayURLFn("https://gw.example.com"))
m := NewBitrixPortalsMethods(pStore, newStubChannelInstanceStore(), gatewayURLFn("https://gw.example.com"), "test-enc-key")
// Tenant A asks for tenant B's portal → NOT_FOUND (not unauthorized — we
// don't want to leak existence of cross-tenant names).
client, ch := gateway.NewCapturingTestClient(permissions.RoleAdmin, tidA, "u", 4)
@@ -526,7 +598,7 @@ func TestBitrixPortals_Delete_BlockedByActiveChannel(t *testing.T) {
{Name: "support-bot", ChannelType: "bitrix24", Config: cfg},
}
m := NewBitrixPortalsMethods(pStore, chStore, gatewayURLFn("https://gw.example.com"))
m := NewBitrixPortalsMethods(pStore, chStore, gatewayURLFn("https://gw.example.com"), "test-enc-key")
client, ch := gateway.NewCapturingTestClient(permissions.RoleAdmin, tid, "u", 4)
m.handleDelete(store.WithTenantID(context.Background(), tid), client, buildBitrixReq(t, protocol.MethodBitrixPortalsDelete, map[string]string{"name": "p"}))
@@ -548,7 +620,7 @@ func TestBitrixPortals_Delete_HappyPath_RemovesRow(t *testing.T) {
pStore := newStubBitrixPortalStore()
_ = pStore.Create(context.Background(), &store.BitrixPortalData{TenantID: tid, Name: "orphan"})
m := NewBitrixPortalsMethods(pStore, newStubChannelInstanceStore(), gatewayURLFn("https://gw.example.com"))
m := NewBitrixPortalsMethods(pStore, newStubChannelInstanceStore(), gatewayURLFn("https://gw.example.com"), "test-enc-key")
client, ch := gateway.NewCapturingTestClient(permissions.RoleAdmin, tid, "u", 4)
m.handleDelete(store.WithTenantID(context.Background(), tid), client, buildBitrixReq(t, protocol.MethodBitrixPortalsDelete, map[string]string{"name": "orphan"}))
@@ -561,12 +633,77 @@ func TestBitrixPortals_Delete_HappyPath_RemovesRow(t *testing.T) {
}
}
// TestBitrixPortals_Delete_UnregistersFromLiveRouter is the symmetric
// counterpart to TestBitrixPortals_Create_RegistersInLiveRouter — without it,
// a portal registered in the live router (at boot, or by a prior create)
// stays routable in-memory after its DB row is deleted, until the next
// gateway restart.
func TestBitrixPortals_Delete_UnregistersFromLiveRouter(t *testing.T) {
tid := uuid.New()
pStore := newStubBitrixPortalStore()
_ = pStore.Create(context.Background(), &store.BitrixPortalData{TenantID: tid, Name: "orphan"})
m := NewBitrixPortalsMethods(pStore, newStubChannelInstanceStore(), gatewayURLFn("https://gw.example.com"), "test-enc-key")
var gotTenant uuid.UUID
var gotName string
calls := 0
m.unregisterPortal = func(tenantID uuid.UUID, name string) {
calls++
gotTenant = tenantID
gotName = name
}
client, ch := gateway.NewCapturingTestClient(permissions.RoleAdmin, tid, "u", 4)
m.handleDelete(store.WithTenantID(context.Background(), tid), client, buildBitrixReq(t, protocol.MethodBitrixPortalsDelete, map[string]string{"name": "orphan"}))
resp := readResponse(t, ch)
if resp.Error != nil {
t.Fatalf("delete failed: %+v", resp.Error)
}
if calls != 1 {
t.Fatalf("unregisterPortal called %d times, want 1", calls)
}
if gotTenant != tid || gotName != "orphan" {
t.Errorf("unregisterPortal called with (%s, %q), want (%s, %q)", gotTenant, gotName, tid, "orphan")
}
}
// TestBitrixPortals_Delete_BlockedByActiveChannel_DoesNotUnregister confirms
// unregisterPortal only fires after a successful delete — a blocked delete
// (portal still in use) must leave the live router untouched.
func TestBitrixPortals_Delete_BlockedByActiveChannel_DoesNotUnregister(t *testing.T) {
tid := uuid.New()
pStore := newStubBitrixPortalStore()
_ = pStore.Create(context.Background(), &store.BitrixPortalData{TenantID: tid, Name: "p"})
chStore := newStubChannelInstanceStore()
cfg, _ := json.Marshal(map[string]string{"portal": "p"})
chStore.instances = []store.ChannelInstanceData{
{Name: "in-use-channel", ChannelType: "bitrix24", Config: cfg},
}
m := NewBitrixPortalsMethods(pStore, chStore, gatewayURLFn("https://gw.example.com"), "test-enc-key")
calls := 0
m.unregisterPortal = func(_ uuid.UUID, _ string) { calls++ }
client, ch := gateway.NewCapturingTestClient(permissions.RoleAdmin, tid, "u", 4)
m.handleDelete(store.WithTenantID(context.Background(), tid), client, buildBitrixReq(t, protocol.MethodBitrixPortalsDelete, map[string]string{"name": "p"}))
resp := readResponse(t, ch)
if resp.Error == nil {
t.Fatal("expected delete to be blocked")
}
if calls != 0 {
t.Errorf("unregisterPortal called %d times, want 0 (delete was blocked)", calls)
}
}
func TestBitrixPortals_Delete_RBAC_OperatorDenied(t *testing.T) {
tid := uuid.New()
pStore := newStubBitrixPortalStore()
_ = pStore.Create(context.Background(), &store.BitrixPortalData{TenantID: tid, Name: "p"})
m := NewBitrixPortalsMethods(pStore, newStubChannelInstanceStore(), gatewayURLFn("https://gw.example.com"))
m := NewBitrixPortalsMethods(pStore, newStubChannelInstanceStore(), gatewayURLFn("https://gw.example.com"), "test-enc-key")
client, ch := gateway.NewCapturingTestClient(permissions.RoleOperator, tid, "u", 4)
m.handleDelete(store.WithTenantID(context.Background(), tid), client, buildBitrixReq(t, protocol.MethodBitrixPortalsDelete, map[string]string{"name": "p"}))