Skip to content

Commit 43a5f15

Browse files
rchinchaCaffe1neAdd1ct
rchincha
and
Caffe1neAdd1ct
authored
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>
1 parent 55b6822 commit 43a5f15

3 files changed

Lines changed: 267 additions & 15 deletions

File tree

‎examples/README.md‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,7 @@ zot can be configured to use the above providers with:
415415
}
416416
```
417417

418+
418419
To login with either provider use http://127.0.0.1:8080/zot/auth/login?provider=\<provider\>&callback_ui=/home
419420
for example to login with github use http://127.0.0.1:8080/zot/auth/login?provider=github&callback_ui=/home
420421

@@ -441,6 +442,29 @@ for example github callback url would be http://127.0.0.1:8080/zot/auth/callback
441442

442443
If network policy doesn't allow inbound connections, this callback wont work!
443444

445+
#### GitHub Teams in Access Control
446+
447+
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.
448+
Team memberships are formatted as `<organization>/<team-slug>` 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`.
449+
Group strings preserve GitHub-provided `login`/`slug` casing (no lowercasing is applied), so policy group values must match that exact casing.
450+
451+
```json
452+
{
453+
"accessControl": {
454+
"repositories": {
455+
"myorg/infrastructure/**": {
456+
"policies": [
457+
{
458+
"groups": ["myorg/infra"],
459+
"actions": ["read", "create", "update", "delete"]
460+
}
461+
]
462+
}
463+
}
464+
}
465+
}
466+
```
467+
444468
dex is an identity service that uses OpenID Connect to drive authentication for other apps https://github.com/dexidp/dex
445469
To setup dex service see https://dexidp.io/docs/getting-started/
446470

‎pkg/api/authn.go‎

Lines changed: 79 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1061,7 +1061,7 @@ func GetGithubUserInfo(ctx context.Context, client *github.Client, log log.Logge
10611061

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

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

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

1083-
return "", []string{}, err
1082+
groups := []string{}
1083+
1084+
orgsOpt := &github.ListOptions{PerPage: 100}
1085+
for {
1086+
orgs, resp, err := client.Organizations.List(ctx, "", orgsOpt)
1087+
if err != nil {
1088+
var ghErr *github.ErrorResponse
1089+
1090+
if errors.As(err, &ghErr) && ghErr.Response != nil && ghErr.Response.StatusCode == http.StatusForbidden {
1091+
log.Warn().Err(err).Msg("skipping github orgs: read:org scope not granted or access denied")
1092+
1093+
break
1094+
}
1095+
1096+
log.Error().Err(err).Msg("failed to fetch github organizations")
1097+
1098+
return "", []string{}, err
1099+
}
1100+
1101+
for _, org := range orgs {
1102+
if org.Login != nil {
1103+
groups = append(groups, *org.Login)
1104+
}
1105+
}
1106+
1107+
log.Debug().Int("orgsInPage", len(orgs)).Int("nextPage", func() int {
1108+
if resp == nil {
1109+
return 0
1110+
}
1111+
1112+
return resp.NextPage
1113+
}()).Msg("processed github organization page")
1114+
1115+
if resp == nil || resp.NextPage == 0 {
1116+
break
1117+
}
1118+
1119+
orgsOpt.Page = resp.NextPage
10841120
}
10851121

1086-
groups := []string{}
1087-
for _, org := range orgs {
1088-
groups = append(groups, *org.Login)
1122+
teamsOpt := &github.ListOptions{PerPage: 100}
1123+
for {
1124+
teams, resp, err := client.Teams.ListUserTeams(ctx, teamsOpt)
1125+
if err != nil {
1126+
var ghErr *github.ErrorResponse
1127+
1128+
if errors.As(err, &ghErr) && ghErr.Response != nil && ghErr.Response.StatusCode == http.StatusForbidden {
1129+
log.Warn().Err(err).Msg("skipping github teams: read:org scope not granted or access denied")
1130+
1131+
break
1132+
}
1133+
1134+
log.Error().Err(err).Msg("failed to fetch user teams")
1135+
1136+
return "", []string{}, err
1137+
}
1138+
1139+
for _, team := range teams {
1140+
if team.Organization != nil && team.Organization.Login != nil && team.Slug != nil {
1141+
groups = append(groups, fmt.Sprintf("%s/%s", *team.Organization.Login, *team.Slug))
1142+
}
1143+
}
1144+
1145+
log.Debug().Int("teamsInPage", len(teams)).Int("nextPage", func() int {
1146+
if resp == nil {
1147+
return 0
1148+
}
1149+
1150+
return resp.NextPage
1151+
}()).Msg("processed github team page")
1152+
1153+
if resp == nil || resp.NextPage == 0 {
1154+
break
1155+
}
1156+
1157+
teamsOpt.Page = resp.NextPage
10891158
}
10901159

1160+
log.Debug().Int("totalGroups", len(groups)).Msg("computed github groups for user")
1161+
10911162
return primaryEmail, groups, nil
10921163
}
10931164

‎pkg/api/controller_test.go‎

Lines changed: 164 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13007,26 +13007,129 @@ func TestGetGithubUserInfo(t *testing.T) {
1300713007
},
1300813008
},
1300913009
),
13010-
mock.WithRequestMatch(
13010+
mock.WithRequestMatchHandler(
1301113011
mock.GetUserOrgs,
13012-
[]github.Organization{
13012+
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
13013+
w.Header().Set("Content-Type", "application/json")
13014+
13015+
page := r.URL.Query().Get("page")
13016+
if page == "" || page == "1" {
13017+
w.Header().Set("Link", `<https://api.github.com/user/orgs?page=2>; rel="next"`)
13018+
_, _ = w.Write([]byte(`[{"login": "MyOrg"}]`))
13019+
13020+
return
13021+
}
13022+
13023+
_, _ = w.Write([]byte(`[{"login": "AnotherOrg"}]`))
13024+
}),
13025+
),
13026+
mock.WithRequestMatchHandler(
13027+
mock.EndpointPattern{
13028+
Pattern: "/user/teams",
13029+
Method: "GET",
13030+
},
13031+
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
13032+
w.Header().Set("Content-Type", "application/json")
13033+
13034+
page := r.URL.Query().Get("page")
13035+
if page == "" || page == "1" {
13036+
w.Header().Set("Link", `<https://api.github.com/user/teams?page=2>; rel="next"`)
13037+
_, _ = w.Write([]byte(`[{"slug": "infra", "organization": {"login": "MyOrg"}}]`))
13038+
13039+
return
13040+
}
13041+
13042+
_, _ = w.Write([]byte(`[{"slug": "platform", "organization": {"login": "MyOrg"}}]`))
13043+
}),
13044+
),
13045+
)
13046+
13047+
client := github.NewClient(mockedHTTPClient)
13048+
13049+
email, groups, err := api.GetGithubUserInfo(context.Background(), client, log.NewTestLogger())
13050+
So(err, ShouldBeNil)
13051+
So(email, ShouldEqual, "test@test")
13052+
So(groups, ShouldContain, "MyOrg")
13053+
So(groups, ShouldContain, "AnotherOrg")
13054+
So(groups, ShouldContain, "MyOrg/infra")
13055+
So(groups, ShouldContain, "MyOrg/platform")
13056+
})
13057+
13058+
Convey("github ListEmails internal server error", t, func() {
13059+
mockedHTTPClient := mock.NewMockedHTTPClient(
13060+
mock.WithRequestMatchHandler(
13061+
mock.GetUserEmails,
13062+
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
13063+
mock.WriteError(
13064+
w,
13065+
http.StatusInternalServerError,
13066+
"github error",
13067+
)
13068+
}),
13069+
),
13070+
)
13071+
13072+
client := github.NewClient(mockedHTTPClient)
13073+
13074+
_, _, err := api.GetGithubUserInfo(context.Background(), client, log.NewTestLogger())
13075+
So(err, ShouldNotBeNil)
13076+
})
13077+
13078+
Convey("github ListOrgs forbidden", t, func() {
13079+
mockedHTTPClient := mock.NewMockedHTTPClient(
13080+
mock.WithRequestMatch(
13081+
mock.GetUserEmails,
13082+
[]github.UserEmail{
1301313083
{
13014-
Login: new("testOrg"),
13084+
Email: new("test@test"),
13085+
Primary: new(true),
1301513086
},
1301613087
},
1301713088
),
13089+
mock.WithRequestMatchHandler(
13090+
mock.GetUserOrgs,
13091+
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
13092+
mock.WriteError(
13093+
w,
13094+
http.StatusForbidden,
13095+
"github error",
13096+
)
13097+
}),
13098+
),
13099+
mock.WithRequestMatchHandler(
13100+
mock.EndpointPattern{
13101+
Pattern: "/user/teams",
13102+
Method: "GET",
13103+
},
13104+
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
13105+
w.Header().Set("Content-Type", "application/json")
13106+
_, _ = w.Write([]byte(`[{"slug": "infra", "organization": {"login": "MyOrg"}}]`))
13107+
}),
13108+
),
1301813109
)
1301913110

1302013111
client := github.NewClient(mockedHTTPClient)
1302113112

13022-
_, _, err := api.GetGithubUserInfo(context.Background(), client, log.NewTestLogger())
13113+
email, groups, err := api.GetGithubUserInfo(context.Background(), client, log.NewTestLogger())
1302313114
So(err, ShouldBeNil)
13115+
So(email, ShouldEqual, "test@test")
13116+
So(groups, ShouldNotContain, "MyOrg")
13117+
So(groups, ShouldContain, "MyOrg/infra")
1302413118
})
1302513119

13026-
Convey("github ListEmails error", t, func() {
13120+
Convey("github ListOrgs internal server error", t, func() {
1302713121
mockedHTTPClient := mock.NewMockedHTTPClient(
13028-
mock.WithRequestMatchHandler(
13122+
mock.WithRequestMatch(
1302913123
mock.GetUserEmails,
13124+
[]github.UserEmail{
13125+
{
13126+
Email: new("test@test"),
13127+
Primary: new(true),
13128+
},
13129+
},
13130+
),
13131+
mock.WithRequestMatchHandler(
13132+
mock.GetUserOrgs,
1303013133
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
1303113134
mock.WriteError(
1303213135
w,
@@ -13043,7 +13146,7 @@ func TestGetGithubUserInfo(t *testing.T) {
1304313146
So(err, ShouldNotBeNil)
1304413147
})
1304513148

13046-
Convey("github ListEmails error", t, func() {
13149+
Convey("github ListUserTeams forbidden", t, func() {
1304713150
mockedHTTPClient := mock.NewMockedHTTPClient(
1304813151
mock.WithRequestMatch(
1304913152
mock.GetUserEmails,
@@ -13054,8 +13157,62 @@ func TestGetGithubUserInfo(t *testing.T) {
1305413157
},
1305513158
},
1305613159
),
13160+
mock.WithRequestMatch(
13161+
mock.GetUserOrgs,
13162+
[]github.Organization{
13163+
{
13164+
Login: new("MyOrg"),
13165+
},
13166+
},
13167+
),
1305713168
mock.WithRequestMatchHandler(
13169+
mock.EndpointPattern{
13170+
Pattern: "/user/teams",
13171+
Method: "GET",
13172+
},
13173+
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
13174+
mock.WriteError(
13175+
w,
13176+
http.StatusForbidden,
13177+
"github error",
13178+
)
13179+
}),
13180+
),
13181+
)
13182+
13183+
client := github.NewClient(mockedHTTPClient)
13184+
13185+
email, groups, err := api.GetGithubUserInfo(context.Background(), client, log.NewTestLogger())
13186+
So(err, ShouldBeNil)
13187+
So(email, ShouldEqual, "test@test")
13188+
So(groups, ShouldContain, "MyOrg")
13189+
So(groups, ShouldNotContain, "MyOrg/infra")
13190+
})
13191+
13192+
Convey("github ListUserTeams internal server error", t, func() {
13193+
mockedHTTPClient := mock.NewMockedHTTPClient(
13194+
mock.WithRequestMatch(
13195+
mock.GetUserEmails,
13196+
[]github.UserEmail{
13197+
{
13198+
Email: new("test@test"),
13199+
Primary: new(true),
13200+
},
13201+
},
13202+
),
13203+
mock.WithRequestMatch(
1305813204
mock.GetUserOrgs,
13205+
[]github.Organization{
13206+
{
13207+
Login: new("MyOrg"),
13208+
},
13209+
},
13210+
),
13211+
mock.WithRequestMatchHandler(
13212+
mock.EndpointPattern{
13213+
Pattern: "/user/teams",
13214+
Method: "GET",
13215+
},
1305913216
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
1306013217
mock.WriteError(
1306113218
w,

0 commit comments

Comments
 (0)