Skip to content

Commit 7200a0d

Browse files
Merge pull request #4720 from linuxfoundation/uncron-active-signature-wrong-uuid-case
Add test coverage for non-v4 and invalid UUIDs and fix v4 APIs to behave as V2 APIs - accept non-v4 UUIDs
2 parents 4379843 + fe03608 commit 7200a0d

File tree

7 files changed

+224
-46
lines changed

7 files changed

+224
-46
lines changed

cla-backend-go/swagger/cla.v2.yaml

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -892,11 +892,7 @@ paths:
892892
/project-compat/{projectID}:
893893
parameters:
894894
- $ref: "#/parameters/x-request-id"
895-
- name: projectID
896-
in: path
897-
type: string
898-
required: true
899-
pattern: '^[a-fA-F0-9]{8}-?[a-fA-F0-9]{4}-?4[a-fA-F0-9]{3}-?[89ab][a-fA-F0-9]{3}-?[a-fA-F0-9]{12}$' # uuidv4
895+
- $ref: "#/parameters/projectPathUuid"
900896
get:
901897
summary: Get project by ID (returns data in the same format as Py V2 API)
902898
security: [ ]
@@ -2448,12 +2444,7 @@ paths:
24482444
/user/{userID}/active-signature:
24492445
parameters:
24502446
- $ref: "#/parameters/x-request-id"
2451-
- name: userID
2452-
description: the user ID
2453-
in: path
2454-
type: string
2455-
required: true
2456-
pattern: '^[a-fA-F0-9]{8}-?[a-fA-F0-9]{4}-?4[a-fA-F0-9]{3}-?[89ab][a-fA-F0-9]{3}-?[a-fA-F0-9]{12}$' # uuidv4
2447+
- $ref: "#/parameters/userPathUuid"
24572448
get:
24582449
summary: |
24592450
Returns all metadata associated with a user's active signature.
@@ -4525,6 +4516,20 @@ responses:
45254516

45264517
# Common parameters
45274518
parameters:
4519+
userPathUuid:
4520+
name: userID
4521+
in: path
4522+
required: true
4523+
description: The user ID (UUID string)
4524+
type: string
4525+
pattern: '^[a-fA-F0-9]{8}-?[a-fA-F0-9]{4}-?[a-fA-F0-9]{4}-?[a-fA-F0-9]{4}-?[a-fA-F0-9]{12}$' # this is any UUID, not only v4
4526+
projectPathUuid:
4527+
name: projectID
4528+
in: path
4529+
required: true
4530+
description: The project ID (UUID string)
4531+
type: string
4532+
pattern: '^[a-fA-F0-9]{8}-?[a-fA-F0-9]{4}-?[a-fA-F0-9]{4}-?[a-fA-F0-9]{4}-?[a-fA-F0-9]{12}$' # this is any UUID, not only v4
45284533
pageSize:
45294534
name: pageSize
45304535
description: The maximum number of results per page, value must be a positive integer value

cla-backend-go/swagger/common/project-compat.yaml

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,7 @@ description: Project Model - in Py V2 - minimal fields needed by FE
88
properties:
99
project_id:
1010
description: Project's UUID
11-
example: '88ee12de-122b-4c46-9046-19422054ed8d'
12-
type: string
11+
$ref: './common/properties/uuid.yaml'
1312
x-omitempty: false
1413
project_name:
1514
description: Project name
@@ -78,8 +77,7 @@ properties:
7877
properties:
7978
cla_group_id:
8079
description: Project's UUID
81-
example: '88ee12de-122b-4c46-9046-19422054ed8d'
82-
type: string
80+
$ref: './common/properties/uuid.yaml'
8381
x-omitempty: false
8482
foundation_sfid:
8583
description: The salesforce foundation ID
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
# Copyright The Linux Foundation and each contributor to CommunityBridge.
2+
# SPDX-License-Identifier: MIT
3+
4+
type: string
5+
example: 'a1b86c26-d8e8-4fd8-9f8d-5c723d5dac9f'
6+
pattern: '^[a-fA-F0-9]{8}-?[a-fA-F0-9]{4}-?[a-fA-F0-9]{4}-?[a-fA-F0-9]{4}-?[a-fA-F0-9]{12}$'

cla-backend-go/swagger/common/user-active-signature.yaml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@ description: >
99
Returns `null` if the user does not have an active signature.
1010
properties:
1111
user_id:
12-
$ref: './common/properties/internal-id.yaml'
12+
$ref: './common/properties/uuid.yaml'
1313
description: The unique internal UUID of the user
1414
project_id:
15-
$ref: './common/properties/internal-id.yaml'
15+
$ref: './common/properties/uuid.yaml'
1616
description: The unique UUID of the associated project
1717
repository_id:
1818
type: string
@@ -24,7 +24,7 @@ properties:
2424
example: '456'
2525
merge_request_id:
2626
type: string
27-
description: The merge request ID related to the signature (optional number stored as string, thsi property can be missing in JSON)
27+
description: The merge request ID related to the signature (optional number stored as string, this property can be missing in JSON)
2828
example: '456'
2929
x-nullable: true
3030
return_url:

cla-backend-go/v2/sign/handlers.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,6 @@ func Configure(api *operations.EasyclaAPI, service Service, userService users.Se
278278
}
279279
return sign.NewGetUserActiveSignatureOK().WithPayload(resp)
280280
})
281-
282281
}
283282

284283
type codedResponse interface {

tests/py2go/README.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,8 @@
2424
- `` DEBUG='' USER_UUID=b817eb57-045a-4fe0-8473-fbb416a01d70 PY_API_URL=https://api.lfcla.dev.platform.linuxfoundation.org go test -v -run '^TestUserActiveSignatureAPI$' ``.
2525
- `` REPO_ID=466156917 PR_ID=3 DEBUG=1 go test -v -run '^TestUserActiveSignatureAPI$' ``.
2626
- `` MAX_PARALLEL=2 DEBUG='' go test -v -run '^TestAllUserActiveSignatureAPI$' ``.
27-
- `` [STAGE=prod PROD=1] REMOTE=1 make ``.
27+
- `` [STAGE=prod] [PY_API_URL=local|dev|prod] [GO_API_URL=local|dev|prod] make ``.
28+
- `` GO_API_URL=dev PY_API_URL=dev go test -v -run '^TestUserActiveSignatureAPIWithNonV4UUID$' ``.
29+
- `` GO_API_URL=dev PY_API_URL=dev go test -v -run '^TestUserActiveSignatureAPIWithInvalidUUID$' ``.
30+
- `` GO_API_URL=dev PY_API_URL=dev go test -v -run '^TestProjectCompatAPIWithNonV4UUID$' ``.
31+
- `` GO_API_URL=dev PY_API_URL=dev go test -v -run '^TestProjectCompatAPIWithInvalidUUID$' ``.

tests/py2go/api_test.go

Lines changed: 192 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,10 @@ import (
2020
)
2121

2222
const (
23-
ExampleRepoID = 466156917
24-
ExamplePRNumber = 3
23+
ExampleRepoID = 466156917
24+
ExamplePRNumber = 3
25+
InvalidUUIDProvided = "Invalid UUID provided"
26+
GoUUIDValidationError = " in path should match '^[a-fA-F0-9]{8}-?[a-fA-F0-9]{4}-?[a-fA-F0-9]{4}-?[a-fA-F0-9]{4}-?[a-fA-F0-9]{12}$'"
2527
)
2628

2729
var (
@@ -30,8 +32,6 @@ var (
3032
PY_API_URL string
3133
GO_API_URL string
3234
DEBUG bool
33-
REMOTE bool
34-
PROD bool
3535
MAX_PARALLEL int
3636
PROJECT_UUID string
3737
USER_UUID string
@@ -102,31 +102,23 @@ var (
102102
func init() {
103103
TOKEN = os.Getenv("TOKEN")
104104
XACL = os.Getenv("XACL")
105-
REMOTE = os.Getenv("REMOTE") != ""
106-
PROD = os.Getenv("PROD") != ""
107105
PY_API_URL = os.Getenv("PY_API_URL")
108-
if PY_API_URL == "" {
109-
if REMOTE {
110-
if PROD {
111-
PY_API_URL = "https://api.easycla.lfx.linuxfoundation.org"
112-
} else {
113-
PY_API_URL = "https://api.lfcla.dev.platform.linuxfoundation.org"
114-
}
115-
} else {
116-
PY_API_URL = "http://127.0.0.1:5000"
117-
}
106+
switch PY_API_URL {
107+
case "local", "":
108+
PY_API_URL = "http://127.0.0.1:5000"
109+
case "dev":
110+
PY_API_URL = "https://api.lfcla.dev.platform.linuxfoundation.org"
111+
case "prod":
112+
PY_API_URL = "https://api.easycla.lfx.linuxfoundation.org"
118113
}
119114
GO_API_URL = os.Getenv("GO_API_URL")
120-
if GO_API_URL == "" {
121-
if REMOTE {
122-
if PROD {
123-
GO_API_URL = "https://api-gw.platform.linuxfoundation.org/cla-service"
124-
} else {
125-
GO_API_URL = "https://api-gw.dev.platform.linuxfoundation.org/cla-service"
126-
}
127-
} else {
128-
GO_API_URL = "http://127.0.0.1:5001"
129-
}
115+
switch GO_API_URL {
116+
case "local", "":
117+
GO_API_URL = "http://127.0.0.1:5001"
118+
case "dev":
119+
GO_API_URL = "https://api-gw.dev.platform.linuxfoundation.org/cla-service"
120+
case "prod":
121+
GO_API_URL = "https://api-gw.platform.linuxfoundation.org/cla-service"
130122
}
131123
DEBUG = os.Getenv("DEBUG") != ""
132124
MAX_PARALLEL = runtime.NumCPU()
@@ -341,6 +333,21 @@ func compareNestedFields(t *testing.T, pyData, goData, keyMapping map[string]int
341333
}
342334
}
343335

336+
func expectedPyInvalidUUID(field string) map[string]interface{} {
337+
return map[string]interface{}{
338+
"errors": map[string]interface{}{
339+
field: InvalidUUIDProvided,
340+
},
341+
}
342+
}
343+
344+
func expectedGoInvalidUUID(field string) map[string]interface{} {
345+
return map[string]interface{}{
346+
"code": float64(605),
347+
"message": field + GoUUIDValidationError,
348+
}
349+
}
350+
344351
func runProjectCompatAPIForProject(t *testing.T, projectId string) {
345352
apiURL := PY_API_URL + fmt.Sprintf(ProjectAPIPath[0], projectId)
346353
Debugf("Py API call: %s\n", apiURL)
@@ -396,6 +403,41 @@ func runProjectCompatAPIForProject(t *testing.T, projectId string) {
396403
}
397404
}
398405

406+
func runProjectCompatAPIForProjectExpectFail(t *testing.T, projectId string) {
407+
apiURL := PY_API_URL + fmt.Sprintf(ProjectAPIPath[0], projectId)
408+
Debugf("Py API call: %s\n", apiURL)
409+
oldResp, err := http.Get(apiURL)
410+
if err != nil {
411+
t.Fatalf("Failed to call API: %v", err)
412+
}
413+
assert.Equal(t, http.StatusBadRequest, oldResp.StatusCode, "Expected 400 from Py API")
414+
defer oldResp.Body.Close()
415+
oldBody, _ := io.ReadAll(oldResp.Body)
416+
var oldJSON interface{}
417+
err = json.Unmarshal(oldBody, &oldJSON)
418+
assert.NoError(t, err)
419+
Debugf("Py raw response: %+v\n", string(oldBody))
420+
Debugf("Py response: %+v\n", oldJSON)
421+
422+
apiURL = GO_API_URL + fmt.Sprintf(ProjectAPIPath[2], projectId)
423+
Debugf("Go API call: %s\n", apiURL)
424+
newResp, err := http.Get(apiURL)
425+
if err != nil {
426+
t.Fatalf("Failed to call API: %v", err)
427+
}
428+
assert.Equal(t, http.StatusUnprocessableEntity, newResp.StatusCode, "Expected 422 from Go API")
429+
defer newResp.Body.Close()
430+
newBody, _ := io.ReadAll(newResp.Body)
431+
var newJSON interface{}
432+
err = json.Unmarshal(newBody, &newJSON)
433+
assert.NoError(t, err)
434+
Debugf("Go raw Response: %+v\n", string(newBody))
435+
Debugf("Go response: %+v\n", newJSON)
436+
437+
assert.Equal(t, expectedPyInvalidUUID("project_id"), oldJSON)
438+
assert.Equal(t, expectedGoInvalidUUID("projectID"), newJSON)
439+
}
440+
399441
func TestProjectCompatAPI(t *testing.T) {
400442
projectId := PROJECT_UUID
401443
if projectId == "" {
@@ -416,6 +458,40 @@ func TestProjectCompatAPI(t *testing.T) {
416458
runProjectCompatAPIForProject(t, projectId)
417459
}
418460

461+
func TestProjectCompatAPIWithNonV4UUID(t *testing.T) {
462+
projectId := "6ba7b810-9dad-11d1-80b4-00c04fd430c8" // Non-v4 UUID
463+
putTestItem("projects", "project_id", projectId, "S", map[string]interface{}{
464+
"project_name": "CNCF",
465+
"project_icla_enabled": true,
466+
"project_ccla_enabled": true,
467+
"project_ccla_requires_icla_signature": true,
468+
"date_created": "2022-11-21T10:31:31Z",
469+
"date_modified": "2023-02-23T13:14:48Z",
470+
"foundation_sfid": "a09410000182dD2AAI",
471+
"version": "2",
472+
}, DEBUG)
473+
defer deleteTestItem("projects", "project_id", projectId, "S", DEBUG)
474+
475+
runProjectCompatAPIForProject(t, projectId)
476+
}
477+
478+
func TestProjectCompatAPIWithInvalidUUID(t *testing.T) {
479+
projectId := "6ba7b810-9dad-11d1-80b4-00c04fd430cg" // Invalid UUID - "g" is not a hex digit
480+
putTestItem("projects", "project_id", projectId, "S", map[string]interface{}{
481+
"project_name": "CNCF",
482+
"project_icla_enabled": true,
483+
"project_ccla_enabled": true,
484+
"project_ccla_requires_icla_signature": true,
485+
"date_created": "2022-11-21T10:31:31Z",
486+
"date_modified": "2023-02-23T13:14:48Z",
487+
"foundation_sfid": "a09410000182dD2AAI",
488+
"version": "2",
489+
}, DEBUG)
490+
defer deleteTestItem("projects", "project_id", projectId, "S", DEBUG)
491+
492+
runProjectCompatAPIForProjectExpectFail(t, projectId)
493+
}
494+
419495
func TestAllProjectsCompatAPI(t *testing.T) {
420496
allProjects := getAllPrimaryKeys("projects", "project_id", "S")
421497

@@ -610,6 +686,96 @@ func runUserActiveSignatureAPIForUser(t *testing.T, userId string) {
610686
}
611687
}
612688

689+
func runUserActiveSignatureAPIForUserExpectFail(t *testing.T, userId string) {
690+
apiURL := PY_API_URL + fmt.Sprintf(UserActiveSignatureAPIPath[0], userId)
691+
Debugf("Py API call: %s\n", apiURL)
692+
oldResp, err := http.Get(apiURL)
693+
if err != nil {
694+
t.Fatalf("Failed to call API: %v", err)
695+
}
696+
assert.Equal(t, http.StatusBadRequest, oldResp.StatusCode, "Expected 400 from Py API")
697+
defer oldResp.Body.Close()
698+
oldBody, _ := io.ReadAll(oldResp.Body)
699+
var oldJSON interface{}
700+
err = json.Unmarshal(oldBody, &oldJSON)
701+
assert.NoError(t, err)
702+
Debugf("Py raw response: %+v\n", string(oldBody))
703+
Debugf("Py response: %+v\n", oldJSON)
704+
705+
apiURL = GO_API_URL + fmt.Sprintf(UserActiveSignatureAPIPath[1], userId)
706+
Debugf("Go API call: %s\n", apiURL)
707+
newResp, err := http.Get(apiURL)
708+
if err != nil {
709+
t.Fatalf("Failed to call API: %v", err)
710+
}
711+
assert.Equal(t, http.StatusUnprocessableEntity, newResp.StatusCode, "Expected 422 from Go API")
712+
defer newResp.Body.Close()
713+
newBody, _ := io.ReadAll(newResp.Body)
714+
var newJSON interface{}
715+
err = json.Unmarshal(newBody, &newJSON)
716+
assert.NoError(t, err)
717+
Debugf("Go raw Response: %+v\n", string(newBody))
718+
Debugf("Go response: %+v\n", newJSON)
719+
720+
assert.Equal(t, expectedPyInvalidUUID("user_id"), oldJSON)
721+
assert.Equal(t, expectedGoInvalidUUID("userID"), newJSON)
722+
}
723+
724+
func TestUserActiveSignatureAPIWithInvalidUUID(t *testing.T) {
725+
userId := "6ba7b810-9dad-11d1-80b4-00c04fd430cg" // Invalid UUID "g" is not a hex digit
726+
projectId := uuid.New().String()
727+
key := "active_signature:" + userId
728+
expire := time.Now().Add(time.Hour).Unix()
729+
iValue := map[string]interface{}{
730+
"user_id": userId,
731+
"project_id": projectId,
732+
"repository_id": fmt.Sprintf("%d", REPO_ID),
733+
"pull_request_id": fmt.Sprintf("%d", PR_ID),
734+
}
735+
value, err := json.Marshal(iValue)
736+
if err != nil {
737+
t.Fatalf("failed to marshal value: %+v", err)
738+
}
739+
putTestItem("projects", "project_id", projectId, "S", map[string]interface{}{}, DEBUG)
740+
putTestItem("store", "key", key, "S", map[string]interface{}{
741+
"value": string(value),
742+
"expire": expire,
743+
}, DEBUG)
744+
defer deleteTestItem("projects", "project_id", projectId, "S", DEBUG)
745+
defer deleteTestItem("store", "key", key, "S", DEBUG)
746+
runUserActiveSignatureAPIForUserExpectFail(t, userId)
747+
}
748+
749+
func TestUserActiveSignatureAPIWithNonV4UUID(t *testing.T) {
750+
userId := "6ba7b810-9dad-11d1-80b4-00c04fd430c8" // Non-v4 UUID
751+
projectId := uuid.New().String()
752+
key := "active_signature:" + userId
753+
expire := time.Now().Add(time.Hour).Unix()
754+
iValue := map[string]interface{}{
755+
"user_id": userId,
756+
"project_id": projectId,
757+
"repository_id": fmt.Sprintf("%d", REPO_ID),
758+
"pull_request_id": fmt.Sprintf("%d", PR_ID),
759+
}
760+
if rand.Intn(2) == 0 {
761+
mrId := rand.Intn(100)
762+
iValue["merge_request_id"] = fmt.Sprintf("%d", mrId)
763+
iValue["return_url"] = fmt.Sprintf("https://gitlab.com/gitlab-org/gitlab/-/merge_requests/%d", mrId)
764+
}
765+
value, err := json.Marshal(iValue)
766+
if err != nil {
767+
t.Fatalf("failed to marshal value: %+v", err)
768+
}
769+
putTestItem("projects", "project_id", projectId, "S", map[string]interface{}{}, DEBUG)
770+
putTestItem("store", "key", key, "S", map[string]interface{}{
771+
"value": string(value),
772+
"expire": expire,
773+
}, DEBUG)
774+
defer deleteTestItem("projects", "project_id", projectId, "S", DEBUG)
775+
defer deleteTestItem("store", "key", key, "S", DEBUG)
776+
runUserActiveSignatureAPIForUser(t, userId)
777+
}
778+
613779
func TestUserActiveSignatureAPI(t *testing.T) {
614780
userId := USER_UUID
615781
if userId == "" {

0 commit comments

Comments
 (0)