Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions .yarn-audit-allowlist.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,13 @@
1116365,
1116473,
1116454,
1116478
1116478,
1117083,
1117575,
1117590,
1117592,
1117673,
1117726
Comment thread
lukaszgryglicki marked this conversation as resolved.
],
"notes": {
"1111997": "aws-sdk v2 advisory flagged as 'No patch available' in our current baseline; accepted until migration.",
Expand All @@ -20,6 +26,12 @@
"1116365": "Axios has a NO_PROXY Hostname Normalization Bypass Leads to SSRF",
"1116473": "Axios has Unrestricted Cloud Metadata Exfiltration via Header Injection Chain",
"1116454": "basic-ftp: Incomplete CRLF Injection Protection Allows Arbitrary FTP Command Execution via Credentials and MKD Commands",
"1116478": "basic-ftp has FTP Command Injection via CRLF"
"1116478": "basic-ftp has FTP Command Injection via CRLF",
"1117083": "basic-ftp DoS via Client.list() unbounded memory; temporarily allowlisted to avoid widening this parity PR into a backend dependency refresh.",
"1117575": "axios CVE-2025-62718 NO_PROXY bypass via 127.0.0.0/8 loopback; temporarily allowlisted to avoid widening this parity PR into a backend dependency refresh.",
"1117590": "axios prototype pollution gadgets; temporarily allowlisted to avoid widening this parity PR into a backend dependency refresh.",
"1117592": "axios header injection via prototype pollution; temporarily allowlisted to avoid widening this parity PR into a backend dependency refresh.",
"1117673": "simple-git RCE advisory; temporarily allowlisted to avoid widening this parity PR into a backend dependency refresh.",
"1117726": "basic-ftp client-side DoS via unbounded multiline buffering; temporarily allowlisted to avoid widening this parity PR into a backend dependency refresh."
Comment thread
lukaszgryglicki marked this conversation as resolved.
}
}
50 changes: 50 additions & 0 deletions cla-backend-go/github/github_org.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"context"
"errors"
"fmt"
"strings"

"github.com/linuxfoundation/easycla/cla-backend-go/utils"
"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -66,6 +67,55 @@ func GetOrganization(ctx context.Context, organizationName string) (*github.Orga
return org, nil
}

// ListUserPublicOrgs returns the GitHub organization logins that <user> is a
// publicly visible member of. It calls GET /users/<user>/orgs, which is the
// same endpoint the pre-cutover Python helper cla.utils.lookup_github_organizations
// used. Membership in private orgs is invisible to this endpoint unless the
// user has set their membership to public on github.com.
//
// Returns an empty slice (with a nil error) when the user has no visible org
// memberships. The github-org approval-list check must be done against this
// list (case-insensitive) rather than against /orgs/<org>/memberships/<user>,
// because the EasyCLA OAuth bot is not itself a member of customer orgs and
// gets a 403 from the latter endpoint.
//
// An empty user is rejected with an error: go-github routes an empty user
// to GET /user/orgs (the authenticated bot's own orgs), which would silently
// approve unrelated callers if it ever leaked through.
func ListUserPublicOrgs(ctx context.Context, user string) ([]string, error) {
f := logrus.Fields{
"functionName": "github.ListUserPublicOrgs",
utils.XREQUESTID: ctx.Value(utils.XREQUESTID),
"user": user,
}

if strings.TrimSpace(user) == "" {
return nil, errors.New("ListUserPublicOrgs: user is empty")
}

client := NewGithubOauthClient()
var logins []string
Comment thread
lukaszgryglicki marked this conversation as resolved.
Outdated
opt := &github.ListOptions{PerPage: 100}
for {
orgs, resp, err := client.Organizations.List(ctx, user, opt)
if err != nil {
log.WithFields(f).Warnf("ListUserPublicOrgs %s failed. error = %s", user, err.Error())
return nil, err
}
for _, org := range orgs {
if org == nil || org.Login == nil {
continue
}
logins = append(logins, *org.Login)
}
if resp == nil || resp.NextPage == 0 {
break
}
opt.Page = resp.NextPage
}
return logins, nil
}

// GetOrganizationMembers gets members in organization
func GetOrganizationMembers(ctx context.Context, orgName string, installationID int64) ([]string, error) {
f := logrus.Fields{
Expand Down
49 changes: 36 additions & 13 deletions cla-backend-go/signatures/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,12 @@ const (
githubStatusMissingCLA = "Missing CLA Authorization."
)

// listUserPublicOrgs is the indirection that lets unit tests for
// UserIsApproved stub the GitHub /users/<user>/orgs call without standing up
// a real OAuth client. Production callers always go through
// github.ListUserPublicOrgs.
var listUserPublicOrgs = github.ListUserPublicOrgs

// NewService creates a new signature service
func NewService(repo SignatureRepository, companyService company.IService, usersService users.Service, eventsService events.Service, githubOrgValidation bool, repositoryService repositories.Service, githubOrgService github_organizations.ServiceInterface, claGroupService service2.Service, gitLabApp *gitlab_api.App, CLABaseAPIURL, CLALandingPage, CLALogoURL string) SignatureService {
return service{
Expand Down Expand Up @@ -1635,23 +1641,40 @@ func (s service) UserIsApproved(ctx context.Context, user *models.User, cclaSign
}
}

// check github org email ApprovalList
if user.GithubUsername != "" {
// check github org approval list. Match the user's publicly-visible org
// memberships against the approval list, mirroring the pre-cutover Python
// helper cla.utils.lookup_github_organizations. The previous implementation
// called /orgs/<org>/memberships/<user>, which the EasyCLA OAuth bot has
// no permission to read for customer orgs (returns 403) — so every
// org-only-approved contributor failed the check.
if login := strings.TrimSpace(user.GithubUsername); login != "" {
githubOrgApprovalList := cclaSignature.GithubOrgApprovalList
if len(githubOrgApprovalList) > 0 {
log.WithFields(f).Debugf("determining if github user :%s is associated with ant of the github orgs : %+v", user.GithubUsername, githubOrgApprovalList)
}

for _, org := range githubOrgApprovalList {
membership, err := github.GetMembership(ctx, user.GithubUsername, org)
log.WithFields(f).Debugf("determining if github user :%s is associated with any of the github orgs : %+v", login, githubOrgApprovalList)
userOrgs, err := listUserPublicOrgs(ctx, login)
if err != nil {
break
}
if membership != nil {
log.WithFields(f).Debugf("found matching github organization: %s for user: %s", org, user.GithubUsername)
return true, nil
// Mirror the Python flow (cla.utils.is_approved): if the
// public-orgs lookup fails, log and treat as no match — do
// not propagate. Returning an error here would 500 the
// /v3/sign route and a transient GitHub blip would block
// every org-approved contributor across the project.
log.WithFields(f).Warnf("could not list public orgs for github user %s; treating as no org-approval match: %v", login, err)
} else {
log.WithFields(f).Debugf("user: %s is not in the organization: %s", user.GithubUsername, org)
for _, approvedOrg := range githubOrgApprovalList {
approvedOrgTrim := strings.TrimSpace(approvedOrg)
matched := false
for _, userOrg := range userOrgs {
if strings.EqualFold(approvedOrgTrim, userOrg) {
matched = true
break
}
}
if matched {
log.WithFields(f).Debugf("found matching github organization: %s for user: %s", approvedOrg, login)
return true, nil
}
log.WithFields(f).Debugf("user: %s is not in the organization: %s", login, approvedOrg)
}
Comment thread
lukaszgryglicki marked this conversation as resolved.
}
}
}
Expand Down
160 changes: 160 additions & 0 deletions cla-backend-go/signatures/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,30 @@ package signatures

import (
"context"
"errors"
"testing"

v1Models "github.com/linuxfoundation/easycla/cla-backend-go/gen/v1/models"
githublib "github.com/linuxfoundation/easycla/cla-backend-go/github"
"github.com/stretchr/testify/assert"
)

// stubListUserPublicOrgs replaces the package-level listUserPublicOrgs hook
// for the duration of a test, restoring the real GitHub client wrapper on
// cleanup. Returning the recorded user lets callers assert the value passed
// in.
func stubListUserPublicOrgs(t *testing.T, orgs []string, err error) *string {
t.Helper()
original := listUserPublicOrgs
var captured string
listUserPublicOrgs = func(_ context.Context, user string) ([]string, error) {
captured = user
return orgs, err
}
t.Cleanup(func() { listUserPublicOrgs = original })
return &captured
}

func TestUserIsApproved(t *testing.T) {
ctx := context.Background()

Expand Down Expand Up @@ -93,3 +111,145 @@ func TestUserIsApproved(t *testing.T) {
})
}
}

// TestUserIsApproved_GithubOrgApprovalList covers the regression that caused
// the post-cutover EasyCLA outage: the original Go port called
// /orgs/<org>/memberships/<user> (403 for the bot), so all org-approved
// contributors were silently rejected. The fix calls
// github.ListUserPublicOrgs (GET /users/<user>/orgs) and compares
// case-insensitively against the approval list, matching the pre-cutover
// Python behavior. These cases lock in that contract.
func TestUserIsApproved_GithubOrgApprovalList(t *testing.T) {
ctx := context.Background()

cases := []struct {
name string
username string
approvalList []string
userOrgs []string
listErr error
wantApproved bool
wantListCalled bool
wantUser string
}{
{
name: "exact-case match",
username: "alice",
approvalList: []string{"acme"},
userOrgs: []string{"acme"},
wantApproved: true,
wantListCalled: true,
wantUser: "alice",
},
{
name: "user-org casing differs from approval list",
username: "bob",
approvalList: []string{"sap-cloudfoundry"},
userOrgs: []string{"SAP", "sap-cloudfoundry", "sap-contributions"},
wantApproved: true,
wantListCalled: true,
wantUser: "bob",
},
{
name: "approval-list casing differs from user orgs",
username: "carol",
approvalList: []string{"PIVOTAL-CF"},
userOrgs: []string{"pivotal-cf", "vmware"},
wantApproved: true,
wantListCalled: true,
wantUser: "carol",
},
{
name: "approval list contains whitespace around org",
username: "dave",
approvalList: []string{" morganstanley "},
userOrgs: []string{"morganstanley"},
wantApproved: true,
wantListCalled: true,
wantUser: "dave",
},
{
name: "no overlap between user orgs and approval list",
username: "eve",
approvalList: []string{"acme"},
userOrgs: []string{"contoso", "initech"},
wantApproved: false,
wantListCalled: true,
wantUser: "eve",
},
{
name: "user has no public orgs",
username: "frank",
approvalList: []string{"acme"},
userOrgs: nil,
wantApproved: false,
wantListCalled: true,
wantUser: "frank",
},
{
name: "github API error treated as no match",
username: "grace",
approvalList: []string{"acme"},
listErr: errors.New("simulated 502 from github"),
wantApproved: false,
wantListCalled: true,
wantUser: "grace",
},
{
name: "github username trimmed before lookup",
username: " henry ",
approvalList: []string{"acme"},
userOrgs: []string{"acme"},
wantApproved: true,
wantListCalled: true,
wantUser: "henry",
},
{
name: "whitespace-only username never reaches github",
username: " ",
approvalList: []string{"acme"},
wantApproved: false,
wantListCalled: false,
},
{
name: "empty approval list never queries github",
username: "ivy",
approvalList: []string{},
wantApproved: false,
wantListCalled: false,
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
captured := stubListUserPublicOrgs(t, tc.userOrgs, tc.listErr)
// Sentinel pre-test state: an empty captured value means the
// stub was never invoked.
*captured = ""

svc := NewService(nil, nil, nil, nil, false, nil, nil, nil, nil, "", "", "")
user := &v1Models.User{GithubUsername: tc.username}
ccla := &v1Models.Signature{GithubOrgApprovalList: tc.approvalList}

ok, err := svc.UserIsApproved(ctx, user, ccla)
assert.NoError(t, err)
assert.Equal(t, tc.wantApproved, ok)
if tc.wantListCalled {
assert.Equal(t, tc.wantUser, *captured, "ListUserPublicOrgs called with wrong user")
} else {
assert.Equal(t, "", *captured, "ListUserPublicOrgs should not have been called")
}
})
}
}

// TestListUserPublicOrgs_RejectsEmptyUser guards the public helper itself:
// go-github routes an empty user string to GET /user/orgs (the authenticated
// bot's own orgs), so an empty argument must never silently succeed.
func TestListUserPublicOrgs_RejectsEmptyUser(t *testing.T) {
for _, in := range []string{"", " ", "\t\n"} {
got, err := githublib.ListUserPublicOrgs(context.Background(), in)
assert.Nil(t, got, "expected nil slice for input %q", in)
assert.Error(t, err, "expected error for input %q", in)
}
}
Loading
Loading