From 43a5f155b8517688f9f122afff00aabc32528e29 Mon Sep 17 00:00:00 2001 From: Ramkumar Chinchani <45800463+rchincha@users.noreply.github.com> Date: Wed, 17 Jun 2026 14:40:31 -0700 Subject: [PATCH] feat: add authz support for GitHub teams (#4139) * feat: fetch github teams for oidc groups claim Signed-off-by: Ramkumar Chinchani * feat: enable GitHub team membership inclusion in access control groups Signed-off-by: Ramkumar Chinchani * 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 * 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 * 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 * 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 * 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 * 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 * 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 * 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 --------- Signed-off-by: Ramkumar Chinchani Co-authored-by: Kevin Andrews --- examples/README.md | 24 +++++ pkg/api/authn.go | 89 +++++++++++++++++-- pkg/api/controller_test.go | 173 +++++++++++++++++++++++++++++++++++-- 3 files changed, 269 insertions(+), 17 deletions(-) diff --git a/examples/README.md b/examples/README.md index f15c6e2d..9992651e 100644 --- a/examples/README.md +++ b/examples/README.md @@ -415,6 +415,7 @@ zot can be configured to use the above providers with: } ``` + To login with either provider use http://127.0.0.1:8080/zot/auth/login?provider=\&callback_ui=/home for example to login with github use http://127.0.0.1:8080/zot/auth/login?provider=github&callback_ui=/home @@ -441,6 +442,29 @@ for example github callback url would be http://127.0.0.1:8080/zot/auth/callback If network policy doesn't allow inbound connections, this callback wont work! +#### GitHub Teams in Access Control + +When authenticating with the GitHub provider, if you include the `read:org` scope, zot will fetch both the user's Organization memberships and their Team memberships. +Team memberships are formatted as `/` and added to the user's groups. You can use these in your access control policies. For example, if a user belongs to the `Infra` team in the `myorg` organization, the group name will be `myorg/infra`. +Group strings preserve GitHub-provided `login`/`slug` casing (no lowercasing is applied), so policy group values must match that exact casing. + +```json +{ + "accessControl": { + "repositories": { + "myorg/infrastructure/**": { + "policies": [ + { + "groups": ["myorg/infra"], + "actions": ["read", "create", "update", "delete"] + } + ] + } + } + } +} +``` + dex is an identity service that uses OpenID Connect to drive authentication for other apps https://github.com/dexidp/dex To setup dex service see https://dexidp.io/docs/getting-started/ diff --git a/pkg/api/authn.go b/pkg/api/authn.go index abfd5551..6f6a6f5f 100644 --- a/pkg/api/authn.go +++ b/pkg/api/authn.go @@ -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 } diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go index 39acf522..f51c2b2f 100644 --- a/pkg/api/controller_test.go +++ b/pkg/api/controller_test.go @@ -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", `; 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", `; 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,