Merge pull request #4961 from linuxfoundation/unicron-4939-handle-lf_email-prod

lukaszgryglicki · web-flow · commit 56ce6b4a47bb · 2026-04-01T06:39:50.000+02:00
Handle lf email per provider (read only) - prod
diff --git a/cla-backend-go/v2/sign/service.go b/cla-backend-go/v2/sign/service.go
@@ -1333,7 +1333,7 @@ func (s *service) RequestIndividualSignature(ctx context.Context, input *models.
 
 	// creating individual default values
 	log.WithFields(f).Debugf("creating individual default values...")
-	defaultValues := s.createDefaultIndividualValues(user, preferredEmail)
+	defaultValues := s.createDefaultIndividualValues(user, preferredEmail, false)
 	log.WithFields(f).Debugf("default values: %+v", defaultValues)
 
 	// 4. Generate signature callback url
@@ -2057,11 +2057,12 @@ func (s *service) populateUserDetails(ctx context.Context, signatureReferenceTyp
 				return nil, errors.New(msg)
 			}
 
+			allowLFEmail := strings.EqualFold(latestSignature.SignatureReturnURLType, "gerrit")
 			if userModel.Username != "" {
 				userSignDetails.userSignatureName = userModel.Username
 			}
-			if getUserEmail(userModel, preferredEmail) != "" {
-				userSignDetails.userSignatureEmail = getUserEmail(userModel, preferredEmail)
+			if userEmail := getUserEmail(userModel, preferredEmail, allowLFEmail); userEmail != "" {
+				userSignDetails.userSignatureEmail = userEmail
 			}
 		}
 	} else {
@@ -2161,13 +2162,16 @@ func getTabsFromDocument(document *v1Models.ClaGroupDocument, documentID string,
 }
 
 // helper function to get user email
-func getUserEmail(user *v1Models.User, preferredEmail string) string {
+func getUserEmail(user *v1Models.User, preferredEmail string, allowLFEmail bool) string {
+	if user == nil {
+		return ""
+	}
 	if preferredEmail != "" {
-		if utils.StringInSlice(preferredEmail, user.Emails) || user.LfEmail == strfmt.Email(preferredEmail) {
+		if utils.StringInSlice(preferredEmail, user.Emails) || (allowLFEmail && user.LfEmail == strfmt.Email(preferredEmail)) {
 			return preferredEmail
 		}
 	}
-	if user.LfEmail != "" {
+	if allowLFEmail && user.LfEmail != "" {
 		return string(user.LfEmail)
 	}
 	if len(user.Emails) > 0 {
@@ -2219,7 +2223,7 @@ func getActiveSignatureReturnURL(userID string, metadata map[string]interface{})
 
 }
 
-func (s *service) createDefaultIndividualValues(user *v1Models.User, preferredEmail string) map[string]interface{} {
+func (s *service) createDefaultIndividualValues(user *v1Models.User, preferredEmail string, allowLFEmail bool) map[string]interface{} {
 	f := logrus.Fields{
 		"functionName": "sign.createDefaultIndiviualValues",
 	}
@@ -2234,10 +2238,8 @@ func (s *service) createDefaultIndividualValues(user *v1Models.User, preferredEm
 		}
 	}
 
-	if preferredEmail != "" {
-		if utils.StringInSlice(preferredEmail, user.Emails) || user.LfEmail == strfmt.Email(preferredEmail) {
-			defaultValues["email"] = preferredEmail
-		}
+	if userEmail := getUserEmail(user, preferredEmail, allowLFEmail); userEmail != "" {
+		defaultValues["email"] = userEmail
 	}
 
 	return defaultValues
@@ -2336,7 +2338,7 @@ func (s *service) RequestIndividualSignatureGerrit(ctx context.Context, input *m
 		preferredEmail = user.Emails[0]
 	}
 
-	defaultValues := s.createDefaultIndividualValues(user, preferredEmail)
+	defaultValues := s.createDefaultIndividualValues(user, preferredEmail, true)
 
 	log.WithFields(f).Debugf("defaultValues: %+v", defaultValues)
 
diff --git a/cla-backend/cla/models/docusign_models.py b/cla-backend/cla/models/docusign_models.py
@@ -177,18 +177,32 @@ def request_individual_signature(self, project_id, user_id, return_url=None, ret
                       format(project))
 
         cla.log.debug('Individual Signature - creating default individual values for user: {}'.format(user))
-        default_cla_values = create_default_individual_values(user)
+        provider_type = (return_url_type or '').lower()
+        if provider_type not in ('github', 'gitlab'):
+            provider_label = provider_type or 'missing'
+            cla.log.warning('Individual Signature - unsupported provider_type "{}" for {}'.format(
+                provider_label, request_info))
+            return {'errors': {'return_url_type': 'unsupported provider_type: {}'.format(provider_label)}}
+        default_cla_values = create_default_individual_values(user, preferred_email=preferred_email,
+                                                              provider_type=provider_type)
         cla.log.debug('Individual Signature - created default individual values: {}'.format(default_cla_values))
 
+        resolved_email = default_cla_values.get('email')
+        if provider_type in ('github', 'gitlab'):
+            resolved_email = resolved_email.strip() if isinstance(resolved_email, str) else None
+            if not resolved_email:
+                return {'errors': {'user_id': f'no {provider_type} user_emails found'}}
+            default_cla_values['email'] = resolved_email
+
         # Generate signature callback url
         cla.log.debug('Individual Signature - get active signature metadata')
         signature_metadata = cla.utils.get_active_signature_metadata(user_id)
         cla.log.debug('Individual Signature - get active signature metadata: {}'.format(signature_metadata))
 
         cla.log.debug('Individual Signature - get individual signature callback url')
-        if return_url_type.lower() == "github":
+        if provider_type == "github":
             callback_url = cla.utils.get_individual_signature_callback_url(user_id, signature_metadata)
-        elif return_url_type.lower() == "gitlab":
+        elif provider_type == "gitlab":
             callback_url = cla.utils.get_individual_signature_callback_url_gitlab(user_id, signature_metadata)
 
         cla.log.debug('Individual Signature - get individual signature callback url: {}'.format(callback_url))
@@ -261,12 +275,12 @@ def request_individual_signature(self, project_id, user_id, return_url=None, ret
                               signature_return_url=return_url,
                               signature_callback_url=callback_url)
         # Set signature ACL
-        if return_url_type.lower() == "github":
+        if provider_type == "github":
             acl = user.get_user_github_id()
-        elif return_url_type.lower() == "gitlab":
+        elif provider_type == "gitlab":
             acl = user.get_user_gitlab_id()
-        cla.log.debug('Individual Signature - setting ACL using user {} id: {}'.format(return_url_type, acl))
-        signature.set_signature_acl('{}:{}'.format(return_url_type.lower(),acl))
+        cla.log.debug('Individual Signature - setting ACL using provider {} id: {}'.format(provider_type, acl))
+        signature.set_signature_acl('{}:{}'.format(provider_type,acl))
 
         # Populate sign url
         self.populate_sign_url(signature, callback_url, default_values=default_cla_values,
@@ -1346,14 +1360,16 @@ def populate_sign_url(self, signature, callback_url=None,
                     cla.log.debug(f'{fn} - {sig_type} - '
                                   f'loading user by reference id: {signature.get_signature_reference_id()}')
                     user.load(signature.get_signature_reference_id())
+                    provider_type = (signature.get_signature_return_url_type() or '').lower()
+                    resolved_user_email = resolve_individual_user_email(user, provider_type, preferred_email)
                     cla.log.debug(f'{fn} - {sig_type} - loaded user by '
                                   f'id: {user.get_user_id()}, '
                                   f'name: {user.get_user_name()}, '
-                                  f'email: {user.get_user_email()}')
+                                  f'email: {resolved_user_email}')
                     if not user.get_user_name() is None:
                         user_signature_name = user.get_user_name()
-                    if not user.get_user_email() is None:
-                        user_signature_email = user.get_user_email()
+                    if resolved_user_email is not None:
+                        user_signature_email = resolved_user_email
                 except DoesNotExist:
                     cla.log.warning(f'{fn} - {sig_type} - no user associated with this signature '
                                     f'id: {signature.get_signature_reference_id()} - can not sign ICLA')
@@ -2406,7 +2422,32 @@ def create_default_company_values(company: Company,
     return values
 
 
-def create_default_individual_values(user: User, preferred_email: str = None) -> Dict[str, Any]:
+def resolve_individual_user_email(user: User, provider_type: str = None,
+                                  preferred_email: str = None) -> Optional[str]:
+    if user is None:
+        return None
+
+    provider_type = (provider_type or '').lower()
+    user_emails = user.get_user_emails() or set()
+
+    if preferred_email and preferred_email in user_emails:
+        return preferred_email
+
+    if provider_type not in ('github', 'gitlab'):
+        lf_email = user.get_lf_email()
+        if preferred_email and lf_email is not None and preferred_email == lf_email:
+            return preferred_email
+        if lf_email is not None:
+            return lf_email
+
+    if len(user_emails) > 0:
+        return next(iter(user_emails), None)
+
+    return None
+
+
+def create_default_individual_values(user: User, preferred_email: str = None,
+                                     provider_type: str = None) -> Dict[str, Any]:
     values = {}
 
     if user is None:
@@ -2416,8 +2457,9 @@ def create_default_individual_values(user: User, preferred_email: str = None) ->
         values['full_name'] = user.get_user_name()
         values['public_name'] = user.get_user_name()
 
-    if user.get_user_email(preferred_email=preferred_email) is not None:
-        values['email'] = user.get_user_email()
+    resolved_email = resolve_individual_user_email(user, provider_type, preferred_email)
+    if resolved_email is not None:
+        values['email'] = resolved_email
 
     return values
 
diff --git a/cla-backend/cla/tests/unit/test_docusign_models.py b/cla-backend/cla/tests/unit/test_docusign_models.py
@@ -6,6 +6,7 @@
 from cla.models.docusign_models import (ClaSignatoryEmailParams,
                                         cla_signatory_email_content,
                                         create_default_company_values,
+                                        create_default_individual_values,
                                         document_signed_email_content,
                                         populate_signature_from_ccla_callback,
                                         populate_signature_from_icla_callback)
@@ -802,3 +803,25 @@ def test_cla_signatory_email_content():
     assert "<p>After you sign, john (as the initial CLA Manager for your company)" in email_body
     assert "and if you approve john as your initial CLA Manager" in email_body
     assert "contact the requester at john@example.com" in email_body
+
+
+def test_create_default_individual_values_github_ignores_lf_email():
+    user = User()
+    user.set_user_name('Example User')
+    user.set_user_emails(['git@example.com'])
+    user.set_lf_email('lf@example.com')
+
+    values = create_default_individual_values(user, preferred_email='lf@example.com', provider_type='github')
+
+    assert values['email'] == 'git@example.com'
+
+
+def test_create_default_individual_values_gerrit_allows_lf_email():
+    user = User()
+    user.set_user_name('Example User')
+    user.set_user_emails(['git@example.com'])
+    user.set_lf_email('lf@example.com')
+
+    values = create_default_individual_values(user, provider_type='gerrit')
+
+    assert values['email'] == 'lf@example.com'
diff --git a/tests/functional/cypress/e2e/v4/github-repositories.cy.ts b/tests/functional/cypress/e2e/v4/github-repositories.cy.ts
@@ -57,6 +57,30 @@ describe('To Validate github-repositories API call', function () {
     }
   });
 
+  // Shared DEV fixture repair:
+  // this repository is sometimes left disabled by earlier/parallel E2E runs.
+  // Re-enable it before the happy-path tests start, but do not assert on the
+  // POST body because the API returns { list: null } when the repo is already
+  // enabled and the add operation becomes a no-op.
+  before(() => {
+    cy.request({
+      method: 'POST',
+      url: `${claEndpoint}`,
+      timeout: timeout,
+      failOnStatusCode: allowFail,
+      headers: getXACLHeader(),
+      auth: { bearer: bearerToken },
+      body: {
+        cla_group_id: '1baf67ab-d894-4edf-b6fc-c5f939db59f7',
+        github_organization_name: 'ApiAutomStandaloneOrg',
+        repository_github_id: '507892593',
+        repository_github_ids: ['507892593'],
+      },
+    }).then((response) => {
+      validate_200_Status(response);
+    });
+  });
+
   it('Get the GitHub repositories of the project which are CLA Enforced- Record should return 200 Response', function () {
     cy.task('log', `--> GET ${claEndpoint}`);
 
diff --git a/tests/functional/package-lock.json b/tests/functional/package-lock.json
diff --git a/tests/functional/package.json b/tests/functional/package.json
@@ -26,7 +26,8 @@
     "@types/node": "^20.5.0",
     "cypress": "^12.17.3",
     "cypress-dotenv": "^3.0.1",
-    "typescript": "^5.1.6"
+    "typescript": "^5.1.6",
+    "dotenv": "^16.4.5"
   },
   "dependencies": {
     "ajv": "^8.18.0",
diff --git a/tests/functional/yarn.lock b/tests/functional/yarn.lock
@@ -582,6 +582,11 @@ dotenv-parse-variables@^2.0.0:
     debug "^4.3.1"
     is-string-and-not-blank "^0.0.2"
 
+dotenv@^16.4.5:
+  version "16.6.1"
+  resolved "https://registry.npmjs.org/dotenv/-/dotenv-16.6.1.tgz"
+  integrity sha512-uBq4egWHTcTt33a72vpSG0z3HnPuIl6NqYcTrKEg2azoEyl2hpW0zqlxysq2pK9HlDIHyHyakeYaYnSAwd8bow==
+
 dunder-proto@^1.0.1:
   version "1.0.1"
   resolved "https://registry.npmjs.org/dunder-proto/-/dunder-proto-1.0.1.tgz"

Original file line number	Diff line number	Diff line change
`@@ -1333,7 +1333,7 @@ func (s service) RequestIndividualSignature(ctx context.Context, input models.`
`1333`	`1333`
`1334`	`1334`	`// creating individual default values`
`1335`	`1335`	`log.WithFields(f).Debugf("creating individual default values...")`
`1336`		`- defaultValues := s.createDefaultIndividualValues(user, preferredEmail)`
	`1336`	`+ defaultValues := s.createDefaultIndividualValues(user, preferredEmail, false)`
`1337`	`1337`	`log.WithFields(f).Debugf("default values: %+v", defaultValues)`
`1338`	`1338`
`1339`	`1339`	`// 4. Generate signature callback url`
`@@ -2057,11 +2057,12 @@ func (s *service) populateUserDetails(ctx context.Context, signatureReferenceTyp`
`2057`	`2057`	`return nil, errors.New(msg)`
`2058`	`2058`	`}`
`2059`	`2059`
	`2060`	`+ allowLFEmail := strings.EqualFold(latestSignature.SignatureReturnURLType, "gerrit")`
`2060`	`2061`	`if userModel.Username != "" {`
`2061`	`2062`	`userSignDetails.userSignatureName = userModel.Username`
`2062`	`2063`	`}`
`2063`		`- if getUserEmail(userModel, preferredEmail) != "" {`
`2064`		`- userSignDetails.userSignatureEmail = getUserEmail(userModel, preferredEmail)`
	`2064`	`+ if userEmail := getUserEmail(userModel, preferredEmail, allowLFEmail); userEmail != "" {`
	`2065`	`+ userSignDetails.userSignatureEmail = userEmail`
`2065`	`2066`	`}`
`2066`	`2067`	`}`
`2067`	`2068`	`} else {`
`@@ -2161,13 +2162,16 @@ func getTabsFromDocument(document *v1Models.ClaGroupDocument, documentID string,`
`2161`	`2162`	`}`
`2162`	`2163`
`2163`	`2164`	`// helper function to get user email`
`2164`		`-func getUserEmail(user *v1Models.User, preferredEmail string) string {`
	`2165`	`+func getUserEmail(user *v1Models.User, preferredEmail string, allowLFEmail bool) string {`
	`2166`	`+ if user == nil {`
	`2167`	`+ return ""`
	`2168`	`+ }`
`2165`	`2169`	`if preferredEmail != "" {`
`2166`		`- if utils.StringInSlice(preferredEmail, user.Emails) \|\| user.LfEmail == strfmt.Email(preferredEmail) {`
	`2170`	`+ if utils.StringInSlice(preferredEmail, user.Emails) \|\| (allowLFEmail && user.LfEmail == strfmt.Email(preferredEmail)) {`
`2167`	`2171`	`return preferredEmail`
`2168`	`2172`	`}`
`2169`	`2173`	`}`
`2170`		`- if user.LfEmail != "" {`
	`2174`	`+ if allowLFEmail && user.LfEmail != "" {`
`2171`	`2175`	`return string(user.LfEmail)`
`2172`	`2176`	`}`
`2173`	`2177`	`if len(user.Emails) > 0 {`
`@@ -2219,7 +2223,7 @@ func getActiveSignatureReturnURL(userID string, metadata map[string]interface{})`
`2219`	`2223`
`2220`	`2224`	`}`
`2221`	`2225`
`2222`		`-func (s service) createDefaultIndividualValues(user v1Models.User, preferredEmail string) map[string]interface{} {`
	`2226`	`+func (s service) createDefaultIndividualValues(user v1Models.User, preferredEmail string, allowLFEmail bool) map[string]interface{} {`
`2223`	`2227`	`f := logrus.Fields{`
`2224`	`2228`	`"functionName": "sign.createDefaultIndiviualValues",`
`2225`	`2229`	`}`
`@@ -2234,10 +2238,8 @@ func (s service) createDefaultIndividualValues(user v1Models.User, preferredEm`
`2234`	`2238`	`}`
`2235`	`2239`	`}`
`2236`	`2240`
`2237`		`- if preferredEmail != "" {`
`2238`		`- if utils.StringInSlice(preferredEmail, user.Emails) \|\| user.LfEmail == strfmt.Email(preferredEmail) {`
`2239`		`- defaultValues["email"] = preferredEmail`
`2240`		`- }`
	`2241`	`+ if userEmail := getUserEmail(user, preferredEmail, allowLFEmail); userEmail != "" {`
	`2242`	`+ defaultValues["email"] = userEmail`
`2241`	`2243`	`}`
`2242`	`2244`
`2243`	`2245`	`return defaultValues`
`@@ -2336,7 +2338,7 @@ func (s service) RequestIndividualSignatureGerrit(ctx context.Context, input m`
`2336`	`2338`	`preferredEmail = user.Emails[0]`
`2337`	`2339`	`}`
`2338`	`2340`
`2339`		`- defaultValues := s.createDefaultIndividualValues(user, preferredEmail)`
	`2341`	`+ defaultValues := s.createDefaultIndividualValues(user, preferredEmail, true)`
`2340`	`2342`
`2341`	`2343`	`log.WithFields(f).Debugf("defaultValues: %+v", defaultValues)`
`2342`	`2344`