diff --git a/cla-backend-go/company/models.go b/cla-backend-go/company/models.go index 60aca59d0..0670289c2 100644 --- a/cla-backend-go/company/models.go +++ b/cla-backend-go/company/models.go @@ -26,6 +26,7 @@ type DBModel struct { Note string `dynamodbav:"note" json:"note"` IsSanctioned bool `dynamodbav:"is_sanctioned" json:"is_sanctioned"` SanctionOrigin string `dynamodbav:"sanction_origin" json:"sanction_origin,omitempty"` + SanctionedDate string `dynamodbav:"sanctioned_date" json:"sanctioned_date,omitempty"` Version string `dynamodbav:"version" json:"version"` } @@ -89,6 +90,7 @@ func (dbCompanyModel *DBModel) toModel() (*models.Company, error) { Note: dbCompanyModel.Note, IsSanctioned: dbCompanyModel.IsSanctioned, SanctionOrigin: dbCompanyModel.SanctionOrigin, + SanctionedDate: dbCompanyModel.SanctionedDate, Version: dbCompanyModel.Version, }, nil } @@ -151,6 +153,7 @@ func toSwaggerModel(dbCompanyModel *DBModel) (*models.Company, error) { SigningEntityName: dbCompanyModel.SigningEntityName, IsSanctioned: dbCompanyModel.IsSanctioned, SanctionOrigin: dbCompanyModel.SanctionOrigin, + SanctionedDate: dbCompanyModel.SanctionedDate, CompanyExternalID: dbCompanyModel.CompanyExternalID, CompanyManagerID: dbCompanyModel.CompanyManagerID, Created: strfmt.DateTime(createdDateTime), diff --git a/cla-backend-go/company/projections.go b/cla-backend-go/company/projections.go index ad40e95e9..b75809bc3 100644 --- a/cla-backend-go/company/projections.go +++ b/cla-backend-go/company/projections.go @@ -20,6 +20,7 @@ func buildCompanyProjection() expression.ProjectionBuilder { expression.Name("note"), expression.Name("is_sanctioned"), expression.Name("sanction_origin"), + expression.Name("sanctioned_date"), expression.Name("version"), ) } diff --git a/cla-backend-go/company/repository.go b/cla-backend-go/company/repository.go index 5b8e2fbc8..5e49ebf04 100644 --- a/cla-backend-go/company/repository.go +++ b/cla-backend-go/company/repository.go @@ -787,6 +787,7 @@ func buildCompanyModels(ctx context.Context, results *dynamodb.ScanOutput) ([]mo Created string `json:"date_created"` Note string `json:"note"` IsSanctioned bool `json:"is_sanctioned"` + SanctionedDate string `json:"sanctioned_date"` Modified string `json:"date_modified"` } @@ -829,6 +830,7 @@ func buildCompanyModels(ctx context.Context, results *dynamodb.ScanOutput) ([]mo Created: strfmt.DateTime(createdDateTime), Note: dbCompany.Note, IsSanctioned: dbCompany.IsSanctioned, + SanctionedDate: dbCompany.SanctionedDate, Updated: strfmt.DateTime(modifiedDateTime), }) } @@ -1282,6 +1284,58 @@ func (repo repository) UpdateCompanyAccessList(ctx context.Context, companyID st // sanctionOriginSSS is the sanction_origin value written by the Sanctions Screening Service. const sanctionOriginSSS = "sss" +// sanctionUpdate is the DynamoDB update for one sanction status change. +type sanctionUpdate struct { + expression string + condition *string + names map[string]*string + values map[string]*dynamodb.AttributeValue +} + +// buildSanctionUpdate assembles the update for UpdateCompanySanctionStatus. All SET assignments +// stay contiguous ahead of any REMOVE, as DynamoDB requires. +func buildSanctionUpdate(sanctioned bool, origin, now string) sanctionUpdate { + update := sanctionUpdate{ + expression: "SET #S = :s, #M = :m", + names: map[string]*string{ + "#S": aws.String("is_sanctioned"), + "#M": aws.String("date_modified"), + "#O": aws.String("sanction_origin"), + }, + values: map[string]*dynamodb.AttributeValue{ + ":s": {BOOL: aws.Bool(sanctioned)}, + ":m": {S: aws.String(now)}, + }, + } + + // Setting the flag stamps sanctioned_date; clearing it leaves the stored date alone. + if sanctioned { + update.names["#D"] = aws.String("sanctioned_date") + update.values[":d"] = &dynamodb.AttributeValue{S: aws.String(now)} + update.expression += ", #D = :d" + } + + if origin != "" { + update.values[":o"] = &dynamodb.AttributeValue{S: aws.String(origin)} + update.expression += ", #O = :o" + } else { + // Manual/admin update: remove any stale SSS-set origin so the record becomes a + // sticky admin block (origin absent) that SSS will never auto-clear. + update.expression += " REMOVE #O" + } + + // When SSS sets a block, never overwrite a manual/admin block (is_sanctioned=true + // with absent or non-"sss" origin). Only set the SSS flag when the company is + // currently unblocked or already SSS-blocked. A ConditionalCheckFailedException + // therefore means a manual/admin block is already in place and must be preserved. + if sanctioned && origin == sanctionOriginSSS { + update.values[":false"] = &dynamodb.AttributeValue{BOOL: aws.Bool(false)} + update.condition = aws.String("attribute_not_exists(#S) OR #S = :false OR #O = :o") + } + + return update +} + // UpdateCompanySanctionStatus sets is_sanctioned and, when origin is non-empty, sanction_origin. // Pass origin="sss" when flagging via SSS; pass origin="" for manual admin updates. func (repo repository) UpdateCompanySanctionStatus(ctx context.Context, companyID string, sanctioned bool, origin string) error { @@ -1294,50 +1348,21 @@ func (repo repository) UpdateCompanySanctionStatus(ctx context.Context, companyI } _, now := utils.CurrentTime() - - names := map[string]*string{ - "#S": aws.String("is_sanctioned"), - "#M": aws.String("date_modified"), - } - values := map[string]*dynamodb.AttributeValue{ - ":s": {BOOL: aws.Bool(sanctioned)}, - ":m": {S: aws.String(now)}, - } - updateExpr := "SET #S = :s, #M = :m" - - if origin != "" { - names["#O"] = aws.String("sanction_origin") - values[":o"] = &dynamodb.AttributeValue{S: aws.String(origin)} - updateExpr += ", #O = :o" - } else { - // Manual/admin update: remove any stale SSS-set origin so the record becomes a - // sticky admin block (origin absent) that SSS will never auto-clear. - names["#O"] = aws.String("sanction_origin") - updateExpr += " REMOVE #O" - } + update := buildSanctionUpdate(sanctioned, origin, now) input := &dynamodb.UpdateItemInput{ - ExpressionAttributeNames: names, - ExpressionAttributeValues: values, + ExpressionAttributeNames: update.names, + ExpressionAttributeValues: update.values, TableName: aws.String(repo.companyTableName), Key: map[string]*dynamodb.AttributeValue{ "company_id": {S: aws.String(companyID)}, }, - UpdateExpression: aws.String(updateExpr), - } - - // When SSS sets a block, never overwrite a manual/admin block (is_sanctioned=true - // with absent or non-"sss" origin). Only set the SSS flag when the company is - // currently unblocked or already SSS-blocked. A ConditionalCheckFailedException - // therefore means a manual/admin block is already in place and must be preserved. - sssSettingBlock := sanctioned && origin == sanctionOriginSSS - if sssSettingBlock { - values[":false"] = &dynamodb.AttributeValue{BOOL: aws.Bool(false)} - input.ConditionExpression = aws.String("attribute_not_exists(#S) OR #S = :false OR #O = :o") + UpdateExpression: aws.String(update.expression), + ConditionExpression: update.condition, } if _, err := repo.dynamoDBClient.UpdateItem(input); err != nil { - if sssSettingBlock { + if update.condition != nil { if aerr, ok := err.(awserr.Error); ok && aerr.Code() == dynamodb.ErrCodeConditionalCheckFailedException { log.WithFields(f).Debugf("company %s already has a manual/admin sanction block; preserving it and not overwriting origin with sss", companyID) return nil diff --git a/cla-backend-go/company/repository_test.go b/cla-backend-go/company/repository_test.go new file mode 100644 index 000000000..bac2ef6df --- /dev/null +++ b/cla-backend-go/company/repository_test.go @@ -0,0 +1,94 @@ +// Copyright The Linux Foundation and each contributor to CommunityBridge. +// SPDX-License-Identifier: MIT + +package company + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestBuildSanctionUpdate(t *testing.T) { + const now = "2026-08-20T10:11:12Z" + + tests := []struct { + name string + sanctioned bool + origin string + expression string + condition string + stampedDate bool + }{ + { + name: "sss flags the company", + sanctioned: true, + origin: sanctionOriginSSS, + expression: "SET #S = :s, #M = :m, #D = :d, #O = :o", + condition: "attribute_not_exists(#S) OR #S = :false OR #O = :o", + stampedDate: true, + }, + { + name: "sss clears the company", + sanctioned: false, + origin: sanctionOriginSSS, + expression: "SET #S = :s, #M = :m, #O = :o", + }, + { + name: "admin flags the company", + sanctioned: true, + origin: "", + expression: "SET #S = :s, #M = :m, #D = :d REMOVE #O", + stampedDate: true, + }, + { + name: "admin clears the company", + sanctioned: false, + origin: "", + expression: "SET #S = :s, #M = :m REMOVE #O", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + update := buildSanctionUpdate(tc.sanctioned, tc.origin, now) + + assert.Equal(t, tc.expression, update.expression) + if tc.condition == "" { + assert.Nil(t, update.condition, "only an SSS-set flag is conditional") + } else { + require.NotNil(t, update.condition) + assert.Equal(t, tc.condition, *update.condition, "the manual/admin block must stay protected") + } + + if tc.stampedDate { + require.Contains(t, update.names, "#D") + assert.Equal(t, "sanctioned_date", *update.names["#D"]) + require.Contains(t, update.values, ":d") + assert.Equal(t, now, *update.values[":d"].S, "the flag and the date are stamped with the same time") + } else { + assert.NotContains(t, update.names, "#D", "clearing the flag leaves the stored date alone") + assert.NotContains(t, update.values, ":d") + } + + assert.Equal(t, tc.sanctioned, *update.values[":s"].BOOL) + assert.Equal(t, now, *update.values[":m"].S) + + // Every declared name and value has to be referenced, or DynamoDB rejects the update. + for name := range update.names { + assert.Contains(t, update.expression+condition(update), name) + } + for value := range update.values { + assert.Contains(t, update.expression+condition(update), value) + } + }) + } +} + +func condition(update sanctionUpdate) string { + if update.condition == nil { + return "" + } + return " " + *update.condition +} diff --git a/cla-backend-go/swagger/common/company.yaml b/cla-backend-go/swagger/common/company.yaml index 53b6fd4a5..a55e3fd13 100644 --- a/cla-backend-go/swagger/common/company.yaml +++ b/cla-backend-go/swagger/common/company.yaml @@ -47,6 +47,10 @@ properties: type: string description: "Source of the sanction flag (e.g. sss)" example: "sss" + sanctionedDate: + type: string + description: "When the sanction flag was last set; kept after it is cleared" + example: "2026-08-20T10:11:12Z" version: type: string diff --git a/cla-backend-go/swagger/common/my-cla.yaml b/cla-backend-go/swagger/common/my-cla.yaml index d02be339c..c8c4bdb60 100644 --- a/cla-backend-go/swagger/common/my-cla.yaml +++ b/cla-backend-go/swagger/common/my-cla.yaml @@ -104,7 +104,7 @@ properties: description: True when the employer is currently flagged by sanctions screening - always present, always false on ICLA rows flaggedAt: type: string - description: When the flag was observed - currently the response time, as no sanction timestamp is stored yet; present only when flagged is true + description: The employer's stored sanctioned_date, stamped at the first live detection - the response time only when no date is stored and stamping it failed; present only when flagged is true flaggedCheck: type: string enum: [live, stored, unavailable] diff --git a/cla-backend-go/v2/my_clas/cla_managers_test.go b/cla-backend-go/v2/my_clas/cla_managers_test.go index 41f51f4f7..2ddab5250 100644 --- a/cla-backend-go/v2/my_clas/cla_managers_test.go +++ b/cla-backend-go/v2/my_clas/cla_managers_test.go @@ -110,7 +110,7 @@ func TestGetMyClasFlaggedAndClaManager(t *testing.T) { userA := &v1Models.User{UserID: "user-a", LfUsername: "someone"} companies := &fakeCompanies{byID: map[string]*v1Models.Company{ "company-1": {CompanyID: "company-1", CompanyName: "Good Corp"}, - "company-2": {CompanyID: "company-2", CompanyName: "Sanctioned Corp", IsSanctioned: true}, + "company-2": {CompanyID: "company-2", CompanyName: "Sanctioned Corp", IsSanctioned: true, SanctionedDate: "2024-01-15T10:11:12.000000+0000"}, }} signaturesService := &fakeSignatures{ cclas: map[string]*v1Models.Signature{ @@ -145,7 +145,7 @@ func TestGetMyClasFlaggedAndClaManager(t *testing.T) { sanctioned := byID["sig-sanctioned"] assert.True(t, sanctioned.Flagged, "a sanctioned employer flags the ECLA") - assert.NotEmpty(t, sanctioned.FlaggedAt) + assert.Equal(t, "2024-01-15T10:11:12Z", sanctioned.FlaggedAt, "the stored sanctioned_date is returned in RFC3339, not the response time") assert.Equal(t, models.MyClaStatusRevoked, sanctioned.Status, "the Revoked state is system-set from sanctions") assert.False(t, sanctioned.Valid) assert.False(t, sanctioned.ClaManager, "a sanctioned employer carries no CLA manager action") @@ -332,6 +332,21 @@ func TestCreateMyClaManagerRequestRecipientDedupe(t *testing.T) { assert.Equal(t, []string{"manager-one@corp.example.org"}, (*sent)[0].recipients) } +func TestCreateMyClaManagerRequestSharedEmailDedupe(t *testing.T) { + repo, signaturesService, companies := managersFixture() + signaturesService.cclas["cla-group-1|company-1"].SignatureACL[1].Emails = []string{"Manager-One@Corp.Example.org"} + svc, _, sent := newRequestTestService(repo, signaturesService, companies) + + result, err := svc.CreateMyClaManagerRequest(context.Background(), &Caller{Username: "someone"}, &Identity{}, "sig-ecla", + requestInput("removal", []string{"manager-one", "manager-two"}, "")) + require.NoError(t, err) + require.NotNil(t, result) + assert.Equal(t, []string{"manager-one", "manager-two"}, result.Recipients, "both selected managers are still reported") + + require.Len(t, *sent, 1) + assert.Equal(t, []string{"manager-one@corp.example.org"}, (*sent)[0].recipients, "two managers sharing an address are mailed once") +} + func TestCreateMyClaManagerRequestZeroManagers(t *testing.T) { repo, _, companies := managersFixture() svc, eventsService, sent := newRequestTestService(repo, &fakeSignatures{}, companies) diff --git a/cla-backend-go/v2/my_clas/sanctions.go b/cla-backend-go/v2/my_clas/sanctions.go index 29293bcea..c950be779 100644 --- a/cla-backend-go/v2/my_clas/sanctions.go +++ b/cla-backend-go/v2/my_clas/sanctions.go @@ -40,8 +40,8 @@ type sssScreener struct { getOrganization func(ctx context.Context, orgID string) (*orgModels.Organization, error) } -// NewSanctionsScreener returns a read-only screener: unlike the signing flow it never persists what -// it observes, so the listing stays a pure read +// NewSanctionsScreener returns a read-only screener: it answers the question and never writes. +// Persisting a first live detection is the caller's job (see service.persistLiveSanction) func NewSanctionsScreener(client *sss.Client, enabled, required bool) SanctionsScreener { screener := &sssScreener{ enabled: enabled, diff --git a/cla-backend-go/v2/my_clas/service.go b/cla-backend-go/v2/my_clas/service.go index 4934a9b60..9aa4409fa 100644 --- a/cla-backend-go/v2/my_clas/service.go +++ b/cla-backend-go/v2/my_clas/service.go @@ -123,6 +123,7 @@ type SignaturesService interface { // CompanyRepository is the company repository subset used to resolve employers type CompanyRepository interface { GetCompany(ctx context.Context, companyID string) (*v1Models.Company, error) + UpdateCompanySanctionStatus(ctx context.Context, companyID string, sanctioned bool, origin string) error } // ProjectsCLAGroupsRepository is the projects-cla-groups subset used to resolve CLA Group names @@ -269,7 +270,12 @@ func (s *service) GetMyClas(ctx context.Context, caller *Caller, requested *Iden row.Flagged = sanction.flagged row.FlaggedCheck = sanction.check if sanction.flagged { - _, row.FlaggedAt = utils.CurrentTime() + if sanction.date != "" { + row.FlaggedAt = utils.FormatTimeString(sanction.date) + } else { + // Flagged with no stored date and none could be stamped; report when it was seen. + _, row.FlaggedAt = utils.CurrentTime() + } } coverage := data.coverage(sig, ref.user, sanction.flagged) row.Valid = sig.SignatureApproved && coverage.covered @@ -411,6 +417,7 @@ func (s *service) CreateMyClaManagerRequest(ctx context.Context, caller *Caller, selectedUsernames := make([]string, 0, len(recipients)) recipientEmails := make([]string, 0, len(recipients)) selected := make(map[string]bool, len(recipients)) + emailed := make(map[string]bool, len(recipients)) for _, recipient := range recipients { key := strings.ToLower(recipient) manager, ok := byUsername[key] @@ -422,7 +429,9 @@ func (s *service) CreateMyClaManagerRequest(ctx context.Context, caller *Caller, } selected[key] = true selectedUsernames = append(selectedUsernames, manager.LfUsername) - if manager.Email != "" { + // Managers can share an address; mail it once, but report both as recipients. + if emailKey := strings.ToLower(manager.Email); emailKey != "" && !emailed[emailKey] { + emailed[emailKey] = true recipientEmails = append(recipientEmails, manager.Email) } } @@ -1056,6 +1065,7 @@ type eclaCoverage struct { type sanctionState struct { flagged bool check string + date string } func (s *service) sanctionsMode() string { @@ -1071,13 +1081,38 @@ func (s *service) companySanctions(ctx context.Context, companyModel *v1Models.C if companyModel == nil { return sanctionState{check: models.MyClaFlaggedCheckUnavailable} } - state := sanctionState{flagged: companyModel.IsSanctioned, check: models.MyClaFlaggedCheckStored} + state := sanctionState{flagged: companyModel.IsSanctioned, check: models.MyClaFlaggedCheckStored, date: companyModel.SanctionedDate} if s.sanctions != nil { state.flagged, state.check = s.sanctions.ScreenCompany(ctx, companyModel) + s.persistLiveSanction(ctx, companyModel, &state) } return state } +// persistLiveSanction stamps sanctioned_date the first time a live screen flags an employer, so +// the reported date stops moving with every listing. A record already carrying the date is left +// alone - restamping it here would drift on each page view - and a failed write only costs this +// employer its stored date, never the listing. +func (s *service) persistLiveSanction(ctx context.Context, companyModel *v1Models.Company, state *sanctionState) { + if !state.flagged || state.check != models.MyClaFlaggedCheckLive || (companyModel.IsSanctioned && companyModel.SanctionedDate != "") { + return + } + f := logrus.Fields{ + "functionName": "v2.my_clas.service.persistLiveSanction", + utils.XREQUESTID: ctx.Value(utils.XREQUESTID), + "companyID": companyModel.CompanyID, + } + if err := s.companyRepo.UpdateCompanySanctionStatus(ctx, companyModel.CompanyID, true, sanctionOriginSSS); err != nil { + log.WithFields(f).WithError(err).Warnf("unable to persist the live sanction for company %s - reporting the observation time instead", companyModel.CompanyID) + // A retained date belongs to the previous, cleared sanction - drop it rather than + // report it as this flag's date. + state.date = "" + return + } + log.WithFields(f).Warnf("live screen flagged company %s, persisted the sanction with origin=%s", companyModel.CompanyID, sanctionOriginSSS) + _, state.date = utils.CurrentTime() +} + // assignMyClaStatus sets the contributor-facing status independently of approved/valid. A // sanctioned employer wins over everything else and carries no user action. func assignMyClaStatus(row *models.MyCla, coverage eclaCoverage) { diff --git a/cla-backend-go/v2/my_clas/service_test.go b/cla-backend-go/v2/my_clas/service_test.go index a99369151..2effb88c4 100644 --- a/cla-backend-go/v2/my_clas/service_test.go +++ b/cla-backend-go/v2/my_clas/service_test.go @@ -117,11 +117,19 @@ func (f *fakeSignatures) EvaluateUserApproval(_ context.Context, user *v1Models. return f.approvedUserIDs[user.UserID], f.orgLookupFailed, nil } +type sanctionWrite struct { + companyID string + sanctioned bool + origin string +} + type fakeCompanies struct { - byID map[string]*v1Models.Company - failIDs map[string]bool - mu sync.Mutex - calls int + byID map[string]*v1Models.Company + failIDs map[string]bool + mu sync.Mutex + calls int + writes []sanctionWrite + writeErr error } func (f *fakeCompanies) GetCompany(_ context.Context, companyID string) (*v1Models.Company, error) { @@ -137,6 +145,16 @@ func (f *fakeCompanies) GetCompany(_ context.Context, companyID string) (*v1Mode return nil, &utils.CompanyNotFound{CompanyID: companyID} } +func (f *fakeCompanies) UpdateCompanySanctionStatus(_ context.Context, companyID string, sanctioned bool, origin string) error { + f.mu.Lock() + defer f.mu.Unlock() + if f.writeErr != nil { + return f.writeErr + } + f.writes = append(f.writes, sanctionWrite{companyID: companyID, sanctioned: sanctioned, origin: origin}) + return nil +} + // fakeScreener stands in for the live SSS screen and records how often each employer was screened type fakeScreener struct { mode string @@ -1110,6 +1128,82 @@ func TestGetMyClasLiveSanctionsScreening(t *testing.T) { assert.Equal(t, models.MyClaStatusRevoked, unavailable.Status) assert.Equal(t, 1, screener.calls["company-1"], "each distinct employer is screened once per response") + assert.Equal(t, []sanctionWrite{{companyID: "company-1", sanctioned: true, origin: sanctionOriginSSS}}, companies.writes, + "only the newly detected sanction is persisted - a live clean and an unusable screen write nothing") +} + +func TestGetMyClasPersistsFirstLiveSanction(t *testing.T) { + const storedDate = "2024-01-15T10:11:12.000000+0000" + + tests := []struct { + name string + company *v1Models.Company + writeErr error + wantWrites int + wantFlaggedAt string + }{ + { + name: "first live detection is persisted", + company: &v1Models.Company{CompanyID: "company-1", CompanyName: "Newly Flagged Corp"}, + wantWrites: 1, + }, + { + name: "an employer flagged again after a clear is restamped", + company: &v1Models.Company{CompanyID: "company-1", CompanyName: "Repeat Corp", SanctionedDate: storedDate}, + wantWrites: 1, + }, + { + name: "an already stamped employer is left alone", + company: &v1Models.Company{CompanyID: "company-1", CompanyName: "Known Corp", IsSanctioned: true, SanctionOrigin: sanctionOriginSSS, SanctionedDate: storedDate}, + wantWrites: 0, + wantFlaggedAt: "2024-01-15T10:11:12Z", + }, + { + name: "a failed write reports the observation, not the cleared episode's date", + company: &v1Models.Company{CompanyID: "company-1", CompanyName: "Unwritable Corp", SanctionedDate: storedDate}, + writeErr: errors.New("dynamodb unavailable"), + wantWrites: 0, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + userA := &v1Models.User{UserID: "user-a", LfUsername: "someone"} + companies := &fakeCompanies{ + byID: map[string]*v1Models.Company{"company-1": tc.company}, + writeErr: tc.writeErr, + } + repo := &fakeRepo{ + byUserID: map[string][]*signatures.ItemSignature{"user-a": {ecla("sig-1", "company-1", "2024-01-01T00:00:00Z", true)}}, + byLFUsername: map[string][]*v1Models.User{"someone": {userA}}, + } + signaturesService := &fakeSignatures{ + cclas: map[string]*v1Models.Signature{"cla-group-1|company-1": {SignatureID: "ccla-1"}}, + approvedUserIDs: map[string]bool{"user-a": true}, + } + svc := newTestService(repo, &fakePlatform{}, signaturesService, companies, &fakeClaGroups{}) + svc.sanctions = &fakeScreener{mode: models.MyClaListSssModeRequired, flagged: map[string]bool{"company-1": true}} + + result, err := svc.GetMyClas(context.Background(), &Caller{Username: "someone"}, &Identity{}) + require.NoError(t, err, "persisting must never fail the listing") + require.Len(t, result.Clas, 1) + row := result.Clas[0] + + assert.Len(t, companies.writes, tc.wantWrites) + if tc.wantWrites > 0 { + assert.Equal(t, sanctionWrite{companyID: "company-1", sanctioned: true, origin: sanctionOriginSSS}, companies.writes[0], + "the listing persists through the same SSS-origin write the signing flow uses") + } + assert.True(t, row.Flagged) + assert.Equal(t, models.MyClaFlaggedCheckLive, row.FlaggedCheck) + if tc.wantFlaggedAt != "" { + assert.Equal(t, tc.wantFlaggedAt, row.FlaggedAt, "the stored date is reported, not the response time") + } else { + assert.NotEmpty(t, row.FlaggedAt) + assert.NotEqual(t, "2024-01-15T10:11:12Z", row.FlaggedAt, "a restamped or unwritten employer reports this observation") + } + }) + } } // countingScreener records how many screens run at once and holds the first want of them open, diff --git a/cla-backend-legacy/internal/api/handlers.go b/cla-backend-legacy/internal/api/handlers.go index ee79b81f8..3cdc4c7c8 100644 --- a/cla-backend-legacy/internal/api/handlers.go +++ b/cla-backend-legacy/internal/api/handlers.go @@ -5175,6 +5175,9 @@ func (h *Handlers) PostCompanyV1(w http.ResponseWriter, r *http.Request) { "date_modified": &types.AttributeValueMemberS{Value: formatPynamoDateTimeUTC(now)}, "version": &types.AttributeValueMemberS{Value: "v1"}, } + if isSanctioned { + item["sanctioned_date"] = &types.AttributeValueMemberS{Value: formatPynamoDateTimeUTC(now)} + } if err := h.companies.PutItem(ctx, item); err != nil { respond.JSON(w, http.StatusInternalServerError, map[string]any{"errors": map[string]any{"server": err.Error()}}) @@ -5260,6 +5263,7 @@ func (h *Handlers) PutCompanyV1(w http.ResponseWriter, r *http.Request) { return } + now := time.Now().UTC() updateStr := "" if req.CompanyName != nil { item["company_name"] = &types.AttributeValueMemberS{Value: *req.CompanyName} @@ -5280,10 +5284,12 @@ func (h *Handlers) PutCompanyV1(w http.ResponseWriter, r *http.Request) { // Manual/admin sanction change: drop any SSS-set origin so this becomes an // admin-controlled state (sticky when true; never later auto-cleared by SSS). delete(item, "sanction_origin") + if *req.IsSanctioned { + item["sanctioned_date"] = &types.AttributeValueMemberS{Value: formatPynamoDateTimeUTC(now)} + } updateStr += fmt.Sprintf("The company is_sanctioned was updated to %t. ", *req.IsSanctioned) } - now := time.Now().UTC() item["date_modified"] = &types.AttributeValueMemberS{Value: formatPynamoDateTimeUTC(now)} if err := h.companies.PutItem(ctx, item); err != nil { diff --git a/cla-backend-legacy/internal/store/companies.go b/cla-backend-legacy/internal/store/companies.go index f79e8efdb..5a6ac81cf 100644 --- a/cla-backend-legacy/internal/store/companies.go +++ b/cla-backend-legacy/internal/store/companies.go @@ -141,59 +141,82 @@ func (s *CompaniesStore) DeleteByID(ctx context.Context, companyID string) error return err } -// UpdateCompanySanctionStatus sets is_sanctioned and, when origin is non-empty, sanction_origin. -// Pass origin="sss" when flagging via SSS; pass origin="" for manual admin updates. -func (s *CompaniesStore) UpdateCompanySanctionStatus(ctx context.Context, companyID string, sanctioned bool, origin string) error { - if s == nil || s.client == nil { - return nil - } - - now := time.Now().UTC().Format("2006-01-02T15:04:05.000000-0700") // Best effort for date_modified parity +// sanctionUpdate is the DynamoDB update for one sanction status change. +type sanctionUpdate struct { + expression string + condition *string + names map[string]string + values map[string]types.AttributeValue +} - names := map[string]string{ - "#S": "is_sanctioned", - "#M": "date_modified", +// buildSanctionUpdate assembles the update for UpdateCompanySanctionStatus. All SET assignments +// stay contiguous ahead of any REMOVE, as DynamoDB requires. +func buildSanctionUpdate(sanctioned bool, origin, now string) sanctionUpdate { + update := sanctionUpdate{ + expression: "SET #S = :s, #M = :m", + names: map[string]string{ + "#S": "is_sanctioned", + "#M": "date_modified", + "#O": "sanction_origin", + }, + values: map[string]types.AttributeValue{ + ":s": &types.AttributeValueMemberBOOL{Value: sanctioned}, + ":m": &types.AttributeValueMemberS{Value: now}, + }, } - values := map[string]types.AttributeValue{ - ":s": &types.AttributeValueMemberBOOL{Value: sanctioned}, - ":m": &types.AttributeValueMemberS{Value: now}, + + // Setting the flag stamps sanctioned_date; clearing it leaves the stored date alone. + if sanctioned { + update.names["#D"] = "sanctioned_date" + update.values[":d"] = &types.AttributeValueMemberS{Value: now} + update.expression += ", #D = :d" } - updateExpr := "SET #S = :s, #M = :m" if origin != "" { - names["#O"] = "sanction_origin" - values[":o"] = &types.AttributeValueMemberS{Value: origin} - updateExpr += ", #O = :o" + update.values[":o"] = &types.AttributeValueMemberS{Value: origin} + update.expression += ", #O = :o" } else { // Manual/admin update: remove any stale SSS-set origin so the record becomes a // sticky admin block (origin absent) that SSS will never auto-clear. - names["#O"] = "sanction_origin" - updateExpr += " REMOVE #O" + update.expression += " REMOVE #O" + } + + // When SSS sets a block, never overwrite a manual/admin block (is_sanctioned=true + // with absent or non-"sss" origin). Only set the SSS flag when the company is + // currently unblocked or already SSS-blocked; a ConditionalCheckFailedException + // means a manual/admin block is already present and must be preserved. + if sanctioned && origin == "sss" { + update.values[":false"] = &types.AttributeValueMemberBOOL{Value: false} + update.condition = aws.String("attribute_not_exists(#S) OR #S = :false OR #O = :o") } + return update +} + +// UpdateCompanySanctionStatus sets is_sanctioned and, when origin is non-empty, sanction_origin. +// Pass origin="sss" when flagging via SSS; pass origin="" for manual admin updates. +func (s *CompaniesStore) UpdateCompanySanctionStatus(ctx context.Context, companyID string, sanctioned bool, origin string) error { + if s == nil || s.client == nil { + return nil + } + + now := time.Now().UTC().Format("2006-01-02T15:04:05.000000-0700") // Best effort for date_modified parity + update := buildSanctionUpdate(sanctioned, origin, now) + input := &dynamodb.UpdateItemInput{ TableName: aws.String(s.table), Key: map[string]types.AttributeValue{ "company_id": &types.AttributeValueMemberS{Value: companyID}, }, - UpdateExpression: aws.String(updateExpr), - ExpressionAttributeNames: names, - ExpressionAttributeValues: values, - } - - // When SSS sets a block, never overwrite a manual/admin block (is_sanctioned=true - // with absent or non-"sss" origin). Only set the SSS flag when the company is - // currently unblocked or already SSS-blocked; a ConditionalCheckFailedException - // means a manual/admin block is already present and must be preserved. - sssSettingBlock := sanctioned && origin == "sss" - if sssSettingBlock { - values[":false"] = &types.AttributeValueMemberBOOL{Value: false} - input.ConditionExpression = aws.String("attribute_not_exists(#S) OR #S = :false OR #O = :o") + UpdateExpression: aws.String(update.expression), + ConditionExpression: update.condition, + ExpressionAttributeNames: update.names, + ExpressionAttributeValues: update.values, } _, err := s.client.UpdateItem(ctx, input) if err != nil { - if sssSettingBlock { + if update.condition != nil { var condErr *types.ConditionalCheckFailedException if errors.As(err, &condErr) { return nil // Preserve the existing manual/admin block diff --git a/cla-backend-legacy/internal/store/companies_test.go b/cla-backend-legacy/internal/store/companies_test.go new file mode 100644 index 000000000..f8aea886d --- /dev/null +++ b/cla-backend-legacy/internal/store/companies_test.go @@ -0,0 +1,102 @@ +// Copyright The Linux Foundation and each contributor to CommunityBridge. +// SPDX-License-Identifier: MIT + +package store + +import ( + "strings" + "testing" + + "github.com/aws/aws-sdk-go-v2/service/dynamodb/types" +) + +// TestBuildSanctionUpdate locks in the sanctioned_date semantics: the date is stamped on every +// flag-set and never touched when the flag is cleared. +func TestBuildSanctionUpdate(t *testing.T) { + const now = "2026-08-20T10:11:12.000000+0000" + + tests := []struct { + name string + sanctioned bool + origin string + expression string + condition string + stampedDate bool + }{ + { + name: "sss flags the company", + sanctioned: true, + origin: "sss", + expression: "SET #S = :s, #M = :m, #D = :d, #O = :o", + condition: "attribute_not_exists(#S) OR #S = :false OR #O = :o", + stampedDate: true, + }, + { + name: "sss clears the company", + sanctioned: false, + origin: "sss", + expression: "SET #S = :s, #M = :m, #O = :o", + }, + { + name: "admin flags the company", + sanctioned: true, + origin: "", + expression: "SET #S = :s, #M = :m, #D = :d REMOVE #O", + stampedDate: true, + }, + { + name: "admin clears the company", + sanctioned: false, + origin: "", + expression: "SET #S = :s, #M = :m REMOVE #O", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + update := buildSanctionUpdate(tc.sanctioned, tc.origin, now) + + if update.expression != tc.expression { + t.Fatalf("expression = %q, want %q", update.expression, tc.expression) + } + switch { + case tc.condition == "" && update.condition != nil: + t.Fatalf("condition = %q, want none: only an SSS-set flag is conditional", *update.condition) + case tc.condition != "" && update.condition == nil: + t.Fatal("missing condition: the manual/admin block must stay protected") + case tc.condition != "" && *update.condition != tc.condition: + t.Fatalf("condition = %q, want %q", *update.condition, tc.condition) + } + + _, hasName := update.names["#D"] + date, hasValue := update.values[":d"] + if hasName != tc.stampedDate || hasValue != tc.stampedDate { + t.Fatalf("sanctioned_date stamped = %v/%v, want %v", hasName, hasValue, tc.stampedDate) + } + if tc.stampedDate { + if update.names["#D"] != "sanctioned_date" { + t.Fatalf("#D = %q, want sanctioned_date", update.names["#D"]) + } + if got := date.(*types.AttributeValueMemberS).Value; got != now { + t.Fatalf("sanctioned_date = %q, want %q: stamped with the same time as the flag", got, now) + } + } + + // Every declared name and value has to be referenced, or DynamoDB rejects the update. + full := update.expression + if update.condition != nil { + full += " " + *update.condition + } + for name := range update.names { + if !strings.Contains(full, name) { + t.Errorf("name %s declared but never referenced", name) + } + } + for value := range update.values { + if !strings.Contains(full, value) { + t.Errorf("value %s declared but never referenced", value) + } + } + }) + } +} diff --git a/docs/MY_CLAS_API.md b/docs/MY_CLAS_API.md index 9540fdefb..b56b194fc 100644 --- a/docs/MY_CLAS_API.md +++ b/docs/MY_CLAS_API.md @@ -45,7 +45,7 @@ Files changed in `easycla`: - `cla-backend-go/v2/my_clas/handlers.go` — swagger operation wiring (`Configure`) - `cla-backend-go/v2/my_clas/service.go` — ownership enforcement, identity resolution, aggregation, validity evaluation - `cla-backend-go/v2/my_clas/prefetch.go` — per-request concurrent prefetch of every distinct external lookup -- `cla-backend-go/v2/my_clas/sanctions.go` — read-only sanctions screener (live SSS lookup, never persists what it observes) +- `cla-backend-go/v2/my_clas/sanctions.go` — sanctions screener (live SSS lookup; the screener never writes, the service persists a first detection) - `cla-backend-go/v2/my_clas/repository.go` — plural, paginated GSI queries for identity resolution and the user's ICLA/ECLA records, plus the single-scan secondary-email lookup - `cla-backend-go/emails/contact_cla_manager_templates.go`, `cla-backend-go/events/event_data.go`, `event_types.go` — the contact-request email and its audit event - `cla-backend-go/v2/my_clas/*_test.go` — unit tests @@ -396,9 +396,15 @@ Sanctions Screening Service for a live answer, and reports how the answer was ob The response-level `sssMode` (`required` / `optional` / `disabled`) tells the consumer how much weight `unavailable` carries: in `required` mode an unverified row is a real gap, in `optional` mode best-effort. **A screening failure never fails this endpoint** in either -mode — unlike the signing flow, which may fail closed. The screener is also strictly -read-only: `v2/sign`'s `checkCompanyCompliance` *persists* what it observes, which a GET must -not do, so the lookup/domain/status logic is duplicated rather than shared. +mode — unlike the signing flow, which may fail closed. The screener itself only answers the +question (the lookup/domain/status logic is duplicated from `v2/sign`'s +`checkCompanyCompliance` rather than shared), but a *first* live detection is persisted — +`is_sanctioned` plus `sanctioned_date` with origin `sss` — so `flaggedAt` stops moving between +listings. An employer *currently* sanctioned and already carrying the date is never restamped +by the listing; one that was cleared but retained its date is restamped on the next live +detection, and the signing and legacy SSS flows restamp on every flagged detection by design. +A failed write costs that employer only its stored date (the row then reports the observation +time), and the listing never clears a flag. ### Response — `200 my-cla-list` @@ -469,7 +475,7 @@ Field reference (`my-cla` rows): | `valid` | bool | Computed as defined above | | `status` | `valid` \| `needs_attention` \| `revoked` \| `invalidated` \| `unknown` | Contributor-facing standing, computed independently of `approved`/`valid` (see step 5). New values may be added in a future spec revision (generated clients validate the enum strictly) | | `statusReason` | `not_on_approval_list` \| `unknown` | Why the standing is not `valid`; omitted for every other status and on every ICLA | -| `flagged` / `flaggedAt` | bool / string | ECLA only: the employer is currently flagged by sanctions screening, and when that was observed (response time — no sanction timestamp is stored yet) | +| `flagged` / `flaggedAt` | bool / string | ECLA only: the employer is currently flagged by sanctions screening, and the company's stored `sanctioned_date` — stamped at the first live detection, so the response time only when no date is stored and stamping it failed | | `flaggedCheck` | `live` \| `stored` \| `unavailable` | ECLA only: how `flagged` was obtained (see step 5). `unavailable` means the value is the persisted flag and may be stale | | `signedVia` / `signedAs` | string | The platform signed via (`github`, `gitlab`, `gerrit` — the last also covers LF SSO signings identified by email) and the account signed as; omitted when the record carries no such identity | | `claManager` | bool | ECLA only: the owning user is a CLA manager of the employer's CCLA for this CLA Group | @@ -681,8 +687,10 @@ the latency envelope is to be confirmed on dev. `ssm delete-parameter` plus the same restart. A read failure other than a missing parameter is logged as a warning and leaves the path disabled — non-admin callers keep having every identity verified per request — and never aborts the other lambdas that load this config. -5. Read-only rollback: revert the ACS sync (or never flip the SS feature flag); the endpoints - write nothing. +5. Read-only rollback: revert the ACS sync (or never flip the SS feature flag). The only write + these endpoints make is a sanction stamp on the company row at the first live detection of + each sanction episode — ordinary company-table data that is safe to leave in place and + never needs reverting. ## Verification performed diff --git a/utils/update_company_is_sanctioned.sh b/utils/update_company_is_sanctioned.sh index d2807abd3..e3241d3ea 100755 --- a/utils/update_company_is_sanctioned.sh +++ b/utils/update_company_is_sanctioned.sh @@ -16,8 +16,17 @@ then echo "$0: you need to value: true|false" exit 2 fi +# Mirrors the backends' admin path: stamp sanctioned_date on set, keep it on clear, and drop +# sanction_origin so the manual state is sticky and SSS never auto-clears it. +upd_expr="SET is_sanctioned = :val REMOVE sanction_origin" +values="{\":val\":{\"BOOL\":${2}}}" +if [ "$2" = "true" ] +then + upd_expr="SET is_sanctioned = :val, sanctioned_date = :now REMOVE sanction_origin" + values="{\":val\":{\"BOOL\":true},\":now\":{\"S\":\"$(date -u '+%Y-%m-%dT%H:%M:%S.%6N+0000')\"}}" +fi if [ ! -z "$DEBUG" ] then - echo aws --profile "lfproduct-$STAGE" dynamodb update-item --table-name "cla-${STAGE}-companies" --key "{\"company_id\":{\"S\":\"${1}\"}}" --update-expression '"SET is_sanctioned = :val"' --expression-attribute-values "{\":val\":{\"BOOL\":${2}}}" + echo aws --profile "lfproduct-$STAGE" dynamodb update-item --table-name "cla-${STAGE}-companies" --key "{\"company_id\":{\"S\":\"${1}\"}}" --update-expression "\"${upd_expr}\"" --expression-attribute-values "$values" fi -aws --profile "lfproduct-$STAGE" dynamodb update-item --table-name "cla-${STAGE}-companies" --key "{\"company_id\":{\"S\":\"${1}\"}}" --update-expression "SET is_sanctioned = :val" --expression-attribute-values "{\":val\":{\"BOOL\":${2}}}" +aws --profile "lfproduct-$STAGE" dynamodb update-item --table-name "cla-${STAGE}-companies" --key "{\"company_id\":{\"S\":\"${1}\"}}" --update-expression "$upd_expr" --expression-attribute-values "$values"