diff --git a/cla-backend-go/swagger/cla.v2.yaml b/cla-backend-go/swagger/cla.v2.yaml index adaa63d89..51b433b48 100644 --- a/cla-backend-go/swagger/cla.v2.yaml +++ b/cla-backend-go/swagger/cla.v2.yaml @@ -3005,7 +3005,7 @@ paths: /self-serve/request-corporate-signature: post: summary: Requests and generates a new Corporate Signature from LFX Self Serve - description: Creates a new corporate signature given project and company IDs, exactly as /request-corporate-signature does, after verifying the two Self Serve attestations - the request is rejected with a 400 unless both authority_acked and embargo_acked are true. A company blocked for trade compliance is rejected with the typed 403 `company_sanctioned` body described by the 403 response + description: Creates a new corporate signature given project and company IDs, exactly as /request-corporate-signature does, after verifying the two Self Serve attestations on self-sign - the request is rejected with a 400 unless both authority_acked and embargo_acked are true, except when send_as_email is true, in which case authority_name and authority_email are required instead and the attestations are not. A company blocked for trade compliance is rejected with the typed 403 `company_sanctioned` body described by the 403 response operationId: selfServeRequestCorporateSignature parameters: - $ref: "#/parameters/authorization" diff --git a/cla-backend-go/swagger/common/self-serve-corporate-signature-input.yaml b/cla-backend-go/swagger/common/self-serve-corporate-signature-input.yaml index 08a4ddded..7c02c6d79 100644 --- a/cla-backend-go/swagger/common/self-serve-corporate-signature-input.yaml +++ b/cla-backend-go/swagger/common/self-serve-corporate-signature-input.yaml @@ -4,7 +4,7 @@ type: object x-nullable: false title: Self Serve Corporate Signature Input -description: The corporate signature request started from LFX Self Serve - the corporate-signature-input fields plus the two Self Serve attestations, both of which must be true +description: The corporate signature request started from LFX Self Serve - the corporate-signature-input fields plus the two Self Serve attestations. Both attestations must be true on self-sign; when send_as_email is true they are not required and authority_name plus authority_email are required instead required: - project_sfid - company_sfid @@ -24,7 +24,7 @@ properties: send_as_email: type: boolean example: false - description: send signing request as email. This should be set to true when requestor is not signatory. + description: send signing request as email. Set to true when the requestor is not the signatory - then authority_name and authority_email are required and the attestations are not authority_name: type: string example: "Derk Miyamoto" @@ -43,9 +43,9 @@ properties: type: boolean x-omitempty: false example: true - description: attestation that the caller is the signing authority for the company, or has named the signing authority via authority_name/authority_email - must be true + description: attestation that the caller is the signing authority for the company - must be true unless send_as_email is true embargo_acked: type: boolean x-omitempty: false example: true - description: attestation that the caller acknowledges the embargo terms - must be true + description: attestation that the caller acknowledges the embargo terms - must be true unless send_as_email is true diff --git a/cla-backend-go/v2/self_serve_sign/handlers.go b/cla-backend-go/v2/self_serve_sign/handlers.go index b35ede5b4..823347cdc 100644 --- a/cla-backend-go/v2/self_serve_sign/handlers.go +++ b/cla-backend-go/v2/self_serve_sign/handlers.go @@ -113,7 +113,7 @@ func requestCorporateSignatureError(reqID string, err error) middleware.Responde return utils.CompanySanctionedResponder(reqID, sanctionedErr) } switch { - case errors.Is(err, ErrAttestationRequired): + case errors.Is(err, ErrAttestationRequired), errors.Is(err, ErrSignatoryRequired): return selfServeSignOps.NewSelfServeRequestCorporateSignatureBadRequest().WithXRequestID(reqID).WithPayload(utils.ErrorResponseBadRequest(reqID, err.Error())) case errors.Is(err, ErrSigningEntityMismatch): return selfServeSignOps.NewSelfServeRequestCorporateSignatureForbidden().WithXRequestID(reqID).WithPayload(utils.ErrorResponseForbidden(reqID, err.Error())) diff --git a/cla-backend-go/v2/self_serve_sign/handlers_test.go b/cla-backend-go/v2/self_serve_sign/handlers_test.go index 0965556bb..b25b4d608 100644 --- a/cla-backend-go/v2/self_serve_sign/handlers_test.go +++ b/cla-backend-go/v2/self_serve_sign/handlers_test.go @@ -383,6 +383,7 @@ func TestSelfServeRequestCorporateSignatureHandlerErrorMapping(t *testing.T) { expectedText string }{ {"attestations missing", ErrAttestationRequired, http.StatusBadRequest, "authority_acked and embargo_acked"}, + {"signatory missing", ErrSignatoryRequired, http.StatusBadRequest, "authority_name and authority_email"}, {"signing entity mismatch", ErrSigningEntityMismatch, http.StatusForbidden, "signing entity name does not belong to the provided company SFID"}, {"company unknown", errors.New("company does not exist"), http.StatusNotFound, "company does not exist"}, {"platform failure", errors.New("internal server error - docusign unavailable"), http.StatusInternalServerError, "internal server error"}, diff --git a/cla-backend-go/v2/self_serve_sign/service.go b/cla-backend-go/v2/self_serve_sign/service.go index 078935bb7..ea73f4f1a 100644 --- a/cla-backend-go/v2/self_serve_sign/service.go +++ b/cla-backend-go/v2/self_serve_sign/service.go @@ -43,9 +43,12 @@ var ErrSigningNotEnabled = errors.New("the cla group has neither an individual n // ErrReturnURLNotSupported is returned when the return URL is not an absolute https URL var ErrReturnURLNotSupported = errors.New("returnUrl must be an absolute https URL") -// ErrAttestationRequired is returned when either Self Serve attestation is missing or false +// ErrAttestationRequired is returned when either Self Serve attestation is missing or false on self-sign var ErrAttestationRequired = errors.New("both the authority_acked and embargo_acked attestations must be true") +// ErrSignatoryRequired is returned when send_as_email is set without a named signatory +var ErrSignatoryRequired = errors.New("send_as_email requires authority_name and authority_email") + // ErrSigningEntityMismatch is returned when the requested signing entity belongs to a different company var ErrSigningEntityMismatch = errors.New("signing entity name does not belong to the provided company SFID") @@ -234,10 +237,11 @@ func (s *service) PrepareSign(ctx context.Context, caller *my_clas.Caller, curre return result, nil } -// RequestCorporateSignature verifies both Self Serve attestations, resolves the signing company -// and the CLA group of the project - rejecting a signing entity that belongs to a different -// company than the one the caller is authorized for - delegates the request verbatim to the -// shared corporate signing service, and echoes the identifiers of the created signature +// RequestCorporateSignature verifies both Self Serve attestations on self-sign, requires the +// named signatory when sending by email, resolves the signing company and the CLA group of +// the project - rejecting a signing entity that belongs to a different company than the one +// the caller is authorized for - delegates the request verbatim to the shared corporate +// signing service, and echoes the identifiers of the created signature func (s *service) RequestCorporateSignature(ctx context.Context, lfUsername, authorizationHeader string, input *models.SelfServeCorporateSignatureInput) (*models.SelfServeCorporateSignatureOutput, error) { f := logrus.Fields{ "functionName": "v2.self_serve_sign.service.RequestCorporateSignature", @@ -246,9 +250,15 @@ func (s *service) RequestCorporateSignature(ctx context.Context, lfUsername, aut "projectSFID": utils.StringValue(input.ProjectSfid), "companySFID": utils.StringValue(input.CompanySfid), "signingEntityName": input.SigningEntityName, + "sendAsEmail": input.SendAsEmail, } - if !input.AuthorityAcked || !input.EmbargoAcked { + if input.SendAsEmail { + if strings.TrimSpace(input.AuthorityName) == "" || strings.TrimSpace(input.AuthorityEmail.String()) == "" { + log.WithFields(f).Warn(ErrSignatoryRequired.Error()) + return nil, ErrSignatoryRequired + } + } else if !input.AuthorityAcked || !input.EmbargoAcked { log.WithFields(f).Warn(ErrAttestationRequired.Error()) return nil, ErrAttestationRequired } diff --git a/cla-backend-go/v2/self_serve_sign/service_test.go b/cla-backend-go/v2/self_serve_sign/service_test.go index 292081aaf..c84e20a94 100644 --- a/cla-backend-go/v2/self_serve_sign/service_test.go +++ b/cla-backend-go/v2/self_serve_sign/service_test.go @@ -735,6 +735,41 @@ func corporateInput() *models.SelfServeCorporateSignatureInput { } func TestRequestCorporateSignatureRequiresBothAttestations(t *testing.T) { + testCases := []struct { + name string + authorityAcked bool + embargoAcked bool + authorityName string + authorityEmail strfmt.Email + }{ + {"both missing", false, false, "", ""}, + {"authority only", true, false, "", ""}, + {"embargo only", false, true, "", ""}, + {"self-sign with a named signatory still requires both", false, false, "Alex Contributor", "contributor@example.org"}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + corporateSign := &fakeCorporateSign{} + companies, projectsClaGroups := corporateFakes() + svc := newCorporateTestService(corporateSign, companies, projectsClaGroups) + input := corporateInput() + input.AuthorityAcked = tc.authorityAcked + input.EmbargoAcked = tc.embargoAcked + input.AuthorityName = tc.authorityName + input.AuthorityEmail = tc.authorityEmail + + result, err := svc.RequestCorporateSignature(context.Background(), "lgryglicki", "Bearer token", input) + + assert.ErrorIs(t, err, ErrAttestationRequired) + assert.Nil(t, result) + assert.Zero(t, corporateSign.calls) + assert.Zero(t, companies.calls) + }) + } +} + +func TestRequestCorporateSignatureSkipsAttestationsWhenSendAsEmail(t *testing.T) { testCases := []struct { name string authorityAcked bool @@ -743,20 +778,62 @@ func TestRequestCorporateSignatureRequiresBothAttestations(t *testing.T) { {"both missing", false, false}, {"authority only", true, false}, {"embargo only", false, true}, + {"both true", true, true}, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - corporateSign := &fakeCorporateSign{} + corporateSign := &fakeCorporateSign{output: &models.CorporateSignatureOutput{SignatureID: testSignatureID, SignURL: testSignURL}} companies, projectsClaGroups := corporateFakes() svc := newCorporateTestService(corporateSign, companies, projectsClaGroups) input := corporateInput() + input.SendAsEmail = true input.AuthorityAcked = tc.authorityAcked input.EmbargoAcked = tc.embargoAcked + input.AuthorityName = "Alex Contributor" + input.AuthorityEmail = "contributor@example.org" result, err := svc.RequestCorporateSignature(context.Background(), "lgryglicki", "Bearer token", input) - assert.ErrorIs(t, err, ErrAttestationRequired) + assert.NoError(t, err) + assert.Equal(t, 1, corporateSign.calls) + assert.True(t, corporateSign.input.SendAsEmail) + assert.Equal(t, "Alex Contributor", corporateSign.input.AuthorityName) + assert.Equal(t, strfmt.Email("contributor@example.org"), corporateSign.input.AuthorityEmail) + assert.Equal(t, testSignatureID, result.SignatureID) + assert.Equal(t, testCLAGroupID, result.ClaGroupID) + }) + } +} + +func TestRequestCorporateSignatureRequiresSignatoryWhenSendAsEmail(t *testing.T) { + testCases := []struct { + name string + authorityName string + authorityEmail strfmt.Email + }{ + {"name missing", "", "contributor@example.org"}, + {"name whitespace", " ", "contributor@example.org"}, + {"email missing", "Alex Contributor", ""}, + {"email whitespace", "Alex Contributor", " "}, + {"both missing", "", ""}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + corporateSign := &fakeCorporateSign{} + companies, projectsClaGroups := corporateFakes() + svc := newCorporateTestService(corporateSign, companies, projectsClaGroups) + input := corporateInput() + input.SendAsEmail = true + input.AuthorityAcked = false + input.EmbargoAcked = false + input.AuthorityName = tc.authorityName + input.AuthorityEmail = tc.authorityEmail + + result, err := svc.RequestCorporateSignature(context.Background(), "lgryglicki", "Bearer token", input) + + assert.ErrorIs(t, err, ErrSignatoryRequired) assert.Nil(t, result) assert.Zero(t, corporateSign.calls) assert.Zero(t, companies.calls) diff --git a/docs/M3_ORG_LENS_API.md b/docs/M3_ORG_LENS_API.md index fe2de9356..676374fe6 100644 --- a/docs/M3_ORG_LENS_API.md +++ b/docs/M3_ORG_LENS_API.md @@ -153,14 +153,16 @@ served from an in-process cache with a 30-minute TTL (`CLA_SEARCH_CACHE_TTL` ove result is company-agnostic by design — a sanctioned company's state is resolved through the cla-groups endpoint above (which only carries it once the company has a CCLA), not here. -## `POST /v4/self-serve/request-corporate-signature` ([lfx-self-serve#2150](https://github.com/linuxfoundation/lfx-self-serve/issues/2150)) +## `POST /v4/self-serve/request-corporate-signature` ([lfx-self-serve#2150](https://github.com/linuxfoundation/lfx-self-serve/issues/2150), [lfx-self-serve#2590](https://github.com/linuxfoundation/lfx-self-serve/issues/2590)) Self Serve front door for starting a CCLA signing session. Input = the corporate-console `corporate-signature-input` fields (`project_sfid`, `company_sfid`, optional `signing_entity_name`, `send_as_email`, `authority_name`, `authority_email`, -`return_url`) plus two required-true attestation booleans `authority_acked` and -`embargo_acked` — HTTP 400 before any DocuSign work unless both are true. With both true -the request is delegated **verbatim** to the same service method behind +`return_url`) plus two attestation booleans `authority_acked` and `embargo_acked`. +On self-sign (`send_as_email` absent or false) both attestations must be true — HTTP 400 +before any DocuSign work otherwise. When `send_as_email` is true the attestations are not +required and `authority_name` plus `authority_email` are required instead. +With a valid body the request is delegated **verbatim** to the same service method behind `/v4/request-corporate-signature` (company/signing-entity resolution, sanctions gate, DocuSign envelope, send-by-email signatory flow all unchanged), so behavior and error statuses match the console endpoint — with one hardening on top: a `signing_entity_name` diff --git a/utils/self_serve_request_corporate_signature.sh b/utils/self_serve_request_corporate_signature.sh index e5389271d..2876e778d 100755 --- a/utils/self_serve_request_corporate_signature.sh +++ b/utils/self_serve_request_corporate_signature.sh @@ -5,7 +5,7 @@ # Calls POST /v4/self-serve/request-corporate-signature through lfx-gateway and reports the HTTP status and total time. # WARNING: a 200 response creates a real DocuSign envelope (and sends a signing email when SEND_AS_EMAIL=true) in the targeted environment. # PROJECT_SFID (or 1st arg) and COMPANY_SFID (or 2nd arg): required. -# AUTHORITY_ACKED / EMBARGO_ACKED: default true - set to false to probe the attestation 400 (no DocuSign side effects). +# AUTHORITY_ACKED / EMBARGO_ACKED: default true - set to false to probe the attestation 400 on self-sign (no DocuSign side effects). SEND_AS_EMAIL=true skips that gate; AUTHORITY_NAME and AUTHORITY_EMAIL are required instead. # SIGNING_ENTITY_NAME, SEND_AS_EMAIL, AUTHORITY_NAME, AUTHORITY_EMAIL, RETURN_URL: optional corporate-signature-input passthrough fields. # TOKEN: bearer access token (env, or ./self_serve_request_corporate_signature.token.secret / ./auth0.token.secret). Get one with ~/get_oauth_token.sh (dev) or ~/get_oauth_token_prod.sh (prod). # STAGE: dev (default) | test | staging | prod - selects the api-gw host.