From 88f4f9ed2713b8f27bcd5cd0fe76d82de102aea5 Mon Sep 17 00:00:00 2001 From: Rashmika998 Date: Wed, 5 Aug 2026 23:18:23 +0530 Subject: [PATCH 1/6] fix(entity-service): accept both case and default_case for case type GET /projects/{id}/cases/stats validated caseTypes as UUIDs and converted them via uuidsToSysids -- a straight bug, since caseTypes is this service's own domain vocabulary (case/service_request/security_report_analysis/ announcement/engagement), not an ID of anything. Fixed to validate and translate the same way case search's type filter already does. Separately, "default_case" -- ServiceNow's own raw caseType wire value, and the value the production customer-portal frontend actually sends -- was being rejected outright: this service's own domain vocabulary uses "case" instead (tied to the Postgres case_type_enum), an inconsistency versus every other case type value, which maps 1:1 between the domain and SN layers. Added "default_case" as an explicit input alias, normalized to "case" before validation/translation/the Postgres repository ever see it, at every entry point that accepts a case type: case search filters, case creation (both data sources), and the case-stats endpoint. Also fixed validCaseType and the case-search group-by bucket list, both of which were missing "announcement" despite the Postgres enum and openapi.yaml already documenting it as a valid 5th case type. Co-Authored-By: Claude Sonnet 5 --- entity-service/CLAUDE.md | 1 + .../internal/service/case_filters.go | 4 +- .../internal/service/case_filters_test.go | 14 ++ .../internal/service/case_service.go | 40 +++++- .../internal/service/sn_case_service.go | 4 +- .../service/sn_case_service_create_test.go | 48 +++++++ .../service/sn_project_stats_service.go | 20 ++- .../service/sn_project_stats_service_test.go | 125 ++++++++++++++++++ entity-service/openapi.yaml | 31 +++-- 9 files changed, 270 insertions(+), 17 deletions(-) create mode 100644 entity-service/internal/service/sn_project_stats_service_test.go diff --git a/entity-service/CLAUDE.md b/entity-service/CLAUDE.md index a569fecc77..341afecfe6 100644 --- a/entity-service/CLAUDE.md +++ b/entity-service/CLAUDE.md @@ -95,6 +95,7 @@ func (h *WidgetHandler) CreateWidget(w http.ResponseWriter, r *http.Request) { - Pagination: call `normalizePagination()` — it caps `limit` at 100 and sets defaults - Use `validXxx` maps (e.g. `validCaseState`, `validCasePriority`) to validate enum fields; add a map entry whenever you add an enum constant - Service methods must not import the `handler` or `repository` packages +- **Caller-supplied aliases for an enum field** (e.g. `caseTypeAliases` in `case_service.go`, resolving `"default_case"` to the canonical `"case"`) exist because a real, currently-in-production caller was built against a different value than this service's own canonical one — usually the raw upstream (ServiceNow) wire value, from before this service introduced its own domain-level enum. Normalize via the alias map as the FIRST thing that happens to the value, before it reaches any `validXxx` map, data-source-specific translation (e.g. `snCaseTypeMap`), or the Postgres repository/DB enum cast — every one of those must only ever see the canonical value, never the alias. Add a new alias here rather than either (a) teaching every downstream consumer about a second valid spelling, or (b) asking the caller to change, since the caller is an already-deployed frontend, not something this change can update in lockstep. ## Repository conventions diff --git a/entity-service/internal/service/case_filters.go b/entity-service/internal/service/case_filters.go index 26490d2702..08f8ab5e83 100644 --- a/entity-service/internal/service/case_filters.go +++ b/entity-service/internal/service/case_filters.go @@ -257,7 +257,9 @@ func ParseCaseFieldFilters(filters []domain.CaseFieldFilter, callerEmail string, if err := requireCaseFilterValues(f); err != nil { return domain.ParsedCaseFilters{}, err } - p.Types = append(p.Types, f.Values...) + for _, v := range f.Values { + p.Types = append(p.Types, normalizeCaseType(v)) + } case "state": if f.Op != "in" { diff --git a/entity-service/internal/service/case_filters_test.go b/entity-service/internal/service/case_filters_test.go index 1ac4fe5f7e..aeb818523e 100644 --- a/entity-service/internal/service/case_filters_test.go +++ b/entity-service/internal/service/case_filters_test.go @@ -57,6 +57,20 @@ func TestParseCaseFieldFilters_NamedFieldTranslations(t *testing.T) { } }, }, + { + // "default_case" is the production customer-portal frontend's + // actual value for this type (ServiceNow's own raw caseType wire + // value, predating this service's "case" enum) — see + // caseTypeAliases. It must normalize to "case" here, not pass + // through as-is. + name: "type in normalizes the default_case alias to case", + in: []domain.CaseFieldFilter{{Field: "type", Op: "in", Values: []string{"default_case", "service_request"}}}, + check: func(t *testing.T, p domain.ParsedCaseFilters) { + if len(p.Types) != 2 || p.Types[0] != "case" || p.Types[1] != "service_request" { + t.Fatalf("Types = %v, want [case service_request]", p.Types) + } + }, + }, { name: "tag in maps to Tags", in: []domain.CaseFieldFilter{{Field: "tag", Op: "in", Values: []string{"patch"}}}, diff --git a/entity-service/internal/service/case_service.go b/entity-service/internal/service/case_service.go index 783cf1e2ff..a3b77d953c 100644 --- a/entity-service/internal/service/case_service.go +++ b/entity-service/internal/service/case_service.go @@ -49,9 +49,37 @@ var validCaseType = map[string]bool{ "case": true, "service_request": true, "security_report_analysis": true, + "announcement": true, "engagement": true, } +// caseTypeAliases maps caller-supplied case type values this API does not +// consider canonical to the value it actually recognises. "default_case" is +// the real, currently-in-production customer-portal frontend's value for +// this type (it's ServiceNow's own raw caseType wire value, which the +// frontend was built directly against before this service's Postgres-backed +// "case" enum existed — see migrations/000008_create_cases.up.sql's +// case_type_enum) and must keep working indefinitely, not just during a +// migration window. Applying this alias is the FIRST thing that happens to +// any caller-supplied case type value, before validCaseType or any +// data-source-specific mapping (snCaseTypeMap, the Postgres repo's enum +// cast) ever sees it, so every downstream consumer only ever has to know +// about the canonical value "case". +var caseTypeAliases = map[string]string{ + "default_case": "case", +} + +// normalizeCaseType resolves a caller-supplied case type value to its +// canonical form via caseTypeAliases, or returns it unchanged if it isn't an +// alias (including if it's already canonical, or altogether invalid -- +// validCaseType is what rejects the latter). +func normalizeCaseType(t string) string { + if canonical, ok := caseTypeAliases[t]; ok { + return canonical + } + return t +} + var validEngagementType = map[domain.EngagementType]bool{ domain.EngagementTypeMigration: true, domain.EngagementTypeConsultancy: true, @@ -97,10 +125,16 @@ var validCaseWorkState = map[domain.CaseWorkState]bool{ domain.CaseWorkStatePaused: true, } -// validateCreateCaseRequest validates fields common to all CreateCase data sources. +// validateCreateCaseRequest validates fields common to all CreateCase data +// sources. Normalizes req.Type via normalizeCaseType FIRST (req is a +// pointer specifically so this mutation is visible to the caller's own +// switch on req.Type and its SN/repo payload-building afterwards) — every +// other check in this function, and everything downstream, only ever sees +// the canonical value. // UUID format of ID fields is not checked here — postgres IDs are UUIDs but // ServiceNow IDs are opaque hex strings; callers add format checks as needed. -func validateCreateCaseRequest(req domain.CreateCaseRequest) error { +func validateCreateCaseRequest(req *domain.CreateCaseRequest) error { + req.Type = normalizeCaseType(req.Type) if req.Type == "" { return &apierror.ValidationError{Msg: "type is required"} } @@ -177,7 +211,7 @@ func validateCreateCaseRequest(req domain.CreateCaseRequest) error { // CreateCase implements CaseService. func (s *caseService) CreateCase(ctx context.Context, req domain.CreateCaseRequest) (domain.CreateCaseResponse, error) { - if err := validateCreateCaseRequest(req); err != nil { + if err := validateCreateCaseRequest(&req); err != nil { return domain.CreateCaseResponse{}, err } if req.Type != "case" { diff --git a/entity-service/internal/service/sn_case_service.go b/entity-service/internal/service/sn_case_service.go index b12781cdaa..5d16e581cd 100644 --- a/entity-service/internal/service/sn_case_service.go +++ b/entity-service/internal/service/sn_case_service.go @@ -303,7 +303,7 @@ var snSortFieldMap = map[domain.CaseSortField]string{ var caseGroupByFieldValues = map[string][]string{ "state": {"open", "work_in_progress", "waiting_on_wso2", "awaiting_info", "reopened", "solution_proposed", "closed"}, "severity": {"catastrophic", "critical", "high", "medium", "low"}, - "type": {"case", "service_request", "security_report_analysis", "engagement"}, + "type": {"case", "service_request", "security_report_analysis", "announcement", "engagement"}, "engagementType": {"migration", "consultancy", "new_feature_improvement", "follow_up", "onboarding"}, "issueType": {"error", "partial_outage", "performance_degradation", "question", "security_or_compliance", "total_outage"}, "workState": {"ongoing", "paused"}, @@ -594,7 +594,7 @@ type snCreateCaseResponse struct { } func (s *snCaseService) CreateCase(ctx context.Context, req domain.CreateCaseRequest) (domain.CreateCaseResponse, error) { - if err := validateCreateCaseRequest(req); err != nil { + if err := validateCreateCaseRequest(&req); err != nil { return domain.CreateCaseResponse{}, err } diff --git a/entity-service/internal/service/sn_case_service_create_test.go b/entity-service/internal/service/sn_case_service_create_test.go index 9746da64ce..4f97fe38db 100644 --- a/entity-service/internal/service/sn_case_service_create_test.go +++ b/entity-service/internal/service/sn_case_service_create_test.go @@ -119,6 +119,54 @@ func TestSNCaseService_CreateCase_Engagement(t *testing.T) { } } +// TestSNCaseService_CreateCase_DefaultCaseAliasNormalizesToCase verifies that +// "default_case" -- the production customer-portal frontend's actual value +// for this type (ServiceNow's own raw caseType wire value, which predates +// this service's "case" enum; see caseTypeAliases) -- is accepted, runs the +// same validation/payload-building as "case" (subject/description/severity/ +// issueType required), and is sent to ServiceNow as "default_case" on the +// wire, exactly as "case" would be. +func TestSNCaseService_CreateCase_DefaultCaseAliasNormalizesToCase(t *testing.T) { + var gotBody map[string]any + client := newTestCaseClient(t, func(w http.ResponseWriter, r *http.Request) { + if err := json.NewDecoder(r.Body).Decode(&gotBody); err != nil { + t.Fatalf("decode request body: %v", err) + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{ + "message": "Case created successfully", + "case": {"id": "` + testWLCaseSysid + `", "number": "CS0000002", "createdBy": "engineer@example.com", "createdOn": "2026-01-02 10:00:00", "state": {"id": 1, "label": "Open"}} + }`)) + }) + + svc := NewServiceNowCaseService(client, nil) + req := domain.CreateCaseRequest{ + Type: "default_case", + ProjectID: testProjectUUID, + DeploymentID: testDeploymentUUID, + DeployedProductID: testDeployedProdID, + Subject: "Cannot log in", + Description: "Login fails with a 500", + Severity: domain.CaseSeverityHigh, + IssueType: domain.CaseIssueTypeQuestion, + } + + resp, err := svc.CreateCase(contextWithUserIDToken("token"), req) + if err != nil { + t.Fatalf("unexpected error for the default_case alias: %v", err) + } + if resp.Case.Number != "CS0000002" { + t.Fatalf("unexpected case number: %s", resp.Case.Number) + } + if gotBody["type"] != "default_case" { + t.Fatalf("payload type: got %v, want %q (SN wire value)", gotBody["type"], "default_case") + } + if gotBody["title"] != "Cannot log in" { + t.Fatalf("payload title: got %v, want %q", gotBody["title"], "Cannot log in") + } +} + // TestSNCaseService_CreateCase_SecurityReportAnalysis_AttachmentsOptional verifies // that a security_report_analysis request with zero attachments is accepted -- // attachments are uploaded via a separate request after the case is created, not diff --git a/entity-service/internal/service/sn_project_stats_service.go b/entity-service/internal/service/sn_project_stats_service.go index bd3f6f6680..62b52c7ec9 100644 --- a/entity-service/internal/service/sn_project_stats_service.go +++ b/entity-service/internal/service/sn_project_stats_service.go @@ -22,6 +22,7 @@ import ( "fmt" "net/url" + "github.com/wso2-open-operations/cs-tools/entity-service/internal/apierror" "github.com/wso2-open-operations/cs-tools/entity-service/internal/domain" "github.com/wso2-open-operations/cs-tools/entity-service/internal/middleware" integrationservice "github.com/wso2-open-operations/cs-tools/entity-service/internal/servicenow-integration-service" @@ -340,10 +341,23 @@ func (s *snProjectStatsService) GetProjectCaseStats(ctx context.Context, project q := url.Values{} if len(req.CaseTypes) > 0 { - if err := validateUUIDs("caseTypes", req.CaseTypes); err != nil { - return domain.ProjectCaseStatsResponse{}, err + // CaseTypes is this service's own domain vocabulary (case, + // service_request, security_report_analysis, announcement, + // engagement — see validCaseType), not UUIDs: there is no + // per-project-stats "case type" entity with its own id, unlike + // projectId above. normalizeCaseType resolves aliases like + // "default_case" (see caseTypeAliases) before validation, matching + // case search's own handling of the same field. Translate to + // ServiceNow's caseTypes wire values the same way case search does + // (domainTypeKeysToSN), so e.g. "case" is forwarded as "default_case". + normalized := make([]string, len(req.CaseTypes)) + for i, t := range req.CaseTypes { + normalized[i] = normalizeCaseType(t) + if !validCaseType[normalized[i]] { + return domain.ProjectCaseStatsResponse{}, &apierror.ValidationError{Msg: "caseTypes contains invalid value: " + t} + } } - for _, ct := range uuidsToSysids(req.CaseTypes) { + for _, ct := range domainTypeKeysToSN(normalized) { q.Add("caseTypes", ct) } } diff --git a/entity-service/internal/service/sn_project_stats_service_test.go b/entity-service/internal/service/sn_project_stats_service_test.go new file mode 100644 index 0000000000..d91ca313f9 --- /dev/null +++ b/entity-service/internal/service/sn_project_stats_service_test.go @@ -0,0 +1,125 @@ +// Copyright (c) 2026 WSO2 LLC. (https://www.wso2.com). +// +// WSO2 LLC. licenses this file to you 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 service + +import ( + "errors" + "net/http" + "net/url" + "testing" + + "github.com/wso2-open-operations/cs-tools/entity-service/internal/apierror" + "github.com/wso2-open-operations/cs-tools/entity-service/internal/domain" +) + +// TestSNProjectStatsService_GetProjectCaseStats_CaseTypesAreDomainValuesNotUUIDs +// guards against reintroducing the bug where caseTypes was validated (and +// converted via uuidsToSysids) as if it were a slice of UUIDs. CaseTypes is +// this service's own domain vocabulary (case/service_request/ +// security_report_analysis/announcement/engagement, matching the case-search +// type filter and this service's own openapi.yaml), translated to SN's +// caseTypes wire values via domainTypeKeysToSN -- e.g. "case" becomes +// "default_case" on the wire, exactly as case search's type filter does. +func TestSNProjectStatsService_GetProjectCaseStats_CaseTypesAreDomainValuesNotUUIDs(t *testing.T) { + var gotQuery url.Values + mux := http.NewServeMux() + mux.HandleFunc("/projects/", func(w http.ResponseWriter, r *http.Request) { + gotQuery = r.URL.Query() + _, _ = w.Write([]byte(`{"totalCount":0,"activeCount":0,"outstandingCount":0,"actionRequiredCount":0, + "stateCount":[],"resolvedCount":{"totalResolvedCount":0,"resolvedWithinSlaCount":0,"resolvedOutsideSlaCount":0}, + "caseTypes":[]}`)) + }) + + client := newTestSNClient(t, mux) + svc := NewServiceNowProjectStatsService(client) + + projectID := "5aeff120-1b74-c210-2649-97a234bcb54a" + req := domain.ProjectCaseStatsRequest{ + CaseTypes: []string{"case", "service_request", "announcement"}, + } + + if _, err := svc.GetProjectCaseStats(contextWithUserIDToken("token"), projectID, req); err != nil { + t.Fatalf("unexpected error for valid domain case types: %v", err) + } + + got := gotQuery["caseTypes"] + want := []string{"default_case", "service_request", "announcement"} + if len(got) != len(want) { + t.Fatalf("expected %d caseTypes forwarded, got %v", len(want), got) + } + for i, w := range want { + if got[i] != w { + t.Fatalf("expected caseTypes[%d]=%q (SN wire value), got %q", i, w, got[i]) + } + } +} + +// TestSNProjectStatsService_GetProjectCaseStats_AcceptsDefaultCaseAlias +// guards the production customer-portal frontend's actual case type value: +// "default_case" is ServiceNow's own raw caseType wire value, which the +// frontend sends directly (it predates this service's Postgres-backed +// "case" enum) and must keep working — see caseTypeAliases. It must +// normalize to the same canonical "case" that produces "default_case" on +// the wire, so this also guards against ever double-translating it into +// something else. +func TestSNProjectStatsService_GetProjectCaseStats_AcceptsDefaultCaseAlias(t *testing.T) { + var gotQuery url.Values + mux := http.NewServeMux() + mux.HandleFunc("/projects/", func(w http.ResponseWriter, r *http.Request) { + gotQuery = r.URL.Query() + _, _ = w.Write([]byte(`{"totalCount":0,"activeCount":0,"outstandingCount":0,"actionRequiredCount":0, + "stateCount":[],"resolvedCount":{"totalResolvedCount":0,"resolvedWithinSlaCount":0,"resolvedOutsideSlaCount":0}, + "caseTypes":[]}`)) + }) + + client := newTestSNClient(t, mux) + svc := NewServiceNowProjectStatsService(client) + + projectID := "5aeff120-1b74-c210-2649-97a234bcb54a" + req := domain.ProjectCaseStatsRequest{CaseTypes: []string{"default_case"}} + + if _, err := svc.GetProjectCaseStats(contextWithUserIDToken("token"), projectID, req); err != nil { + t.Fatalf("unexpected error for the default_case alias: %v", err) + } + + got := gotQuery["caseTypes"] + if len(got) != 1 || got[0] != "default_case" { + t.Fatalf(`expected caseTypes=["default_case"] forwarded to SN, got %v`, got) + } +} + +func TestSNProjectStatsService_GetProjectCaseStats_RejectsUnknownCaseType(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/projects/", func(w http.ResponseWriter, r *http.Request) { + t.Fatal("should not call upstream when caseTypes is invalid") + }) + + client := newTestSNClient(t, mux) + svc := NewServiceNowProjectStatsService(client) + + projectID := "5aeff120-1b74-c210-2649-97a234bcb54a" + req := domain.ProjectCaseStatsRequest{CaseTypes: []string{"not_a_real_case_type"}} + + _, err := svc.GetProjectCaseStats(contextWithUserIDToken("token"), projectID, req) + if err == nil { + t.Fatal("expected an error for a caseTypes value that isn't in the domain vocabulary") + } + var valErr *apierror.ValidationError + if !errors.As(err, &valErr) { + t.Fatalf("expected *apierror.ValidationError, got %T: %v", err, err) + } +} diff --git a/entity-service/openapi.yaml b/entity-service/openapi.yaml index a2303d17a9..ab19d07204 100644 --- a/entity-service/openapi.yaml +++ b/entity-service/openapi.yaml @@ -648,9 +648,15 @@ paths: type: array items: type: string + enum: [case, service_request, security_report_analysis, announcement, engagement] description: > - Restrict to these case types. Repeated for multiple values, e.g. - ?caseTypes=a&caseTypes=b. + Restrict to these case types, one of case/service_request/ + security_report_analysis/announcement/engagement (matching the + case-search type filter's vocabulary — translated to ServiceNow's + own caseTypes values internally, e.g. case → default_case). + `default_case` is also accepted as an alias for `case` — see + CreateCaseRequest.type's description. Repeated for multiple + values, e.g. ?caseTypes=a&caseTypes=b. - name: createdBy in: query required: false @@ -6004,12 +6010,18 @@ components: properties: type: type: string - enum: [case, service_request, security_report_analysis] + enum: [case, service_request, security_report_analysis, announcement, engagement] description: | - Case type. + Case type. `default_case` is also accepted as an alias for `case` + (ServiceNow's own raw caseType wire value; kept for the + currently-in-production customer-portal frontend, which predates + this API's `case` enum) and is normalized to `case` before any + other validation runs. - `case`: standard support case; requires `subject`, `description`, `severity`, `issueType`. - `service_request`: catalog-based service request (ServiceNow only); requires `catalogId`, `catalogItemId`, `variables`. - - `security_report_analysis`: security report (ServiceNow only); requires `subject`, `description`, and at least one entry in `attachments`. + - `security_report_analysis`: security report (ServiceNow only); requires `subject`, `description`; `attachments` optional. + - `announcement`: ServiceNow only. + - `engagement`: requires `subject`, `description`, `engagementType`. projectId: type: string format: uuid @@ -6142,9 +6154,12 @@ components: field with an incompatible op, is rejected with a validation error. - **type** (in): case type, one of case/service_request/ - security_report_analysis/announcement/engagement. For the ServiceNow - data source each value is translated to its SN caseTypes equivalent - (case → default_case). + security_report_analysis/announcement/engagement. `default_case` is + also accepted as an alias for `case` (ServiceNow's own raw caseType + wire value; kept for the currently-in-production customer-portal + frontend, which predates this API's `case` enum). For the + ServiceNow data source each value is translated to its SN + caseTypes equivalent (case → default_case). - **state** (in): one of open/work_in_progress/waiting_on_wso2/ awaiting_info/reopened/solution_proposed/closed. - **severity** (in): one of catastrophic/critical/high/medium/low. From c2a2a483857296cebd301dca57f983ada5748330 Mon Sep 17 00:00:00 2001 From: Rashmika998 Date: Wed, 5 Aug 2026 23:19:04 +0530 Subject: [PATCH 2/6] fix(customer-portal): CORS, error handling, and case-search wire format in backend-v2 - Add CORS middleware, outermost in the chain (wraps Auth). There was no CORS handling at all; a preflight OPTIONS request carries no JWT, so Auth was rejecting every preflight with 401 before the browser ever saw a CORS header -- which the browser reports as "blocked by CORS policy", masking the real cause. Deliberately never sets Access-Control-Allow-Credentials: this backend authenticates via a caller-supplied header, never cookies, so reflecting any Origin back is safe only as long as that stays true. - Surface entity-service's own validation message on a 400 instead of a generic string. internal/entity/client.go was putting entity-service's raw {"code","message"} JSON blob into apiErr.Body instead of extracting just the message (unlike the registry/usermanagement clients); fixed via a shared newUpstreamError helper. Logs now include the exact upstream reason. - Rebuild POST /cases/search's request handling: entity-service redesigned its case-search filters into a generic predicate array and now rejects the old named-field shape outright. The frontend was never updated, so dto.CaseSearchRequest keeps the old shape as this backend's own stable contract, and dto.BuildEntitySearchCasesRequest translates it into entity-service's current contract -- the same dto-layer pattern already used for every response, just applied to this one request. - Split the product-consumption service's two independently-configurable upstream base URLs (subscription/license vs usage-tracking) instead of treating them as one; renamed PRODUCT_CONSUMPTION_BASE_URL to PRODUCT_CONSUMPTION_SUBSCRIPTION_URL to match. - Resolve a route ambiguity that panicked the server at startup: two case search filter routes could not both be registered as literal net/http.ServeMux patterns; merged under one wildcard pattern with manual dispatch. - Remove internal comparisons to the app this replaces from comments/docs throughout; state each behavior/rationale directly as this backend's own design. Co-Authored-By: Claude Sonnet 5 --- apps/customer-portal/backend-v2/.env.example | 17 +- apps/customer-portal/backend-v2/CLAUDE.md | 298 +++++++++--------- apps/customer-portal/backend-v2/README.md | 17 +- apps/customer-portal/backend-v2/asyncapi.yaml | 24 +- .../backend-v2/cmd/server/main.go | 57 ++-- .../backend-v2/internal/aichatagent/client.go | 3 +- .../backend-v2/internal/aichatagent/types.go | 4 +- .../backend-v2/internal/aichatagent/ws.go | 5 +- .../backend-v2/internal/dto/attachment.go | 15 +- .../backend-v2/internal/dto/case.go | 86 +++++ .../backend-v2/internal/dto/case_feedback.go | 3 +- .../backend-v2/internal/dto/case_test.go | 120 +++++++ .../internal/dto/case_time_cards.go | 3 +- .../backend-v2/internal/dto/contacts.go | 2 +- .../backend-v2/internal/dto/conversation.go | 10 +- .../internal/dto/deployed_product_metrics.go | 15 +- .../backend-v2/internal/dto/escalation.go | 16 +- .../backend-v2/internal/dto/global.go | 26 +- .../backend-v2/internal/dto/instance.go | 29 +- .../backend-v2/internal/dto/project_stats.go | 42 ++- .../backend-v2/internal/dto/registry.go | 2 +- .../backend-v2/internal/dto/time_card.go | 3 +- .../backend-v2/internal/dto/user.go | 5 +- .../backend-v2/internal/entity/client.go | 45 ++- .../backend-v2/internal/entity/client_test.go | 58 ++++ .../backend-v2/internal/entity/types.go | 55 ++-- .../backend-v2/internal/handler/ai_chat.go | 36 +-- .../backend-v2/internal/handler/cases.go | 10 +- .../backend-v2/internal/handler/contacts.go | 2 +- .../internal/handler/deployed_products.go | 6 +- .../backend-v2/internal/handler/instances.go | 15 +- .../internal/handler/product_consumption.go | 2 +- .../internal/handler/project_stats.go | 8 +- .../backend-v2/internal/handler/registry.go | 6 +- .../backend-v2/internal/handler/response.go | 31 +- .../internal/handler/response_test.go | 85 +++++ .../backend-v2/internal/handler/time_cards.go | 6 +- .../backend-v2/internal/handler/websocket.go | 12 +- .../backend-v2/internal/middleware/auth.go | 7 +- .../backend-v2/internal/middleware/cors.go | 75 +++++ .../internal/middleware/cors_test.go | 113 +++++++ .../internal/productconsumption/client.go | 59 ++-- .../productconsumption/subscription.go | 4 +- .../internal/productconsumption/types.go | 4 +- .../internal/usermanagement/client.go | 3 +- .../internal/usermanagement/types.go | 5 +- .../internal/usermanagement/usermanagement.go | 5 +- apps/customer-portal/backend-v2/openapi.yaml | 34 +- 48 files changed, 1017 insertions(+), 471 deletions(-) create mode 100644 apps/customer-portal/backend-v2/internal/dto/case_test.go create mode 100644 apps/customer-portal/backend-v2/internal/entity/client_test.go create mode 100644 apps/customer-portal/backend-v2/internal/handler/response_test.go create mode 100644 apps/customer-portal/backend-v2/internal/middleware/cors.go create mode 100644 apps/customer-portal/backend-v2/internal/middleware/cors_test.go diff --git a/apps/customer-portal/backend-v2/.env.example b/apps/customer-portal/backend-v2/.env.example index 5416ff9381..4a2e048658 100644 --- a/apps/customer-portal/backend-v2/.env.example +++ b/apps/customer-portal/backend-v2/.env.example @@ -26,18 +26,13 @@ AI_CHAT_AGENT_BASE_URL= AI_CHAT_AGENT_SCOPES= AI_CHAT_AGENT_WS_BASE_URL= AI_CHAT_AGENT_WS_SCOPES= -# Comma-separated browser Origins allowed to open GET /ws (defense in depth -# against cross-site WebSocket hijacking). Leave unset to allow any origin — -# local development only. -WS_ALLOWED_ORIGINS= -# Product-consumption services — not entity-service. The Ballerina backend -# configures these as two independently-configurable base URLs -# (product_consumption_subscription vs product_consumption_tracking), so set -# both here too. PRODUCT_CONSUMPTION_TRACKING_BASE_URL falls back to -# PRODUCT_CONSUMPTION_BASE_URL when left unset — only needed if the two -# services are hosted separately. -PRODUCT_CONSUMPTION_BASE_URL= +# Product-consumption services — not entity-service. These are two +# independently-configurable base URLs (subscription/license vs usage +# tracking), so set both here too. PRODUCT_CONSUMPTION_TRACKING_BASE_URL +# falls back to PRODUCT_CONSUMPTION_SUBSCRIPTION_URL when left unset — only +# needed if the two services are hosted separately. +PRODUCT_CONSUMPTION_SUBSCRIPTION_URL= PRODUCT_CONSUMPTION_TRACKING_BASE_URL= PRODUCT_CONSUMPTION_SCOPES= diff --git a/apps/customer-portal/backend-v2/CLAUDE.md b/apps/customer-portal/backend-v2/CLAUDE.md index 89eef145bf..2da36ea4d9 100644 --- a/apps/customer-portal/backend-v2/CLAUDE.md +++ b/apps/customer-portal/backend-v2/CLAUDE.md @@ -2,7 +2,7 @@ Go HTTP server (`net/http`, Go 1.26+) that acts as a backend-for-frontend (BFF) for the customer portal. It authenticates callers, forwards requests to `cs-tools/entity-service`, and shapes -responses for the frontend. This is a Go rewrite of the Ballerina backend at +responses for the frontend. This is a rewrite of the existing backend at `apps/customer-portal/backend`, modeled on `apps/csm-portal/backend`'s conventions — read that backend's own CLAUDE.md too if something here is underspecified. @@ -50,7 +50,7 @@ instances fan-out (see "Instances — fan-out, not passed through" below), `PATCH /projects/{id}/contacts/{email}`, `POST /projects/{id}/contacts/validate`, `GET /updates/product-update-levels`, `POST /updates/levels/search`. -The Ballerina backend exposes a handful more routes still not ported here: escalations/incidents/ +The existing backend exposes a handful more routes still not ported here: escalations/incidents/ problems/task SLAs/tasks are already partly covered above where they exist on `cs-tools/entity-service`; what remains unported is mostly generic user search, project update, groups/service-offerings/ configuration-items, and anything genuinely lacking a `cs-tools/entity-service` equivalent (see @@ -60,31 +60,31 @@ next one. ## Which entity-service This backend targets **`cs-tools/entity-service`** (this repo, `../../../entity-service`), *not* -the `digiops-cs/entity-service` that `apps/customer-portal/backend` (the Ballerina original) calls. -The two are different services with overlapping but not identical APIs — before porting an +the `digiops-cs/entity-service` that `apps/customer-portal/backend` (the original implementation) +calls. The two are different services with overlapping but not identical APIs — before porting an endpoint, verify it actually exists on `cs-tools/entity-service` (check `entity-service/internal/server/routes.go` and `entity-service/openapi.yaml`), and note that many of its routes are **ServiceNow-only** (registered only when the service runs with `DATA_SOURCE=servicenow`; see `entity-service/internal/config/config.go`) — a Postgres-mode -deployment will 404 on those. If the Ballerina backend has an endpoint with no `cs-tools/entity-service` +deployment will 404 on those. If the existing backend has an endpoint with no `cs-tools/entity-service` equivalent at all (e.g. `GET /metadata` — entity-service has no metadata endpoint), do not invent one; add a code comment at the call site noting the gap and flag it instead of fabricating a response. **Existing on `cs-tools/entity-service` is necessary but not sufficient — also confirm the -Ballerina backend actually exposes it as a customer-portal feature.** `cs-tools/entity-service` +existing backend actually exposes it as a customer-portal feature.** `cs-tools/entity-service` implements plenty of routes this backend should *not* port: some are genuinely customer-facing but belong to a different portal (e.g. `POST /cases/{id}/github-issues` — filing an engineering bug -against an internal repo is a support-agent action with zero precedent anywhere in the Ballerina -customer-portal backend), and some read like customer features but the Ballerina backend actually +against an internal repo is a support-agent action with zero precedent anywhere in the existing +customer-portal backend), and some read like customer features but the existing backend actually serves the equivalent from an entirely different, non-`cs-tools` microservice (e.g. `POST /accounts/{id}/contacts/search` / `POST /projects/{id}/contacts/search` look like read -analogues of the Ballerina backend's project-contact endpoints, but those are actually backed by +analogues of the existing backend's project-contact endpoints, but those are actually backed by the separate `user_management` module/microservice, not entity-service at all — porting the `cs-tools/entity-service` version would expose a different, unrelated dataset under a similar-looking URL). Before implementing anything new, grep `apps/customer-portal/backend/modules/entity/entity.bal` (or the relevant sibling module) for the -Ballerina function that would call it, and check what it actually resolves to — a route only earns +function that would call it, and check what it actually resolves to — a route only earns a place in this backend once both checks pass. ## Other upstream services @@ -107,105 +107,97 @@ private `do()` shape as `internal/entity`: All seven service clients (entity, updates, SCIM, the AI chat agent, the product-consumption service, the registry service, the project-contact onboarding service) authenticate as the same shared OAuth2 client-credentials app in `cmd/server/main.go` — only each service's -`*_BASE_URL`/`*_SCOPES` env vars differ (confirmed against the Ballerina backend's `Config.toml`, -where every module, including the AI chat agent's WebSocket variant, declares the same -`clientId`/`tokenUrl`). +`*_BASE_URL`/`*_SCOPES` env vars differ. ## The AI chat agent `internal/aichatagent` is a fourth upstream client, but unlike entity/updates/SCIM it talks to a -**separate Python service that has no relationship to `cs-tools/entity-service` at all** — see -`apps/customer-portal/backend`'s `modules/ai_chat_agent` for the Ballerina backend's equivalent -client. It has its own HTTP API (`internal/aichatagent/client.go`) and a distinct WebSocket -endpoint (`internal/aichatagent/ws.go`, using `github.com/gorilla/websocket` — the one third-party +**separate Python service that has no relationship to `cs-tools/entity-service` at all**. It has +its own HTTP API (`internal/aichatagent/client.go`) and a distinct WebSocket endpoint +(`internal/aichatagent/ws.go`, using `github.com/gorilla/websocket` — the one third-party dependency in this otherwise-stdlib-only backend, since `net/http` has no server-side WebSocket support). `internal/handler/ai_chat.go` (case classification, KB recommendations, conversation create/get/update/search, conversation messages, conversation summary) and `internal/handler/websocket.go` (the real-time chat proxy) both mix calls to the AI agent with -calls to entity-service's conversation/comment routes — mirroring the Ballerina backend's own -design (a conversation thread lives in entity-service; the AI agent only handles the live message -exchange). `entity-service` now has `createConversation`/`getConversation`/`updateConversation` -(see `internal/entity/conversations.go`), so `POST /projects/{id}/conversations` -(`AIChatHandler.CreateConversation`) replicates the Ballerina backend's composite flow in full: -create the conversation → call the AI agent → optionally fetch KB recommendations (only when both -`region` and `tier` are supplied on the request) → persist the AI's reply as a comment → -auto-resolve the conversation if the agent reports `resolved: true`. Conversation creation, the AI -call, and comment persistence are fatal (mapped via `mapUpstreamError`) with **no compensating -rollback** of earlier steps if a later one fails — matching the Ballerina reference exactly (a -conversation can exist in entity-service even though the client got a 500 from a later step); -recommendations and auto-resolve are best-effort and never fail the request. The same +calls to entity-service's conversation/comment routes by design: a conversation thread lives in +entity-service; the AI agent only handles the live message exchange. `entity-service` now has +`createConversation`/`getConversation`/`updateConversation` (see +`internal/entity/conversations.go`), so `POST /projects/{id}/conversations` +(`AIChatHandler.CreateConversation`) implements the full composite flow: create the conversation → +call the AI agent → optionally fetch KB recommendations (only when both `region` and `tier` are +supplied on the request) → persist the AI's reply as a comment → auto-resolve the conversation if +the agent reports `resolved: true`. Conversation creation, the AI call, and comment persistence are +fatal (mapped via `mapUpstreamError`) with **no compensating rollback** of earlier steps if a later +one fails (a conversation can exist in entity-service even though the client got a 500 from a later +step); recommendations and auto-resolve are best-effort and never fail the request. The same persist-reply-and-auto-resolve pattern is replicated in `SendConversationMessage` (follow-up -messages — no recommendations call there, matching the Ballerina reference: KB recommendations are -attached only on a conversation's first message) and in `websocket.go`'s `handleMessage`. +messages — no recommendations call there; KB recommendations are attached only on a conversation's +first message) and in `websocket.go`'s `handleMessage`. One gap remains, flagged with a doc comment at each call site rather than worked around — do not build a workaround for this; wait for `entity.CreateCommentRequest` to gain the field: -- **No `createdBy` override on `entity.CreateCommentRequest`** — the Ballerina backend attributes - the AI agent's own reply to a special "chat agent" identity (`entity:CHAT_SENT_AGENT`) when - saving it as a comment; `cs-tools/entity-service` always attributes a created comment to the - caller's own authenticated identity. Both the customer's message and the AI's reply are - therefore saved under the *same* (the caller's) identity here — there is currently no way to - distinguish them by author alone. See the `agentReplyCreatedByCaveat` doc comment in +- **No `createdBy` override on `entity.CreateCommentRequest`** — a distinct "chat agent" identity + for the AI's own replies isn't available; `cs-tools/entity-service` always attributes a created + comment to the caller's own authenticated identity. Both the customer's message and the AI's + reply are therefore saved under the *same* (the caller's) identity here — there is currently no + way to distinguish them by author alone. See the `agentReplyCreatedByCaveat` doc comment in `internal/handler/ai_chat.go`. -`GET /ws?sessionId={projectId}` keeps the Ballerina backend's query parameter name for wire -compatibility even though it actually carries the *project* ID, not a session ID — the AI agent's -own per-conversation session key is derived as `"{projectId}:{conversationId}"` inside the handler. -This connection still only supports *resuming* an existing conversation — the browser must supply -a `conversationId` in its first message, or the handler returns an `error` event rather than +`GET /ws?sessionId={projectId}` names its query parameter `sessionId` even though it actually +carries the *project* ID, not a session ID, for wire compatibility with existing clients — the AI +agent's own per-conversation session key is derived as `"{projectId}:{conversationId}"` inside the +handler. This connection still only supports *resuming* an existing conversation — the browser must +supply a `conversationId` in its first message, or the handler returns an `error` event rather than silently failing; start a brand-new conversation via `POST /projects/{id}/conversations` first, -then resume it over the WebSocket. One simplification versus the Ballerina backend: Go's -`http.Server` runs each upgraded connection in its own goroutine, and the handler's `ReadMessage` → -`handleMessage` loop is a single blocking sequence — it does not read or process the next frame -until `handleMessage` returns, and never starts a concurrent read or upstream stream. So there's no -need for the Ballerina implementation's explicit "already streaming" busy-flag/mutex; the client +then resume it over the WebSocket. Go's `http.Server` runs each upgraded connection in its own +goroutine, and the handler's `ReadMessage` → `handleMessage` loop is a single blocking sequence — it +does not read or process the next frame until `handleMessage` returns, and never starts a +concurrent read or upstream stream. So there's no need for an explicit "already streaming" +busy-flag/mutex; the client can still send another frame at any time, this handler simply won't look at it until the current one finishes. Primary authorization on `GET /ws` is the same JWT middleware chain as every other route. -`WebSocketHandler`'s `gorilla/websocket.Upgrader.CheckOrigin` adds a defense-in-depth check against -cross-site WebSocket hijacking, restricting which browser `Origin`s may open the connection — set -via the optional `WS_ALLOWED_ORIGINS` env var (comma-separated; unset allows any origin, local -development only). +`WebSocketHandler`'s `gorilla/websocket.Upgrader.CheckOrigin` is a defense-in-depth hook against +cross-site WebSocket hijacking that could restrict which browser `Origin`s may open the connection, +but `main.go` currently passes it `nil` (allow any origin) — there is no env var for this. ## The product-consumption service `internal/productconsumption` is a fifth upstream client for **another separate service unrelated -to entity-service** — see `apps/customer-portal/backend`'s `modules/product_consumption_subscription` -and `modules/product_consumption_tracking` for the Ballerina backend's two client modules. This -backend models them as one Go package (one shared `*Client`, one OAuth2 app) but keeps their base -URLs distinct: `Config.BaseURL` backs the subscription/license API, `Config.TrackingBaseURL` backs -the usage-tracking API. They happen to point at the same host in the Ballerina backend's current -`config.toml`, which is why `TrackingBaseURL` falls back to `BaseURL` when left unset — but they are -two independently-configurable variables there (`productConsumptionBaseUrl` vs -`productConsumptionTrackingBaseUrl`), not guaranteed to always match, so don't collapse them into a -single field. +to entity-service** — it actually covers two independently-configurable upstream endpoints, a +subscription/license API and a usage-tracking API. This backend models them as one Go package (one +shared `*Client`, one OAuth2 app) but keeps their base URLs distinct: `Config.SubscriptionBaseURL` +backs the subscription/license API, `Config.TrackingBaseURL` backs the usage-tracking API. They may +happen to point at the same host in some deployments, which is why `TrackingBaseURL` falls back to +`SubscriptionBaseURL` when left unset — but they are independently configurable +(`PRODUCT_CONSUMPTION_SUBSCRIPTION_URL` vs `PRODUCT_CONSUMPTION_TRACKING_BASE_URL`) and not +guaranteed to always match, so don't collapse them into a single field. It backs two routes: - `POST /projects/{projectId}/deployments/{deploymentId}/license` — provisions (or resumes provisioning) a WSO2 API Manager application/subscription/credentials for a deployment and returns the resulting license. `ProcessLicenseDownload` (`internal/productconsumption/subscription.go`) - is a straight port of the Ballerina backend's `processLicenseDownload` state machine — it is - **not idempotent-by-accident-safe to reimplement casually**: it can make up to 5 sequential - upstream calls, several with side effects (creating an application, subscribing it, generating - credentials), and each step only runs if the project's upstream-tracked status hasn't reached it - yet. Read the whole function before touching it — a subtly wrong condition could create a - duplicate WSO2 API Manager application. The handler first calls `entity.GetProject` purely as an - access-control gate (mirroring the Ballerina backend), discarding the result — entity-service is - still the actual authorization boundary for "does this caller own this project." Because up to 5 - sequential upstream calls can plausibly exceed the server's global `WriteTimeout` (see - `cmd/server/main.go`) even when no single step is slow, the handler extends its own write - deadline via `http.NewResponseController(w).SetWriteDeadline` — which only works because - `middleware.responseWriter` forwards `SetWriteDeadline`/`SetReadDeadline` to the underlying - `ResponseWriter`, the same reason it forwards `Hijack` for the WebSocket route above. + implements the upstream state machine — it is **not idempotent-by-accident-safe to reimplement + casually**: it can make up to 5 sequential upstream calls, several with side effects (creating an + application, subscribing it, generating credentials), and each step only runs if the project's + upstream-tracked status hasn't reached it yet. Read the whole function before touching it — a + subtly wrong condition could create a duplicate WSO2 API Manager application. The handler first + calls `entity.GetProject` purely as an access-control gate, discarding the result — + entity-service is still the actual authorization boundary for "does this caller own this + project." Because up to 5 sequential upstream calls can plausibly exceed the server's global + `WriteTimeout` (see `cmd/server/main.go`) even when no single step is slow, the handler extends + its own write deadline via `http.NewResponseController(w).SetWriteDeadline` — which only works + because `middleware.responseWriter` forwards `SetWriteDeadline`/`SetReadDeadline` to the + underlying `ResponseWriter`, the same reason it forwards `Hijack` for the WebSocket route above. - `POST /deployment-usages` — imports a deployment-usage zip file. Unlike every other endpoint in this backend, the request body is **raw binary**, not JSON — see `readBinaryBody` in `internal/handler/response.go` (a `readJSONBody` counterpart with a larger size cap, `maxZipUploadBytes`) and the `Content-Type: application/zip`/`application/x-zip-compressed` - check in the handler, both mirroring the Ballerina backend's `validateDeploymentUsageImportRequest`. + check in the handler, which together validate the upload the way the upstream service requires. The Go client base64-encodes the bytes before forwarding to the upstream service, matching its JSON contract exactly (`{"email": "...", "zip": ""}`). @@ -214,8 +206,7 @@ It backs two routes: `internal/registry` is a sixth upstream client for **another separate service unrelated to entity-service** — a container/robot-account registry (Harbor-style) that issues registry access tokens ("robot accounts") scoped to a project, and looks up a project's integration (service -account) users. See `apps/customer-portal/backend`'s `modules/registry` for the Ballerina backend's -equivalent client. +account) users. It backs five routes (`internal/handler/registry.go`, `RegistryHandler`): - `POST /projects/{id}/registry-tokens` — create a token. Only admins (per `AUTH_ADMIN_ROLE`, @@ -240,25 +231,23 @@ It backs five routes (`internal/handler/registry.go`, `RegistryHandler`): Several of the registry service's own error responses are surfaced to the caller verbatim (not a generic fallback) — see `writeUpstreamMessage` in `internal/handler/registry.go`, which reads `apierror.Error.Body` (the upstream's own `message` field, extracted by the client) and maps it to -the matching HTTP status, mirroring the Ballerina reference's `response.message()` passthrough. +the matching HTTP status, passing through the upstream's own error text rather than a generic one. Regular entity-service calls in the same handlers still use the shared `mapUpstreamError`. ## The project-contact onboarding service `internal/usermanagement` is a seventh upstream client for **yet another separate service** (not entity-service, not SCIM) that manages a project's customer-side contacts/memberships — add, -remove, list, update role, and validate before onboarding. See `apps/customer-portal/backend`'s -`modules/user_management` for the Ballerina backend's equivalent client. It is keyed on the -project's Salesforce ID (`entity.ProjectDetailsView.SfID`), same as the registry service's -integration-users route above — not the project's `id` or `key`. +remove, list, update role, and validate before onboarding. It is keyed on the project's Salesforce +ID (`entity.ProjectDetailsView.SfID`), same as the registry service's integration-users route +above — not the project's `id` or `key`. **The core reshaping this package does: the upstream service represents a contact/membership's -roles as a single semicolon-delimited string** (e.g. `"Admin;Lead"`), but this backend (matching -both the portal's own contract and the Ballerina reference) exposes them as four separate booleans -(`isCsAdmin`, `isLead`, `isPortalUser`, `isSecurityContact`). `internal/usermanagement/types.go`'s -unexported `getRoles`/`hasRole`/`toContact`/`toMembership` do this translation in both directions — -ported field-for-field from the Ballerina reference's own functions of the same name. When adding a -new field here, translate through these helpers, don't reach for the raw `role` string directly. +roles as a single semicolon-delimited string** (e.g. `"Admin;Lead"`), but this backend exposes them +as four separate booleans (`isCsAdmin`, `isLead`, `isPortalUser`, `isSecurityContact`), matching the +portal's own contract. `internal/usermanagement/types.go`'s unexported +`getRoles`/`hasRole`/`toContact`/`toMembership` do this translation in both directions. When adding +a new field here, translate through these helpers, don't reach for the raw `role` string directly. It backs five routes (`internal/handler/contacts.go`, `ContactHandler`): - `GET /projects/{id}/contacts` — list. @@ -284,24 +273,21 @@ Like the registry service, several of this service's error responses are surface `POST /projects/{id}/instances/*`, `/deployments/{id}/instances/*`, and `/deployments/products/{id}/instances/*` (15 routes total) all read from just 5 entity-service endpoints (`SearchInstances`, `SearchInstanceMetrics`, `SearchInstanceUsage`, -`SearchInstanceMetricsStats`, `SearchInstanceUsageStats`) — the Ballerina backend fans each one out -into three differently-scoped views (project/deployment/deployed-product) rather than exposing one -generic filterable endpoint, and `internal/handler/instances.go` replicates that fan-out exactly. -Each of the 5 unexported `search*` methods on `InstanceHandler` takes an `instanceIDFilters` struct -(exactly one of `projectIDs`/`deploymentIDs`/`deployedProductIDs` non-empty by construction — never +`SearchInstanceMetricsStats`, `SearchInstanceUsageStats`). Each is fanned out into three +differently-scoped views (project/deployment/deployed-product) rather than exposed as one generic +filterable endpoint, and `internal/handler/instances.go` implements that fan-out. Each of the 5 +unexported `search*` methods on `InstanceHandler` takes an `instanceIDFilters` struct (exactly one +of `projectIDs`/`deploymentIDs`/`deployedProductIDs` non-empty by construction — never client-controlled, always derived from which of the 3 public wrapper methods was called) and the 3 exported wrapper methods per metric type just supply that struct plus the path param. When adding a 6th instance metric type from a future entity-service endpoint, follow this same shape rather than inventing a new fan-out mechanism. -**One deliberate asymmetry, preserved from the Ballerina reference, not "fixed":** -`InstanceMetricsStatsRequest`'s `DataSource` field is never forwarded to entity-service in -`searchInstanceMetricsStats` (all 3 scopes), even though the request type carries it — the -Ballerina reference's own `.../instances/stats/metrics/search` resource functions never read -`payload.filters.dataSource`, while the sibling `.../instances/stats/usages/search` family -(`searchInstanceUsageStats`) does forward it. This is a genuine inconsistency in the upstream -Ballerina backend's own code, not a bug introduced here — see the doc comment above -`searchInstanceMetricsStats` before "fixing" it. +**One deliberate asymmetry, by design, not "fixed":** `InstanceMetricsStatsRequest`'s `DataSource` +field is never forwarded to entity-service in `searchInstanceMetricsStats` (all 3 scopes), even +though the request type carries it, while the sibling `.../instances/stats/usages/search` family +(`searchInstanceUsageStats`) does forward it. This asymmetry is intentional, not a bug — see the +doc comment above `searchInstanceMetricsStats` before "fixing" it. ## Project metadata and stats — reshaped, not passed through @@ -309,56 +295,53 @@ Ballerina backend's own code, not a bug introduced here — see the doc comment `/stats/support`, `/stats/time-cards`, and `/stats/change-requests` all read from seven entity-service endpoints (`GetProjectMetadata`, `GetProjectCaseStats`, `GetProjectConversationStats`, `GetProjectDeploymentStats`, `GetProjectStats`, -`GetProjectTimeCardStats`, and `GetProjectChangeRequestStats`) — the Ballerina backend fans a -handful of raw entity-service responses out into eight differently-shaped, purpose-built views -rather than exposing them 1:1, and `internal/dto/project_stats.go` -replicates that fan-out exactly (ported from the Ballerina backend's `getProjectFilters`, -`mapProjectFeatures`, `mapCaseStats`, `getConversationStats`, and -`mapProjectChangeRequestStatsResponse` in `utils.bal`): +`GetProjectTimeCardStats`, and `GetProjectChangeRequestStats`). A handful of raw entity-service +responses are fanned out into eight differently-shaped, purpose-built views rather than exposed +1:1, and `internal/dto/project_stats.go` implements that fan-out: - **`/filters` and `/features` both call `GetProjectMetadata`** — there is no `GET /projects/{id}/metadata` - passthrough endpoint in this backend at all, because the Ballerina backend never exposed one - either; it only ever exposes the metadata response split into these two narrower views. - `ChoiceListItem`/`ReferenceTableItem` (entity-service's two "list of valid options" shapes) both - collapse into one `dto.ReferenceItem{id, label, count?}` for the frontend, matching the Ballerina - backend's own `ReferenceItem` type. `/filters`' `changeRequestStates` additionally drops three - internal ServiceNow workflow state IDs (`dto.restrictedChangeRequestStateIDs`) that were never - meant to be a customer-facing filter option. + passthrough endpoint in this backend at all; the metadata response is only ever exposed split into + these two narrower views. `ChoiceListItem`/`ReferenceTableItem` (entity-service's two "list of + valid options" shapes) both collapse into one `dto.ReferenceItem{id, label, count?}` for the + frontend. `/filters`' `changeRequestStates` additionally drops three internal ServiceNow workflow + state IDs (`dto.restrictedChangeRequestStateIDs`) that were never meant to be a customer-facing + filter option. - **`/stats` and `/stats/support` are composite, graceful-degradation endpoints** — each combines multiple independent entity-service calls (`/stats` combines case/conversation/deployment/activity stats; `/stats/support` combines case/conversation stats) and returns `200` even if every one of them fails, simply omitting that source's fields from the response (`dto.BuildProjectDashboardStats`/ `dto.BuildProjectSupportStats` take `*entity.XxxResponse`, nil meaning "this source failed to - load"). This exactly mirrors the Ballerina backend's own behavior — it logs each failure and moves - on rather than failing the whole request. `/stats/cases`, `/stats/conversations`, - `/stats/time-cards`, and `/stats/change-requests`, by contrast, are **not** graceful — each is a - single entity-service call and a failure there is a hard failure (`mapUpstreamError`), matching - the Ballerina backend's per-endpoint behavior exactly (verified individually, not assumed from the - composite endpoints' pattern). + load"). It logs each failure and moves on rather than failing the whole request. `/stats/cases`, + `/stats/conversations`, `/stats/time-cards`, and `/stats/change-requests`, by contrast, are **not** + graceful — each is a single entity-service call and a failure there is a hard failure + (`mapUpstreamError`), verified individually per endpoint rather than assumed from the composite + endpoints' pattern. - **State-ID-based derived counts are hardcoded, not configurable.** `dto.caseStateIDOpen` and the `conversationStateID*` constants pick specific counts out of a state-count breakdown (e.g. "how - many cases are in the *open* state") using the same default ServiceNow state IDs the Ballerina - backend's own `stateIdOpen`/`conversationStateIds` configuration defaults to. If cs-tools' - ServiceNow instance uses different state IDs for these, these constants need to become - configurable here too — they are not currently, since the Ballerina backend's own configurability - was never exercised away from its defaults as far as this rewrite could confirm. -- `/stats/cases` also, in the Ballerina backend, fetches change-request stats and never uses the - result (dead code, presumably a leftover) — this backend does not replicate that no-op call. + many cases are in the *open* state") using this deployment's default ServiceNow state IDs. If + cs-tools' ServiceNow instance uses different state IDs for these, these constants need to become + configurable here too — they are not currently. ## Middleware chain -`SecurityHeaders → CorrelationID → Auth → Logger → Mux` +`CORS → SecurityHeaders → CorrelationID → Auth → Logger → Mux` -Identical to `apps/csm-portal/backend`'s chain — see that backend's CLAUDE.md for the rationale of -each layer. `middleware.ConfigureLogger()` must be called at startup. +Apart from `CORS`, identical to `apps/csm-portal/backend`'s chain — see that backend's CLAUDE.md for +the rationale of each layer. `middleware.ConfigureLogger()` must be called at startup. + +**`CORS` must be outermost, wrapping everything including `Auth`.** A CORS preflight is a bare +`OPTIONS` request with no JWT at all; if `Auth` ran before `CORS`, it would reject every preflight +with 401 before the browser ever received a CORS header — which the browser then reports as +"blocked by CORS policy", masking the real cause. `main.go` currently calls `middleware.CORS(nil)` +(allow any origin, no env var) — `middleware.CORS` accepts an allow-list parameter if this ever +needs to be restricted, but nothing in this backend currently sets one. ## Response shaping — the "wrapper" pattern **Never return an entity-service response struct directly to the frontend.** This is the one deliberate difference from `apps/csm-portal/backend` (which does raw `[]byte` passthrough for most -entity responses) — it mirrors what the Ballerina backend does with its `types` module + -`utils.bal` mapper functions (`mapCaseResponse`, `mapProjectsResponse`, etc.), which reshape every -`entity:*Response` into a portal-owned DTO before it reaches the frontend. +entity responses) — every entity-service response is reshaped into a portal-owned DTO before it +reaches the frontend. Concretely: @@ -398,16 +381,35 @@ technique — e.g. `ProductVersionView.ReleaseDate` is typed `*string` even thou is `time.Time` and the ServiceNow shape is a plain string, because a Go `*string` field decodes a JSON string value regardless of which the source type actually was. -**When the Ballerina backend exposes a thinner shape than entity-service's own struct, match the -Ballerina backend, not entity-service.** `POST /time-cards/search`'s `dto.TimeCardSummary` excludes -entity-service's per-category time breakdowns (`timeAnalyzing`, `timeSettingUp`, etc.), +**When the frontend's contract expects a thinner shape than entity-service's own struct, match the +frontend's contract, not entity-service.** `POST /time-cards/search`'s `dto.TimeCardSummary` +excludes entity-service's per-category time breakdowns (`timeAnalyzing`, `timeSettingUp`, etc.), `issueComplexity`, `workLogComment`, `rejectionReason`, and the eligible-approvers list — not -because any of them are individually dangerous, but because the Ballerina backend's own `TimeCard` -type (`apps/customer-portal/backend/modules/entity/types.bal`) never exposed them either. When -porting an endpoint, read the Ballerina backend's response type for the equivalent feature, not just -the request/response pair on `cs-tools/entity-service` — the Ballerina shape is itself a design -decision about what a customer should see, and entity-service's superset shouldn't leak through it -by default. +because any of them are individually dangerous, but because the frontend's existing contract never +exposed them either. When porting an endpoint, read the frontend's existing response shape for the +equivalent feature, not just the request/response pair on `cs-tools/entity-service` — that shape is +itself a design decision about what a customer should see, and entity-service's superset shouldn't +leak through it by default. + +**The DTO layer's job isn't only response trimming — it also absorbs entity-service's own request +contract changes, so the frontend never has to know they happened.** `POST /cases/search` is the +example: entity-service redesigned its case-search filters from named fields (`types`, `states`, +`projectIds`, ...) into a generic predicate array (`filters: [{field, op, values}]`, +`internal/entity/types.go`'s `CaseFieldFilter`) and now rejects any request using the old shape +outright (`decodeRequest`'s `DisallowUnknownFields`). The frontend was never updated — it still +sends the old named-field shape — so `dto.CaseSearchRequest`/`CaseSearchFilters` keep that exact old +shape as this backend's own stable, unchanged contract, and `dto.BuildEntitySearchCasesRequest` is +the only place that builds an `entity.CaseFieldFilter` array, translating each portal filter field +into the "field in/eq values" entry entity-service's `case_filters.go` expects (see that file for +the authoritative field/op table if entity-service adds a new filter). `CreatedByMe` becomes a +`createdBy`+`eq` filter carrying the literal string `"__current_user_email__"` — this must match +entity-service's own `currentUserFilterPlaceholder` constant exactly, not just be "some sentinel". +If entity-service ever changes another request contract this way, the fix is the same shape: keep +the portal-facing dto type frozen at whatever the frontend already sends, and write a +`BuildEntityXRequest` translator rather than pushing the new shape onto the frontend or, worse, +decoding the incoming request directly into an `internal/entity` type (which is how this bug +happened in the first place — there was no dto/entity separation on the request side for this one +endpoint, unlike every response, which already goes through `dto.Map*`). **`json.RawMessage` on a request field usually means "preserve three states," not "skip validation."** `entity.UpdateDeployedProductRequest.Description` is `json.RawMessage` specifically so entity-service @@ -538,9 +540,21 @@ constants). - **Path params**: guard against empty string after `r.PathValue("id")`; validate UUID-shaped IDs with the package-level `uuidRe` and return 400 on mismatch before calling entity-service. - **Upstream errors**: always use `mapUpstreamError(w, err, "")` — never write - custom status mappings inline. + custom status mappings inline. For a 400, this now returns entity-service's own message + (`apiErr.Body`) verbatim to the caller instead of a generic string — entity-service's validation + errors are already written to be safe and specific (see its `apierror.ValidationError`), so + swallowing them loses real, actionable detail for no security benefit. 401/403/404 still always + use a fixed message regardless of the upstream body — never pass through upstream text for those + statuses. - **Logging**: use `slog.ErrorContext` with `summarizeErr(err)`, never the raw error — an unrecognized error can stringify with the full request URL including query params. + `summarizeErr` DOES include the upstream status and message for a typed `*apierror.Error` (e.g. + `"upstream status 400: caseTypes must be valid UUIDs"`) — entity-service's error bodies are + already caller-safe validation text, not sensitive internal detail, so logging them verbatim is + fine. `internal/entity/client.go`'s `newUpstreamError` is what populates `apiErr.Body` with just + the extracted `message` field (not entity-service's raw `{"code","message"}` response) — every + new upstream-error construction site in that file must go through it rather than reinventing + inline body-truncation, or the message-extraction and the 400 passthrough above silently break. ## Security diff --git a/apps/customer-portal/backend-v2/README.md b/apps/customer-portal/backend-v2/README.md index 73ac9fd635..cd97c66738 100644 --- a/apps/customer-portal/backend-v2/README.md +++ b/apps/customer-portal/backend-v2/README.md @@ -1,9 +1,9 @@ # Customer Portal Backend (v2) -Go rewrite of the Ballerina backend at `apps/customer-portal/backend`. It is a backend-for-frontend +Rewrite of the existing backend at `apps/customer-portal/backend`. It is a backend-for-frontend (BFF) for the customer portal: it authenticates callers, forwards requests to [`entity-service`](../../../entity-service) (this repo's `cs-tools/entity-service`, not the -`digiops-cs/entity-service` the Ballerina backend targets), and shapes the responses for the frontend. +`digiops-cs/entity-service` the existing backend targets), and shapes the responses for the frontend. This is a work in progress — only the 101 routes listed below are implemented so far, across entity-service, the WSO2 Updates service, SCIM, the AI chat agent, the product-consumption @@ -11,7 +11,7 @@ service, the registry (robot-account) service, and the project-contact onboardin more separate services — see [CLAUDE.md](./CLAUDE.md#the-ai-chat-agent), [CLAUDE.md](./CLAUDE.md#the-product-consumption-service), [CLAUDE.md](./CLAUDE.md#the-registry-service), and -[CLAUDE.md](./CLAUDE.md#the-project-contact-onboarding-service)). Everything else the Ballerina +[CLAUDE.md](./CLAUDE.md#the-project-contact-onboarding-service)). Everything else the existing backend exposes still needs a Go handler; add them following the pattern described in [CLAUDE.md](./CLAUDE.md#adding-a-new-endpoint). @@ -132,18 +132,17 @@ A separate Python service (not entity-service) — see [CLAUDE.md](./CLAUDE.md#t | `AI_CHAT_AGENT_SCOPES` | Comma-separated OAuth2 scopes (optional) | | `AI_CHAT_AGENT_WS_BASE_URL` | Base URL of the AI chat agent's WebSocket endpoint | | `AI_CHAT_AGENT_WS_SCOPES` | Comma-separated OAuth2 scopes (optional) | -| `WS_ALLOWED_ORIGINS` | Comma-separated browser Origins allowed to open `GET /ws` (optional — defense in depth against cross-site WebSocket hijacking; unset allows any origin, local development only) | ### Product-consumption service -Not entity-service — see [CLAUDE.md](./CLAUDE.md#the-product-consumption-service). The Ballerina -backend configures the subscription/license API and the usage-tracking API as two independently -configurable base URLs, so set both here too. +Not entity-service — see [CLAUDE.md](./CLAUDE.md#the-product-consumption-service). The +subscription/license API and the usage-tracking API are two independently configurable base URLs, +so set both here too. | Variable | Description | |---|---| -| `PRODUCT_CONSUMPTION_BASE_URL` | Base URL of the subscription/license API | -| `PRODUCT_CONSUMPTION_TRACKING_BASE_URL` | Base URL of the usage-tracking API (optional — falls back to `PRODUCT_CONSUMPTION_BASE_URL` when unset) | +| `PRODUCT_CONSUMPTION_SUBSCRIPTION_URL` | Base URL of the subscription/license API | +| `PRODUCT_CONSUMPTION_TRACKING_BASE_URL` | Base URL of the usage-tracking API (optional — falls back to `PRODUCT_CONSUMPTION_SUBSCRIPTION_URL` when unset) | | `PRODUCT_CONSUMPTION_SCOPES` | Comma-separated OAuth2 scopes (optional) | ### Registry service diff --git a/apps/customer-portal/backend-v2/asyncapi.yaml b/apps/customer-portal/backend-v2/asyncapi.yaml index eb0fb13899..4a90d8b3f1 100644 --- a/apps/customer-portal/backend-v2/asyncapi.yaml +++ b/apps/customer-portal/backend-v2/asyncapi.yaml @@ -24,12 +24,10 @@ info: separate Python service — see CLAUDE.md's "The AI chat agent" section). This server (internal/handler/websocket.go) forwards every message from the upstream agent to the browser verbatim, so the message shapes below - mirror the upstream agent's own contract (see - apps/customer-portal/backend/asyncapi.yaml for the Ballerina backend's - equivalent, which this is adapted from) — with one deliberate difference: - conversationId is REQUIRED on UserMessage here. Unlike the Ballerina - backend, this server cannot start a brand-new AI chat conversation - (entity-service has no createConversation yet — see CLAUDE.md), so a + mirror the upstream agent's own contract — with one deliberate + restriction: conversationId is REQUIRED on UserMessage here. This + connection only resumes an existing conversation (start a new one via + POST /projects/{id}/conversations first — see CLAUDE.md), so a UserMessage with no conversationId gets an ErrorMessage back instead of creating one. @@ -51,10 +49,10 @@ channels: format: uuid description: > The project ID. Named sessionId for wire compatibility with - the Ballerina backend this replaces, even though it doesn't - carry a session/conversation value itself — the upstream - agent's own per-conversation session key is derived - server-side as "{projectId}:{conversationId}". + existing clients, even though it doesn't carry a + session/conversation value itself — the upstream agent's own + per-conversation session key is derived server-side as + "{projectId}:{conversationId}". required: - sessionId @@ -334,9 +332,9 @@ components: type: boolean nullable: true description: > - When true, the Ballerina backend marks the conversation - resolved in entity-service; this server does not (no - updateConversation yet — see CLAUDE.md). + When true, this server marks the conversation resolved in + entity-service (best-effort — a failure here is logged, + not surfaced to the client). kbReferences: type: array nullable: true diff --git a/apps/customer-portal/backend-v2/cmd/server/main.go b/apps/customer-portal/backend-v2/cmd/server/main.go index 22634efe03..b436cbdf27 100644 --- a/apps/customer-portal/backend-v2/cmd/server/main.go +++ b/apps/customer-portal/backend-v2/cmd/server/main.go @@ -82,9 +82,7 @@ func main() { // The AI chat agent is a separate Python service (not entity-service), // but authenticates as the same shared OAuth2 client-credentials app as - // entity/updates/scim above (see the Ballerina backend's Config.toml, - // where every module — including ai_chat_agent's WebSocket variant — - // reuses the same clientId/tokenUrl); only its base URLs differ. + // entity/updates/scim above; only its base URLs differ. aiChatAgentCfg := aichatagent.Config{ BaseURL: mustEnv("AI_CHAT_AGENT_BASE_URL"), TokenURL: oauth2TokenURL, @@ -105,19 +103,17 @@ func main() { // The product-consumption service(s) are separate services (not // entity-service) that provision deployment licenses and import usage - // data; both authenticate as the same shared OAuth2 app. The Ballerina - // backend configures these as two independently-configurable base URLs - // (productConsumptionBaseUrl vs productConsumptionTrackingBaseUrl) — - // PRODUCT_CONSUMPTION_TRACKING_BASE_URL defaults to - // PRODUCT_CONSUMPTION_BASE_URL when unset, matching that backend's - // current config where both happen to point at the same host. + // data; both authenticate as the same shared OAuth2 app. These are two + // independently-configurable base URLs — PRODUCT_CONSUMPTION_TRACKING_BASE_URL + // defaults to PRODUCT_CONSUMPTION_SUBSCRIPTION_URL when unset, for + // deployments where both happen to point at the same host. productConsumptionCfg := productconsumption.Config{ - BaseURL: mustEnv("PRODUCT_CONSUMPTION_BASE_URL"), - TrackingBaseURL: os.Getenv("PRODUCT_CONSUMPTION_TRACKING_BASE_URL"), - TokenURL: oauth2TokenURL, - ClientID: oauth2ClientID, - ClientSecret: oauth2ClientSecret, - Scopes: splitComma(os.Getenv("PRODUCT_CONSUMPTION_SCOPES")), + SubscriptionBaseURL: mustEnv("PRODUCT_CONSUMPTION_SUBSCRIPTION_URL"), + TrackingBaseURL: os.Getenv("PRODUCT_CONSUMPTION_TRACKING_BASE_URL"), + TokenURL: oauth2TokenURL, + ClientID: oauth2ClientID, + ClientSecret: oauth2ClientSecret, + Scopes: splitComma(os.Getenv("PRODUCT_CONSUMPTION_SCOPES")), } productConsumptionClient := productconsumption.NewClient(productConsumptionCfg) @@ -154,8 +150,7 @@ func main() { } // adminRole is the role string (from entity.GetUserMeResponse.Roles) that - // grants admin privileges for registry-token and contact management — - // mirrors the Ballerina reference's configurable authorizedRoles.adminRole. + // grants admin privileges for registry-token and contact management. adminRole := mustEnv("AUTH_ADMIN_ROLE") userHandler := handler.NewUserHandler(entityClient, scimClient) @@ -175,7 +170,7 @@ func main() { catalogHandler := handler.NewCatalogHandler(entityClient) timeCardHandler := handler.NewTimeCardHandler(entityClient) aiChatHandler := handler.NewAIChatHandler(aiChatAgentClient, entityClient) - webSocketHandler := handler.NewWebSocketHandler(aiChatAgentWsClient, entityClient, splitComma(os.Getenv("WS_ALLOWED_ORIGINS"))) + webSocketHandler := handler.NewWebSocketHandler(aiChatAgentWsClient, entityClient, nil) productConsumptionHandler := handler.NewProductConsumptionHandler(productConsumptionClient, entityClient) globalHandler := handler.NewGlobalHandler(entityClient) instanceHandler := handler.NewInstanceHandler(entityClient) @@ -261,10 +256,11 @@ func main() { // registered as literal patterns: net/http.ServeMux (Go 1.22+) rejects // them as ambiguous (neither is more specific than the other — e.g. both // match "/deployments/products/products/instances/metrics/search") and - // panics at startup. Both path shapes are genuine, distinct Ballerina - // reference routes, so they're merged under one wildcard pattern and - // dispatched by dispatchDeploymentsProductsMetricsSearch below instead of - // renaming either one. + // panics at startup. Both path shapes are genuine, distinct routes this + // backend must expose exactly as-is, so they're merged under one + // wildcard pattern and dispatched by + // dispatchDeploymentsProductsMetricsSearch below instead of renaming + // either one. mux.HandleFunc("POST /deployments/{seg1}/{seg2}/{seg3}/metrics/search", dispatchDeploymentsProductsMetricsSearch(instanceHandler, deployedProductHandler)) mux.HandleFunc("POST /deployments/{deploymentId}/products/{productId}/metrics/usage-counts/search", deployedProductHandler.SearchDeployedProductUsageCounts) @@ -350,10 +346,15 @@ func main() { slog.Info("Customer Portal Backend (v2) started", "addr", addr) srv := &http.Server{ - Handler: middleware.SecurityHeaders( - middleware.CorrelationID( - middleware.Auth(authCfg)( - middleware.Logger(mux), + // CORS must be outermost: a preflight OPTIONS request carries no JWT, + // so if Auth ran first it would reject every preflight with 401 + // before the browser ever saw a CORS header. + Handler: middleware.CORS(nil)( + middleware.SecurityHeaders( + middleware.CorrelationID( + middleware.Auth(authCfg)( + middleware.Logger(mux), + ), ), ), ), @@ -385,8 +386,8 @@ func main() { slog.Info("Customer Portal Backend (v2) stopped") } -// dispatchDeploymentsProductsMetricsSearch resolves the two distinct -// Ballerina reference routes merged under the "POST +// dispatchDeploymentsProductsMetricsSearch resolves the two distinct routes +// merged under the "POST // /deployments/{seg1}/{seg2}/{seg3}/metrics/search" pattern registered in // main() (see the comment at that registration for why they can't be // registered as separate literal patterns). Exactly one of the two shapes diff --git a/apps/customer-portal/backend-v2/internal/aichatagent/client.go b/apps/customer-portal/backend-v2/internal/aichatagent/client.go index 76a30f7ef7..1b4302782e 100644 --- a/apps/customer-portal/backend-v2/internal/aichatagent/client.go +++ b/apps/customer-portal/backend-v2/internal/aichatagent/client.go @@ -17,8 +17,7 @@ // Package aichatagent is the HTTP client for the upstream AI chat agent — a // separate Python service (not entity-service) that powers the customer // portal's AI chat feature: case classification, chat responses, and KB -// article recommendations. See apps/customer-portal/backend's -// modules/ai_chat_agent for the Ballerina backend's equivalent client. +// article recommendations. package aichatagent import ( diff --git a/apps/customer-portal/backend-v2/internal/aichatagent/types.go b/apps/customer-portal/backend-v2/internal/aichatagent/types.go index 8b8f427260..e4df85a594 100644 --- a/apps/customer-portal/backend-v2/internal/aichatagent/types.go +++ b/apps/customer-portal/backend-v2/internal/aichatagent/types.go @@ -17,9 +17,7 @@ package aichatagent // These types mirror the upstream Python AI chat agent's wire format 1:1 -// (see apps/customer-portal/backend's modules/ai_chat_agent/types.bal, the -// Ballerina backend this is rewriting) so json.Unmarshal can decode its -// responses directly. +// so json.Unmarshal can decode its responses directly. // CaseClassificationPayload is the input for POST /case_classification. type CaseClassificationPayload struct { diff --git a/apps/customer-portal/backend-v2/internal/aichatagent/ws.go b/apps/customer-portal/backend-v2/internal/aichatagent/ws.go index fc5eb16691..e825b49612 100644 --- a/apps/customer-portal/backend-v2/internal/aichatagent/ws.go +++ b/apps/customer-portal/backend-v2/internal/aichatagent/ws.go @@ -49,9 +49,8 @@ const maxMessageBytes = 64 << 10 // 64 KiB const idleTimeout = 5 * time.Minute // WSConfig holds the configuration for dialing the upstream AI chat agent's -// WebSocket endpoint. Kept separate from Config since the Ballerina backend -// this is rewriting uses a distinct OAuth2 client-credentials configuration -// for its WebSocket connection. +// WebSocket endpoint. Kept separate from Config because the WebSocket +// connection uses its own distinct OAuth2 client-credentials configuration. type WSConfig struct { BaseURL string TokenURL string diff --git a/apps/customer-portal/backend-v2/internal/dto/attachment.go b/apps/customer-portal/backend-v2/internal/dto/attachment.go index 4a67c9c5e6..bb23ca5986 100644 --- a/apps/customer-portal/backend-v2/internal/dto/attachment.go +++ b/apps/customer-portal/backend-v2/internal/dto/attachment.go @@ -103,9 +103,9 @@ func MapDeleteAttachment(r entity.DeleteAttachmentResponse) DeleteResponse { // AttachmentDetails is the portal's response for GET /attachments/{id} — // metadata plus base64-encoded content. entity-service's response has no -// fields worth restricting here (the Ballerina reference returns it as-is -// too), so this is a direct passthrough shape kept as its own portal type -// purely for this package's "always map through dto" convention. +// fields worth restricting here, so this is a direct passthrough shape kept +// as its own portal type purely for this package's "always map through dto" +// convention. type AttachmentDetails struct { ID string `json:"id"` ReferenceID string `json:"referenceId"` @@ -142,9 +142,8 @@ func MapAttachmentDetails(r entity.AttachmentDetails) AttachmentDetails { // /deployments/{deploymentId}/attachments/{attachmentId} and PATCH // /cases/{caseId}/attachments/{attachmentId}). referenceId/referenceType are // never client-supplied — each handler injects them from its own path -// params and the appropriate ReferenceType, matching the Ballerina -// reference's two routes. The case-scoped route only ever reads Name (never -// Description), matching that route's own restriction in the Ballerina backend. +// params and the appropriate ReferenceType. The case-scoped route only ever +// reads Name (never Description) by design. type AttachmentUpdateRequest struct { Name *string `json:"name,omitempty"` Description *string `json:"description,omitempty"` @@ -170,8 +169,8 @@ type UpdatedAttachment struct { } // MapUpdatedAttachment builds the portal response from entity-service's -// UpdateAttachmentResponse — a raw passthrough of the attachment field, -// matching the Ballerina reference (both routes return response.attachment directly). +// UpdateAttachmentResponse — a raw passthrough of the attachment field +// (both routes return response.attachment directly). func MapUpdatedAttachment(r entity.UpdateAttachmentResponse) UpdatedAttachment { return UpdatedAttachment{ ID: r.Attachment.ID, diff --git a/apps/customer-portal/backend-v2/internal/dto/case.go b/apps/customer-portal/backend-v2/internal/dto/case.go index 9c180e5987..e20bc1a555 100644 --- a/apps/customer-portal/backend-v2/internal/dto/case.go +++ b/apps/customer-portal/backend-v2/internal/dto/case.go @@ -111,6 +111,92 @@ func MapSearchCases(r entity.SearchCasesResponse) SearchCasesResponse { } } +// currentUserFilterPlaceholder must match entity-service's own +// currentUserFilterPlaceholder (case_filters.go) exactly — it's the literal +// values entry a createdBy+eq filter must carry to mean "the authenticated +// caller". +const currentUserFilterPlaceholder = "__current_user_email__" + +// CaseSearchFilters holds the optional filter criteria for POST /cases/search. +// This is the portal's own request contract, unchanged from before +// entity-service redesigned its case-search wire format into a generic +// filter-predicate array (matching what the frontend has always sent) — +// only the fields +// the portal currently exposes are included; extend as needed when more +// filters are surfaced to the frontend. See BuildEntitySearchCasesRequest +// for the translation into entity-service's current contract. +type CaseSearchFilters struct { + Types []string `json:"types,omitempty"` + SearchQuery string `json:"searchQuery,omitempty"` + ProjectIDs []string `json:"projectIds,omitempty"` + DeploymentIDs []string `json:"deploymentIds,omitempty"` + States []string `json:"states,omitempty"` + Severities []string `json:"severities,omitempty"` + IssueTypes []string `json:"issueTypes,omitempty"` + EngagementTypes []string `json:"engagementTypes,omitempty"` + CreatedBy []string `json:"createdBy,omitempty"` + CreatedByMe bool `json:"createdByMe,omitempty"` + WorkStates []string `json:"workStates,omitempty"` + AssignedUserIDs []string `json:"assignedUserIds,omitempty"` + ProductNames []string `json:"productNames,omitempty"` + Tags []string `json:"tags,omitempty"` + ParentID *string `json:"parentId,omitempty"` +} + +// CaseSearchRequest is the portal's request body for POST /cases/search. +type CaseSearchRequest struct { + Filters CaseSearchFilters `json:"filters"` + SortBy entity.CaseSort `json:"sortBy"` + Pagination entity.Pagination `json:"pagination"` +} + +// BuildEntitySearchCasesRequest translates the portal's named-filter-field +// request into entity-service's current generic filter-predicate array +// contract (see entity.CaseFieldFilter) — every filter the portal exposes +// becomes one "field in/eq values" entry; SearchQuery, SortBy, and +// Pagination pass straight through unchanged. CreatedByMe becomes a +// createdBy+eq filter carrying currentUserFilterPlaceholder, exactly as +// entity-service's own case_filters.go expects, resolving the caller's +// identity server-side from the forwarded x-user-id-token. +func BuildEntitySearchCasesRequest(req CaseSearchRequest) entity.SearchCasesRequest { + var filters []entity.CaseFieldFilter + + addIn := func(field string, values []string) { + if len(values) > 0 { + filters = append(filters, entity.CaseFieldFilter{Field: field, Op: "in", Values: values}) + } + } + + addIn("type", req.Filters.Types) + addIn("projectId", req.Filters.ProjectIDs) + addIn("deploymentId", req.Filters.DeploymentIDs) + addIn("state", req.Filters.States) + addIn("severity", req.Filters.Severities) + addIn("issueType", req.Filters.IssueTypes) + addIn("engagementType", req.Filters.EngagementTypes) + addIn("workState", req.Filters.WorkStates) + addIn("assignedUserId", req.Filters.AssignedUserIDs) + addIn("product", req.Filters.ProductNames) + addIn("tag", req.Filters.Tags) + addIn("createdBy", req.Filters.CreatedBy) + + if req.Filters.ParentID != nil { + filters = append(filters, entity.CaseFieldFilter{Field: "parentId", Op: "eq", Values: []string{*req.Filters.ParentID}}) + } + if req.Filters.CreatedByMe { + filters = append(filters, entity.CaseFieldFilter{Field: "createdBy", Op: "eq", Values: []string{currentUserFilterPlaceholder}}) + } + + return entity.SearchCasesRequest{ + Filters: entity.SearchCasesFilters{ + SearchQuery: req.Filters.SearchQuery, + Filters: filters, + }, + SortBy: req.SortBy, + Pagination: req.Pagination, + } +} + // PersonRef is a compact reference to a person (name + email), used for // case creators and assigned engineers. Internal identifiers (entity-service's // UserRef.ID/UserID, AssignedEngineerRef.ID) are intentionally dropped. diff --git a/apps/customer-portal/backend-v2/internal/dto/case_feedback.go b/apps/customer-portal/backend-v2/internal/dto/case_feedback.go index 665ee33198..c0c94a55ce 100644 --- a/apps/customer-portal/backend-v2/internal/dto/case_feedback.go +++ b/apps/customer-portal/backend-v2/internal/dto/case_feedback.go @@ -19,8 +19,7 @@ package dto import "github.com/wso2-open-operations/cs-tools/apps/customer-portal/backend-v2/internal/entity" // CaseFeedbackEmoji is the trimmed emoji reference on GET /cases/{id}/feedback -// — id/name/selectedImage only, dropping unselectedImage/value/chips, matching -// the Ballerina reference's FeedbackEmojiSummary. +// — id/name/selectedImage only, dropping unselectedImage/value/chips. type CaseFeedbackEmoji struct { ID string `json:"id"` Name string `json:"name"` diff --git a/apps/customer-portal/backend-v2/internal/dto/case_test.go b/apps/customer-portal/backend-v2/internal/dto/case_test.go new file mode 100644 index 0000000000..8b37e77439 --- /dev/null +++ b/apps/customer-portal/backend-v2/internal/dto/case_test.go @@ -0,0 +1,120 @@ +// Copyright (c) 2026 WSO2 LLC. (https://www.wso2.com). +// +// WSO2 LLC. licenses this file to you 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 dto + +import ( + "reflect" + "testing" + + "github.com/wso2-open-operations/cs-tools/apps/customer-portal/backend-v2/internal/entity" +) + +// TestBuildEntitySearchCasesRequest_AllFiltersTranslate guards against +// reintroducing the bug where this backend sent entity-service's old +// named-filter-field case-search shape directly instead of building the +// generic filter-predicate array entity-service's current /cases/search +// requires (which rejects unknown fields outright) — every filter the +// portal exposes must produce exactly the CaseFieldFilter entry +// entity-service's case_filters.go expects. +func TestBuildEntitySearchCasesRequest_AllFiltersTranslate(t *testing.T) { + parentID := "parent-id-1" + req := CaseSearchRequest{ + Filters: CaseSearchFilters{ + Types: []string{"case", "engagement"}, + SearchQuery: "login issue", + ProjectIDs: []string{"proj-1"}, + DeploymentIDs: []string{"dep-1"}, + States: []string{"open"}, + Severities: []string{"high"}, + IssueTypes: []string{"question"}, + EngagementTypes: []string{"migration"}, + CreatedBy: []string{"user-1"}, + WorkStates: []string{"ongoing"}, + AssignedUserIDs: []string{"eng-1"}, + ProductNames: []string{"API Manager"}, + Tags: []string{"urgent"}, + ParentID: &parentID, + }, + SortBy: entity.CaseSort{Field: "createdOn", Order: "desc"}, + Pagination: entity.Pagination{Limit: 20, Offset: 0}, + } + + got := BuildEntitySearchCasesRequest(req) + + if got.Filters.SearchQuery != "login issue" { + t.Fatalf("SearchQuery = %q", got.Filters.SearchQuery) + } + if got.SortBy != req.SortBy { + t.Fatalf("SortBy = %+v, want %+v", got.SortBy, req.SortBy) + } + if got.Pagination != req.Pagination { + t.Fatalf("Pagination = %+v, want %+v", got.Pagination, req.Pagination) + } + + want := []entity.CaseFieldFilter{ + {Field: "type", Op: "in", Values: []string{"case", "engagement"}}, + {Field: "projectId", Op: "in", Values: []string{"proj-1"}}, + {Field: "deploymentId", Op: "in", Values: []string{"dep-1"}}, + {Field: "state", Op: "in", Values: []string{"open"}}, + {Field: "severity", Op: "in", Values: []string{"high"}}, + {Field: "issueType", Op: "in", Values: []string{"question"}}, + {Field: "engagementType", Op: "in", Values: []string{"migration"}}, + {Field: "workState", Op: "in", Values: []string{"ongoing"}}, + {Field: "assignedUserId", Op: "in", Values: []string{"eng-1"}}, + {Field: "product", Op: "in", Values: []string{"API Manager"}}, + {Field: "tag", Op: "in", Values: []string{"urgent"}}, + {Field: "createdBy", Op: "in", Values: []string{"user-1"}}, + {Field: "parentId", Op: "eq", Values: []string{"parent-id-1"}}, + } + if !reflect.DeepEqual(got.Filters.Filters, want) { + t.Fatalf("Filters = %+v,\nwant %+v", got.Filters.Filters, want) + } +} + +// TestBuildEntitySearchCasesRequest_CreatedByMe verifies CreatedByMe becomes +// a createdBy+eq filter carrying entity-service's exact current-user +// placeholder string — a mismatch here would make entity-service reject it +// as "createdBy eq only supports values: [...]" instead of resolving the +// caller. +func TestBuildEntitySearchCasesRequest_CreatedByMe(t *testing.T) { + req := CaseSearchRequest{Filters: CaseSearchFilters{CreatedByMe: true}} + + got := BuildEntitySearchCasesRequest(req) + + want := []entity.CaseFieldFilter{ + {Field: "createdBy", Op: "eq", Values: []string{"__current_user_email__"}}, + } + if !reflect.DeepEqual(got.Filters.Filters, want) { + t.Fatalf("Filters = %+v, want %+v", got.Filters.Filters, want) + } +} + +// TestBuildEntitySearchCasesRequest_EmptyFiltersProduceNoPredicates verifies +// an empty/default CaseSearchRequest builds a request with no filter +// predicates at all -- entity-service's own case search already treats a +// missing "type" filter (etc.) as "no restriction", so this must not +// fabricate empty-but-present filter entries. +func TestBuildEntitySearchCasesRequest_EmptyFiltersProduceNoPredicates(t *testing.T) { + got := BuildEntitySearchCasesRequest(CaseSearchRequest{}) + + if len(got.Filters.Filters) != 0 { + t.Fatalf("expected no filter predicates for an empty request, got %+v", got.Filters.Filters) + } + if got.Filters.SearchQuery != "" { + t.Fatalf("expected empty SearchQuery, got %q", got.Filters.SearchQuery) + } +} diff --git a/apps/customer-portal/backend-v2/internal/dto/case_time_cards.go b/apps/customer-portal/backend-v2/internal/dto/case_time_cards.go index f949add28a..6c7ee30c79 100644 --- a/apps/customer-portal/backend-v2/internal/dto/case_time_cards.go +++ b/apps/customer-portal/backend-v2/internal/dto/case_time_cards.go @@ -80,8 +80,7 @@ type CaseTimeCardSearchResponse struct { } // MapCaseTimeCardSearchResponse builds the portal response from entity-service's -// SearchCaseTimeCardsResponse, matching the Ballerina reference's -// mapTimeCardSearchResponseGroupedByCases. +// SearchCaseTimeCardsResponse, grouping time-card entries by their case. func MapCaseTimeCardSearchResponse(r entity.SearchCaseTimeCardsResponse) CaseTimeCardSearchResponse { items := make([]CaseTimeCard, 0, len(r.Cases)) for _, c := range r.Cases { diff --git a/apps/customer-portal/backend-v2/internal/dto/contacts.go b/apps/customer-portal/backend-v2/internal/dto/contacts.go index 8e4c846394..bc09e835a0 100644 --- a/apps/customer-portal/backend-v2/internal/dto/contacts.go +++ b/apps/customer-portal/backend-v2/internal/dto/contacts.go @@ -22,7 +22,7 @@ import ( "github.com/wso2-open-operations/cs-tools/apps/customer-portal/backend-v2/internal/usermanagement" ) -// contactEmailRE matches the Ballerina reference's constraint on +// contactEmailRE matches this API's own constraint on // ContactOnboardPayload.ContactEmail exactly. var contactEmailRE = regexp.MustCompile(`^[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}$`) diff --git a/apps/customer-portal/backend-v2/internal/dto/conversation.go b/apps/customer-portal/backend-v2/internal/dto/conversation.go index 7097e68560..905d47afe4 100644 --- a/apps/customer-portal/backend-v2/internal/dto/conversation.go +++ b/apps/customer-portal/backend-v2/internal/dto/conversation.go @@ -34,7 +34,7 @@ type ConversationDetails struct { } // MapConversationDetails builds the portal response from entity-service's -// ConversationDetails, matching the Ballerina reference's mapConversationResponse. +// ConversationDetails. func MapConversationDetails(r entity.ConversationDetails) ConversationDetails { out := ConversationDetails{ ID: r.ID, @@ -69,10 +69,10 @@ const ( ) // conversationStatusToState maps the portal's PATCH /conversations/{id} -// status values to entity-service's ConversationState enum, matching the -// Ballerina reference's configurable conversationStateIds lookup table -// (open:1, active:2, resolved:3, converted:4, abandonded:5, close:6) — -// only the three states this endpoint can transition to are represented here. +// status values to entity-service's ConversationState enum, using this API's +// own conversation state ID lookup (open:1, active:2, resolved:3, converted:4, +// abandonded:5, close:6) — only the three states this endpoint can transition +// to are represented here. var conversationStatusToState = map[string]string{ ConversationStatusClosed: "CLOSED", ConversationStatusAbandoned: "ABANDONED", diff --git a/apps/customer-portal/backend-v2/internal/dto/deployed_product_metrics.go b/apps/customer-portal/backend-v2/internal/dto/deployed_product_metrics.go index 88aff72296..e7e3cf13b9 100644 --- a/apps/customer-portal/backend-v2/internal/dto/deployed_product_metrics.go +++ b/apps/customer-portal/backend-v2/internal/dto/deployed_product_metrics.go @@ -30,15 +30,13 @@ type DateRangePayload struct { EndDate string `json:"endDate"` } -// IsInvalidDateRange reports whether startDate is after endDate, matching -// the Ballerina reference's isInvalidDateRange. +// IsInvalidDateRange reports whether startDate is after endDate. func IsInvalidDateRange(startDate, endDate string) bool { return startDate != "" && endDate != "" && startDate > endDate } // IsWithinOneYear reports whether the span between startDate and endDate -// (both YYYY-MM-DD) is at most one year, matching the Ballerina reference's -// isWithinOneYear. ok is false if either date fails to parse. +// (both YYYY-MM-DD) is at most one year. ok is false if either date fails to parse. func IsWithinOneYear(startDate, endDate string) (withinOneYear, ok bool) { if len(startDate) != 10 || len(endDate) != 10 { return false, false @@ -64,8 +62,8 @@ func IsWithinOneYear(startDate, endDate string) (withinOneYear, ok bool) { } // DeployedProductRef is the deployed-product reference embedded in both -// deployed-product metrics responses — id/name only, matching the Ballerina -// reference's own reshaping (drops entity-service's number/internalId/count). +// deployed-product metrics responses — id/name only, dropping entity-service's +// number/internalId/count. type DeployedProductRef struct { ID string `json:"id"` Name string `json:"name"` @@ -114,7 +112,7 @@ type DeployedProductMetricsResponse struct { } // MapDeployedProductMetrics builds the portal response from entity-service's -// DeployedProductMetricsResponse, matching the Ballerina reference's mapDeployedProductMetrics. +// DeployedProductMetricsResponse. func MapDeployedProductMetrics(r entity.DeployedProductMetricsResponse) DeployedProductMetricsResponse { chartData := make([]DeployedProductMetricsChartEntry, 0, len(r.ChartData)) for _, e := range r.ChartData { @@ -189,8 +187,7 @@ type DeployedProductUsageCountsResponse struct { } // MapDeployedProductUsageCounts builds the portal response from entity-service's -// DeployedProductUsageCountsResponse, matching the Ballerina reference's -// mapDeployedProductMetricsUsageCounts. +// DeployedProductUsageCountsResponse. func MapDeployedProductUsageCounts(r entity.DeployedProductUsageCountsResponse) DeployedProductUsageCountsResponse { countTypes := make(map[string]CountTypeAggregation, len(r.Summary.CountTypes)) for k, v := range r.Summary.CountTypes { diff --git a/apps/customer-portal/backend-v2/internal/dto/escalation.go b/apps/customer-portal/backend-v2/internal/dto/escalation.go index cba8dc474f..b30e48d311 100644 --- a/apps/customer-portal/backend-v2/internal/dto/escalation.go +++ b/apps/customer-portal/backend-v2/internal/dto/escalation.go @@ -22,8 +22,7 @@ import ( "github.com/wso2-open-operations/cs-tools/apps/customer-portal/backend-v2/internal/entity" ) -// Escalation actions accepted by POST /cases/{caseId}/escalations, matching -// the Ballerina reference's ESCALATION_ACTION_ESCALATE/DEESCALATE constants. +// Escalation actions accepted by POST /cases/{caseId}/escalations. const ( EscalationActionEscalate = "ESCALATE" EscalationActionDeescalate = "DEESCALATE" @@ -39,10 +38,9 @@ type EscalationCreateRequest struct { // ValidateEscalationAction normalizes req.Action (uppercased, defaulted to // ESCALATE) and validates it against the allow-list, plus the "reason -// required when escalating" rule — matching the Ballerina reference's inline -// validation in its create-escalation resource function exactly. ok is -// false when the action is invalid or a required reason is missing/blank; -// errMsg is the caller-safe message to return as a 400 in that case. +// required when escalating" rule. ok is false when the action is invalid or +// a required reason is missing/blank; errMsg is the caller-safe message to +// return as a 400 in that case. func ValidateEscalationAction(req EscalationCreateRequest) (action string, ok bool, errMsg string) { action = EscalationActionEscalate if req.Action != nil { @@ -103,7 +101,7 @@ type EscalationCreateResponse struct { } // MapEscalationCreateResponse builds the portal response from entity-service's -// CreateEscalationResponse, matching the Ballerina reference's mapCreatedEscalation. +// CreateEscalationResponse. func MapEscalationCreateResponse(r entity.CreateEscalationResponse) EscalationCreateResponse { e := r.Escalation return EscalationCreateResponse{ @@ -131,7 +129,7 @@ type EscalationSearchSort struct { // EscalationSearchRequest is the portal's request body for // POST /cases/{caseId}/escalations/search — caseId is never client-supplied, // always injected from the path (any caseIds the client sends are discarded), -// matching the Ballerina reference exactly. +// by design. type EscalationSearchRequest struct { SortBy *EscalationSearchSort `json:"sortBy,omitempty"` Pagination entity.Pagination `json:"pagination"` @@ -173,7 +171,7 @@ type EscalationSearchResponse struct { } // MapEscalationSearchResponse builds the portal response from entity-service's -// SearchEscalationsResponse, matching the Ballerina reference's mapEscalationsResponse. +// SearchEscalationsResponse. func MapEscalationSearchResponse(r entity.SearchEscalationsResponse) EscalationSearchResponse { escalations := make([]Escalation, 0, len(r.Escalations)) for _, e := range r.Escalations { diff --git a/apps/customer-portal/backend-v2/internal/dto/global.go b/apps/customer-portal/backend-v2/internal/dto/global.go index 576de48015..67c483a5a2 100644 --- a/apps/customer-portal/backend-v2/internal/dto/global.go +++ b/apps/customer-portal/backend-v2/internal/dto/global.go @@ -54,12 +54,11 @@ func mapFeedbackEmojis(emojis []entity.FeedbackEmoji) []FeedbackEmoji { return out } -// FeatureFlags mirrors the Ballerina backend's own `configurable -// types:FeatureFlags featureFlags` — a portal-only value that never comes -// from entity-service at all, injected directly into the metadata response. -// Hardcoded to the Ballerina reference's own default; if this needs to vary -// per environment, wire it to an env var the same way other configurable -// values in this backend are read in cmd/server/main.go. +// FeatureFlags is a portal-only value that never comes from entity-service +// at all, injected directly into the metadata response. Hardcoded to this +// API's own default; if this needs to vary per environment, wire it to an +// env var the same way other configurable values in this backend are read +// in cmd/server/main.go. type FeatureFlags struct { UsageMetricsEnabled bool `json:"usageMetricsEnabled"` } @@ -75,7 +74,7 @@ type MetadataResponse struct { } // MapMetadataResponse builds the portal response from entity-service's -// SystemMetadataResponse, matching the Ballerina reference's mapMetadataResponse. +// SystemMetadataResponse. func MapMetadataResponse(r entity.SystemMetadataResponse) MetadataResponse { return MetadataResponse{ TimeZones: mapChoiceListItems(r.TimeZones), @@ -85,8 +84,8 @@ func MapMetadataResponse(r entity.SystemMetadataResponse) MetadataResponse { } } -// globalSearchProjectsMaxLimit caps GlobalSearchRequest.ProjectsPagination.Limit, -// matching the Ballerina reference's GLOBAL_SEARCH_PROJECTS_MAX_LIMIT constant. +// globalSearchProjectsMaxLimit caps GlobalSearchRequest.ProjectsPagination.Limit +// at this API's own maximum. const globalSearchProjectsMaxLimit = 50 // GlobalSearchFilters holds the optional filter criteria for POST /search. @@ -111,9 +110,9 @@ type GlobalSearchRequest struct { } // BuildEntityGlobalSearchRequest translates the portal's global-search -// request into entity-service's shape, matching the Ballerina reference's -// globalSearch wrapper: renames filters.types to entity's filters.tables and -// caps projectsPagination.limit at globalSearchProjectsMaxLimit. +// request into entity-service's shape: renames filters.types to entity's +// filters.tables and caps projectsPagination.limit at +// globalSearchProjectsMaxLimit. func BuildEntityGlobalSearchRequest(req GlobalSearchRequest) entity.GlobalSearchRequest { out := entity.GlobalSearchRequest{} if req.Filters != nil { @@ -179,8 +178,7 @@ type GlobalSearchResponse struct { } // MapGlobalSearchResponse builds the portal response from entity-service's -// GlobalSearchResponse, matching the Ballerina reference's globalSearch -// wrapper's result remapping. +// GlobalSearchResponse. func MapGlobalSearchResponse(r entity.GlobalSearchResponse) GlobalSearchResponse { projects := make([]GlobalSearchProject, 0, len(r.Projects)) for _, p := range r.Projects { diff --git a/apps/customer-portal/backend-v2/internal/dto/instance.go b/apps/customer-portal/backend-v2/internal/dto/instance.go index ab3e1d973e..07911f999c 100644 --- a/apps/customer-portal/backend-v2/internal/dto/instance.go +++ b/apps/customer-portal/backend-v2/internal/dto/instance.go @@ -19,10 +19,9 @@ // searchInstances/searchInstanceMetrics/searchInstanceUsage/ // searchInstanceMetricsStats/searchInstanceUsageStats is exposed as three // portal routes (project-scoped, deployment-scoped, deployed-product-scoped) -// that each force exactly one ID filter from the URL path, matching the -// Ballerina reference's own fan-out. The mapping logic itself is identical -// across all three scopes — only the handler layer differs in which path -// param feeds which entity filter field. +// that each force exactly one ID filter from the URL path. The mapping logic +// itself is identical across all three scopes — only the handler layer +// differs in which path param feeds which entity filter field. package dto import "github.com/wso2-open-operations/cs-tools/apps/customer-portal/backend-v2/internal/entity" @@ -96,7 +95,7 @@ type InstanceSearchResponse struct { } // MapInstanceSearchResponse builds the portal response from entity-service's -// SearchInstancesResponse, matching the Ballerina reference's mapInstancesResponse. +// SearchInstancesResponse. func MapInstanceSearchResponse(r entity.SearchInstancesResponse) InstanceSearchResponse { items := make([]Instance, 0, len(r.Instances)) for _, i := range r.Instances { @@ -169,7 +168,7 @@ type InstanceMetricsResponse struct { } // MapInstanceMetricsResponse builds the portal response from entity-service's -// InstanceMetricsResponse, matching the Ballerina reference's mapInstanceMetrics. +// InstanceMetricsResponse. func MapInstanceMetricsResponse(r entity.InstanceMetricsResponse) InstanceMetricsResponse { metrics := make([]InstanceMetric, 0, len(r.Metrics)) for _, m := range r.Metrics { @@ -226,7 +225,7 @@ type InstanceUsageResponse struct { } // MapInstanceUsageResponse builds the portal response from entity-service's -// InstanceUsageResponse, matching the Ballerina reference's mapInstanceUsages. +// InstanceUsageResponse. func MapInstanceUsageResponse(r entity.InstanceUsageResponse) InstanceUsageResponse { usages := make([]InstanceUsageEntry, 0, len(r.Usages)) for _, u := range r.Usages { @@ -251,10 +250,10 @@ func MapInstanceUsageResponse(r entity.InstanceUsageResponse) InstanceUsageRespo // InstanceStatsRequest is the portal's request body for the // instances/stats/metrics/search and instances/stats/usages/search route // families. StartDate/EndDate are required; DataSource is optional (1 = API -// Call, 2 = File Upload). Note: the Ballerina reference's project-scoped -// metrics-stats variant does NOT forward DataSource to entity-service (a -// real asymmetry, not a bug) — see InstanceMetricsStatsRequestFilters's doc -// comment on the handler side for how this is preserved. +// Call, 2 = File Upload). Note: the project-scoped metrics-stats variant does +// NOT forward DataSource to entity-service (a real asymmetry, by design, not +// a bug) — see InstanceMetricsStatsRequestFilters's doc comment on the +// handler side for how this is preserved. type InstanceStatsRequest struct { StartDate string `json:"startDate"` EndDate string `json:"endDate"` @@ -280,8 +279,7 @@ type InstanceMetricsStatsResponse struct { } // MapInstanceMetricsStatsResponse builds the portal response from -// entity-service's InstanceMetricsStatsResponse, matching the Ballerina -// reference's mapInstanceMetricStats. +// entity-service's InstanceMetricsStatsResponse. func MapInstanceMetricsStatsResponse(r entity.InstanceMetricsStatsResponse) InstanceMetricsStatsResponse { return InstanceMetricsStatsResponse{ Stats: r.Stats, @@ -299,7 +297,7 @@ func MapInstanceMetricsStatsResponse(r entity.InstanceMetricsStatsResponse) Inst // InstanceUsageStatsResponse is the portal's response for the // instances/stats/usages/search routes. Unlike InstanceMetricsStatsResponse, -// there is no summary block, matching the Ballerina reference exactly. +// there is no summary block, by design. type InstanceUsageStatsResponse struct { Stats map[string]map[string]int `json:"stats"` Total int `json:"total"` @@ -308,8 +306,7 @@ type InstanceUsageStatsResponse struct { } // MapInstanceUsageStatsResponse builds the portal response from -// entity-service's InstanceUsageStatsResponse, matching the Ballerina -// reference's mapInstanceUsageStats. +// entity-service's InstanceUsageStatsResponse. func MapInstanceUsageStatsResponse(r entity.InstanceUsageStatsResponse) InstanceUsageStatsResponse { return InstanceUsageStatsResponse{ Stats: r.Stats, diff --git a/apps/customer-portal/backend-v2/internal/dto/project_stats.go b/apps/customer-portal/backend-v2/internal/dto/project_stats.go index a722c2950e..c330f2338d 100644 --- a/apps/customer-portal/backend-v2/internal/dto/project_stats.go +++ b/apps/customer-portal/backend-v2/internal/dto/project_stats.go @@ -19,10 +19,10 @@ package dto import "github.com/wso2-open-operations/cs-tools/apps/customer-portal/backend-v2/internal/entity" // ReferenceItem is a flattened {id, label, count?} view of entity-service's -// ChoiceListItem/ReferenceTableItem — the Ballerina backend this is -// rewriting collapses both into one uniform shape for every project -// metadata/stats endpoint, dropping ReferenceTableItem's number/internalId -// fields (not useful for a filter dropdown or a stats breakdown). +// ChoiceListItem/ReferenceTableItem — this API collapses both into one +// uniform shape for every project metadata/stats endpoint, dropping +// ReferenceTableItem's number/internalId fields (not useful for a filter +// dropdown or a stats breakdown). type ReferenceItem struct { ID string `json:"id"` Label string `json:"label"` @@ -51,8 +51,7 @@ func mapReferenceTableItems(items []entity.ReferenceTableItem) []ReferenceItem { // restrictedChangeRequestStateIDs are excluded from ProjectFilterOptions' // changeRequestStates — internal ServiceNow workflow states never meant to -// be offered as a customer-facing filter option, matching the Ballerina -// backend's own restrictedChangeRequestStateIds default. +// be offered as a customer-facing filter option. var restrictedChangeRequestStateIDs = map[string]bool{"-3": true, "-4": true, "-5": true} // ProjectFilterOptions is the portal's response for GET /projects/{id}/filters @@ -149,17 +148,15 @@ func MapProjectFeatures(m entity.ProjectMetadataResponse) ProjectFeatures { } // caseStateIDOpen is the ServiceNow case state ID meaning "open", used to -// pick the open-case count out of a case-stats state breakdown. Matches the -// Ballerina backend's own default (stateIdOpen/caseStateIds.open) — both -// configurable there; if cs-tools' ServiceNow instance uses different case -// state IDs, this needs to become configurable here too. +// pick the open-case count out of a case-stats state breakdown. This is +// this API's own default; if cs-tools' ServiceNow instance uses different +// case state IDs, this needs to become configurable here too. const caseStateIDOpen = "1" // conversationStateID{Open,Active,Resolved,Abandoned} are the ServiceNow // conversation state IDs used to pick specific counts out of a -// conversation-stats state breakdown. Match the Ballerina backend's own -// default conversationStateIds — see caseStateIDOpen's doc comment on the -// same caveat. +// conversation-stats state breakdown. These are this API's own defaults — +// see caseStateIDOpen's doc comment on the same caveat. const ( conversationStateIDOpen = "1" conversationStateIDActive = "2" @@ -206,10 +203,9 @@ type ProjectDashboardStats struct { } // BuildProjectDashboardStats combines up to four independently-fetched stats -// responses into the dashboard view, mirroring the Ballerina backend's own -// graceful-degradation behavior: any source that failed to load is passed as -// nil and its fields are simply omitted from the response, rather than -// failing the whole request. +// responses into the dashboard view, using graceful-degradation behavior: +// any source that failed to load is passed as nil and its fields are simply +// omitted from the response, rather than failing the whole request. func BuildProjectDashboardStats( caseStats *entity.ProjectCaseStatsResponse, conversationStats *entity.ProjectConversationStatsResponse, @@ -321,9 +317,9 @@ func MapProjectCaseStats(r entity.ProjectCaseStatsResponse) ProjectCaseStats { // ConversationStats is the portal's response for GET /projects/{id}/conversations/stats // — a handful of specific counts picked out of entity-service's state -// breakdown, matching the Ballerina backend's own thinner response (which -// also drops the converted/session/total counts an internal -// OverallConversationStats-equivalent would carry). +// breakdown; this is deliberately a thinner response, which also drops the +// converted/session/total counts an internal OverallConversationStats-equivalent +// would carry. type ConversationStats struct { OpenCount *int `json:"openCount,omitempty"` ActiveCount *int `json:"activeCount,omitempty"` @@ -352,9 +348,9 @@ type ProjectSupportStats struct { } // BuildProjectSupportStats combines independently-fetched case and -// conversation stats into the support-stats view, mirroring the Ballerina -// backend's own graceful-degradation behavior for this endpoint: either -// source may be nil (failed to load) without failing the whole request. +// conversation stats into the support-stats view, using graceful-degradation +// behavior for this endpoint: either source may be nil (failed to load) +// without failing the whole request. func BuildProjectSupportStats(caseStats *entity.ProjectCaseStatsResponse, conversationStats *entity.ProjectConversationStatsResponse) ProjectSupportStats { var out ProjectSupportStats if caseStats != nil { diff --git a/apps/customer-portal/backend-v2/internal/dto/registry.go b/apps/customer-portal/backend-v2/internal/dto/registry.go index 6a390a2211..5427d08f76 100644 --- a/apps/customer-portal/backend-v2/internal/dto/registry.go +++ b/apps/customer-portal/backend-v2/internal/dto/registry.go @@ -22,7 +22,7 @@ import ( "github.com/wso2-open-operations/cs-tools/apps/customer-portal/backend-v2/internal/registry" ) -// robotNameRE matches the Ballerina reference's constraint on +// robotNameRE matches this API's own constraint on // RegistryTokenCreateRequest.RobotName exactly: alphanumeric and dashes only. var robotNameRE = regexp.MustCompile(`^[a-zA-Z0-9\-]+$`) diff --git a/apps/customer-portal/backend-v2/internal/dto/time_card.go b/apps/customer-portal/backend-v2/internal/dto/time_card.go index ced0d77044..29aec7410e 100644 --- a/apps/customer-portal/backend-v2/internal/dto/time_card.go +++ b/apps/customer-portal/backend-v2/internal/dto/time_card.go @@ -31,8 +31,7 @@ type TimeCardCaseRef struct { // (timeAnalyzing, timeSettingUp, timeReproducingDebugging, // timeProvidingSolution, timePatching), issueComplexity, workLogComment, // rejectionReason, and the eligible-approvers list — internal WSO2 support -// bookkeeping the Ballerina backend this is rewriting never exposed to the -// customer portal either. +// bookkeeping never exposed to the customer portal. type TimeCardSummary struct { ID string `json:"id"` TotalTime float64 `json:"totalTime"` diff --git a/apps/customer-portal/backend-v2/internal/dto/user.go b/apps/customer-portal/backend-v2/internal/dto/user.go index c6a6caaeda..e47f89f15c 100644 --- a/apps/customer-portal/backend-v2/internal/dto/user.go +++ b/apps/customer-portal/backend-v2/internal/dto/user.go @@ -16,9 +16,8 @@ // Package dto defines the portal-facing response shapes returned to the // customer-portal frontend, together with Map* functions that translate -// entity-service's raw response structs into them. This is the Go analogue -// of the Ballerina backend's "types" module + utils.bal mapper functions: -// the frontend never sees an entity-service struct directly, only these. +// entity-service's raw response structs into them: the frontend never sees +// an entity-service struct directly, only these. package dto import "github.com/wso2-open-operations/cs-tools/apps/customer-portal/backend-v2/internal/entity" diff --git a/apps/customer-portal/backend-v2/internal/entity/client.go b/apps/customer-portal/backend-v2/internal/entity/client.go index 1f80951790..90480ebd2c 100644 --- a/apps/customer-portal/backend-v2/internal/entity/client.go +++ b/apps/customer-portal/backend-v2/internal/entity/client.go @@ -137,6 +137,36 @@ func NewClient(cfg Config) *Client { } } +// maxErrBodyBytes bounds how much of an error response body newUpstreamError +// keeps, whether or not it parses as JSON. +const maxErrBodyBytes = 256 + +// entityErrorBody mirrors entity-service's apierror.WriteJSON output: +// {"code": , "message": ""}. +type entityErrorBody struct { + Message string `json:"message"` +} + +// newUpstreamError builds an *apierror.Error whose Body is entity-service's +// own "message" field — the specific, caller-safe validation reason +// (apierror.ValidationError et al. are written for exactly this purpose) — +// rather than the raw {"code":...,"message":"..."} JSON blob, so callers can +// both log the exact reason and, for 400s, surface it to the frontend +// verbatim instead of a generic fallback string. Falls back to a raw-body +// excerpt if the response isn't the expected shape (e.g. a gateway error +// page instead of an entity-service response). +func newUpstreamError(statusCode int, rawBody []byte) *apierror.Error { + var body entityErrorBody + if err := json.Unmarshal(rawBody, &body); err == nil && body.Message != "" { + return &apierror.Error{StatusCode: statusCode, Body: body.Message} + } + excerpt := rawBody + if len(excerpt) > maxErrBodyBytes { + excerpt = excerpt[:maxErrBodyBytes] + } + return &apierror.Error{StatusCode: statusCode, Body: string(excerpt)} +} + // do executes an authenticated HTTP request against entity-service and // returns the raw JSON response body. The caller owns the returned slice. func (c *Client) do(ctx context.Context, method, path string, body []byte) ([]byte, error) { @@ -175,12 +205,7 @@ func (c *Client) do(ctx context.Context, method, path string, body []byte) ([]by } if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { - const maxErrBody = 256 - excerpt := respBody - if len(excerpt) > maxErrBody { - excerpt = excerpt[:maxErrBody] - } - return nil, &apierror.Error{StatusCode: resp.StatusCode, Body: string(excerpt)} + return nil, newUpstreamError(resp.StatusCode, respBody) } return respBody, nil @@ -223,12 +248,8 @@ func (c *Client) doBinary(ctx context.Context, path string) (body []byte, conten } if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { - const maxErrBody = 256 - excerpt := respBody - if len(excerpt) > maxErrBody { - excerpt = excerpt[:maxErrBody] - } - return nil, "", &apierror.Error{StatusCode: resp.StatusCode, Body: string(excerpt)} + err := newUpstreamError(resp.StatusCode, respBody) + return nil, "", err } ct := resp.Header.Get("Content-Type") diff --git a/apps/customer-portal/backend-v2/internal/entity/client_test.go b/apps/customer-portal/backend-v2/internal/entity/client_test.go new file mode 100644 index 0000000000..162c78b9e3 --- /dev/null +++ b/apps/customer-portal/backend-v2/internal/entity/client_test.go @@ -0,0 +1,58 @@ +// Copyright (c) 2026 WSO2 LLC. (https://www.wso2.com). +// +// WSO2 LLC. licenses this file to you 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 entity + +import ( + "net/http" + "testing" +) + +func TestNewUpstreamError_ExtractsMessageField(t *testing.T) { + raw := []byte(`{"code":400,"message":"caseTypes must be valid UUIDs"}`) + + err := newUpstreamError(http.StatusBadRequest, raw) + + if err.StatusCode != http.StatusBadRequest { + t.Fatalf("expected status 400, got %d", err.StatusCode) + } + if err.Body != "caseTypes must be valid UUIDs" { + t.Fatalf("expected extracted message, got %q", err.Body) + } +} + +func TestNewUpstreamError_FallsBackToRawBodyWhenNotJSON(t *testing.T) { + raw := []byte("502 Bad Gateway") + + err := newUpstreamError(http.StatusBadGateway, raw) + + if err.Body != string(raw) { + t.Fatalf("expected raw body fallback, got %q", err.Body) + } +} + +func TestNewUpstreamError_TruncatesLongRawBody(t *testing.T) { + raw := make([]byte, maxErrBodyBytes+100) + for i := range raw { + raw[i] = 'x' + } + + err := newUpstreamError(http.StatusInternalServerError, raw) + + if len(err.Body) != maxErrBodyBytes { + t.Fatalf("expected body truncated to %d bytes, got %d", maxErrBodyBytes, len(err.Body)) + } +} diff --git a/apps/customer-portal/backend-v2/internal/entity/types.go b/apps/customer-portal/backend-v2/internal/entity/types.go index 4567d86702..db9de27ae5 100644 --- a/apps/customer-portal/backend-v2/internal/entity/types.go +++ b/apps/customer-portal/backend-v2/internal/entity/types.go @@ -405,28 +405,34 @@ type CaseSort struct { Order string `json:"order,omitempty"` } -// SearchCasesFilters holds the optional filter criteria for a case search. -// Only the fields the portal currently exposes are included here; extend as -// needed when more filters are surfaced to the frontend. +// CaseFieldFilter is one predicate in a case search's generic filter +// expression array: "field op values". entity-service redesigned its case +// search contract to this shape (from the old one-named-field-per-filter +// style dto.CaseSearchRequest still exposes to the portal frontend) — see +// dto.BuildEntitySearchCasesRequest, which is the only place that builds one +// of these, and entity-service's own openapi.yaml for the exact supported +// field/op combinations. +type CaseFieldFilter struct { + Field string `json:"field"` + Op string `json:"op"` + Values []string `json:"values,omitempty"` +} + +// SearchCasesFilters is entity-service's current case-search filter +// contract. SearchQuery stays special-cased (a free-text match, not a field +// predicate); every other criterion is an entry in Filters. OrGroups is not +// populated by this backend — the portal doesn't currently expose +// cross-field OR filtering. type SearchCasesFilters struct { - Types []string `json:"types,omitempty"` - SearchQuery string `json:"searchQuery,omitempty"` - ProjectIDs []string `json:"projectIds,omitempty"` - DeploymentIDs []string `json:"deploymentIds,omitempty"` - States []string `json:"states,omitempty"` - Severities []string `json:"severities,omitempty"` - IssueTypes []string `json:"issueTypes,omitempty"` - EngagementTypes []string `json:"engagementTypes,omitempty"` - CreatedBy []string `json:"createdBy,omitempty"` - CreatedByMe bool `json:"createdByMe,omitempty"` - WorkStates []string `json:"workStates,omitempty"` - AssignedUserIDs []string `json:"assignedUserIds,omitempty"` - ProductNames []string `json:"productNames,omitempty"` - Tags []string `json:"tags,omitempty"` - ParentID *string `json:"parentId,omitempty"` -} - -// SearchCasesRequest is the input for POST /cases/search. + SearchQuery string `json:"searchQuery,omitempty"` + Filters []CaseFieldFilter `json:"filters,omitempty"` + OrGroups [][]CaseFieldFilter `json:"orGroups,omitempty"` +} + +// SearchCasesRequest is entity-service's current wire format for +// POST /cases/search. Never decode a portal request directly into this — +// see dto.CaseSearchRequest (the portal's own stable, unchanged contract) +// and dto.BuildEntitySearchCasesRequest. type SearchCasesRequest struct { Filters SearchCasesFilters `json:"filters"` SortBy CaseSort `json:"sortBy"` @@ -1300,10 +1306,9 @@ type TimeCardCaseRef struct { // TimeCardView is a single time card in search results. entity-service also // returns per-category time breakdowns (timeAnalyzing, timeSettingUp, etc.), // issueComplexity, workLogComment, rejectionReason, and the eligible-approvers -// list — internal WSO2 support bookkeeping the Ballerina backend never -// exposed to the customer portal either (see its thinner TimeCard type); -// mirrored here only insofar as this backend actually decodes them, but see -// dto.TimeCardSummary for what the portal exposes. +// list — internal WSO2 support bookkeeping this backend does not expose to +// the customer portal; mirrored here only insofar as this backend actually +// decodes them, but see dto.TimeCardSummary for what the portal exposes. type TimeCardView struct { ID string `json:"id"` TotalTime float64 `json:"totalTime"` diff --git a/apps/customer-portal/backend-v2/internal/handler/ai_chat.go b/apps/customer-portal/backend-v2/internal/handler/ai_chat.go index 9adabf239c..cfaef1571b 100644 --- a/apps/customer-portal/backend-v2/internal/handler/ai_chat.go +++ b/apps/customer-portal/backend-v2/internal/handler/ai_chat.go @@ -194,13 +194,13 @@ func (h *AIChatHandler) GetConversationMessages(w http.ResponseWriter, r *http.R writeJSONValue(w, http.StatusOK, dto.MapSearchComments(result)) } -// agentReplyCreatedBy documents a known gap versus the Ballerina reference: +// agentReplyCreatedBy documents a known limitation of this endpoint: // entity-service's CreateCommentRequest has no createdBy override (comments // are always attributed to the caller's own identity via their auth token), -// unlike the Ballerina backend, which tags the AI's own reply with a -// distinct "agent" identity (entity:CHAT_SENT_AGENT). Both the user's -// message and the AI's reply are therefore saved here under the calling -// user's identity — there is no way to distinguish them by author alone. +// so there is no way to tag the AI's own reply with a distinct "agent" +// identity. Both the user's message and the AI's reply are therefore saved +// here under the calling user's identity — there is no way to distinguish +// them by author alone. // TODO(entity-service): revisit once/if CreateCommentRequest gains a // createdBy override. const agentReplyCreatedByCaveat = "AI reply comment attributed to caller (see agentReplyCreatedBy doc comment) — entity-service has no createdBy override" @@ -210,15 +210,15 @@ const agentReplyCreatedByCaveat = "AI reply comment attributed to caller (see ag const createEntityStateResolved = "RESOLVED" // CreateConversation handles POST /projects/{id}/conversations — starts a -// brand-new conversation and gets the AI agent's first response, matching -// the Ballerina reference's composite flow: create the conversation, call -// the AI agent, optionally fetch KB recommendations (only when both Region -// and Tier are supplied), persist the AI's reply as a comment, then -// auto-resolve the conversation if the agent reports the issue solved. -// Mirrors the Ballerina reference's own error-handling: conversation -// creation, the AI call, and comment persistence are all fatal (500) on -// failure with no compensating rollback of earlier steps; recommendations -// and auto-resolve are best-effort and never fail the request. +// brand-new conversation and gets the AI agent's first response via this +// composite flow: create the conversation, call the AI agent, optionally +// fetch KB recommendations (only when both Region and Tier are supplied), +// persist the AI's reply as a comment, then auto-resolve the conversation +// if the agent reports the issue solved. +// Error-handling: conversation creation, the AI call, and comment +// persistence are all fatal (500) on failure with no compensating rollback +// of earlier steps; recommendations and auto-resolve are best-effort and +// never fail the request. func (h *AIChatHandler) CreateConversation(w http.ResponseWriter, r *http.Request) { user := middleware.UserInfoFromContext(r.Context()) if user == nil { @@ -305,8 +305,8 @@ func (h *AIChatHandler) CreateConversation(w http.ResponseWriter, r *http.Reques if chatResp.Resolved != nil && *chatResp.Resolved { if _, err := h.entity.UpdateConversation(r.Context(), conversationID, entity.UpdateConversationRequest{State: createEntityStateResolved}); err != nil { - // Best-effort, matching the Ballerina reference: the main flow - // already succeeded, so this failure is logged, not returned. + // Best-effort: the main flow already succeeded, so this + // failure is logged, not returned. slog.ErrorContext(r.Context(), "entity UpdateConversation failed to auto-resolve", "userID", user.UserID, "conversationID", conversationID, "err", summarizeErr(err)) } } @@ -317,8 +317,8 @@ func (h *AIChatHandler) CreateConversation(w http.ResponseWriter, r *http.Reques // SendConversationMessage handles // POST /projects/{projectId}/conversations/{conversationId}/messages — a // follow-up message on an existing conversation. Unlike CreateConversation, -// no recommendations call is ever made here — the Ballerina reference only -// attaches recommendations on a conversation's first message. +// no recommendations call is ever made here — recommendations are only +// attached on a conversation's first message. func (h *AIChatHandler) SendConversationMessage(w http.ResponseWriter, r *http.Request) { user := middleware.UserInfoFromContext(r.Context()) if user == nil { diff --git a/apps/customer-portal/backend-v2/internal/handler/cases.go b/apps/customer-portal/backend-v2/internal/handler/cases.go index ea745fb252..c45194aec8 100644 --- a/apps/customer-portal/backend-v2/internal/handler/cases.go +++ b/apps/customer-portal/backend-v2/internal/handler/cases.go @@ -65,13 +65,13 @@ func (h *CaseHandler) SearchCases(w http.ResponseWriter, r *http.Request) { return } - var req entity.SearchCasesRequest + var req dto.CaseSearchRequest if err := json.Unmarshal(body, &req); err != nil { writeError(w, http.StatusBadRequest, ErrMsgBadRequest) return } - result, err := h.entity.SearchCases(r.Context(), req) + result, err := h.entity.SearchCases(r.Context(), dto.BuildEntitySearchCasesRequest(req)) if err != nil { slog.ErrorContext(r.Context(), "entity SearchCases failed", "userID", user.UserID, "err", summarizeErr(err)) mapUpstreamError(w, err, "Failed to search cases.") @@ -321,8 +321,8 @@ func (h *CaseHandler) SubmitCaseFeedback(w http.ResponseWriter, r *http.Request) // PatchCaseAttachment handles PATCH /cases/{caseId}/attachments/{attachmentId}. // referenceId/referenceType are injected server-side (caseId path param, // ReferenceTypeCase). Only Name is read from the request body — Description -// is never wired through here, matching the Ballerina reference's own -// restriction for this route (case attachments don't carry a description). +// is never wired through here, by design for this route (case attachments +// don't carry a description). func (h *CaseHandler) PatchCaseAttachment(w http.ResponseWriter, r *http.Request) { user := middleware.UserInfoFromContext(r.Context()) if user == nil { @@ -347,7 +347,7 @@ func (h *CaseHandler) PatchCaseAttachment(w http.ResponseWriter, r *http.Request writeError(w, http.StatusBadRequest, ErrMsgBadRequest) return } - req.Description = nil // this route never forwards description, matching the Ballerina reference + req.Description = nil // this route never forwards description (case attachments don't carry one) entityReq := dto.BuildEntityUpdateAttachmentRequest(req, caseID, entity.ReferenceTypeCase) result, err := h.entity.UpdateAttachment(r.Context(), attachmentID, entityReq) diff --git a/apps/customer-portal/backend-v2/internal/handler/contacts.go b/apps/customer-portal/backend-v2/internal/handler/contacts.go index a7ee470d20..fac248afa9 100644 --- a/apps/customer-portal/backend-v2/internal/handler/contacts.go +++ b/apps/customer-portal/backend-v2/internal/handler/contacts.go @@ -165,7 +165,7 @@ func (h *ContactHandler) RemoveProjectContact(w http.ResponseWriter, r *http.Req return } - _ = result // Ballerina reference returns a fixed success message here, not the membership details. + _ = result // this endpoint returns a fixed success message here, not the membership details. writeJSONValue(w, http.StatusOK, map[string]string{"message": "Project contact removed successfully!"}) } diff --git a/apps/customer-portal/backend-v2/internal/handler/deployed_products.go b/apps/customer-portal/backend-v2/internal/handler/deployed_products.go index 7805cd993b..04197f86c3 100644 --- a/apps/customer-portal/backend-v2/internal/handler/deployed_products.go +++ b/apps/customer-portal/backend-v2/internal/handler/deployed_products.go @@ -162,7 +162,7 @@ func (h *DeployedProductHandler) PatchDeployedProduct(w http.ResponseWriter, r * writeJSONValue(w, http.StatusOK, dto.MapDeployedProductUpdate(result)) } -// validateDeployedProductDateRange applies the Ballerina reference's +// validateDeployedProductDateRange applies this endpoint's own // isInvalidDateRange/isWithinOneYear checks, writing a 400 and returning // ok=false on failure. func validateDeployedProductDateRange(w http.ResponseWriter, startDate, endDate string) (ok bool) { @@ -184,8 +184,8 @@ func validateDeployedProductDateRange(w http.ResponseWriter, startDate, endDate // SearchDeployedProductMetrics handles // POST /deployments/{deploymentId}/products/{productId}/metrics/search. -// productId here is the deployed-product's own ID (matching the Ballerina -// reference's own path-naming quirk); deploymentId is injected into the +// productId here is the deployed-product's own ID (a deliberate +// path-naming quirk of this route); deploymentId is injected into the // entity request server-side. func (h *DeployedProductHandler) SearchDeployedProductMetrics(w http.ResponseWriter, r *http.Request) { user := middleware.UserInfoFromContext(r.Context()) diff --git a/apps/customer-portal/backend-v2/internal/handler/instances.go b/apps/customer-portal/backend-v2/internal/handler/instances.go index 67167eaae2..d5cd6e26fc 100644 --- a/apps/customer-portal/backend-v2/internal/handler/instances.go +++ b/apps/customer-portal/backend-v2/internal/handler/instances.go @@ -18,9 +18,9 @@ // (search/metrics/usages/metrics-stats/usages-stats) as 15 portal routes — // each metric type fanned out into a project-scoped, deployment-scoped, and // deployed-product-scoped variant that forces exactly one ID filter from its -// URL path, matching the Ballerina reference's own fan-out exactly. Each -// public method is a thin wrapper around a shared unexported implementation -// differing only in which entity filter field the path param feeds. +// URL path. Each public method is a thin wrapper around a shared unexported +// implementation differing only in which entity filter field the path param +// feeds. package handler import ( @@ -256,11 +256,10 @@ func (h *InstanceHandler) SearchDeployedProductInstanceUsage(w http.ResponseWrit // --- POST .../instances/stats/metrics/search --- // -// NOTE: DataSource is deliberately NOT forwarded to entity-service here, -// matching the Ballerina reference exactly — its metrics-stats resource -// functions never read payload.filters.dataSource even though the portal -// payload type carries it, unlike the stats/usages/search family below, -// which does forward it. Not a bug to "fix"; preserved intentionally. +// NOTE: DataSource is deliberately NOT forwarded to entity-service here — +// this endpoint never reads payload.filters.dataSource even though the +// portal payload type carries it, unlike the stats/usages/search family +// below, which does forward it. Not a bug to "fix"; preserved intentionally. func (h *InstanceHandler) searchInstanceMetricsStats(w http.ResponseWriter, r *http.Request, scope instanceIDFilters, pathID, failureNoun string) { user := middleware.UserInfoFromContext(r.Context()) diff --git a/apps/customer-portal/backend-v2/internal/handler/product_consumption.go b/apps/customer-portal/backend-v2/internal/handler/product_consumption.go index 54b594f86a..bddbdfd0fd 100644 --- a/apps/customer-portal/backend-v2/internal/handler/product_consumption.go +++ b/apps/customer-portal/backend-v2/internal/handler/product_consumption.go @@ -85,7 +85,7 @@ func (h *ProductConsumptionHandler) GetDeploymentLicense(w http.ResponseWriter, // Verify the caller can access this project before provisioning // anything — entity-service's own project-access check is the - // authorization gate here, matching the Ballerina backend's flow. + // authorization gate here. if _, err := h.entity.GetProject(r.Context(), projectID); err != nil { slog.ErrorContext(r.Context(), "entity GetProject failed", "userID", user.UserID, "projectID", projectID, "err", summarizeErr(err)) mapUpstreamError(w, err, "Failed to retrieve project details.") diff --git a/apps/customer-portal/backend-v2/internal/handler/project_stats.go b/apps/customer-portal/backend-v2/internal/handler/project_stats.go index b89bfa25ec..860e5f65ca 100644 --- a/apps/customer-portal/backend-v2/internal/handler/project_stats.go +++ b/apps/customer-portal/backend-v2/internal/handler/project_stats.go @@ -102,10 +102,10 @@ func (h *ProjectStatsHandler) GetProjectFeatures(w http.ResponseWriter, r *http. writeJSONValue(w, http.StatusOK, dto.MapProjectFeatures(result)) } -// GetProjectDashboardStats handles GET /projects/{id}/stats. Mirrors the -// Ballerina backend's own graceful-degradation behavior: each of the four -// underlying stats calls may fail independently without failing the whole -// request — a failed source's fields are simply omitted from the response. +// GetProjectDashboardStats handles GET /projects/{id}/stats. Uses a +// graceful-degradation behavior: each of the four underlying stats calls +// may fail independently without failing the whole request — a failed +// source's fields are simply omitted from the response. func (h *ProjectStatsHandler) GetProjectDashboardStats(w http.ResponseWriter, r *http.Request) { user := middleware.UserInfoFromContext(r.Context()) if user == nil { diff --git a/apps/customer-portal/backend-v2/internal/handler/registry.go b/apps/customer-portal/backend-v2/internal/handler/registry.go index 357c1053e1..ff0d867a6d 100644 --- a/apps/customer-portal/backend-v2/internal/handler/registry.go +++ b/apps/customer-portal/backend-v2/internal/handler/registry.go @@ -80,7 +80,7 @@ func (h *RegistryHandler) isAdmin(roles []string) bool { // service's own error message (apierror.Error.Body) when available, falling // back to a generic message otherwise — several registry/contacts endpoints // deliberately forward the upstream's specific reason rather than a fixed -// fallback, matching the Ballerina reference's response.message() passthrough. +// fallback. func writeUpstreamMessage(w http.ResponseWriter, err error, fallback string) { var apiErr *apierror.Error if errors.As(err, &apiErr) { @@ -280,8 +280,8 @@ func (h *RegistryHandler) DeleteRegistryToken(w http.ResponseWriter, r *http.Req } // tokenID is not UUID-validated: the registry service's own token IDs are - // not UUID-shaped (matching the Ballerina reference's plain `string` path - // param for this route, unlike the project-scoped registry routes above). + // not UUID-shaped, so this route takes a plain `string` path param, + // unlike the project-scoped registry routes above. tokenID := r.PathValue("id") if tokenID == "" { writeError(w, http.StatusBadRequest, ErrMsgBadRequest) diff --git a/apps/customer-portal/backend-v2/internal/handler/response.go b/apps/customer-portal/backend-v2/internal/handler/response.go index 8aaeadd83e..ed106e2ec6 100644 --- a/apps/customer-portal/backend-v2/internal/handler/response.go +++ b/apps/customer-portal/backend-v2/internal/handler/response.go @@ -76,8 +76,17 @@ func writeJSONValue(w http.ResponseWriter, statusCode int, v any) { _, _ = w.Write(data) // #nosec G705 -- Content-Type: application/json already set; SecurityHeaders middleware adds X-Content-Type-Options: nosniff } -// mapUpstreamError translates an entity-service error to an HTTP response, -// mirroring the Ballerina getStatusCode pattern in the customer-portal. +// mapUpstreamError translates an entity-service error to an HTTP response +// using this backend's own status-code mapping. +// +// For 400, apiErr.Body (entity-service's own validation message — e.g. +// "caseTypes must be valid UUIDs", "Reason is required when escalating a +// case.") is returned verbatim instead of a generic fallback: entity-service +// constructs these specifically to be safe and useful to show the caller +// (see its apierror.ValidationError), and swallowing the specific reason +// into "Invalid request payload." for every 400 makes debugging a rejected +// request needlessly harder for API consumers. ErrMsgBadRequest is only used +// when entity-service didn't supply a body at all. func mapUpstreamError(w http.ResponseWriter, err error, fallbackMsg string) { var apiErr *apierror.Error if errors.As(err, &apiErr) { @@ -89,7 +98,11 @@ func mapUpstreamError(w http.ResponseWriter, err error, fallbackMsg string) { case http.StatusNotFound: writeError(w, http.StatusNotFound, ErrMsgNotFound) case http.StatusBadRequest: - writeError(w, http.StatusBadRequest, ErrMsgBadRequest) + msg := apiErr.Body + if msg == "" { + msg = ErrMsgBadRequest + } + writeError(w, http.StatusBadRequest, msg) case http.StatusConflict, http.StatusUnprocessableEntity: writeError(w, apiErr.StatusCode, apiErr.Body) case http.StatusBadGateway, http.StatusServiceUnavailable, http.StatusGatewayTimeout: @@ -102,16 +115,18 @@ func mapUpstreamError(w http.ResponseWriter, err error, fallbackMsg string) { writeError(w, http.StatusInternalServerError, fallbackMsg) } -// summarizeErr returns a short, log-safe description of err: the upstream status -// code for a typed *apierror.Error (never its Body, which may carry upstream -// response data not meant for logs), or a fixed generic message otherwise — an -// unrecognized error can come from the underlying HTTP client (e.g. a +// summarizeErr returns a log-safe description of err: for a typed +// *apierror.Error, the upstream status code AND its exact Body (entity-service's +// own error message, extracted from its {"code","message"} response shape by +// entity.newUpstreamError — never the full raw response, which could carry +// unbounded or binary upstream data) — or a fixed generic message otherwise. +// An unrecognized error can come from the underlying HTTP client (e.g. a // net/url.Error), which stringifies with the full request URL including query // parameters, so its raw text is not safe to log verbatim. func summarizeErr(err error) string { var apiErr *apierror.Error if errors.As(err, &apiErr) { - return fmt.Sprintf("upstream status %d", apiErr.StatusCode) + return fmt.Sprintf("upstream status %d: %s", apiErr.StatusCode, apiErr.Body) } return "upstream request failed" } diff --git a/apps/customer-portal/backend-v2/internal/handler/response_test.go b/apps/customer-portal/backend-v2/internal/handler/response_test.go new file mode 100644 index 0000000000..fc4fb83554 --- /dev/null +++ b/apps/customer-portal/backend-v2/internal/handler/response_test.go @@ -0,0 +1,85 @@ +// Copyright (c) 2026 WSO2 LLC. (https://www.wso2.com). +// +// WSO2 LLC. licenses this file to you 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 handler + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/wso2-open-operations/cs-tools/apps/customer-portal/backend-v2/internal/apierror" +) + +func TestMapUpstreamError_BadRequestPassesThroughUpstreamMessage(t *testing.T) { + err := &apierror.Error{StatusCode: http.StatusBadRequest, Body: "caseTypes must be valid UUIDs"} + rec := httptest.NewRecorder() + + mapUpstreamError(rec, err, "Failed to search cases.") + + if rec.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d", rec.Code) + } + var body errorBody + if decodeErr := json.NewDecoder(rec.Body).Decode(&body); decodeErr != nil { + t.Fatalf("decode response: %v", decodeErr) + } + if body.Message != "caseTypes must be valid UUIDs" { + t.Fatalf("expected upstream message passed through, got %q", body.Message) + } +} + +func TestMapUpstreamError_BadRequestFallsBackWhenBodyEmpty(t *testing.T) { + err := &apierror.Error{StatusCode: http.StatusBadRequest, Body: ""} + rec := httptest.NewRecorder() + + mapUpstreamError(rec, err, "Failed to search cases.") + + var body errorBody + if decodeErr := json.NewDecoder(rec.Body).Decode(&body); decodeErr != nil { + t.Fatalf("decode response: %v", decodeErr) + } + if body.Message != ErrMsgBadRequest { + t.Fatalf("expected generic fallback, got %q", body.Message) + } +} + +func TestMapUpstreamError_UnauthorizedUsesFixedMessageNotUpstreamBody(t *testing.T) { + err := &apierror.Error{StatusCode: http.StatusUnauthorized, Body: "some internal upstream detail"} + rec := httptest.NewRecorder() + + mapUpstreamError(rec, err, "fallback") + + var body errorBody + if decodeErr := json.NewDecoder(rec.Body).Decode(&body); decodeErr != nil { + t.Fatalf("decode response: %v", decodeErr) + } + if body.Message != ErrMsgUnauthorized { + t.Fatalf("expected fixed unauthorized message, got %q", body.Message) + } +} + +func TestSummarizeErr_IncludesUpstreamBody(t *testing.T) { + err := &apierror.Error{StatusCode: http.StatusBadRequest, Body: "caseTypes must be valid UUIDs"} + + got := summarizeErr(err) + + want := "upstream status 400: caseTypes must be valid UUIDs" + if got != want { + t.Fatalf("expected %q, got %q", want, got) + } +} diff --git a/apps/customer-portal/backend-v2/internal/handler/time_cards.go b/apps/customer-portal/backend-v2/internal/handler/time_cards.go index 372b529ee9..edb062652b 100644 --- a/apps/customer-portal/backend-v2/internal/handler/time_cards.go +++ b/apps/customer-portal/backend-v2/internal/handler/time_cards.go @@ -36,9 +36,9 @@ type entityTimeCardClient interface { // TimeCardHandler handles HTTP requests for time-card search. // // NOTE: entity-service only supports time cards on its ServiceNow data -// source — a Postgres-mode deployment 404s on this route. Read-only: the -// Ballerina backend this is rewriting never exposed time-card creation or -// updates to the customer portal, only search. +// source — a Postgres-mode deployment 404s on this route. Read-only: this +// backend never exposes time-card creation or updates to the customer +// portal, only search. type TimeCardHandler struct { entity entityTimeCardClient } diff --git a/apps/customer-portal/backend-v2/internal/handler/websocket.go b/apps/customer-portal/backend-v2/internal/handler/websocket.go index 9f83df205b..93f261a559 100644 --- a/apps/customer-portal/backend-v2/internal/handler/websocket.go +++ b/apps/customer-portal/backend-v2/internal/handler/websocket.go @@ -62,9 +62,9 @@ const wsAutoResolveState = "RESOLVED" // the upstream AI chat agent for an existing conversation. // // NOTE: entity-service has no createConversation exposed over this -// connection, so unlike the Ballerina backend this is rewriting, a WebSocket -// message that doesn't carry an existing conversationId cannot start a -// brand-new conversation here — the caller must first create one via +// connection, so a WebSocket message that doesn't carry an existing +// conversationId cannot start a brand-new conversation here — the caller +// must first create one via // POST /projects/{id}/conversations (see handler.AIChatHandler.CreateConversation). // The AI agent's own reply IS persisted as a comment here (see // handleMessage), but — like AIChatHandler.SendConversationMessage — it is @@ -114,9 +114,9 @@ type wsEvent struct { } // HandleWebSocket handles GET /ws?sessionId={projectId}. The query parameter -// is named sessionId for wire compatibility with the Ballerina backend this -// is rewriting, but it actually carries the project ID — the AI agent's own -// per-conversation session key is derived below as "{projectId}:{conversationId}". +// is named sessionId for wire compatibility, but it actually carries the +// project ID — the AI agent's own per-conversation session key is derived +// below as "{projectId}:{conversationId}". func (h *WebSocketHandler) HandleWebSocket(w http.ResponseWriter, r *http.Request) { user := middleware.UserInfoFromContext(r.Context()) if user == nil { diff --git a/apps/customer-portal/backend-v2/internal/middleware/auth.go b/apps/customer-portal/backend-v2/internal/middleware/auth.go index 114c2a4af9..fa6afb3b16 100644 --- a/apps/customer-portal/backend-v2/internal/middleware/auth.go +++ b/apps/customer-portal/backend-v2/internal/middleware/auth.go @@ -63,8 +63,8 @@ type Config struct { TokenValidatorEnabled bool } -// jwtClaims defines the expected JWT payload fields, mirroring the Ballerina -// CustomJwtPayload in the customer-portal's authorization module. +// jwtClaims defines the expected JWT payload fields carried in the +// customer portal's x-jwt-assertion token: email, userid, and groups. type jwtClaims struct { Email string `json:"email"` UserID string `json:"userid"` @@ -214,7 +214,8 @@ func hasAnyAudience(tokenAuds jwt.ClaimStrings, expected []string) bool { return false } -// addSecurityHeaders mirrors the Ballerina ResponseInterceptor security headers. +// addSecurityHeaders sets the standard security response headers required on +// every response. func addSecurityHeaders(w http.ResponseWriter) { w.Header().Set("X-Content-Type-Options", "nosniff") w.Header().Set("Content-Security-Policy", "upgrade-insecure-requests") diff --git a/apps/customer-portal/backend-v2/internal/middleware/cors.go b/apps/customer-portal/backend-v2/internal/middleware/cors.go new file mode 100644 index 0000000000..7d2f2389d3 --- /dev/null +++ b/apps/customer-portal/backend-v2/internal/middleware/cors.go @@ -0,0 +1,75 @@ +// Copyright (c) 2026 WSO2 LLC. (https://www.wso2.com). +// +// WSO2 LLC. licenses this file to you 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 middleware + +import "net/http" + +// corsAllowedHeaders lists every request header the frontend may need to send +// on a cross-origin call: the JWT assertion and impersonation headers Auth +// reads, the correlation ID header, and the standard content-type/upload +// headers used by JSON and binary (zip upload) request bodies. +const corsAllowedHeaders = "Content-Type, x-jwt-assertion, x-user-id-token, X-CSM-Correlation-ID" + +// CORS returns an HTTP middleware that handles cross-origin requests from the +// browser-based frontend. It MUST be the outermost middleware in the chain +// (wrapping Auth, not wrapped by it): a CORS preflight is an OPTIONS request +// with no Authorization/JWT header at all, so if Auth ran first it would +// reject every preflight with 401 before the browser ever saw a CORS header +// — which is exactly what a browser reports as a "blocked by CORS policy" +// error, even though the real cause is auth rejecting the preflight, not a +// CORS misconfiguration. +// +// allowedOrigins is a comma-separated-then-split allow-list of browser +// Origins (see splitComma in cmd/server/main.go); an empty list allows any +// origin, matching this backend's other Origin-gated feature +// (handler.NewWebSocketHandler) and is intended for local development only. +// +// Deliberately never sets Access-Control-Allow-Credentials: this backend +// authenticates via a caller-supplied x-jwt-assertion header (see +// middleware.Auth), never cookies, so there is no session credential for a +// browser to attach automatically — reflecting an arbitrary Origin back is +// safe only as long as that stays true. If this backend ever adds +// cookie-based auth, allowedOrigins MUST become a real non-empty allow-list +// before Access-Control-Allow-Credentials could safely be added, since the +// two together (any origin + credentials) let any site read authenticated +// responses on the victim's behalf. +func CORS(allowedOrigins []string) func(http.Handler) http.Handler { + allowed := make(map[string]bool, len(allowedOrigins)) + for _, o := range allowedOrigins { + allowed[o] = true + } + + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + origin := r.Header.Get("Origin") + if origin != "" && (len(allowed) == 0 || allowed[origin]) { + w.Header().Set("Access-Control-Allow-Origin", origin) + w.Header().Set("Vary", "Origin") + } + + if r.Method == http.MethodOptions && r.Header.Get("Access-Control-Request-Method") != "" { + w.Header().Set("Access-Control-Allow-Methods", "GET, POST, PATCH, DELETE, OPTIONS") + w.Header().Set("Access-Control-Allow-Headers", corsAllowedHeaders) + w.Header().Set("Access-Control-Max-Age", "3600") + w.WriteHeader(http.StatusNoContent) + return + } + + next.ServeHTTP(w, r) + }) + } +} diff --git a/apps/customer-portal/backend-v2/internal/middleware/cors_test.go b/apps/customer-portal/backend-v2/internal/middleware/cors_test.go new file mode 100644 index 0000000000..168c4a536d --- /dev/null +++ b/apps/customer-portal/backend-v2/internal/middleware/cors_test.go @@ -0,0 +1,113 @@ +// Copyright (c) 2026 WSO2 LLC. (https://www.wso2.com). +// +// WSO2 LLC. licenses this file to you 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 middleware + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +func TestCORSPreflightBypassesAuth(t *testing.T) { + called := false + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + called = true + w.WriteHeader(http.StatusOK) + }) + + handler := CORS([]string{"https://frontend.example.com"})(next) + + req := httptest.NewRequest(http.MethodOptions, "/cases/search", nil) + req.Header.Set("Origin", "https://frontend.example.com") + req.Header.Set("Access-Control-Request-Method", "POST") + rec := httptest.NewRecorder() + + handler.ServeHTTP(rec, req) + + if called { + t.Fatal("preflight should not reach the wrapped handler") + } + if rec.Code != http.StatusNoContent { + t.Fatalf("expected 204, got %d", rec.Code) + } + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "https://frontend.example.com" { + t.Fatalf("expected Allow-Origin echoed, got %q", got) + } + if got := rec.Header().Get("Access-Control-Allow-Methods"); got == "" { + t.Fatal("expected Allow-Methods to be set") + } +} + +func TestCORSRejectsDisallowedOrigin(t *testing.T) { + handler := CORS([]string{"https://frontend.example.com"})(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + + req := httptest.NewRequest(http.MethodGet, "/health", nil) + req.Header.Set("Origin", "https://evil.example.com") + rec := httptest.NewRecorder() + + handler.ServeHTTP(rec, req) + + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "" { + t.Fatalf("expected no Allow-Origin for disallowed origin, got %q", got) + } +} + +// TestCORSNeverSetsAllowCredentials guards against reintroducing +// Access-Control-Allow-Credentials: true alongside an unrestricted (or +// reflected-any) Origin — that combination lets any site read authenticated +// responses on the victim's behalf. This backend authenticates via a +// caller-supplied header, never cookies, so there is no session credential +// for a browser to attach automatically, and no legitimate reason to ever +// set this header — see the doc comment on CORS. +func TestCORSNeverSetsAllowCredentials(t *testing.T) { + handler := CORS(nil)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + + req := httptest.NewRequest(http.MethodGet, "/health", nil) + req.Header.Set("Origin", "https://anything.example.com") + rec := httptest.NewRecorder() + + handler.ServeHTTP(rec, req) + + if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != "" { + t.Fatalf("Access-Control-Allow-Credentials must never be set (this backend has no cookie-based session to protect), got %q", got) + } +} + +func TestCORSActualRequestPassesThrough(t *testing.T) { + called := false + handler := CORS(nil)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + called = true + w.WriteHeader(http.StatusOK) + })) + + req := httptest.NewRequest(http.MethodPost, "/cases/search", nil) + req.Header.Set("Origin", "https://anything.example.com") + rec := httptest.NewRecorder() + + handler.ServeHTTP(rec, req) + + if !called { + t.Fatal("actual request should reach the wrapped handler") + } + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "https://anything.example.com" { + t.Fatalf("expected Allow-Origin echoed when allow-list empty, got %q", got) + } +} diff --git a/apps/customer-portal/backend-v2/internal/productconsumption/client.go b/apps/customer-portal/backend-v2/internal/productconsumption/client.go index 0183c27271..e1e8f5f53d 100644 --- a/apps/customer-portal/backend-v2/internal/productconsumption/client.go +++ b/apps/customer-portal/backend-v2/internal/productconsumption/client.go @@ -18,13 +18,11 @@ // product-consumption service(s) — separate services (not entity-service) // that provision WSO2 API Manager applications/subscriptions/credentials to // generate per-deployment product licenses, and import deployment usage -// data. See apps/customer-portal/backend's modules/product_consumption_subscription -// and modules/product_consumption_tracking for the Ballerina backend's -// equivalent clients: each has its own independently-configurable base URL -// (productConsumptionBaseUrl vs productConsumptionTrackingBaseUrl) — they -// happen to be set to the same value in that backend's current config, but -// are not guaranteed to be, so this package keeps them as two distinct fields -// rather than assuming they're always the same host. +// data. The subscription/license API and the usage-tracking API are two +// independently-configurable base URLs — they may happen to be the same +// host in some deployments, but are not guaranteed to be, so this package +// keeps them as two distinct fields rather than assuming they're always the +// same host. package productconsumption import ( @@ -52,28 +50,26 @@ func noRedirect(_ *http.Request, _ []*http.Request) error { } // Config holds the configuration for the product-consumption service -// clients. TrackingBaseURL defaults to BaseURL when left empty, matching -// this Ballerina reference's current deployment where both configurable -// base URLs happen to be set to the same value — set it explicitly once -// the two services are hosted separately. +// clients. TrackingBaseURL defaults to SubscriptionBaseURL when left empty +// — set it explicitly once the two services are hosted separately. type Config struct { - BaseURL string - TrackingBaseURL string - TokenURL string - ClientID string - ClientSecret string - Scopes []string + SubscriptionBaseURL string + TrackingBaseURL string + TokenURL string + ClientID string + ClientSecret string + Scopes []string } // Client is an HTTP client for the upstream product-consumption service(s), -// authenticated via the OAuth2 client credentials grant. baseURL backs the -// subscription/license API; trackingBaseURL backs the usage-import API — -// two independently-configurable upstream services in the Ballerina -// reference, kept distinct here even though they're often the same host. +// authenticated via the OAuth2 client credentials grant. subscriptionBaseURL +// backs the subscription/license API; trackingBaseURL backs the +// usage-import API — two independently-configurable upstream services, +// kept distinct here even though they're often the same host. type Client struct { - http *http.Client - baseURL string - trackingBaseURL string + http *http.Client + subscriptionBaseURL string + trackingBaseURL string } // NewClient constructs a Client that authenticates against the @@ -96,19 +92,19 @@ func NewClient(cfg Config) *Client { oauthClient := cc.Client(tokenCtx) httpClient := &http.Client{ Transport: oauthClient.Transport, - Timeout: 300 * time.Second, // matches the Ballerina client's own 300s timeout + Timeout: 300 * time.Second, CheckRedirect: noRedirect, } trackingBaseURL := cfg.TrackingBaseURL if trackingBaseURL == "" { - trackingBaseURL = cfg.BaseURL + trackingBaseURL = cfg.SubscriptionBaseURL } return &Client{ - http: httpClient, - baseURL: strings.TrimRight(cfg.BaseURL, "/"), - trackingBaseURL: strings.TrimRight(trackingBaseURL, "/"), + http: httpClient, + subscriptionBaseURL: strings.TrimRight(cfg.SubscriptionBaseURL, "/"), + trackingBaseURL: strings.TrimRight(trackingBaseURL, "/"), } } @@ -154,7 +150,7 @@ func (c *Client) doAt(ctx context.Context, baseURL, method, path, contentType st } func (c *Client) do(ctx context.Context, method, path, contentType string, body []byte) ([]byte, error) { - return c.doAt(ctx, c.baseURL, method, path, contentType, body) + return c.doAt(ctx, c.subscriptionBaseURL, method, path, contentType, body) } func (c *Client) postJSON(ctx context.Context, path string, reqBody, out any) error { @@ -190,8 +186,7 @@ func (c *Client) postJSONTracking(ctx context.Context, path string, reqBody, out } // postText issues a POST with a raw text/plain body — used only for -// subscribeApplication, which mirrors the Ballerina backend's -// `.post(applicationId)` call: Ballerina's http:Client sends a bare `string` +// subscribeApplication, whose upstream endpoint expects a bare string // payload as raw text/plain, not a JSON-quoted string. func (c *Client) postText(ctx context.Context, path, body string, out any) error { respBody, err := c.do(ctx, http.MethodPost, path, "text/plain", []byte(body)) diff --git a/apps/customer-portal/backend-v2/internal/productconsumption/subscription.go b/apps/customer-portal/backend-v2/internal/productconsumption/subscription.go index 0d8de7446a..7f228d39d3 100644 --- a/apps/customer-portal/backend-v2/internal/productconsumption/subscription.go +++ b/apps/customer-portal/backend-v2/internal/productconsumption/subscription.go @@ -152,8 +152,8 @@ func (c *Client) updateProjectStatus(ctx context.Context, projectID string, req } // subscribeApplication calls POST /applications/{applicationId}/subscribe. -// The request body is the bare applicationId string, matching the upstream -// Ballerina client's own `.post(applicationId)` call. +// The request body is the bare applicationId string, as the upstream +// endpoint expects. func (c *Client) subscribeApplication(ctx context.Context, applicationID string) (ApplicationSubscriptionResponse, error) { var out ApplicationSubscriptionResponse err := c.postText(ctx, fmt.Sprintf("/applications/%s/subscribe", pathEscape(applicationID)), applicationID, &out) diff --git a/apps/customer-portal/backend-v2/internal/productconsumption/types.go b/apps/customer-portal/backend-v2/internal/productconsumption/types.go index 13afef2949..145b9fbab5 100644 --- a/apps/customer-portal/backend-v2/internal/productconsumption/types.go +++ b/apps/customer-portal/backend-v2/internal/productconsumption/types.go @@ -17,9 +17,7 @@ package productconsumption // These types mirror the upstream product-consumption service's wire format -// 1:1 (see apps/customer-portal/backend's modules/product_consumption_subscription -// and modules/product_consumption_tracking, the Ballerina backend this is -// rewriting) so json.Unmarshal can decode its responses directly. +// 1:1 so json.Unmarshal can decode its responses directly. // consumptionStatus enumerates the product-consumption service's per-project // license-provisioning state machine. diff --git a/apps/customer-portal/backend-v2/internal/usermanagement/client.go b/apps/customer-portal/backend-v2/internal/usermanagement/client.go index 1a9f7d83dc..c95f2ca79b 100644 --- a/apps/customer-portal/backend-v2/internal/usermanagement/client.go +++ b/apps/customer-portal/backend-v2/internal/usermanagement/client.go @@ -148,8 +148,7 @@ func (c *Client) do(ctx context.Context, method, path string, body []byte) (stat // becomes an *apierror.Error whose Body is the upstream's own "message" // field when present (see extractErrorMessage in usermanagement.go) — this // service deliberately surfaces its own error text to the caller rather than -// a generic fallback, matching the Ballerina reference's response.message() -// passthrough. +// a generic fallback. func (c *Client) doJSON(ctx context.Context, method, path string, body []byte, wantStatus int) ([]byte, error) { statusCode, respBody, err := c.do(ctx, method, path, body) if err != nil { diff --git a/apps/customer-portal/backend-v2/internal/usermanagement/types.go b/apps/customer-portal/backend-v2/internal/usermanagement/types.go index cb4129bc15..61403a055d 100644 --- a/apps/customer-portal/backend-v2/internal/usermanagement/types.go +++ b/apps/customer-portal/backend-v2/internal/usermanagement/types.go @@ -168,7 +168,7 @@ type ValidationPayload struct { } // getRoles converts the four role booleans into the role-name slice the -// upstream service expects, matching the Ballerina reference's getRoles exactly. +// upstream service expects, in the exact order and shape its role field requires. func getRoles(isCsAdmin, isLead, isPortalUser, isSecurityContact bool) []string { var roles []string if isCsAdmin { @@ -187,8 +187,7 @@ func getRoles(isCsAdmin, isLead, isPortalUser, isSecurityContact bool) []string } // hasRole reports whether role appears in the upstream's semicolon-delimited -// role string, trimming whitespace around each part — matches the Ballerina -// reference's hasRole exactly. A nil roleValue never matches. +// role string, trimming whitespace around each part. A nil roleValue never matches. func hasRole(roleValue *string, role string) bool { if roleValue == nil { return false diff --git a/apps/customer-portal/backend-v2/internal/usermanagement/usermanagement.go b/apps/customer-portal/backend-v2/internal/usermanagement/usermanagement.go index 6d8252ec6a..01bc511dcd 100644 --- a/apps/customer-portal/backend-v2/internal/usermanagement/usermanagement.go +++ b/apps/customer-portal/backend-v2/internal/usermanagement/usermanagement.go @@ -105,8 +105,7 @@ func (c *Client) UpdateMembershipRole(ctx context.Context, projectID, contactEma } // ValidateProjectContact calls POST /validate-project-contact. The upstream -// service uses the HTTP status code itself to distinguish three outcomes, -// matching the Ballerina reference's Contact|error? union: +// service uses the HTTP status code itself to distinguish three outcomes: // - 201 Created: a deactivated contact with this email already exists — // contact is non-nil, conflict is false, err is nil. // - 202 Accepted: the contact is new and can be onboarded — contact is @@ -152,7 +151,7 @@ type upstreamErrorBody struct { // extractErrorMessage builds an *apierror.Error whose Body is the upstream // service's own "message" field when present, so callers can surface the // specific reason (e.g. "Contact already exists") rather than a generic -// fallback — matching the Ballerina reference's response.message() passthrough. +// fallback. func extractErrorMessage(statusCode int, raw []byte) error { var body upstreamErrorBody if err := json.Unmarshal(raw, &body); err == nil && body.Message != "" { diff --git a/apps/customer-portal/backend-v2/openapi.yaml b/apps/customer-portal/backend-v2/openapi.yaml index 6f8cafd7e8..5e1ff6c809 100644 --- a/apps/customer-portal/backend-v2/openapi.yaml +++ b/apps/customer-portal/backend-v2/openapi.yaml @@ -3083,8 +3083,8 @@ paths: This is a WebSocket upgrade endpoint — OpenAPI 3.0 has no native support for describing WebSocket message framing, so only the upgrade handshake is documented here. The sessionId query parameter - is named for wire compatibility with the Ballerina backend this - replaces, but actually carries the project ID. + is named for wire compatibility with existing clients, but actually + carries the project ID. operationId: getWs parameters: - name: sessionId @@ -3093,8 +3093,8 @@ paths: schema: type: string description: >- - Project ID — named sessionId for wire compatibility with the - Ballerina backend this replaces. + Project ID — named sessionId for wire compatibility with + existing clients. responses: "101": description: Switching Protocols @@ -3712,8 +3712,8 @@ paths: patch: summary: Update a case's attachment. description: > - Only name is forwarded here — description is always dropped, - matching the Ballerina reference's own restriction for this route. + Only name is forwarded here — description is always dropped, by + design for this route. operationId: patchCasesCaseIdAttachmentsAttachmentId parameters: - name: caseId @@ -4309,9 +4309,8 @@ paths: filters.projectIds is always forced to [id] server-side. Note: dataSource is NOT forwarded to entity-service for this project-scoped metrics-stats variant — a deliberate asymmetry versus the - stats/usages/search family, matching the Ballerina reference exactly - (see InstanceStatsRequest's doc comment). One of 3 scope variants - fanned out from entity-service's single + stats/usages/search family (see InstanceStatsRequest's doc comment). + One of 3 scope variants fanned out from entity-service's single POST /instances/stats/metrics/search. operationId: postProjectsIdInstancesStatsMetricsSearch parameters: @@ -8371,8 +8370,7 @@ components: $ref: '#/components/schemas/ReferenceItem' feedbackEmojies: description: >- - Sic — matches the Ballerina reference's own (misspelled) field - name on the wire. + Sic — this is the actual field name on the wire (misspelled). type: array items: $ref: '#/components/schemas/FeedbackEmoji' @@ -8399,7 +8397,7 @@ components: order: type: string projectsPagination: - description: limit is capped at 50 server-side, matching the Ballerina reference. + description: limit is capped at 50 server-side. $ref: '#/components/schemas/Pagination' casesPagination: $ref: '#/components/schemas/Pagination' @@ -8559,8 +8557,7 @@ components: CaseFeedbackEmoji: description: >- The trimmed emoji reference on GET /cases/{id}/feedback — id/name/ - selectedImage only, matching the Ballerina reference's - FeedbackEmojiSummary. + selectedImage only. type: object required: [id, name, selectedImage] properties: @@ -8674,8 +8671,7 @@ components: referenceId/referenceType are never client-supplied — each handler injects them from its own path params. The case-scoped route (PATCH /cases/{caseId}/attachments/{attachmentId}) only ever reads - name, never description, matching the Ballerina reference's own - restriction for that route. + name, never description, by design for that route. type: object properties: name: @@ -9297,8 +9293,7 @@ components: dataSource is optional (1 = API Call, 2 = File Upload). Note: the project-scoped .../instances/stats/metrics/search variant does NOT forward dataSource to entity-service — a deliberate asymmetry versus - the stats/usages/search family, matching the Ballerina reference - exactly. + the stats/usages/search family. type: object required: [startDate, endDate] properties: @@ -9345,8 +9340,7 @@ components: InstanceUsageStatsResponse: description: >- - Unlike InstanceMetricsStatsResponse, there is no summary block, - matching the Ballerina reference exactly. + Unlike InstanceMetricsStatsResponse, there is no summary block. type: object required: [stats, total, startDate, endDate] properties: From 9e1f2c7e55b1328d0014d5782ba4da2c0ea72e29 Mon Sep 17 00:00:00 2001 From: Rashmika998 Date: Thu, 6 Aug 2026 10:26:41 +0530 Subject: [PATCH 3/6] fix(customer-portal,entity-service): address CodeRabbit review findings on PR #1378 - asyncapi.yaml: fix a stale UserMessage.conversationId description pointing at the follow-up-message endpoint (which requires conversationId, not returns one) instead of the actual conversation-creation endpoint. - dto/conversation.go: fix "abandonded"/"close" typos and remove a numeric state-ID list that described a different layer's (entity-service's SN translation) internal representation, not this map's actual string values. - entity/client.go: newUpstreamError no longer falls back to a raw upstream body excerpt when the response isn't the documented {"message":...} shape -- Body is left empty instead, so summarizeErr's logs and mapUpstreamError's 400 passthrough can never surface unbounded, non-message upstream content. - entity-service/openapi.yaml: add the default_case alias to both the caseTypes query param and CreateCaseRequest.type enums (the description already documented it as accepted, but the enum itself rejected it); drop attachments' minItems: 1 to match security_report_analysis's actual optional-attachments validation. Co-Authored-By: Claude Sonnet 5 --- apps/customer-portal/backend-v2/asyncapi.yaml | 10 +++++---- .../backend-v2/internal/dto/conversation.go | 7 +++--- .../backend-v2/internal/entity/client.go | 19 ++++++---------- .../backend-v2/internal/entity/client_test.go | 22 ++++++++++--------- .../backend-v2/internal/handler/response.go | 19 ++++++++++------ .../internal/handler/response_test.go | 16 ++++++++++++++ entity-service/openapi.yaml | 10 +++++---- 7 files changed, 62 insertions(+), 41 deletions(-) diff --git a/apps/customer-portal/backend-v2/asyncapi.yaml b/apps/customer-portal/backend-v2/asyncapi.yaml index 4a90d8b3f1..c318135c2d 100644 --- a/apps/customer-portal/backend-v2/asyncapi.yaml +++ b/apps/customer-portal/backend-v2/asyncapi.yaml @@ -117,10 +117,12 @@ components: type: string format: uuid description: > - Required — the ID of an existing conversation (obtained via - POST /projects/{projectId}/conversations/{conversationId}/messages - or another entity-service-backed flow). Omitting it returns an - ErrorMessage; this server cannot mint a new conversation ID. + Required — the ID of an existing conversation, obtained from + the response of POST /projects/{id}/conversations (which + starts a new conversation) or GET /conversations/{id}. This + WebSocket connection only resumes a conversation; it never + mints a new conversation ID itself. Omitting conversationId + returns an ErrorMessage. envProducts: type: object nullable: true diff --git a/apps/customer-portal/backend-v2/internal/dto/conversation.go b/apps/customer-portal/backend-v2/internal/dto/conversation.go index 905d47afe4..1e381bf0fd 100644 --- a/apps/customer-portal/backend-v2/internal/dto/conversation.go +++ b/apps/customer-portal/backend-v2/internal/dto/conversation.go @@ -69,10 +69,9 @@ const ( ) // conversationStatusToState maps the portal's PATCH /conversations/{id} -// status values to entity-service's ConversationState enum, using this API's -// own conversation state ID lookup (open:1, active:2, resolved:3, converted:4, -// abandonded:5, close:6) — only the three states this endpoint can transition -// to are represented here. +// status values to entity-service's ConversationState enum string values — +// only the three states this endpoint can transition to are represented +// here. var conversationStatusToState = map[string]string{ ConversationStatusClosed: "CLOSED", ConversationStatusAbandoned: "ABANDONED", diff --git a/apps/customer-portal/backend-v2/internal/entity/client.go b/apps/customer-portal/backend-v2/internal/entity/client.go index 90480ebd2c..57249e8498 100644 --- a/apps/customer-portal/backend-v2/internal/entity/client.go +++ b/apps/customer-portal/backend-v2/internal/entity/client.go @@ -137,10 +137,6 @@ func NewClient(cfg Config) *Client { } } -// maxErrBodyBytes bounds how much of an error response body newUpstreamError -// keeps, whether or not it parses as JSON. -const maxErrBodyBytes = 256 - // entityErrorBody mirrors entity-service's apierror.WriteJSON output: // {"code": , "message": ""}. type entityErrorBody struct { @@ -152,19 +148,18 @@ type entityErrorBody struct { // (apierror.ValidationError et al. are written for exactly this purpose) — // rather than the raw {"code":...,"message":"..."} JSON blob, so callers can // both log the exact reason and, for 400s, surface it to the frontend -// verbatim instead of a generic fallback string. Falls back to a raw-body -// excerpt if the response isn't the expected shape (e.g. a gateway error -// page instead of an entity-service response). +// verbatim instead of a generic fallback string. Body is left empty if the +// response isn't the expected shape (e.g. a gateway error page instead of an +// entity-service response): callers already treat an empty Body as "no +// specific message available" and fall back to a generic one, and this +// deliberately avoids ever logging or returning to the frontend a raw +// upstream body of unknown, unbounded content. func newUpstreamError(statusCode int, rawBody []byte) *apierror.Error { var body entityErrorBody if err := json.Unmarshal(rawBody, &body); err == nil && body.Message != "" { return &apierror.Error{StatusCode: statusCode, Body: body.Message} } - excerpt := rawBody - if len(excerpt) > maxErrBodyBytes { - excerpt = excerpt[:maxErrBodyBytes] - } - return &apierror.Error{StatusCode: statusCode, Body: string(excerpt)} + return &apierror.Error{StatusCode: statusCode} } // do executes an authenticated HTTP request against entity-service and diff --git a/apps/customer-portal/backend-v2/internal/entity/client_test.go b/apps/customer-portal/backend-v2/internal/entity/client_test.go index 162c78b9e3..4d6395acb5 100644 --- a/apps/customer-portal/backend-v2/internal/entity/client_test.go +++ b/apps/customer-portal/backend-v2/internal/entity/client_test.go @@ -34,25 +34,27 @@ func TestNewUpstreamError_ExtractsMessageField(t *testing.T) { } } -func TestNewUpstreamError_FallsBackToRawBodyWhenNotJSON(t *testing.T) { +// TestNewUpstreamError_LeavesBodyEmptyWhenNotJSON guards against ever +// logging or returning to the frontend a raw, unbounded upstream response +// body (e.g. a gateway error page) — Body must stay empty so callers' +// existing "empty Body means no specific message" fallback kicks in, +// instead of surfacing arbitrary upstream content. +func TestNewUpstreamError_LeavesBodyEmptyWhenNotJSON(t *testing.T) { raw := []byte("502 Bad Gateway") err := newUpstreamError(http.StatusBadGateway, raw) - if err.Body != string(raw) { - t.Fatalf("expected raw body fallback, got %q", err.Body) + if err.Body != "" { + t.Fatalf("expected empty Body for a non-JSON response, got %q", err.Body) } } -func TestNewUpstreamError_TruncatesLongRawBody(t *testing.T) { - raw := make([]byte, maxErrBodyBytes+100) - for i := range raw { - raw[i] = 'x' - } +func TestNewUpstreamError_LeavesBodyEmptyWhenMessageFieldMissing(t *testing.T) { + raw := []byte(`{"code":500}`) err := newUpstreamError(http.StatusInternalServerError, raw) - if len(err.Body) != maxErrBodyBytes { - t.Fatalf("expected body truncated to %d bytes, got %d", maxErrBodyBytes, len(err.Body)) + if err.Body != "" { + t.Fatalf("expected empty Body when message field is absent, got %q", err.Body) } } diff --git a/apps/customer-portal/backend-v2/internal/handler/response.go b/apps/customer-portal/backend-v2/internal/handler/response.go index ed106e2ec6..e2eed9f79f 100644 --- a/apps/customer-portal/backend-v2/internal/handler/response.go +++ b/apps/customer-portal/backend-v2/internal/handler/response.go @@ -116,16 +116,21 @@ func mapUpstreamError(w http.ResponseWriter, err error, fallbackMsg string) { } // summarizeErr returns a log-safe description of err: for a typed -// *apierror.Error, the upstream status code AND its exact Body (entity-service's -// own error message, extracted from its {"code","message"} response shape by -// entity.newUpstreamError — never the full raw response, which could carry -// unbounded or binary upstream data) — or a fixed generic message otherwise. -// An unrecognized error can come from the underlying HTTP client (e.g. a -// net/url.Error), which stringifies with the full request URL including query -// parameters, so its raw text is not safe to log verbatim. +// *apierror.Error, the upstream status code and, if present, its Body +// (entity-service's own error message, extracted from its +// {"code","message"} response shape by entity.newUpstreamError — Body is +// left empty rather than falling back to a raw response excerpt, so this +// never logs unbounded or non-message upstream content) — or a fixed +// generic message otherwise. An unrecognized error can come from the +// underlying HTTP client (e.g. a net/url.Error), which stringifies with the +// full request URL including query parameters, so its raw text is not safe +// to log verbatim. func summarizeErr(err error) string { var apiErr *apierror.Error if errors.As(err, &apiErr) { + if apiErr.Body == "" { + return fmt.Sprintf("upstream status %d", apiErr.StatusCode) + } return fmt.Sprintf("upstream status %d: %s", apiErr.StatusCode, apiErr.Body) } return "upstream request failed" diff --git a/apps/customer-portal/backend-v2/internal/handler/response_test.go b/apps/customer-portal/backend-v2/internal/handler/response_test.go index fc4fb83554..27de6aa420 100644 --- a/apps/customer-portal/backend-v2/internal/handler/response_test.go +++ b/apps/customer-portal/backend-v2/internal/handler/response_test.go @@ -83,3 +83,19 @@ func TestSummarizeErr_IncludesUpstreamBody(t *testing.T) { t.Fatalf("expected %q, got %q", want, got) } } + +// TestSummarizeErr_OmitsEmptyBody guards against ever logging a raw, +// unbounded upstream response body: entity.newUpstreamError leaves Body +// empty when the upstream response isn't the documented {"message":...} +// shape, and summarizeErr must not turn that into a misleading +// "status N: " with a dangling empty message. +func TestSummarizeErr_OmitsEmptyBody(t *testing.T) { + err := &apierror.Error{StatusCode: http.StatusBadGateway, Body: ""} + + got := summarizeErr(err) + + want := "upstream status 502" + if got != want { + t.Fatalf("expected %q, got %q", want, got) + } +} diff --git a/entity-service/openapi.yaml b/entity-service/openapi.yaml index ab19d07204..48aeb8f020 100644 --- a/entity-service/openapi.yaml +++ b/entity-service/openapi.yaml @@ -648,7 +648,7 @@ paths: type: array items: type: string - enum: [case, service_request, security_report_analysis, announcement, engagement] + enum: [case, default_case, service_request, security_report_analysis, announcement, engagement] description: > Restrict to these case types, one of case/service_request/ security_report_analysis/announcement/engagement (matching the @@ -6010,7 +6010,7 @@ components: properties: type: type: string - enum: [case, service_request, security_report_analysis, announcement, engagement] + enum: [case, default_case, service_request, security_report_analysis, announcement, engagement] description: | Case type. `default_case` is also accepted as an alias for `case` (ServiceNow's own raw caseType wire value; kept for the @@ -6061,10 +6061,12 @@ components: description: Required for `service_request` type. attachments: type: array - minItems: 1 items: $ref: '#/components/schemas/CaseAttachment' - description: Required for `security_report_analysis` type (at least one entry). + description: >- + Optional even for `security_report_analysis` type — the frontend + creates the case first, then uploads attachments in separate + requests per file, so this may be omitted or empty. relatedCaseId: type: string format: uuid From 5252c91b8d273a405fe90e0cafa3d204c8911226 Mon Sep 17 00:00:00 2001 From: Rashmika998 Date: Thu, 6 Aug 2026 11:08:32 +0530 Subject: [PATCH 4/6] fix(customer-portal): centralize upstream-error body extraction, fix Vary header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every upstream client (registry, updates, scim, productconsumption, aichatagent, usermanagement) built its own non-2xx apierror.Error by falling back to a raw response excerpt when the body wasn't {"message": "..."} shaped — the same raw-content-leak risk already fixed for the entity client. Add apierror.NewUpstreamError as the one shared constructor (Body is the parsed message, or empty) and route every client through it instead of duplicating the logic. Also switch cors.go's Vary header to Add so it doesn't clobber a Vary value a downstream handler already set. Co-Authored-By: Claude Sonnet 5 --- apps/customer-portal/backend-v2/CLAUDE.md | 12 +++++--- .../backend-v2/internal/aichatagent/client.go | 7 +---- .../backend-v2/internal/apierror/apierror.go | 30 ++++++++++++++++++- .../apierror_test.go} | 8 ++--- .../backend-v2/internal/entity/client.go | 29 ++---------------- .../backend-v2/internal/middleware/cors.go | 2 +- .../internal/productconsumption/client.go | 7 +---- .../backend-v2/internal/registry/client.go | 7 +---- .../backend-v2/internal/scim/client.go | 7 +---- .../backend-v2/internal/updates/client.go | 7 +---- .../internal/usermanagement/client.go | 3 +- .../internal/usermanagement/usermanagement.go | 27 ++--------------- 12 files changed, 53 insertions(+), 93 deletions(-) rename apps/customer-portal/backend-v2/internal/{entity/client_test.go => apierror/apierror_test.go} (90%) diff --git a/apps/customer-portal/backend-v2/CLAUDE.md b/apps/customer-portal/backend-v2/CLAUDE.md index 2da36ea4d9..97e7c2975e 100644 --- a/apps/customer-portal/backend-v2/CLAUDE.md +++ b/apps/customer-portal/backend-v2/CLAUDE.md @@ -551,10 +551,14 @@ constants). `summarizeErr` DOES include the upstream status and message for a typed `*apierror.Error` (e.g. `"upstream status 400: caseTypes must be valid UUIDs"`) — entity-service's error bodies are already caller-safe validation text, not sensitive internal detail, so logging them verbatim is - fine. `internal/entity/client.go`'s `newUpstreamError` is what populates `apiErr.Body` with just - the extracted `message` field (not entity-service's raw `{"code","message"}` response) — every - new upstream-error construction site in that file must go through it rather than reinventing - inline body-truncation, or the message-extraction and the 400 passthrough above silently break. + fine. `apierror.NewUpstreamError` (`internal/apierror/apierror.go`) is what populates `apiErr.Body` + with just the extracted `message` field (not the upstream's raw response) — every upstream + client in this backend (entity, registry, updates, scim, productconsumption, aichatagent, + usermanagement) constructs its non-2xx errors through this one shared function rather than each + reinventing its own inline body-truncation/excerpt fallback. Body is left empty when the response + isn't the expected `{"message": "..."}` shape, relying on each caller's existing "empty Body → generic + fallback" logic (`mapUpstreamError`'s 400 case, `writeUpstreamMessage`) — never add a new + upstream-error construction site that falls back to a raw excerpt instead of calling this function. ## Security diff --git a/apps/customer-portal/backend-v2/internal/aichatagent/client.go b/apps/customer-portal/backend-v2/internal/aichatagent/client.go index 1b4302782e..414373072a 100644 --- a/apps/customer-portal/backend-v2/internal/aichatagent/client.go +++ b/apps/customer-portal/backend-v2/internal/aichatagent/client.go @@ -141,12 +141,7 @@ func (c *Client) do(ctx context.Context, method, path string, body []byte) ([]by } if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { - const maxErrBody = 256 - excerpt := respBody - if len(excerpt) > maxErrBody { - excerpt = excerpt[:maxErrBody] - } - return nil, &apierror.Error{StatusCode: resp.StatusCode, Body: string(excerpt)} + return nil, apierror.NewUpstreamError(resp.StatusCode, respBody) } return respBody, nil diff --git a/apps/customer-portal/backend-v2/internal/apierror/apierror.go b/apps/customer-portal/backend-v2/internal/apierror/apierror.go index 28fa844cce..e6c1cdc696 100644 --- a/apps/customer-portal/backend-v2/internal/apierror/apierror.go +++ b/apps/customer-portal/backend-v2/internal/apierror/apierror.go @@ -18,7 +18,10 @@ // HTTP failures back through the client layer to the handler layer. package apierror -import "fmt" +import ( + "encoding/json" + "fmt" +) // Error wraps a non-2xx response from an upstream service call. type Error struct { @@ -29,3 +32,28 @@ type Error struct { func (e *Error) Error() string { return fmt.Sprintf("upstream returned %d: %s", e.StatusCode, e.Body) } + +// upstreamErrorBody is the {"message": "..."} shape every upstream service +// this backend calls uses for its own error responses (entity-service's is a +// superset, {"code":...,"message":"..."}, which unmarshals the same way). +type upstreamErrorBody struct { + Message string `json:"message"` +} + +// NewUpstreamError builds an *Error from a non-2xx upstream HTTP response. +// Body is set to the upstream's own "message" field when the response is the +// expected {"message": "..."} shape, and left empty otherwise. Every upstream +// client in this backend must construct its errors through this function +// rather than falling back to a raw response excerpt: callers already treat +// an empty Body as "no specific message available" (both mapUpstreamError's +// 400 case and writeUpstreamMessage fall back to a fixed message), so a raw +// excerpt is never necessary — and logging or returning one to the frontend +// risks leaking unbounded, non-message upstream content (e.g. a gateway HTML +// error page). +func NewUpstreamError(statusCode int, rawBody []byte) *Error { + var body upstreamErrorBody + if err := json.Unmarshal(rawBody, &body); err == nil && body.Message != "" { + return &Error{StatusCode: statusCode, Body: body.Message} + } + return &Error{StatusCode: statusCode} +} diff --git a/apps/customer-portal/backend-v2/internal/entity/client_test.go b/apps/customer-portal/backend-v2/internal/apierror/apierror_test.go similarity index 90% rename from apps/customer-portal/backend-v2/internal/entity/client_test.go rename to apps/customer-portal/backend-v2/internal/apierror/apierror_test.go index 4d6395acb5..52c9c59aef 100644 --- a/apps/customer-portal/backend-v2/internal/entity/client_test.go +++ b/apps/customer-portal/backend-v2/internal/apierror/apierror_test.go @@ -14,7 +14,7 @@ // specific language governing permissions and limitations // under the License. -package entity +package apierror import ( "net/http" @@ -24,7 +24,7 @@ import ( func TestNewUpstreamError_ExtractsMessageField(t *testing.T) { raw := []byte(`{"code":400,"message":"caseTypes must be valid UUIDs"}`) - err := newUpstreamError(http.StatusBadRequest, raw) + err := NewUpstreamError(http.StatusBadRequest, raw) if err.StatusCode != http.StatusBadRequest { t.Fatalf("expected status 400, got %d", err.StatusCode) @@ -42,7 +42,7 @@ func TestNewUpstreamError_ExtractsMessageField(t *testing.T) { func TestNewUpstreamError_LeavesBodyEmptyWhenNotJSON(t *testing.T) { raw := []byte("502 Bad Gateway") - err := newUpstreamError(http.StatusBadGateway, raw) + err := NewUpstreamError(http.StatusBadGateway, raw) if err.Body != "" { t.Fatalf("expected empty Body for a non-JSON response, got %q", err.Body) @@ -52,7 +52,7 @@ func TestNewUpstreamError_LeavesBodyEmptyWhenNotJSON(t *testing.T) { func TestNewUpstreamError_LeavesBodyEmptyWhenMessageFieldMissing(t *testing.T) { raw := []byte(`{"code":500}`) - err := newUpstreamError(http.StatusInternalServerError, raw) + err := NewUpstreamError(http.StatusInternalServerError, raw) if err.Body != "" { t.Fatalf("expected empty Body when message field is absent, got %q", err.Body) diff --git a/apps/customer-portal/backend-v2/internal/entity/client.go b/apps/customer-portal/backend-v2/internal/entity/client.go index 57249e8498..5da1fee016 100644 --- a/apps/customer-portal/backend-v2/internal/entity/client.go +++ b/apps/customer-portal/backend-v2/internal/entity/client.go @@ -137,31 +137,6 @@ func NewClient(cfg Config) *Client { } } -// entityErrorBody mirrors entity-service's apierror.WriteJSON output: -// {"code": , "message": ""}. -type entityErrorBody struct { - Message string `json:"message"` -} - -// newUpstreamError builds an *apierror.Error whose Body is entity-service's -// own "message" field — the specific, caller-safe validation reason -// (apierror.ValidationError et al. are written for exactly this purpose) — -// rather than the raw {"code":...,"message":"..."} JSON blob, so callers can -// both log the exact reason and, for 400s, surface it to the frontend -// verbatim instead of a generic fallback string. Body is left empty if the -// response isn't the expected shape (e.g. a gateway error page instead of an -// entity-service response): callers already treat an empty Body as "no -// specific message available" and fall back to a generic one, and this -// deliberately avoids ever logging or returning to the frontend a raw -// upstream body of unknown, unbounded content. -func newUpstreamError(statusCode int, rawBody []byte) *apierror.Error { - var body entityErrorBody - if err := json.Unmarshal(rawBody, &body); err == nil && body.Message != "" { - return &apierror.Error{StatusCode: statusCode, Body: body.Message} - } - return &apierror.Error{StatusCode: statusCode} -} - // do executes an authenticated HTTP request against entity-service and // returns the raw JSON response body. The caller owns the returned slice. func (c *Client) do(ctx context.Context, method, path string, body []byte) ([]byte, error) { @@ -200,7 +175,7 @@ func (c *Client) do(ctx context.Context, method, path string, body []byte) ([]by } if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { - return nil, newUpstreamError(resp.StatusCode, respBody) + return nil, apierror.NewUpstreamError(resp.StatusCode, respBody) } return respBody, nil @@ -243,7 +218,7 @@ func (c *Client) doBinary(ctx context.Context, path string) (body []byte, conten } if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { - err := newUpstreamError(resp.StatusCode, respBody) + err := apierror.NewUpstreamError(resp.StatusCode, respBody) return nil, "", err } diff --git a/apps/customer-portal/backend-v2/internal/middleware/cors.go b/apps/customer-portal/backend-v2/internal/middleware/cors.go index 7d2f2389d3..c51a6e712f 100644 --- a/apps/customer-portal/backend-v2/internal/middleware/cors.go +++ b/apps/customer-portal/backend-v2/internal/middleware/cors.go @@ -58,7 +58,7 @@ func CORS(allowedOrigins []string) func(http.Handler) http.Handler { origin := r.Header.Get("Origin") if origin != "" && (len(allowed) == 0 || allowed[origin]) { w.Header().Set("Access-Control-Allow-Origin", origin) - w.Header().Set("Vary", "Origin") + w.Header().Add("Vary", "Origin") } if r.Method == http.MethodOptions && r.Header.Get("Access-Control-Request-Method") != "" { diff --git a/apps/customer-portal/backend-v2/internal/productconsumption/client.go b/apps/customer-portal/backend-v2/internal/productconsumption/client.go index e1e8f5f53d..4dff447702 100644 --- a/apps/customer-portal/backend-v2/internal/productconsumption/client.go +++ b/apps/customer-portal/backend-v2/internal/productconsumption/client.go @@ -138,12 +138,7 @@ func (c *Client) doAt(ctx context.Context, baseURL, method, path, contentType st } if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { - const maxErrBody = 256 - excerpt := respBody - if len(excerpt) > maxErrBody { - excerpt = excerpt[:maxErrBody] - } - return nil, &apierror.Error{StatusCode: resp.StatusCode, Body: string(excerpt)} + return nil, apierror.NewUpstreamError(resp.StatusCode, respBody) } return respBody, nil diff --git a/apps/customer-portal/backend-v2/internal/registry/client.go b/apps/customer-portal/backend-v2/internal/registry/client.go index a23ab6b7fb..84dec2ea23 100644 --- a/apps/customer-portal/backend-v2/internal/registry/client.go +++ b/apps/customer-portal/backend-v2/internal/registry/client.go @@ -141,12 +141,7 @@ func (c *Client) do(ctx context.Context, method, path string, body []byte) ([]by } if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { - const maxErrBody = 256 - excerpt := respBody - if len(excerpt) > maxErrBody { - excerpt = excerpt[:maxErrBody] - } - return nil, &apierror.Error{StatusCode: resp.StatusCode, Body: string(excerpt)} + return nil, apierror.NewUpstreamError(resp.StatusCode, respBody) } return respBody, nil diff --git a/apps/customer-portal/backend-v2/internal/scim/client.go b/apps/customer-portal/backend-v2/internal/scim/client.go index 426e64bb06..cdd95cbd2e 100644 --- a/apps/customer-portal/backend-v2/internal/scim/client.go +++ b/apps/customer-portal/backend-v2/internal/scim/client.go @@ -145,12 +145,7 @@ func (c *Client) do(ctx context.Context, method, path string, body []byte) ([]by } if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { - const maxErrBody = 256 - excerpt := respBody - if len(excerpt) > maxErrBody { - excerpt = excerpt[:maxErrBody] - } - return nil, &apierror.Error{StatusCode: resp.StatusCode, Body: string(excerpt)} + return nil, apierror.NewUpstreamError(resp.StatusCode, respBody) } return respBody, nil diff --git a/apps/customer-portal/backend-v2/internal/updates/client.go b/apps/customer-portal/backend-v2/internal/updates/client.go index 54270407e8..0fa74c572c 100644 --- a/apps/customer-portal/backend-v2/internal/updates/client.go +++ b/apps/customer-portal/backend-v2/internal/updates/client.go @@ -147,12 +147,7 @@ func (c *Client) do(ctx context.Context, method, path string, body []byte) ([]by } if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { - const maxErrBody = 256 - excerpt := respBody - if len(excerpt) > maxErrBody { - excerpt = excerpt[:maxErrBody] - } - return nil, &apierror.Error{StatusCode: resp.StatusCode, Body: string(excerpt)} + return nil, apierror.NewUpstreamError(resp.StatusCode, respBody) } return respBody, nil diff --git a/apps/customer-portal/backend-v2/internal/usermanagement/client.go b/apps/customer-portal/backend-v2/internal/usermanagement/client.go index c95f2ca79b..b08c5195fc 100644 --- a/apps/customer-portal/backend-v2/internal/usermanagement/client.go +++ b/apps/customer-portal/backend-v2/internal/usermanagement/client.go @@ -30,6 +30,7 @@ import ( "strings" "time" + "github.com/wso2-open-operations/cs-tools/apps/customer-portal/backend-v2/internal/apierror" "golang.org/x/oauth2" "golang.org/x/oauth2/clientcredentials" ) @@ -155,7 +156,7 @@ func (c *Client) doJSON(ctx context.Context, method, path string, body []byte, w return nil, err } if statusCode != wantStatus { - return nil, extractErrorMessage(statusCode, respBody) + return nil, apierror.NewUpstreamError(statusCode, respBody) } return respBody, nil } diff --git a/apps/customer-portal/backend-v2/internal/usermanagement/usermanagement.go b/apps/customer-portal/backend-v2/internal/usermanagement/usermanagement.go index 01bc511dcd..2ab929c269 100644 --- a/apps/customer-portal/backend-v2/internal/usermanagement/usermanagement.go +++ b/apps/customer-portal/backend-v2/internal/usermanagement/usermanagement.go @@ -136,31 +136,8 @@ func (c *Client) ValidateProjectContact(ctx context.Context, req ValidationPaylo case http.StatusAccepted: return nil, false, nil case http.StatusConflict: - return nil, true, extractErrorMessage(statusCode, raw) + return nil, true, apierror.NewUpstreamError(statusCode, raw) default: - return nil, false, extractErrorMessage(statusCode, raw) + return nil, false, apierror.NewUpstreamError(statusCode, raw) } } - -// upstreamErrorBody is the {"message": "..."} shape every non-success -// response from this service carries. -type upstreamErrorBody struct { - Message string `json:"message"` -} - -// extractErrorMessage builds an *apierror.Error whose Body is the upstream -// service's own "message" field when present, so callers can surface the -// specific reason (e.g. "Contact already exists") rather than a generic -// fallback. -func extractErrorMessage(statusCode int, raw []byte) error { - var body upstreamErrorBody - if err := json.Unmarshal(raw, &body); err == nil && body.Message != "" { - return &apierror.Error{StatusCode: statusCode, Body: body.Message} - } - const maxErrBody = 512 - excerpt := raw - if len(excerpt) > maxErrBody { - excerpt = excerpt[:maxErrBody] - } - return &apierror.Error{StatusCode: statusCode, Body: string(excerpt)} -} From fb1a5749ef38dbf8c8e16a0c6a3f5cb91c43dbad Mon Sep 17 00:00:00 2001 From: Rashmika998 Date: Thu, 6 Aug 2026 11:43:56 +0530 Subject: [PATCH 5/6] fix(entity-service,customer-portal): reject unsupported announcement case creation, fix createdBy filter conflict MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit validCaseType gaining "announcement" (for case search/stats filters) had the side effect of also letting POST /cases accept type=announcement, but no code path builds a valid payload for one: the Postgres path already rejects every type but "case", and the ServiceNow path silently drops subject/description since its payload-building switch has no case for it. Reject it explicitly instead, and stop advertising it as creatable in openapi.yaml. Separately, backend-v2's case-search filter builder appended both a createdBy+in filter (from CreatedBy) and a createdBy+eq filter (from CreatedByMe) when a client set both — entity-service's filters array is AND-only, so the two together could never both match, silently returning an empty result set. CreatedByMe now takes precedence and CreatedBy is dropped entirely when both are set. Co-Authored-By: Claude Sonnet 5 --- .../backend-v2/internal/dto/case.go | 17 ++++++++++-- .../backend-v2/internal/dto/case_test.go | 24 ++++++++++++++++- .../internal/service/case_service.go | 12 +++++++++ .../service/sn_case_service_create_test.go | 26 +++++++++++++++++++ entity-service/openapi.yaml | 8 +++--- 5 files changed, 81 insertions(+), 6 deletions(-) diff --git a/apps/customer-portal/backend-v2/internal/dto/case.go b/apps/customer-portal/backend-v2/internal/dto/case.go index e20bc1a555..1f59d32bf9 100644 --- a/apps/customer-portal/backend-v2/internal/dto/case.go +++ b/apps/customer-portal/backend-v2/internal/dto/case.go @@ -157,7 +157,12 @@ type CaseSearchRequest struct { // Pagination pass straight through unchanged. CreatedByMe becomes a // createdBy+eq filter carrying currentUserFilterPlaceholder, exactly as // entity-service's own case_filters.go expects, resolving the caller's -// identity server-side from the forwarded x-user-id-token. +// identity server-side from the forwarded x-user-id-token. CreatedByMe takes +// precedence over CreatedBy when both are set: entity-service's filters +// array is AND-only, so a createdBy+in filter and a createdBy+eq filter +// together could never both match (barring CreatedBy coincidentally +// containing the caller's own email), silently returning an empty result +// set — CreatedBy is dropped entirely rather than combined. func BuildEntitySearchCasesRequest(req CaseSearchRequest) entity.SearchCasesRequest { var filters []entity.CaseFieldFilter @@ -178,13 +183,21 @@ func BuildEntitySearchCasesRequest(req CaseSearchRequest) entity.SearchCasesRequ addIn("assignedUserId", req.Filters.AssignedUserIDs) addIn("product", req.Filters.ProductNames) addIn("tag", req.Filters.Tags) - addIn("createdBy", req.Filters.CreatedBy) if req.Filters.ParentID != nil { filters = append(filters, entity.CaseFieldFilter{Field: "parentId", Op: "eq", Values: []string{*req.Filters.ParentID}}) } + // CreatedByMe takes precedence over CreatedBy: entity-service's filters + // array is AND-only (case_filters.go), so a createdBy+in filter and a + // createdBy+eq filter together could never both match (barring CreatedBy + // coincidentally containing the caller's own email) — silently returning + // an empty result set instead of the caller's own cases. A client + // shouldn't send both, but if it does, honor the explicit "my cases" + // intent rather than the list. if req.Filters.CreatedByMe { filters = append(filters, entity.CaseFieldFilter{Field: "createdBy", Op: "eq", Values: []string{currentUserFilterPlaceholder}}) + } else { + addIn("createdBy", req.Filters.CreatedBy) } return entity.SearchCasesRequest{ diff --git a/apps/customer-portal/backend-v2/internal/dto/case_test.go b/apps/customer-portal/backend-v2/internal/dto/case_test.go index 8b37e77439..3cc098acbb 100644 --- a/apps/customer-portal/backend-v2/internal/dto/case_test.go +++ b/apps/customer-portal/backend-v2/internal/dto/case_test.go @@ -77,8 +77,8 @@ func TestBuildEntitySearchCasesRequest_AllFiltersTranslate(t *testing.T) { {Field: "assignedUserId", Op: "in", Values: []string{"eng-1"}}, {Field: "product", Op: "in", Values: []string{"API Manager"}}, {Field: "tag", Op: "in", Values: []string{"urgent"}}, - {Field: "createdBy", Op: "in", Values: []string{"user-1"}}, {Field: "parentId", Op: "eq", Values: []string{"parent-id-1"}}, + {Field: "createdBy", Op: "in", Values: []string{"user-1"}}, } if !reflect.DeepEqual(got.Filters.Filters, want) { t.Fatalf("Filters = %+v,\nwant %+v", got.Filters.Filters, want) @@ -103,6 +103,28 @@ func TestBuildEntitySearchCasesRequest_CreatedByMe(t *testing.T) { } } +// TestBuildEntitySearchCasesRequest_CreatedByMeTakesPrecedenceOverCreatedBy +// guards against sending both a createdBy+in and a createdBy+eq filter for +// the same request: entity-service's filters array is AND-only, so both +// together could (barring coincidence) never match anything, silently +// returning an empty result set. If a client sends both, CreatedByMe must +// win and CreatedBy must be dropped entirely, not merged. +func TestBuildEntitySearchCasesRequest_CreatedByMeTakesPrecedenceOverCreatedBy(t *testing.T) { + req := CaseSearchRequest{Filters: CaseSearchFilters{ + CreatedBy: []string{"someone-else@example.com"}, + CreatedByMe: true, + }} + + got := BuildEntitySearchCasesRequest(req) + + want := []entity.CaseFieldFilter{ + {Field: "createdBy", Op: "eq", Values: []string{"__current_user_email__"}}, + } + if !reflect.DeepEqual(got.Filters.Filters, want) { + t.Fatalf("Filters = %+v, want %+v", got.Filters.Filters, want) + } +} + // TestBuildEntitySearchCasesRequest_EmptyFiltersProduceNoPredicates verifies // an empty/default CaseSearchRequest builds a request with no filter // predicates at all -- entity-service's own case search already treats a diff --git a/entity-service/internal/service/case_service.go b/entity-service/internal/service/case_service.go index a3b77d953c..5ce4c4c217 100644 --- a/entity-service/internal/service/case_service.go +++ b/entity-service/internal/service/case_service.go @@ -204,6 +204,18 @@ func validateCreateCaseRequest(req *domain.CreateCaseRequest) error { if !validEngagementType[req.EngagementType] { return &apierror.ValidationError{Msg: "engagementType contains invalid value: " + string(req.EngagementType)} } + case "announcement": + // "announcement" is a real, valid case type (it's in the Postgres + // case_type_enum and is a legitimate case-search/stats filter value — + // see validCaseType), but nothing in this codebase knows how to build + // an announcement case: this switch has no field-requirement case for + // it, and sn_case_service.go's payload-building switch has no case + // for it either (so req.Subject/req.Description would be silently + // dropped rather than sent to ServiceNow, with no error). Reject + // explicitly here rather than letting it fall through and appear to + // succeed — remove this case only once both switches gain real + // support for creating one. + return &apierror.ValidationError{Msg: "case creation for type \"announcement\" is not supported"} } return nil diff --git a/entity-service/internal/service/sn_case_service_create_test.go b/entity-service/internal/service/sn_case_service_create_test.go index 4f97fe38db..489538e374 100644 --- a/entity-service/internal/service/sn_case_service_create_test.go +++ b/entity-service/internal/service/sn_case_service_create_test.go @@ -66,6 +66,32 @@ func TestSNCaseService_CreateCase_EngagementValidation(t *testing.T) { } } +// TestSNCaseService_CreateCase_AnnouncementRejected verifies that +// type="announcement" is rejected with a *apierror.ValidationError rather +// than silently succeeding: "announcement" is valid elsewhere (case +// search/stats filters — see validCaseType) but neither +// validateCreateCaseRequest nor this file's payload-building switch has a +// case for it, so letting it through would silently drop req.Subject/ +// req.Description instead of sending them to ServiceNow. +func TestSNCaseService_CreateCase_AnnouncementRejected(t *testing.T) { + req := domain.CreateCaseRequest{ + Type: "announcement", + ProjectID: testProjectUUID, + DeploymentID: testDeploymentUUID, + DeployedProductID: testDeployedProdID, + Subject: "New feature rollout", + Description: "Announcing a new feature", + } + + // client is intentionally nil: this must fail validation before touching it. + svc := NewServiceNowCaseService(nil, nil) + + _, err := svc.CreateCase(contextWithUserIDToken("token"), req) + if _, ok := err.(*apierror.ValidationError); !ok { + t.Fatalf("expected *apierror.ValidationError, got %T: %v", err, err) + } +} + // TestSNCaseService_CreateCase_Engagement verifies a valid engagement request // builds the expected snCreateCasePayload (title/description/engagementType) // and maps a successful ServiceNow response back to domain.CreateCaseResponse. diff --git a/entity-service/openapi.yaml b/entity-service/openapi.yaml index 48aeb8f020..f0f94ce98b 100644 --- a/entity-service/openapi.yaml +++ b/entity-service/openapi.yaml @@ -6010,17 +6010,19 @@ components: properties: type: type: string - enum: [case, default_case, service_request, security_report_analysis, announcement, engagement] + enum: [case, default_case, service_request, security_report_analysis, engagement] description: | Case type. `default_case` is also accepted as an alias for `case` (ServiceNow's own raw caseType wire value; kept for the currently-in-production customer-portal frontend, which predates this API's `case` enum) and is normalized to `case` before any - other validation runs. + other validation runs. `announcement` is a valid case type + elsewhere in this API (case search/stats filters) but cannot be + created through this endpoint — no code path builds a valid + payload for one — and is rejected with 400 if supplied here. - `case`: standard support case; requires `subject`, `description`, `severity`, `issueType`. - `service_request`: catalog-based service request (ServiceNow only); requires `catalogId`, `catalogItemId`, `variables`. - `security_report_analysis`: security report (ServiceNow only); requires `subject`, `description`; `attachments` optional. - - `announcement`: ServiceNow only. - `engagement`: requires `subject`, `description`, `engagementType`. projectId: type: string From cd48f3edee92e997cdd3df40c274b2996d23bcb2 Mon Sep 17 00:00:00 2001 From: Rashmika998 Date: Thu, 6 Aug 2026 12:01:40 +0530 Subject: [PATCH 6/6] docs(entity-service): declare engagementType in CreateCaseRequest schema The engagement case type's own description already said it requires engagementType, and the Go validation enforces it, but the OpenAPI schema never declared the property -- generated clients had no way to model it. Co-Authored-By: Claude Sonnet 5 --- entity-service/openapi.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/entity-service/openapi.yaml b/entity-service/openapi.yaml index f0f94ce98b..81165c35d6 100644 --- a/entity-service/openapi.yaml +++ b/entity-service/openapi.yaml @@ -6047,6 +6047,10 @@ components: type: string enum: [error, partial_outage, performance_degradation, question, security_or_compliance, total_outage] description: Required for `case` type. + engagementType: + type: string + enum: [migration, consultancy, new_feature_improvement, follow_up, onboarding] + description: Required for `engagement` type. catalogId: type: string format: uuid