diff --git a/cla-backend-legacy/internal/api/github_oauth.go b/cla-backend-legacy/internal/api/github_oauth.go index 04e4ac965..d9bacb733 100644 --- a/cla-backend-legacy/internal/api/github_oauth.go +++ b/cla-backend-legacy/internal/api/github_oauth.go @@ -15,6 +15,7 @@ import ( "strings" "time" + "github.com/aws/aws-sdk-go-v2/service/dynamodb/types" "github.com/go-chi/chi/v5" "github.com/google/uuid" @@ -233,9 +234,12 @@ func (h *Handlers) githubGetOrCreateUser(ctx context.Context, sess middleware.Se if err != nil { return nil, &httpErr{status: http.StatusInternalServerError, payload: map[string]any{"errors": err.Error()}, err: err} } - var userItem map[string]any + // Operate on the raw AttributeValue map so we never round-trip pynamodb + // types through InterfaceMapToItem's isNumericString heuristic, which + // can silently coerce digit-only S fields to N. + var userAV map[string]types.AttributeValue if len(items) > 0 { - userItem = store.ItemToInterfaceMap(items[0]) + userAV = items[0] } else { // Fallback: look up by email. for _, e := range emails { @@ -252,7 +256,7 @@ func (h *Handlers) githubGetOrCreateUser(ctx context.Context, sess middleware.Se } } if len(its) > 0 { - userItem = store.ItemToInterfaceMap(its[0]) + userAV = its[0] break } } @@ -264,51 +268,58 @@ func (h *Handlers) githubGetOrCreateUser(ctx context.Context, sess middleware.Se githubName = strings.TrimSpace(githubName) now := time.Now().UTC() - if userItem != nil { + if userAV != nil { // Update existing user: set github id, username, display name and emails if githubLogin != "" { - userItem["user_github_username"] = githubLogin + userAV["user_github_username"] = &types.AttributeValueMemberS{Value: githubLogin} } if githubName != "" { - userItem["user_name"] = githubName + userAV["user_name"] = &types.AttributeValueMemberS{Value: githubName} } - userItem["user_emails"] = emails - userItem["user_github_id"] = githubID - userItem["date_modified"] = formatPynamoDateTimeUTC(now) - - // Persist. - userItemAV, convErr := store.InterfaceMapToItem(userItem) - if convErr != nil { - return nil, &httpErr{status: http.StatusInternalServerError, payload: map[string]any{"errors": convErr.Error()}, err: convErr} - } - if err := h.users.PutItem(ctx, userItemAV); err != nil { + // PatchedUnicodeSetAttribute on the Python side. emails is guaranteed + // non-empty by the len(emails) < 1 guard above, so SS is always valid. + userAV["user_emails"] = &types.AttributeValueMemberSS{Value: emails} + userAV["user_github_id"] = &types.AttributeValueMemberN{Value: strconv.FormatInt(githubID, 10)} + userAV["date_modified"] = &types.AttributeValueMemberS{Value: formatPynamoDateTimeUTC(now)} + + if err := h.users.PutItem(ctx, userAV); err != nil { return nil, &httpErr{status: http.StatusInternalServerError, payload: map[string]any{"errors": err.Error()}, err: err} } - return normalizeUserDict(userItem), nil + result := store.ItemToInterfaceMap(userAV) + // Preserve the pre-cutover wire shape for the OAuth callers + // (/v2/github/auth/callback no-redirect branch and /v2/user-from-session): + // pynamodb User.to_dict() returned user_github_id as an int, and the + // previous Go code mirrored that by overwriting the map entry with an + // int64. ItemToInterfaceMap converts N to a string, so re-apply the + // int64 here so JSON consumers continue to see a number. + result["user_github_id"] = githubID + return normalizeUserDict(result), nil } // Create new user. newID := uuid.New().String() - item := map[string]any{ - "user_id": newID, - "version": "v1", - "date_created": formatPynamoDateTimeUTC(now), - "date_modified": formatPynamoDateTimeUTC(now), - "user_github_id": githubID, - "user_github_username": githubLogin, - "user_emails": emails, + itemAV := map[string]types.AttributeValue{ + "user_id": &types.AttributeValueMemberS{Value: newID}, + "version": &types.AttributeValueMemberS{Value: "v1"}, + "date_created": &types.AttributeValueMemberS{Value: formatPynamoDateTimeUTC(now)}, + "date_modified": &types.AttributeValueMemberS{Value: formatPynamoDateTimeUTC(now)}, + "user_github_id": &types.AttributeValueMemberN{Value: strconv.FormatInt(githubID, 10)}, + "user_emails": &types.AttributeValueMemberSS{Value: emails}, } - if githubName != "" { - item["user_name"] = githubName + if githubLogin != "" { + itemAV["user_github_username"] = &types.AttributeValueMemberS{Value: githubLogin} } - itemAV, convErr := store.InterfaceMapToItem(item) - if convErr != nil { - return nil, &httpErr{status: http.StatusInternalServerError, payload: map[string]any{"errors": convErr.Error()}, err: convErr} + if githubName != "" { + itemAV["user_name"] = &types.AttributeValueMemberS{Value: githubName} } if err := h.users.PutItem(ctx, itemAV); err != nil { return nil, &httpErr{status: http.StatusInternalServerError, payload: map[string]any{"errors": err.Error()}, err: err} } - return normalizeUserDict(item), nil + result := store.ItemToInterfaceMap(itemAV) + // See the update branch above: keep user_github_id as int64 in the + // response to match pre-cutover Python wire shape. + result["user_github_id"] = githubID + return normalizeUserDict(result), nil } func (h *Handlers) setActiveSignatureMetadata(ctx context.Context, userID, projectID, repositoryID, pullRequestID string, returnURLs ...string) error { diff --git a/cla-backend-legacy/internal/api/handlers.go b/cla-backend-legacy/internal/api/handlers.go index 55ead35b1..045bc05c2 100644 --- a/cla-backend-legacy/internal/api/handlers.go +++ b/cla-backend-legacy/internal/api/handlers.go @@ -5843,23 +5843,19 @@ func (h *Handlers) PostProjectV1(w http.ResponseWriter, r *http.Request) { now := time.Now().UTC() projectID := uuid.New().String() - proj := map[string]any{ - "project_id": projectID, - "project_external_id": req.ProjectExternalID, - "project_name": req.ProjectName, - "project_icla_enabled": *req.ProjectICLAEnabled, - "project_ccla_enabled": *req.ProjectCCLAEnabled, - "project_ccla_requires_icla_signature": *req.ProjectCCLARequiresICLASignature, - "project_acl": []string{authUser.Username}, - "date_created": formatPynamoDateTimeUTC(now), - "date_modified": formatPynamoDateTimeUTC(now), - "version": "v1", - } - - item, err := store.InterfaceMapToItem(proj) - if err != nil { - respond.JSON(w, http.StatusInternalServerError, map[string]any{"errors": map[string]any{"server": err.Error()}}) - return + // Build the AttributeValue map directly so InterfaceMapToItem's + // isNumericString heuristic cannot coerce an all-digit project_name to N. + item := map[string]types.AttributeValue{ + "project_id": &types.AttributeValueMemberS{Value: projectID}, + "project_external_id": &types.AttributeValueMemberS{Value: req.ProjectExternalID}, + "project_name": &types.AttributeValueMemberS{Value: req.ProjectName}, + "project_icla_enabled": &types.AttributeValueMemberBOOL{Value: *req.ProjectICLAEnabled}, + "project_ccla_enabled": &types.AttributeValueMemberBOOL{Value: *req.ProjectCCLAEnabled}, + "project_ccla_requires_icla_signature": &types.AttributeValueMemberBOOL{Value: *req.ProjectCCLARequiresICLASignature}, + "project_acl": &types.AttributeValueMemberSS{Value: []string{authUser.Username}}, + "date_created": &types.AttributeValueMemberS{Value: formatPynamoDateTimeUTC(now)}, + "date_modified": &types.AttributeValueMemberS{Value: formatPynamoDateTimeUTC(now)}, + "version": &types.AttributeValueMemberS{Value: "v1"}, } if err := h.projects.PutItem(ctx, item); err != nil { respond.JSON(w, http.StatusInternalServerError, map[string]any{"errors": map[string]any{"server": err.Error()}}) @@ -5876,7 +5872,7 @@ func (h *Handlers) PostProjectV1(w http.ResponseWriter, r *http.Request) { ContainsPII: false, }) - respond.JSON(w, http.StatusOK, proj) + respond.JSON(w, http.StatusOK, store.ItemToInterfaceMap(item)) } // PUT /v1/project @@ -5962,39 +5958,36 @@ func (h *Handlers) PutProjectV1(w http.ResponseWriter, r *http.Request) { return } - proj := store.ItemToInterfaceMap(item) + // Patch the AttributeValue map directly so we never round-trip pynamodb + // types through InterfaceMapToItem's isNumericString heuristic, which + // can silently coerce digit-only S fields to N. updatedString := " " if req.ProjectExternalID != nil { - proj["project_external_id"] = *req.ProjectExternalID + item["project_external_id"] = &types.AttributeValueMemberS{Value: *req.ProjectExternalID} updatedString += fmt.Sprintf("project_external_id changed to %s \n", *req.ProjectExternalID) } if req.ProjectName != nil { - proj["project_name"] = *req.ProjectName + item["project_name"] = &types.AttributeValueMemberS{Value: *req.ProjectName} updatedString += fmt.Sprintf("project_name changed to %s \n", *req.ProjectName) } if req.ProjectICLAEnabled != nil { - proj["project_icla_enabled"] = *req.ProjectICLAEnabled + item["project_icla_enabled"] = &types.AttributeValueMemberBOOL{Value: *req.ProjectICLAEnabled} updatedString += fmt.Sprintf("project_icla_enabled changed to %s \n", boolString(*req.ProjectICLAEnabled)) } if req.ProjectCCLAEnabled != nil { - proj["project_ccla_enabled"] = *req.ProjectCCLAEnabled + item["project_ccla_enabled"] = &types.AttributeValueMemberBOOL{Value: *req.ProjectCCLAEnabled} updatedString += fmt.Sprintf("project_ccla_enabled changed to %s \n", boolString(*req.ProjectCCLAEnabled)) } if req.ProjectCCLARequiresICLASignature != nil { - proj["project_ccla_requires_icla_signature"] = *req.ProjectCCLARequiresICLASignature + item["project_ccla_requires_icla_signature"] = &types.AttributeValueMemberBOOL{Value: *req.ProjectCCLARequiresICLASignature} updatedString += fmt.Sprintf("project_ccla_requires_icla_signature changed to %s \n", boolString(*req.ProjectCCLARequiresICLASignature)) } now := time.Now().UTC() - proj["date_modified"] = formatPynamoDateTimeUTC(now) + item["date_modified"] = &types.AttributeValueMemberS{Value: formatPynamoDateTimeUTC(now)} - putItem, err := store.InterfaceMapToItem(proj) - if err != nil { - respond.JSON(w, http.StatusInternalServerError, map[string]any{"errors": map[string]any{"server": err.Error()}}) - return - } - if err := h.projects.PutItem(ctx, putItem); err != nil { + if err := h.projects.PutItem(ctx, item); err != nil { respond.JSON(w, http.StatusInternalServerError, map[string]any{"errors": map[string]any{"server": err.Error()}}) return } @@ -6008,7 +6001,7 @@ func (h *Handlers) PutProjectV1(w http.ResponseWriter, r *http.Request) { ContainsPII: false, }) - respond.JSON(w, http.StatusOK, proj) + respond.JSON(w, http.StatusOK, store.ItemToInterfaceMap(item)) } // DELETE /v1/project/{project_id}