feat: add authz support for GitHub teams (#4139)

* feat: fetch github teams for oidc groups claim Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * feat: enable GitHub team membership inclusion in access control groups Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * feat(auth): paginate org/team groups and tolerate missing read:org scope - apply the same optional-scope strategy to org lookup: paginate org pages and treat 403 Forbidden as non-fatal - keep non-403 org/team API errors as hard failures - preserve provider-returned casing for org/team-derived group values - add anonymized debug logging (counts/page metadata only) - extend tests for org pagination, org 403 optional behavior, team pagination, team 403 optional behavior, and team 5xx hard-fail behavior Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * test(auth): align GitHub user info test names and org-forbidden assertion - rename two Convey blocks so names match the mocked failing API call - assert org-forbidden case does not include "MyOrg" (real org group) instead of "testOrg" Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * test(auth): keep org login casing consistent in paginated teams mock Use MyOrg consistently across mocked /user/orgs and /user/teams payloads in the same success scenario, and align expected team-derived group assertions. Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * test(auth): align ListOrgs-forbidden teams casing with case-sensitive group checks Use MyOrg in the mocked /user/teams payload for the ListOrgs-forbidden scenario and assert MyOrg/infra accordingly to keep test casing semantics consistent. Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * test(auth): use consistent MyOrg casing in teams-forbidden assertion Align negative team-group assertion with MyOrg casing used by org mocks and other case-sensitive authz group checks. Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * docs(auth): align GitHub teams example casing with login-derived groups Use consistent org casing in the README example (myorg -> myorg/infra) to reflect that group strings follow GitHub login values and are not lowercased by zot. Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * docs(auth): clarify GitHub group casing is preserved Document that org/team group strings use GitHub login/slug casing as-is (no normalization), so policy entries must match exact case. Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * fix(auth): improve GitHub ListEmails failure logging Log the underlying error and use an operation-accurate message when client.Users.ListEmails fails in GetGithubUserInfo. Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> --------- Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> Co-authored-by: Kevin Andrews <kevin@nforced.uk>
2026-06-18 13:37:57 +08:00 · 2026-06-17 14:40:31 -07:00
parent 55b68228da
commit 43a5f155b8
3 changed files with 269 additions and 17 deletions
@@ -1061,7 +1061,7 @@ func GetGithubUserInfo(ctx context.Context, client *github.Client, log log.Logge

 	userEmails, _, err := client.Users.ListEmails(ctx, nil)
 	if err != nil {
-		log.Error().Msg("failed to set user record for empty email value")
+		log.Error().Err(err).Msg("failed to fetch github user emails")

 		return "", []string{}, err
 	}
@@ -1076,18 +1076,89 @@ func GetGithubUserInfo(ctx context.Context, client *github.Client, log log.Logge
 		}
 	}

-	orgs, _, err := client.Organizations.List(ctx, "", nil)
-	if err != nil {
-		log.Error().Msg("failed to set user record for empty email value")
-
-		return "", []string{}, err
-	}
+	log.Debug().Int("emailCount", len(userEmails)).Bool("hasPrimaryEmail", primaryEmail != "").
+		Msg("fetched github user emails")

 	groups := []string{}
-	for _, org := range orgs {
-		groups = append(groups, *org.Login)
+
+	orgsOpt := &github.ListOptions{PerPage: 100}
+	for {
+		orgs, resp, err := client.Organizations.List(ctx, "", orgsOpt)
+		if err != nil {
+			var ghErr *github.ErrorResponse
+
+			if errors.As(err, &ghErr) && ghErr.Response != nil && ghErr.Response.StatusCode == http.StatusForbidden {
+				log.Warn().Err(err).Msg("skipping github orgs: read:org scope not granted or access denied")
+
+				break
+			}
+
+			log.Error().Err(err).Msg("failed to fetch github organizations")
+
+			return "", []string{}, err
+		}
+
+		for _, org := range orgs {
+			if org.Login != nil {
+				groups = append(groups, *org.Login)
+			}
+		}
+
+		log.Debug().Int("orgsInPage", len(orgs)).Int("nextPage", func() int {
+			if resp == nil {
+				return 0
+			}
+
+			return resp.NextPage
+		}()).Msg("processed github organization page")
+
+		if resp == nil || resp.NextPage == 0 {
+			break
+		}
+
+		orgsOpt.Page = resp.NextPage
 	}

+	teamsOpt := &github.ListOptions{PerPage: 100}
+	for {
+		teams, resp, err := client.Teams.ListUserTeams(ctx, teamsOpt)
+		if err != nil {
+			var ghErr *github.ErrorResponse
+
+			if errors.As(err, &ghErr) && ghErr.Response != nil && ghErr.Response.StatusCode == http.StatusForbidden {
+				log.Warn().Err(err).Msg("skipping github teams: read:org scope not granted or access denied")
+
+				break
+			}
+
+			log.Error().Err(err).Msg("failed to fetch user teams")
+
+			return "", []string{}, err
+		}
+
+		for _, team := range teams {
+			if team.Organization != nil && team.Organization.Login != nil && team.Slug != nil {
+				groups = append(groups, fmt.Sprintf("%s/%s", *team.Organization.Login, *team.Slug))
+			}
+		}
+
+		log.Debug().Int("teamsInPage", len(teams)).Int("nextPage", func() int {
+			if resp == nil {
+				return 0
+			}
+
+			return resp.NextPage
+		}()).Msg("processed github team page")
+
+		if resp == nil || resp.NextPage == 0 {
+			break
+		}
+
+		teamsOpt.Page = resp.NextPage
+	}
+
+	log.Debug().Int("totalGroups", len(groups)).Msg("computed github groups for user")
+
 	return primaryEmail, groups, nil
 }

@@ -13007,23 +13007,55 @@ func TestGetGithubUserInfo(t *testing.T) {
 					},
 				},
 			),
-			mock.WithRequestMatch(
+			mock.WithRequestMatchHandler(
 				mock.GetUserOrgs,
-				[]github.Organization{
-					{
-						Login: new("testOrg"),
-					},
+				http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+					w.Header().Set("Content-Type", "application/json")
+
+					page := r.URL.Query().Get("page")
+					if page == "" || page == "1" {
+						w.Header().Set("Link", `<https://api.github.com/user/orgs?page=2>; rel="next"`)
+						_, _ = w.Write([]byte(`[{"login": "MyOrg"}]`))
+
+						return
+					}
+
+					_, _ = w.Write([]byte(`[{"login": "AnotherOrg"}]`))
+				}),
+			),
+			mock.WithRequestMatchHandler(
+				mock.EndpointPattern{
+					Pattern: "/user/teams",
+					Method:  "GET",
 				},
+				http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+					w.Header().Set("Content-Type", "application/json")
+
+					page := r.URL.Query().Get("page")
+					if page == "" || page == "1" {
+						w.Header().Set("Link", `<https://api.github.com/user/teams?page=2>; rel="next"`)
+						_, _ = w.Write([]byte(`[{"slug": "infra", "organization": {"login": "MyOrg"}}]`))
+
+						return
+					}
+
+					_, _ = w.Write([]byte(`[{"slug": "platform", "organization": {"login": "MyOrg"}}]`))
+				}),
 			),
 		)

 		client := github.NewClient(mockedHTTPClient)

-		_, _, err := api.GetGithubUserInfo(context.Background(), client, log.NewTestLogger())
+		email, groups, err := api.GetGithubUserInfo(context.Background(), client, log.NewTestLogger())
 		So(err, ShouldBeNil)
+		So(email, ShouldEqual, "test@test")
+		So(groups, ShouldContain, "MyOrg")
+		So(groups, ShouldContain, "AnotherOrg")
+		So(groups, ShouldContain, "MyOrg/infra")
+		So(groups, ShouldContain, "MyOrg/platform")
 	})

-	Convey("github ListEmails error", t, func() {
+	Convey("github ListEmails internal server error", t, func() {
 		mockedHTTPClient := mock.NewMockedHTTPClient(
 			mock.WithRequestMatchHandler(
 				mock.GetUserEmails,
@@ -13043,7 +13075,7 @@ func TestGetGithubUserInfo(t *testing.T) {
 		So(err, ShouldNotBeNil)
 	})

-	Convey("github ListEmails error", t, func() {
+	Convey("github ListOrgs forbidden", t, func() {
 		mockedHTTPClient := mock.NewMockedHTTPClient(
 			mock.WithRequestMatch(
 				mock.GetUserEmails,
@@ -13056,6 +13088,131 @@ func TestGetGithubUserInfo(t *testing.T) {
 			),
 			mock.WithRequestMatchHandler(
 				mock.GetUserOrgs,
+				http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+					mock.WriteError(
+						w,
+						http.StatusForbidden,
+						"github error",
+					)
+				}),
+			),
+			mock.WithRequestMatchHandler(
+				mock.EndpointPattern{
+					Pattern: "/user/teams",
+					Method:  "GET",
+				},
+				http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+					w.Header().Set("Content-Type", "application/json")
+					_, _ = w.Write([]byte(`[{"slug": "infra", "organization": {"login": "MyOrg"}}]`))
+				}),
+			),
+		)
+
+		client := github.NewClient(mockedHTTPClient)
+
+		email, groups, err := api.GetGithubUserInfo(context.Background(), client, log.NewTestLogger())
+		So(err, ShouldBeNil)
+		So(email, ShouldEqual, "test@test")
+		So(groups, ShouldNotContain, "MyOrg")
+		So(groups, ShouldContain, "MyOrg/infra")
+	})
+
+	Convey("github ListOrgs internal server error", t, func() {
+		mockedHTTPClient := mock.NewMockedHTTPClient(
+			mock.WithRequestMatch(
+				mock.GetUserEmails,
+				[]github.UserEmail{
+					{
+						Email:   new("test@test"),
+						Primary: new(true),
+					},
+				},
+			),
+			mock.WithRequestMatchHandler(
+				mock.GetUserOrgs,
+				http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+					mock.WriteError(
+						w,
+						http.StatusInternalServerError,
+						"github error",
+					)
+				}),
+			),
+		)
+
+		client := github.NewClient(mockedHTTPClient)
+
+		_, _, err := api.GetGithubUserInfo(context.Background(), client, log.NewTestLogger())
+		So(err, ShouldNotBeNil)
+	})
+
+	Convey("github ListUserTeams forbidden", t, func() {
+		mockedHTTPClient := mock.NewMockedHTTPClient(
+			mock.WithRequestMatch(
+				mock.GetUserEmails,
+				[]github.UserEmail{
+					{
+						Email:   new("test@test"),
+						Primary: new(true),
+					},
+				},
+			),
+			mock.WithRequestMatch(
+				mock.GetUserOrgs,
+				[]github.Organization{
+					{
+						Login: new("MyOrg"),
+					},
+				},
+			),
+			mock.WithRequestMatchHandler(
+				mock.EndpointPattern{
+					Pattern: "/user/teams",
+					Method:  "GET",
+				},
+				http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+					mock.WriteError(
+						w,
+						http.StatusForbidden,
+						"github error",
+					)
+				}),
+			),
+		)
+
+		client := github.NewClient(mockedHTTPClient)
+
+		email, groups, err := api.GetGithubUserInfo(context.Background(), client, log.NewTestLogger())
+		So(err, ShouldBeNil)
+		So(email, ShouldEqual, "test@test")
+		So(groups, ShouldContain, "MyOrg")
+		So(groups, ShouldNotContain, "MyOrg/infra")
+	})
+
+	Convey("github ListUserTeams internal server error", t, func() {
+		mockedHTTPClient := mock.NewMockedHTTPClient(
+			mock.WithRequestMatch(
+				mock.GetUserEmails,
+				[]github.UserEmail{
+					{
+						Email:   new("test@test"),
+						Primary: new(true),
+					},
+				},
+			),
+			mock.WithRequestMatch(
+				mock.GetUserOrgs,
+				[]github.Organization{
+					{
+						Login: new("MyOrg"),
+					},
+				},
+			),
+			mock.WithRequestMatchHandler(
+				mock.EndpointPattern{
+					Pattern: "/user/teams",
+					Method:  "GET",
+				},
 				http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 					mock.WriteError(
 						w,