[v10.4.x] Chore: Improve domain validation for Google OAuth - Backport 83229 to v10.4.x (#83726)
* Chore: Query oauth info from a new instance (#83229)
* query OAuth info from a new instance
* add `hd` validation flag
* add `disable_hd_validation` to settings map
* update documentation
---------
Co-authored-by: Jo <joao.guerreiro@grafana.com>
(cherry picked from commit b02ae375ba)
This commit is contained in:
@@ -17,19 +17,23 @@ import (
|
||||
ssoModels "github.com/grafana/grafana/pkg/services/ssosettings/models"
|
||||
"github.com/grafana/grafana/pkg/services/ssosettings/validation"
|
||||
"github.com/grafana/grafana/pkg/setting"
|
||||
"github.com/grafana/grafana/pkg/util/errutil"
|
||||
)
|
||||
|
||||
const (
|
||||
legacyAPIURL = "https://www.googleapis.com/oauth2/v1/userinfo"
|
||||
googleIAMGroupsEndpoint = "https://content-cloudidentity.googleapis.com/v1/groups/-/memberships:searchDirectGroups"
|
||||
googleIAMScope = "https://www.googleapis.com/auth/cloud-identity.groups.readonly"
|
||||
validateHDKey = "validate_hd"
|
||||
)
|
||||
|
||||
var _ social.SocialConnector = (*SocialGoogle)(nil)
|
||||
var _ ssosettings.Reloadable = (*SocialGoogle)(nil)
|
||||
var ExtraGoogleSettingKeys = []string{validateHDKey}
|
||||
|
||||
type SocialGoogle struct {
|
||||
*SocialBase
|
||||
validateHD bool
|
||||
}
|
||||
|
||||
type googleUserData struct {
|
||||
@@ -37,12 +41,14 @@ type googleUserData struct {
|
||||
Email string `json:"email"`
|
||||
Name string `json:"name"`
|
||||
EmailVerified bool `json:"email_verified"`
|
||||
HD string `json:"hd"`
|
||||
rawJSON []byte `json:"-"`
|
||||
}
|
||||
|
||||
func NewGoogleProvider(info *social.OAuthInfo, cfg *setting.Cfg, ssoSettings ssosettings.Service, features featuremgmt.FeatureToggles) *SocialGoogle {
|
||||
provider := &SocialGoogle{
|
||||
SocialBase: newSocialBase(social.GoogleProviderName, info, features, cfg),
|
||||
validateHD: MustBool(info.Extra[validateHDKey], false),
|
||||
}
|
||||
|
||||
if strings.HasPrefix(info.ApiUrl, legacyAPIURL) {
|
||||
@@ -87,6 +93,7 @@ func (s *SocialGoogle) Reload(ctx context.Context, settings ssoModels.SSOSetting
|
||||
defer s.reloadMutex.Unlock()
|
||||
|
||||
s.SocialBase = newSocialBase(social.GoogleProviderName, newInfo, s.features, s.cfg)
|
||||
s.validateHD = MustBool(newInfo.Extra[validateHDKey], false)
|
||||
|
||||
return nil
|
||||
}
|
||||
@@ -115,6 +122,10 @@ func (s *SocialGoogle) UserInfo(ctx context.Context, client *http.Client, token
|
||||
return nil, fmt.Errorf("user email is not verified")
|
||||
}
|
||||
|
||||
if err := s.isHDAllowed(data.HD, info); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
groups, errPage := s.retrieveGroups(ctx, client, data)
|
||||
if errPage != nil {
|
||||
s.log.Warn("Error retrieving groups", "error", errPage)
|
||||
@@ -157,6 +168,7 @@ type googleAPIData struct {
|
||||
Name string `json:"name"`
|
||||
Email string `json:"email"`
|
||||
EmailVerified bool `json:"verified_email"`
|
||||
HD string `json:"hd"`
|
||||
}
|
||||
|
||||
func (s *SocialGoogle) extractFromAPI(ctx context.Context, client *http.Client) (*googleUserData, error) {
|
||||
@@ -178,6 +190,7 @@ func (s *SocialGoogle) extractFromAPI(ctx context.Context, client *http.Client)
|
||||
Name: data.Name,
|
||||
Email: data.Email,
|
||||
EmailVerified: data.EmailVerified,
|
||||
HD: data.HD,
|
||||
rawJSON: response.Body,
|
||||
}, nil
|
||||
}
|
||||
@@ -290,3 +303,21 @@ func (s *SocialGoogle) getGroupsPage(ctx context.Context, client *http.Client, u
|
||||
|
||||
return &data, nil
|
||||
}
|
||||
|
||||
func (s *SocialGoogle) isHDAllowed(hd string, info *social.OAuthInfo) error {
|
||||
if s.validateHD {
|
||||
return nil
|
||||
}
|
||||
|
||||
if len(info.AllowedDomains) == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
for _, allowedDomain := range info.AllowedDomains {
|
||||
if hd == allowedDomain {
|
||||
return nil
|
||||
}
|
||||
}
|
||||
|
||||
return errutil.Forbidden("the hd claim found in the ID token is not present in the allowed domains", errutil.WithPublicMessage("Invalid domain"))
|
||||
}
|
||||
|
||||
@@ -890,3 +890,55 @@ func TestSocialGoogle_Reload(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsHDAllowed(t *testing.T) {
|
||||
testCases := []struct {
|
||||
name string
|
||||
email string
|
||||
allowedDomains []string
|
||||
expectedErrorMessage string
|
||||
validateHD bool
|
||||
}{
|
||||
{
|
||||
name: "should not fail if no allowed domains are set",
|
||||
email: "mycompany.com",
|
||||
allowedDomains: []string{},
|
||||
expectedErrorMessage: "",
|
||||
},
|
||||
{
|
||||
name: "should not fail if email is from allowed domain",
|
||||
email: "mycompany.com",
|
||||
allowedDomains: []string{"grafana.com", "mycompany.com", "example.com"},
|
||||
expectedErrorMessage: "",
|
||||
},
|
||||
{
|
||||
name: "should fail if email is not from allowed domain",
|
||||
email: "mycompany.com",
|
||||
allowedDomains: []string{"grafana.com", "example.com"},
|
||||
expectedErrorMessage: "the hd claim found in the ID token is not present in the allowed domains",
|
||||
},
|
||||
{
|
||||
name: "should not fail if the HD validation is disabled and the email not being from an allowed domain",
|
||||
email: "mycompany.com",
|
||||
allowedDomains: []string{"grafana.com", "example.com"},
|
||||
validateHD: true,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
info := &social.OAuthInfo{}
|
||||
info.AllowedDomains = tc.allowedDomains
|
||||
s := NewGoogleProvider(info, &setting.Cfg{}, &ssosettingstests.MockService{}, featuremgmt.WithFeatures())
|
||||
s.validateHD = tc.validateHD
|
||||
err := s.isHDAllowed(tc.email, info)
|
||||
|
||||
if tc.expectedErrorMessage != "" {
|
||||
require.Error(t, err)
|
||||
require.Contains(t, err.Error(), tc.expectedErrorMessage)
|
||||
} else {
|
||||
require.NoError(t, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user