From 21855f47a59805ea0dbb2b8c9d3a878b25281ffa Mon Sep 17 00:00:00 2001 From: Joshua French Date: Mon, 6 Jan 2025 17:13:39 +0000 Subject: [PATCH 1/6] Update ListSecurityManagerTeams to use org roles APIs --- github/orgs_security_managers.go | 45 ++++++++++++++++++++------- github/orgs_security_managers_test.go | 7 ++++- 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/github/orgs_security_managers.go b/github/orgs_security_managers.go index 08037727320..1ff3c740ad5 100644 --- a/github/orgs_security_managers.go +++ b/github/orgs_security_managers.go @@ -10,26 +10,47 @@ import ( "fmt" ) +func (s *OrganizationsService) GetSecurityManagerRole(ctx context.Context, org string) (*CustomOrgRoles, *Response, error) { + roles, resp, err := s.ListRoles(ctx, org) + if err != nil { + return nil, resp, err + } + + for _, role := range roles.CustomRepoRoles { + if *role.Name == "security_manager" { + return role, resp, nil + } + } + + return nil, resp, fmt.Errorf("security manager role not found") +} + // ListSecurityManagerTeams lists all security manager teams for an organization. // -// GitHub API docs: https://docs.github.com/rest/orgs/security-managers#list-security-manager-teams +// GitHub API docs: https://docs.github.com/en/rest/orgs/organization-roles#list-teams-that-are-assigned-to-an-organization-role // -//meta:operation GET /orgs/{org}/security-managers +//meta:operation GET /orgs/{org}/organization-roles/{security_manager_role_id}/teams func (s *OrganizationsService) ListSecurityManagerTeams(ctx context.Context, org string) ([]*Team, *Response, error) { - u := fmt.Sprintf("orgs/%v/security-managers", org) - - req, err := s.client.NewRequest("GET", u, nil) - if err != nil { - return nil, nil, err - } - - var teams []*Team - resp, err := s.client.Do(ctx, req, &teams) + securityManagerRole, resp, err := s.GetSecurityManagerRole(ctx, org) if err != nil { return nil, resp, err } - return teams, resp, nil + options := &ListOptions{PerPage: 100} + securityManagerTeams := make([]*Team, 0) + for { + teams, resp, err := s.ListTeamsAssignedToOrgRole(ctx, org, securityManagerRole.GetID(), options) + if err != nil { + return nil, resp, err + } + + securityManagerTeams = append(securityManagerTeams, teams...) + if resp.NextPage == 0 { + return securityManagerTeams, resp, nil + } + + options.Page = resp.NextPage + } } // AddSecurityManagerTeam adds a team to the list of security managers for an organization. diff --git a/github/orgs_security_managers_test.go b/github/orgs_security_managers_test.go index 3ce67d24c03..49c7d2f53e5 100644 --- a/github/orgs_security_managers_test.go +++ b/github/orgs_security_managers_test.go @@ -18,7 +18,12 @@ func TestOrganizationsService_ListSecurityManagerTeams(t *testing.T) { t.Parallel() client, mux, _ := setup(t) - mux.HandleFunc("/orgs/o/security-managers", func(w http.ResponseWriter, r *http.Request) { + mux.HandleFunc("/orgs/o/organization-roles", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "GET") + fmt.Fprint(w, `{"roles":[{"id":138,"name":"security_manager"}]}`) + }) + + mux.HandleFunc("/orgs/o/organization-roles/138/teams", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") fmt.Fprint(w, `[{"id":1}]`) }) From 6e962d9b86184a4703c9dd01ba8079c893c63729 Mon Sep 17 00:00:00 2001 From: Joshua French Date: Mon, 6 Jan 2025 17:25:36 +0000 Subject: [PATCH 2/6] Update AddSecurityManagerTeam to use org roles APIs --- github/orgs_security_managers.go | 11 +++++------ github/orgs_security_managers_test.go | 22 ++++++++++++++-------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/github/orgs_security_managers.go b/github/orgs_security_managers.go index 1ff3c740ad5..d17afe2c9c3 100644 --- a/github/orgs_security_managers.go +++ b/github/orgs_security_managers.go @@ -55,17 +55,16 @@ func (s *OrganizationsService) ListSecurityManagerTeams(ctx context.Context, org // AddSecurityManagerTeam adds a team to the list of security managers for an organization. // -// GitHub API docs: https://docs.github.com/rest/orgs/security-managers#add-a-security-manager-team +// GitHub API docs: https://docs.github.com/en/rest/orgs/organization-roles#assign-an-organization-role-to-a-team // -//meta:operation PUT /orgs/{org}/security-managers/teams/{team_slug} +//meta:operation PUT /orgs/{org}/organization-roles/teams/{team_slug}/{security_manager_role_id} func (s *OrganizationsService) AddSecurityManagerTeam(ctx context.Context, org, team string) (*Response, error) { - u := fmt.Sprintf("orgs/%v/security-managers/teams/%v", org, team) - req, err := s.client.NewRequest("PUT", u, nil) + securityManagerRole, resp, err := s.GetSecurityManagerRole(ctx, org) if err != nil { - return nil, err + return resp, err } - return s.client.Do(ctx, req, nil) + return s.AssignOrgRoleToTeam(ctx, org, team, securityManagerRole.GetID()) } // RemoveSecurityManagerTeam removes a team from the list of security managers for an organization. diff --git a/github/orgs_security_managers_test.go b/github/orgs_security_managers_test.go index 49c7d2f53e5..925900409aa 100644 --- a/github/orgs_security_managers_test.go +++ b/github/orgs_security_managers_test.go @@ -18,11 +18,7 @@ func TestOrganizationsService_ListSecurityManagerTeams(t *testing.T) { t.Parallel() client, mux, _ := setup(t) - mux.HandleFunc("/orgs/o/organization-roles", func(w http.ResponseWriter, r *http.Request) { - testMethod(t, r, "GET") - fmt.Fprint(w, `{"roles":[{"id":138,"name":"security_manager"}]}`) - }) - + handleGetSecurityManagerRole(t, mux, "o") mux.HandleFunc("/orgs/o/organization-roles/138/teams", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") fmt.Fprint(w, `[{"id":1}]`) @@ -67,7 +63,8 @@ func TestOrganizationsService_AddSecurityManagerTeam(t *testing.T) { t.Parallel() client, mux, _ := setup(t) - mux.HandleFunc("/orgs/o/security-managers/teams/t", func(w http.ResponseWriter, r *http.Request) { + handleGetSecurityManagerRole(t, mux, "o") + mux.HandleFunc("/orgs/o/organization-roles/teams/t/138", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "PUT") }) @@ -99,10 +96,12 @@ func TestOrganizationsService_AddSecurityManagerTeam_invalidOrg(t *testing.T) { func TestOrganizationsService_AddSecurityManagerTeam_invalidTeam(t *testing.T) { t.Parallel() - client, _, _ := setup(t) + client, mux, _ := setup(t) + + handleGetSecurityManagerRole(t, mux, "o") ctx := context.Background() - _, err := client.Organizations.AddSecurityManagerTeam(ctx, "%", "t") + _, err := client.Organizations.AddSecurityManagerTeam(ctx, "o", "%") testURLParseError(t, err) } @@ -148,3 +147,10 @@ func TestOrganizationsService_RemoveSecurityManagerTeam_invalidTeam(t *testing.T _, err := client.Organizations.RemoveSecurityManagerTeam(ctx, "%", "t") testURLParseError(t, err) } + +func handleGetSecurityManagerRole(t *testing.T, mux *http.ServeMux, org string) { + mux.HandleFunc(fmt.Sprintf("/orgs/%s/organization-roles", org), func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "GET") + fmt.Fprintf(w, `{"roles":[{"id":138,"name":"security_manager"}]}`) + }) +} From c59f0d2ad5b0551d1acd723e4321d25fb0dac030 Mon Sep 17 00:00:00 2001 From: Joshua French Date: Mon, 6 Jan 2025 17:31:55 +0000 Subject: [PATCH 3/6] Update RemoveSecurityManagerTeam to use org roles APIs --- github/orgs_security_managers.go | 11 +++++------ github/orgs_security_managers_test.go | 9 ++++++--- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/github/orgs_security_managers.go b/github/orgs_security_managers.go index d17afe2c9c3..50a6ea9fb4b 100644 --- a/github/orgs_security_managers.go +++ b/github/orgs_security_managers.go @@ -69,15 +69,14 @@ func (s *OrganizationsService) AddSecurityManagerTeam(ctx context.Context, org, // RemoveSecurityManagerTeam removes a team from the list of security managers for an organization. // -// GitHub API docs: https://docs.github.com/rest/orgs/security-managers#remove-a-security-manager-team +// GitHub API docs: https://docs.github.com/en/rest/orgs/organization-roles#remove-an-organization-role-from-a-team // -//meta:operation DELETE /orgs/{org}/security-managers/teams/{team_slug} +//meta:operation DELETE /orgs/{org}/organization-roles/teams/{team_slug}/{security_manager_role_id} func (s *OrganizationsService) RemoveSecurityManagerTeam(ctx context.Context, org, team string) (*Response, error) { - u := fmt.Sprintf("orgs/%v/security-managers/teams/%v", org, team) - req, err := s.client.NewRequest("DELETE", u, nil) + securityManagerRole, resp, err := s.GetSecurityManagerRole(ctx, org) if err != nil { - return nil, err + return resp, err } - return s.client.Do(ctx, req, nil) + return s.RemoveOrgRoleFromTeam(ctx, org, team, securityManagerRole.GetID()) } diff --git a/github/orgs_security_managers_test.go b/github/orgs_security_managers_test.go index 925900409aa..4f708ff8ee7 100644 --- a/github/orgs_security_managers_test.go +++ b/github/orgs_security_managers_test.go @@ -109,7 +109,8 @@ func TestOrganizationsService_RemoveSecurityManagerTeam(t *testing.T) { t.Parallel() client, mux, _ := setup(t) - mux.HandleFunc("/orgs/o/security-managers/teams/t", func(w http.ResponseWriter, r *http.Request) { + handleGetSecurityManagerRole(t, mux, "o") + mux.HandleFunc("/orgs/o/organization-roles/teams/t/138", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "DELETE") }) @@ -141,10 +142,12 @@ func TestOrganizationsService_RemoveSecurityManagerTeam_invalidOrg(t *testing.T) func TestOrganizationsService_RemoveSecurityManagerTeam_invalidTeam(t *testing.T) { t.Parallel() - client, _, _ := setup(t) + client, mux, _ := setup(t) + + handleGetSecurityManagerRole(t, mux, "o") ctx := context.Background() - _, err := client.Organizations.RemoveSecurityManagerTeam(ctx, "%", "t") + _, err := client.Organizations.RemoveSecurityManagerTeam(ctx, "o", "%") testURLParseError(t, err) } From d980320f58c1d9ae1a9efd149d2d061f8b1ecf77 Mon Sep 17 00:00:00 2001 From: Joshua French Date: Mon, 6 Jan 2025 17:53:49 +0000 Subject: [PATCH 4/6] Fix linting errors --- github/orgs_security_managers.go | 4 ++-- github/orgs_security_managers_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/github/orgs_security_managers.go b/github/orgs_security_managers.go index 50a6ea9fb4b..1bf0693a1ad 100644 --- a/github/orgs_security_managers.go +++ b/github/orgs_security_managers.go @@ -7,7 +7,7 @@ package github import ( "context" - "fmt" + "errors" ) func (s *OrganizationsService) GetSecurityManagerRole(ctx context.Context, org string) (*CustomOrgRoles, *Response, error) { @@ -22,7 +22,7 @@ func (s *OrganizationsService) GetSecurityManagerRole(ctx context.Context, org s } } - return nil, resp, fmt.Errorf("security manager role not found") + return nil, resp, errors.New("security manager role not found") } // ListSecurityManagerTeams lists all security manager teams for an organization. diff --git a/github/orgs_security_managers_test.go b/github/orgs_security_managers_test.go index 4f708ff8ee7..d230dc7f6e2 100644 --- a/github/orgs_security_managers_test.go +++ b/github/orgs_security_managers_test.go @@ -154,6 +154,6 @@ func TestOrganizationsService_RemoveSecurityManagerTeam_invalidTeam(t *testing.T func handleGetSecurityManagerRole(t *testing.T, mux *http.ServeMux, org string) { mux.HandleFunc(fmt.Sprintf("/orgs/%s/organization-roles", org), func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") - fmt.Fprintf(w, `{"roles":[{"id":138,"name":"security_manager"}]}`) + fmt.Fprint(w, `{"roles":[{"id":138,"name":"security_manager"}]}`) }) } From 1b33bfc23ea7fb67901a71014ac80912cb6e1430 Mon Sep 17 00:00:00 2001 From: Joshua French Date: Mon, 6 Jan 2025 18:09:23 +0000 Subject: [PATCH 5/6] Fixing meta:operations --- github/orgs_security_managers.go | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/github/orgs_security_managers.go b/github/orgs_security_managers.go index 1bf0693a1ad..77cb5da4859 100644 --- a/github/orgs_security_managers.go +++ b/github/orgs_security_managers.go @@ -10,6 +10,11 @@ import ( "errors" ) +// GetSecurityManagerRole retrieves the security manager role for an organization. +// +// GitHub API docs: https://docs.github.com/rest/orgs/organization-roles#get-all-organization-roles-for-an-organization +// +//meta:operation GET /orgs/{org}/organization-roles func (s *OrganizationsService) GetSecurityManagerRole(ctx context.Context, org string) (*CustomOrgRoles, *Response, error) { roles, resp, err := s.ListRoles(ctx, org) if err != nil { @@ -27,9 +32,11 @@ func (s *OrganizationsService) GetSecurityManagerRole(ctx context.Context, org s // ListSecurityManagerTeams lists all security manager teams for an organization. // -// GitHub API docs: https://docs.github.com/en/rest/orgs/organization-roles#list-teams-that-are-assigned-to-an-organization-role +// GitHub API docs: https://docs.github.com/rest/orgs/organization-roles#get-all-organization-roles-for-an-organization +// GitHub API docs: https://docs.github.com/rest/orgs/organization-roles#list-teams-that-are-assigned-to-an-organization-role // -//meta:operation GET /orgs/{org}/organization-roles/{security_manager_role_id}/teams +//meta:operation GET /orgs/{org}/organization-roles +//meta:operation GET /orgs/{org}/organization-roles/{role_id}/teams func (s *OrganizationsService) ListSecurityManagerTeams(ctx context.Context, org string) ([]*Team, *Response, error) { securityManagerRole, resp, err := s.GetSecurityManagerRole(ctx, org) if err != nil { @@ -55,9 +62,11 @@ func (s *OrganizationsService) ListSecurityManagerTeams(ctx context.Context, org // AddSecurityManagerTeam adds a team to the list of security managers for an organization. // -// GitHub API docs: https://docs.github.com/en/rest/orgs/organization-roles#assign-an-organization-role-to-a-team +// GitHub API docs: https://docs.github.com/rest/orgs/organization-roles#assign-an-organization-role-to-a-team +// GitHub API docs: https://docs.github.com/rest/orgs/organization-roles#get-all-organization-roles-for-an-organization // -//meta:operation PUT /orgs/{org}/organization-roles/teams/{team_slug}/{security_manager_role_id} +//meta:operation GET /orgs/{org}/organization-roles +//meta:operation PUT /orgs/{org}/organization-roles/teams/{team_slug}/{role_id} func (s *OrganizationsService) AddSecurityManagerTeam(ctx context.Context, org, team string) (*Response, error) { securityManagerRole, resp, err := s.GetSecurityManagerRole(ctx, org) if err != nil { @@ -69,9 +78,11 @@ func (s *OrganizationsService) AddSecurityManagerTeam(ctx context.Context, org, // RemoveSecurityManagerTeam removes a team from the list of security managers for an organization. // -// GitHub API docs: https://docs.github.com/en/rest/orgs/organization-roles#remove-an-organization-role-from-a-team +// GitHub API docs: https://docs.github.com/rest/orgs/organization-roles#get-all-organization-roles-for-an-organization +// GitHub API docs: https://docs.github.com/rest/orgs/organization-roles#remove-an-organization-role-from-a-team // -//meta:operation DELETE /orgs/{org}/organization-roles/teams/{team_slug}/{security_manager_role_id} +//meta:operation GET /orgs/{org}/organization-roles +//meta:operation DELETE /orgs/{org}/organization-roles/teams/{team_slug}/{role_id} func (s *OrganizationsService) RemoveSecurityManagerTeam(ctx context.Context, org, team string) (*Response, error) { securityManagerRole, resp, err := s.GetSecurityManagerRole(ctx, org) if err != nil { From a5241285f70022179f17d0cc28777546519292c2 Mon Sep 17 00:00:00 2001 From: Joshua French Date: Mon, 6 Jan 2025 18:30:14 +0000 Subject: [PATCH 6/6] Add test coverage for GetSecurityManagerRole --- github/orgs_security_managers_test.go | 41 +++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/github/orgs_security_managers_test.go b/github/orgs_security_managers_test.go index d230dc7f6e2..f84b93481e9 100644 --- a/github/orgs_security_managers_test.go +++ b/github/orgs_security_managers_test.go @@ -14,6 +14,47 @@ import ( "github.com/google/go-cmp/cmp" ) +func TestOrganizationService_GetSecurityManagerRole(t *testing.T) { + t.Parallel() + client, mux, _ := setup(t) + + handleGetSecurityManagerRole(t, mux, "o") + + ctx := context.Background() + role, _, err := client.Organizations.GetSecurityManagerRole(ctx, "o") + if err != nil { + t.Errorf("Organizations.GetSecurityManagerRole returned error: %v", err) + } + + want := &CustomOrgRoles{ID: Ptr(int64(138)), Name: Ptr("security_manager")} + if !cmp.Equal(role, want) { + t.Errorf("Organizations.GetSecurityManagerRole returned %+v, want %+v", role, want) + } + + const methodName = "GetSecurityManagerRole" + testBadOptions(t, methodName, func() (err error) { + _, _, err = client.Organizations.GetSecurityManagerRole(ctx, "\n") + return err + }) + + testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { + got, resp, err := client.Organizations.GetSecurityManagerRole(ctx, "o") + if got != nil { + t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) + } + return resp, err + }) +} + +func TestOrganizationsService_GetSecurityManagerRole_invalidOrg(t *testing.T) { + t.Parallel() + client, _, _ := setup(t) + + ctx := context.Background() + _, _, err := client.Organizations.GetSecurityManagerRole(ctx, "%") + testURLParseError(t, err) +} + func TestOrganizationsService_ListSecurityManagerTeams(t *testing.T) { t.Parallel() client, mux, _ := setup(t)