From 5fb87de321675ecadfda1b72b066c4808d63fe8e Mon Sep 17 00:00:00 2001 From: Nihal <38865967+wasim-nihal@users.noreply.github.com> Date: Tue, 7 May 2024 21:49:20 +0530 Subject: [PATCH] Auth: Added support to filter for parent teams in GitHub connector's team membership filter (#86754) * added changes for parent teams in team_ids. see https://github.com/grafana/grafana/issues/85916 Signed-off-by: Syed Nihal * added unit test Signed-off-by: Syed Nihal * addressed review comments to consider case where parent object can be null Signed-off-by: Syed Nihal * addressed review comment Signed-off-by: Syed Nihal --------- Signed-off-by: Syed Nihal --- pkg/login/social/connectors/github_oauth.go | 5 ++- .../social/connectors/github_oauth_test.go | 42 ++++++++++++++++--- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/pkg/login/social/connectors/github_oauth.go b/pkg/login/social/connectors/github_oauth.go index 18e2175bcc1..29b1f61628f 100644 --- a/pkg/login/social/connectors/github_oauth.go +++ b/pkg/login/social/connectors/github_oauth.go @@ -45,6 +45,9 @@ type GithubTeam struct { Organization struct { Login string `json:"login"` } `json:"organization"` + Parent *struct { + Id int `json:"id"` + } `json:"parent"` } var ( @@ -144,7 +147,7 @@ func (s *SocialGithub) isTeamMember(ctx context.Context, client *http.Client) bo for _, teamId := range s.teamIds { for _, membership := range teamMemberships { - if teamId == membership.Id { + if teamId == membership.Id || (membership.Parent != nil && teamId == membership.Parent.Id) { return true } } diff --git a/pkg/login/social/connectors/github_oauth_test.go b/pkg/login/social/connectors/github_oauth_test.go index e7c8edfa7e4..d4f0d0862fd 100644 --- a/pkg/login/social/connectors/github_oauth_test.go +++ b/pkg/login/social/connectors/github_oauth_test.go @@ -34,7 +34,6 @@ const testGHUserTeamsJSON = `[ "permission": "admin", "members_url": "https://api.github.com/teams/1/members{/member}", "repositories_url": "https://api.github.com/teams/1/repos", - "parent": null, "members_count": 3, "repos_count": 10, "created_at": "2017-07-14T16:53:42Z", @@ -68,7 +67,21 @@ const testGHUserTeamsJSON = `[ "created_at": "2008-01-14T04:33:35Z", "updated_at": "2017-08-17T12:37:15Z", "type": "Organization" - } + }, + "parent": { + "name": "DC", + "id": 99, + "node_id": "bm9kZTIyCg==", + "slug": "dc", + "description": "", + "privacy": "closed", + "notification_setting": "notifications_enabled", + "url": "https://api.github.com/organizations/github/team/2", + "html_url": "https://github.com/orgs/github/teams/dc", + "members_url": "https://api.github.com/orgs/github/members{/member}", + "repositories_url": "https://api.github.com/teams/2/repos", + "permission": "pull" + } } ]` @@ -132,6 +145,7 @@ func TestSocialGitHub_UserInfo(t *testing.T) { autoAssignOrgRole string want *social.BasicUserInfo wantErr bool + oAuthExtraInfo map[string]string }{ { name: "Basic User info", @@ -225,6 +239,25 @@ func TestSocialGitHub_UserInfo(t *testing.T) { Groups: []string{"https://github.com/orgs/github/teams/justice-league", "@github/justice-league"}, }, }, + { + // see: https://github.com/grafana/grafana/issues/85916 + name: "should check parent team id for team membership", + roleAttributePath: "", + userRawJSON: testGHUserJSON, + autoAssignOrgRole: "Editor", + userTeamsRawJSON: testGHUserTeamsJSON, + oAuthExtraInfo: map[string]string{ + "team_ids": "99", + }, + want: &social.BasicUserInfo{ + Id: "1", + Name: "monalisa octocat", + Email: "octocat@github.com", + Login: "octocat", + Role: "Editor", + Groups: []string{"https://github.com/orgs/github/teams/justice-league", "@github/justice-league"}, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -250,10 +283,7 @@ func TestSocialGitHub_UserInfo(t *testing.T) { ApiUrl: server.URL + "/user", RoleAttributePath: tt.roleAttributePath, SkipOrgRoleSync: tt.settingSkipOrgRoleSync, - Extra: map[string]string{ - "allowed_organizations": "", - "team_ids": "", - }, + Extra: tt.oAuthExtraInfo, }, &setting.Cfg{ AutoAssignOrgRole: tt.autoAssignOrgRole, }, &ssosettingstests.MockService{},