IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

OAuth: Fix misleading warn log related to oauth and increase logged content (#57336)

Browse Source 
* only emit warn log when sync is not skipped. Fix warn log that had wrong recommendation

* log presence of refresh token
This commit is contained in:
Jo 2022-10-20 09:50:12 +00:00 committed by GitHub
parent 3ee450e66b
commit 7f3536a6d2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 32 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
pkg/api/login_oauth.go
View File

@ -195,10 +195,15 @@ func (hs *HTTPServer) OAuthLogin(ctx *models.ReqContext) {
token.TokenType = "Bearer"
if hs.Cfg.Env != setting.Dev {
oauthLogger.Debug("OAuthLogin: got token", "expiry", fmt.Sprintf("%v", token.Expiry))
oauthLogger.Debug("OAuthLogin: got token",
"expiry", fmt.Sprintf("%v", token.Expiry),
"type", token.TokenType,
"has_refresh_token", token.RefreshToken != "",
)
} else {
oauthLogger.Debug("OAuthLogin: got token",
"expiry", fmt.Sprintf("%v", token.Expiry),
"type", token.TokenType,
"access_token", fmt.Sprintf("%v", token.AccessToken),
"refresh_token", fmt.Sprintf("%v", token.RefreshToken),
)

26
pkg/login/social/azuread_oauth_test.go
View File

@ -54,7 +54,7 @@ func TestSocialAzureAD_UserInfo(t *testing.T) {
ID: "1234",
},
fields: fields{
SocialBase: newSocialBase("azuread", &oauth2.Config{}, &OAuthInfo{}, "Viewer"),
SocialBase: newSocialBase("azuread", &oauth2.Config{}, &OAuthInfo{}, "Viewer", false),
},
want: &BasicUserInfo{
Id: "1234",
@ -93,7 +93,7 @@ func TestSocialAzureAD_UserInfo(t *testing.T) {
ID: "1234",
},
fields: fields{
SocialBase: newSocialBase("azuread", &oauth2.Config{}, &OAuthInfo{}, "Viewer"),
SocialBase: newSocialBase("azuread", &oauth2.Config{}, &OAuthInfo{}, "Viewer", false),
},
want: &BasicUserInfo{
Id: "1234",
@ -143,7 +143,7 @@ func TestSocialAzureAD_UserInfo(t *testing.T) {
{
name: "Only other roles",
fields: fields{
SocialBase: newSocialBase("azuread", &oauth2.Config{}, &OAuthInfo{}, "Viewer"),
SocialBase: newSocialBase("azuread", &oauth2.Config{}, &OAuthInfo{}, "Viewer", false),
},
claims: &azureClaims{
Email: "me@example.com",
@ -171,7 +171,7 @@ func TestSocialAzureAD_UserInfo(t *testing.T) {
ID: "1234",
},
fields: fields{
SocialBase: newSocialBase("azuread", &oauth2.Config{}, &OAuthInfo{}, "Editor"),
SocialBase: newSocialBase("azuread", &oauth2.Config{}, &OAuthInfo{}, "Editor", false),
},
want: &BasicUserInfo{
Id: "1234",
@ -220,7 +220,7 @@ func TestSocialAzureAD_UserInfo(t *testing.T) {
},
{
name: "Grafana Admin but setting is disabled",
fields: fields{SocialBase: newSocialBase("azuread", &oauth2.Config{}, &OAuthInfo{AllowAssignGrafanaAdmin: false}, "Editor")},
fields: fields{SocialBase: newSocialBase("azuread", &oauth2.Config{}, &OAuthInfo{AllowAssignGrafanaAdmin: false}, "Editor", false)},
claims: &azureClaims{
Email: "me@example.com",
PreferredUsername: "",
@ -242,7 +242,7 @@ func TestSocialAzureAD_UserInfo(t *testing.T) {
name: "Editor roles in claim and GrafanaAdminAssignment enabled",
fields: fields{
SocialBase: newSocialBase("azuread",
&oauth2.Config{}, &OAuthInfo{AllowAssignGrafanaAdmin: true}, "")},
&oauth2.Config{}, &OAuthInfo{AllowAssignGrafanaAdmin: true}, "", false)},
claims: &azureClaims{
Email: "me@example.com",
PreferredUsername: "",
@ -263,7 +263,7 @@ func TestSocialAzureAD_UserInfo(t *testing.T) {
{
name: "Grafana Admin and Editor roles in claim",
fields: fields{SocialBase: newSocialBase("azuread",
&oauth2.Config{}, &OAuthInfo{AllowAssignGrafanaAdmin: true}, "")},
&oauth2.Config{}, &OAuthInfo{AllowAssignGrafanaAdmin: true}, "", false)},
claims: &azureClaims{
Email: "me@example.com",
PreferredUsername: "",
@ -302,7 +302,7 @@ func TestSocialAzureAD_UserInfo(t *testing.T) {
fields: fields{
allowedGroups: []string{"foo", "bar"},
SocialBase: newSocialBase("azuread",
&oauth2.Config{}, &OAuthInfo{AllowAssignGrafanaAdmin: false}, "Viewer"),
&oauth2.Config{}, &OAuthInfo{AllowAssignGrafanaAdmin: false}, "Viewer", false),
},
claims: &azureClaims{
Email: "me@example.com",
@ -324,7 +324,7 @@ func TestSocialAzureAD_UserInfo(t *testing.T) {
{
name: "Fetch groups when ClaimsNames and ClaimsSources is set",
fields: fields{
SocialBase: newSocialBase("azuread", &oauth2.Config{}, &OAuthInfo{}, ""),
SocialBase: newSocialBase("azuread", &oauth2.Config{}, &OAuthInfo{}, "", false),
},
claims: &azureClaims{
ID: "1",
@ -349,7 +349,7 @@ func TestSocialAzureAD_UserInfo(t *testing.T) {
{
name: "Fetch groups when forceUseGraphAPI is set",
fields: fields{
SocialBase: newSocialBase("azuread", &oauth2.Config{}, &OAuthInfo{}, ""),
SocialBase: newSocialBase("azuread", &oauth2.Config{}, &OAuthInfo{}, "", false),
forceUseGraphAPI: true,
},
claims: &azureClaims{
@ -376,7 +376,7 @@ func TestSocialAzureAD_UserInfo(t *testing.T) {
{
name: "Fetch empty role when strict attribute role is true and no match",
fields: fields{
SocialBase: newSocialBase("azuread", &oauth2.Config{}, &OAuthInfo{RoleAttributeStrict: true}, ""),
SocialBase: newSocialBase("azuread", &oauth2.Config{}, &OAuthInfo{RoleAttributeStrict: true}, "", false),
},
claims: &azureClaims{
Email: "me@example.com",
@ -392,7 +392,7 @@ func TestSocialAzureAD_UserInfo(t *testing.T) {
{
name: "Fetch empty role when strict attribute role is true and no role claims returned",
fields: fields{
SocialBase: newSocialBase("azuread", &oauth2.Config{}, &OAuthInfo{RoleAttributeStrict: true}, ""),
SocialBase: newSocialBase("azuread", &oauth2.Config{}, &OAuthInfo{RoleAttributeStrict: true}, "", false),
},
claims: &azureClaims{
Email: "me@example.com",
@ -416,7 +416,7 @@ func TestSocialAzureAD_UserInfo(t *testing.T) {
}
if tt.fields.SocialBase == nil {
s.SocialBase = newSocialBase("azuread", &oauth2.Config{}, &OAuthInfo{}, "")
s.SocialBase = newSocialBase("azuread", &oauth2.Config{}, &OAuthInfo{}, "", false)
}
key := []byte("secret")

2
pkg/login/social/github_oauth_test.go
View File

@ -202,7 +202,7 @@ func TestSocialGitHub_UserInfo(t *testing.T) {
s := &SocialGithub{
SocialBase: newSocialBase("github", &oauth2.Config{},
&OAuthInfo{RoleAttributePath: tt.roleAttributePath}, tt.autoAssignOrgRole),
&OAuthInfo{RoleAttributePath: tt.roleAttributePath}, tt.autoAssignOrgRole, false),
allowedOrganizations: []string{},
apiUrl: server.URL + "/user",
teamIds: []int{},

21
pkg/login/social/social.go
View File

@ -139,7 +139,7 @@ func ProvideService(cfg *setting.Cfg) *SocialService {
// GitHub.
if name == "github" {
ss.socialMap["github"] = &SocialGithub{
SocialBase: newSocialBase(name, &config, info, cfg.AutoAssignOrgRole),
SocialBase: newSocialBase(name, &config, info, cfg.AutoAssignOrgRole, cfg.OAuthSkipOrgRoleUpdateSync),
apiUrl: info.ApiUrl,
teamIds: sec.Key("team_ids").Ints(","),
allowedOrganizations: util.SplitString(sec.Key("allowed_organizations").String()),
@ -149,7 +149,7 @@ func ProvideService(cfg *setting.Cfg) *SocialService {
// GitLab.
if name == "gitlab" {
ss.socialMap["gitlab"] = &SocialGitlab{
SocialBase: newSocialBase(name, &config, info, cfg.AutoAssignOrgRole),
SocialBase: newSocialBase(name, &config, info, cfg.AutoAssignOrgRole, cfg.OAuthSkipOrgRoleUpdateSync),
apiUrl: info.ApiUrl,
allowedGroups: util.SplitString(sec.Key("allowed_groups").String()),
}
@ -158,7 +158,7 @@ func ProvideService(cfg *setting.Cfg) *SocialService {
// Google.
if name == "google" {
ss.socialMap["google"] = &SocialGoogle{
SocialBase: newSocialBase(name, &config, info, cfg.AutoAssignOrgRole),
SocialBase: newSocialBase(name, &config, info, cfg.AutoAssignOrgRole, cfg.OAuthSkipOrgRoleUpdateSync),
hostedDomain: info.HostedDomain,
apiUrl: info.ApiUrl,
}
@ -167,7 +167,7 @@ func ProvideService(cfg *setting.Cfg) *SocialService {
// AzureAD.
if name == "azuread" {
ss.socialMap["azuread"] = &SocialAzureAD{
SocialBase: newSocialBase(name, &config, info, cfg.AutoAssignOrgRole),
SocialBase: newSocialBase(name, &config, info, cfg.AutoAssignOrgRole, cfg.OAuthSkipOrgRoleUpdateSync),
allowedGroups: util.SplitString(sec.Key("allowed_groups").String()),
forceUseGraphAPI: sec.Key("force_use_graph_api").MustBool(false),
}
@ -176,7 +176,7 @@ func ProvideService(cfg *setting.Cfg) *SocialService {
// Okta
if name == "okta" {
ss.socialMap["okta"] = &SocialOkta{
SocialBase: newSocialBase(name, &config, info, cfg.AutoAssignOrgRole),
SocialBase: newSocialBase(name, &config, info, cfg.AutoAssignOrgRole, cfg.OAuthSkipOrgRoleUpdateSync),
apiUrl: info.ApiUrl,
allowedGroups: util.SplitString(sec.Key("allowed_groups").String()),
}
@ -185,7 +185,7 @@ func ProvideService(cfg *setting.Cfg) *SocialService {
// Generic - Uses the same scheme as GitHub.
if name == "generic_oauth" {
ss.socialMap["generic_oauth"] = &SocialGenericOAuth{
SocialBase: newSocialBase(name, &config, info, cfg.AutoAssignOrgRole),
SocialBase: newSocialBase(name, &config, info, cfg.AutoAssignOrgRole, cfg.OAuthSkipOrgRoleUpdateSync),
apiUrl: info.ApiUrl,
teamsUrl: info.TeamsUrl,
emailAttributeName: info.EmailAttributeName,
@ -215,7 +215,7 @@ func ProvideService(cfg *setting.Cfg) *SocialService {
ss.socialMap[grafanaCom] = &SocialGrafanaCom{
SocialBase: newSocialBase(name, &config, info,
cfg.AutoAssignOrgRole),
cfg.AutoAssignOrgRole, cfg.OAuthSkipOrgRoleUpdateSync),
url: cfg.GrafanaComURL,
allowedOrganizations: util.SplitString(sec.Key("allowed_organizations").String()),
}
@ -261,6 +261,7 @@ type SocialBase struct {
roleAttributePath string
roleAttributeStrict bool
autoAssignOrgRole string
skipOrgRoleSync bool
}
type Error struct {
@ -294,6 +295,7 @@ func newSocialBase(name string,
config *oauth2.Config,
info *OAuthInfo,
autoAssignOrgRole string,
skipOrgRoleSync bool,
) *SocialBase {
logger := log.New("oauth." + name)
@ -306,6 +308,7 @@ func newSocialBase(name string,
autoAssignOrgRole: autoAssignOrgRole,
roleAttributePath: info.RoleAttributePath,
roleAttributeStrict: info.RoleAttributeStrict,
skipOrgRoleSync: skipOrgRoleSync,
}
}
@ -346,10 +349,10 @@ func (s *SocialBase) defaultRole(legacy bool) org.RoleType {
return org.RoleType(s.autoAssignOrgRole)
}
if legacy {
if legacy && !s.skipOrgRoleSync {
s.log.Warn("No valid role found. Skipping role sync. " +
"In Grafana 10, this will result in the user being assigned the default role and overriding manual assignment. " +
"If role sync is not desired, set oauth_skip_org_role_update_sync to false")
"If role sync is not desired, set oauth_skip_org_role_update_sync to true")
}
return ""