This is an automated email from the ASF dual-hosted git repository. lahirujayathilake pushed a commit to branch access-integration-v3 in repository https://gitbox.apache.org/repos/asf/airavata-custos.git
commit b3bd3b242244da7b0fad87fa6fa4184739a1f560 Author: lahiruj <[email protected]> AuthorDate: Thu May 21 04:15:46 2026 -0400 Fix AMIE delete/merge handler semantics --- .../AMIE-Processor/handler/request_person_merge.go | 3 +- .../AMIE-Processor/handler/request_user_modify.go | 62 ++++++++++++++++++++-- .../AMIE-Processor/mock-server/mock-amie-server.py | 24 +++++++++ pkg/models/project.go | 1 - pkg/service/user_merge.go | 9 +++- 5 files changed, 91 insertions(+), 8 deletions(-) diff --git a/connectors/ACCESS/AMIE-Processor/handler/request_person_merge.go b/connectors/ACCESS/AMIE-Processor/handler/request_person_merge.go index 1248bbb96..cfafb4158 100644 --- a/connectors/ACCESS/AMIE-Processor/handler/request_person_merge.go +++ b/connectors/ACCESS/AMIE-Processor/handler/request_person_merge.go @@ -24,6 +24,7 @@ import ( "log/slog" "github.com/apache/airavata-custos/connectors/ACCESS/AMIE-Processor/model" + "github.com/apache/airavata-custos/pkg/models" "github.com/apache/airavata-custos/pkg/service" ) @@ -71,7 +72,7 @@ func (h *RequestPersonMergeHandler) Handle(ctx context.Context, tx *sql.Tx, pack } activeMemberships := make([]string, 0, len(memberships)) for _, m := range memberships { - if m.MembershipStatus == "ACTIVE" { + if m.MembershipStatus == models.ACTIVE { activeMemberships = append(activeMemberships, m.ID) } } diff --git a/connectors/ACCESS/AMIE-Processor/handler/request_user_modify.go b/connectors/ACCESS/AMIE-Processor/handler/request_user_modify.go index a2fe2fe68..75964901a 100644 --- a/connectors/ACCESS/AMIE-Processor/handler/request_user_modify.go +++ b/connectors/ACCESS/AMIE-Processor/handler/request_user_modify.go @@ -23,6 +23,7 @@ import ( "encoding/json" "errors" "fmt" + "log/slog" "strings" "github.com/apache/airavata-custos/connectors/ACCESS/AMIE-Processor/model" @@ -30,6 +31,13 @@ import ( "github.com/apache/airavata-custos/pkg/service" ) +var handledModifyTags = map[string]struct{}{ + "ActionType": {}, + "PersonID": {}, + "UserGlobalID": {}, + "DnList": {}, +} + type RequestUserModifyHandler struct { svc *service.Service amieClient AmieClient @@ -73,11 +81,12 @@ func (h *RequestUserModifyHandler) Handle(ctx context.Context, tx *sql.Tx, packe } case strings.EqualFold(actionType, "delete"): if user != nil { - if _, err := h.svc.UpdateUserStatus(ctx, user.ID, models.UserRemoved); err != nil { - return fmt.Errorf("request_user_modify: soft-remove user: %w", err) + if err := h.deleteDNs(ctx, tx, packet, eventID, body, user.ID); err != nil { + return err } - if err := h.auditSvc.Log(ctx, tx, packet.ID, eventID, model.AuditDeletePerson, "user", user.ID, "status=REMOVED"); err != nil { - return fmt.Errorf("request_user_modify: audit DELETE_PERSON: %w", err) + if unhandled := unhandledModifyTags(body); len(unhandled) > 0 { + slog.WarnContext(ctx, "request_user_modify delete carries unhandled tags", + "user_id", user.ID, "tags", unhandled) } } default: @@ -163,6 +172,51 @@ func (h *RequestUserModifyHandler) updateExternalIdentity(ctx context.Context, b return h.svc.UpdateExternalIdentity(ctx, ext) } +func (h *RequestUserModifyHandler) deleteDNs(ctx context.Context, tx *sql.Tx, packet *model.Packet, eventID string, body map[string]any, userID string) error { + dns := getDNList(body) + if len(dns) == 0 { + return nil + } + target := make(map[string]struct{}, len(dns)) + for _, dn := range dns { + target[dn] = struct{}{} + } + existing, err := h.svc.ListUserDNs(ctx, userID) + if err != nil { + return fmt.Errorf("list user DNs: %w", err) + } + removed := 0 + for _, e := range existing { + if _, hit := target[e.DN]; !hit { + continue + } + if err := h.svc.RemoveUserDN(ctx, e.ID); err != nil { + return fmt.Errorf("remove DN %s: %w", e.DN, err) + } + removed++ + } + if removed > 0 { + if err := h.auditSvc.Log(ctx, tx, packet.ID, eventID, model.AuditPersistDNs, "user", userID, + fmt.Sprintf("DN delete: -%d", removed)); err != nil { + return fmt.Errorf("audit PERSIST_DNS: %w", err) + } + } + return nil +} + +func unhandledModifyTags(body map[string]any) []string { + if len(body) == 0 { + return nil + } + var out []string + for k := range body { + if _, known := handledModifyTags[k]; !known { + out = append(out, k) + } + } + return out +} + // syncDNs reconciles the user's DN list with the packet body's UserDnList: // new DNs are added, DNs missing from the packet are removed. AMIE's // request_user_modify with ActionType=replace is the authoritative source. diff --git a/connectors/ACCESS/AMIE-Processor/mock-server/mock-amie-server.py b/connectors/ACCESS/AMIE-Processor/mock-server/mock-amie-server.py index c3e495c08..625417335 100644 --- a/connectors/ACCESS/AMIE-Processor/mock-server/mock-amie-server.py +++ b/connectors/ACCESS/AMIE-Processor/mock-server/mock-amie-server.py @@ -130,6 +130,19 @@ def gen_valid_user_modify(): }) +def gen_valid_user_modify_delete(): + """request_user_modify — delete a DN from a known user's DN list.""" + gid = str(random.randint(100000, 999999)) + return make_packet("request_user_modify", { + "ActionType": "delete", + "PersonID": f"person-{uuid.uuid4().hex[:8]}", + "UserGlobalID": gid, + "DnList": [ + f"/C=US/O=Mock Institute/CN=User {gid}", + ], + }) + + def gen_valid_data_account_create(): """data_account_create — pass DNs for an existing user (looked up by GlobalID).""" gid = str(random.randint(100000, 999999)) @@ -333,6 +346,7 @@ SUCCESS_GENERATORS = [ (gen_valid_project_create, 2), (gen_valid_account_create, 2), (gen_valid_user_modify, 1), + (gen_valid_user_modify_delete, 1), (gen_valid_data_account_create, 1), (gen_valid_data_project_create, 1), (gen_valid_project_inactivate, 1), @@ -409,6 +423,7 @@ def generate_all_handlers_once(): gen_valid_project_create(), gen_valid_account_create(), gen_valid_user_modify(), + gen_valid_user_modify_delete(), make_packet("data_account_create", { "ProjectID": f"PRJ-MOCK{random.randint(1000, 9999)}", "PersonID": f"person-{uuid.uuid4().hex[:8]}", @@ -532,6 +547,15 @@ def gen_baseline_scenario(): "DeletePersonID": "bl-delete-person", "MergeReason": "Duplicate person records", }), + # Remove one of the survivor's DNs via the delete ActionType. + make_packet("request_user_modify", { + "ActionType": "delete", + "PersonID": "bl-pi-001-person", + "UserGlobalID": "bl-pi-001", + "DnList": [ + "/DC=EDU/CN=patfirst", + ], + }), make_packet("inform_transaction_complete", { "StatusCode": "Success", "Message": "Baseline complete", diff --git a/pkg/models/project.go b/pkg/models/project.go index 8d0d5720b..38de362a0 100644 --- a/pkg/models/project.go +++ b/pkg/models/project.go @@ -10,7 +10,6 @@ const ( UserInactive UserStatus = "INACTIVE" UserSuspended UserStatus = "SUSPENDED" UserMerged UserStatus = "MERGED" - UserRemoved UserStatus = "REMOVED" ) // ProjectStatus enumerates the lifecycle states a Project may occupy. diff --git a/pkg/service/user_merge.go b/pkg/service/user_merge.go index a2ffe2ad7..fa558c36e 100644 --- a/pkg/service/user_merge.go +++ b/pkg/service/user_merge.go @@ -57,8 +57,9 @@ func (s *Service) MergeUsers(ctx context.Context, survivingID, retiringID, reaso if survivor == nil { return nil, fmt.Errorf("%w: surviving user %q does not exist", ErrInvalidInput, survivingID) } - if survivor.Status == models.UserMerged { - return nil, fmt.Errorf("%w: surviving user %q is itself merged", ErrInvalidInput, survivingID) + if survivor.Status != models.UserActive { + return nil, fmt.Errorf("%w: surviving user %q must be ACTIVE (got %s)", + ErrInvalidInput, survivingID, survivor.Status) } retiring, err := s.users.FindByID(ctx, retiringID) if err != nil { @@ -83,6 +84,10 @@ func (s *Service) MergeUsers(ctx context.Context, survivingID, retiringID, reaso ErrAlreadyExists, retiringID, prior.SurvivingUserID) } } + if retiring.Status != models.UserActive { + return nil, fmt.Errorf("%w: retiring user %q must be ACTIVE (got %s)", + ErrInvalidInput, retiringID, retiring.Status) + } if err := s.inTx(ctx, func(tx *sql.Tx) error { if err := s.extIDs.ReassignUser(ctx, tx, retiringID, survivingID); err != nil {
