From ec71e444fec90ad8c1e3e20d87ee21633f9c5493 Mon Sep 17 00:00:00 2001 From: Justin Hines Date: Wed, 19 Jun 2019 11:58:56 -0400 Subject: [PATCH 1/2] auth: update google endpoint versions --- internal/auth/authenticator_test.go | 4 +-- internal/auth/providers/google.go | 47 ++++++++++++++------------ internal/auth/providers/google_test.go | 28 +++++++-------- 3 files changed, 42 insertions(+), 37 deletions(-) diff --git a/internal/auth/authenticator_test.go b/internal/auth/authenticator_test.go index df2fafc7..75054a67 100644 --- a/internal/auth/authenticator_test.go +++ b/internal/auth/authenticator_test.go @@ -1608,9 +1608,9 @@ func TestGoogleProviderApiSettings(t *testing.T) { t.Fatalf("unexpected err generating google provider: %v", err) } p := provider.Data() - testutil.Equal(t, "https://accounts.google.com/o/oauth2/auth?access_type=offline", + testutil.Equal(t, "https://accounts.google.com/o/oauth2/v2/auth", p.SignInURL.String()) - testutil.Equal(t, "https://www.googleapis.com/oauth2/v3/token", + testutil.Equal(t, "https://www.googleapis.com/oauth2/v4/token", p.RedeemURL.String()) testutil.Equal(t, "", p.ProfileURL.String()) diff --git a/internal/auth/providers/google.go b/internal/auth/providers/google.go index f8842416..1150501f 100644 --- a/internal/auth/providers/google.go +++ b/internal/auth/providers/google.go @@ -35,13 +35,11 @@ func NewGoogleProvider(p *ProviderData, impersonateUser, credsFilePath string) ( p.ProviderName = "Google" p.SignInURL = &url.URL{Scheme: "https", Host: "accounts.google.com", - Path: "/o/oauth2/auth", - // to get a refresh token. see https://developers.google.com/identity/protocols/OAuth2WebServer#offline - RawQuery: "access_type=offline", + Path: "/o/oauth2/v2/auth", } p.RedeemURL = &url.URL{Scheme: "https", Host: "www.googleapis.com", - Path: "/oauth2/v3/token", + Path: "/oauth2/v4/token", } p.RevokeURL = &url.URL{Scheme: "https", Host: "accounts.google.com", @@ -51,9 +49,13 @@ func NewGoogleProvider(p *ProviderData, impersonateUser, credsFilePath string) ( Host: "www.googleapis.com", Path: "/oauth2/v3/tokeninfo", } + if p.Scope == "" { p.Scope = "profile email" } + if p.ApprovalPrompt == "" { + p.ApprovalPrompt = "consent" + } // not used for google p.ProfileURL = &url.URL{} @@ -113,16 +115,14 @@ func (p *GoogleProvider) ValidateSessionState(s *sessions.SessionState) bool { return false } - var endpoint url.URL - endpoint = *p.ValidateURL - q := endpoint.Query() - q.Add("access_token", s.AccessToken) - endpoint.RawQuery = q.Encode() + params := url.Values{} + params.Set("access_token", s.AccessToken) - err := p.googleRequest("POST", endpoint.String(), nil, []string{"action:validate"}, nil) + err := p.googleRequest("POST", p.ValidateURL.String(), params, []string{"action:validate"}, nil) if err != nil { return false } + return true } @@ -130,13 +130,16 @@ func (p *GoogleProvider) ValidateSessionState(s *sessions.SessionState) bool { func (p *GoogleProvider) GetSignInURL(redirectURI, state string) string { var a url.URL a = *p.SignInURL - params, _ := url.ParseQuery(a.RawQuery) - params.Set("redirect_uri", redirectURI) - params.Set("approval_prompt", p.ApprovalPrompt) - params.Add("scope", p.Scope) + + params := url.Values{} params.Set("client_id", p.ClientID) params.Set("response_type", "code") + params.Set("redirect_uri", redirectURI) + params.Set("scope", p.Scope) + params.Set("access_token", "offline") params.Add("state", state) + params.Set("prompt", p.ApprovalPrompt) + a.RawQuery = params.Encode() return a.String() } @@ -287,11 +290,12 @@ func (p *GoogleProvider) Redeem(redirectURL, code string) (*sessions.SessionStat if code == "" { return nil, ErrBadRequest } + params := url.Values{} - params.Add("redirect_uri", redirectURL) + params.Add("code", code) params.Add("client_id", p.ClientID) params.Add("client_secret", p.ClientSecret) - params.Add("code", code) + params.Add("redirect_uri", redirectURL) params.Add("grant_type", "authorization_code") var response struct { @@ -311,6 +315,7 @@ func (p *GoogleProvider) Redeem(redirectURL, code string) (*sessions.SessionStat if err != nil { return nil, err } + return &sessions.SessionState{ AccessToken: response.AccessToken, RefreshToken: response.RefreshToken, @@ -398,10 +403,10 @@ func (p *GoogleProvider) RefreshAccessToken(refreshToken string) (token string, // https://developers.google.com/identity/protocols/OAuth2WebServer#refresh params := url.Values{} - params.Add("client_id", p.ClientID) - params.Add("client_secret", p.ClientSecret) - params.Add("refresh_token", refreshToken) - params.Add("grant_type", "refresh_token") + params.Set("client_id", p.ClientID) + params.Set("client_secret", p.ClientSecret) + params.Set("refresh_token", refreshToken) + params.Set("grant_type", "refresh_token") var response struct { AccessToken string `json:"access_token"` @@ -421,7 +426,7 @@ func (p *GoogleProvider) RefreshAccessToken(refreshToken string) (token string, // Revoke revokes the access token a given session state. func (p *GoogleProvider) Revoke(s *sessions.SessionState) error { params := url.Values{} - params.Add("token", s.AccessToken) + params.Set("token", s.AccessToken) err := p.googleRequest("GET", p.RevokeURL.String(), params, []string{"action:revoke"}, nil) diff --git a/internal/auth/providers/google_test.go b/internal/auth/providers/google_test.go index d57dd973..044cf21d 100644 --- a/internal/auth/providers/google_test.go +++ b/internal/auth/providers/google_test.go @@ -54,8 +54,8 @@ func TestGoogleProviderDefaults(t *testing.T) { }{ { name: "defaults", - signInURL: "https://accounts.google.com/o/oauth2/auth?access_type=offline", - redeemURL: "https://www.googleapis.com/oauth2/v3/token", + signInURL: "https://accounts.google.com/o/oauth2/v2/auth", + redeemURL: "https://www.googleapis.com/oauth2/v4/token", revokeURL: "https://accounts.google.com/o/oauth2/revoke", validateURL: "https://www.googleapis.com/oauth2/v3/tokeninfo", scope: "profile email", @@ -71,38 +71,38 @@ func TestGoogleProviderDefaults(t *testing.T) { t.Errorf("expected provider name Google, got %s", p.Data().ProviderName) } if p.Data().SignInURL.String() != expected.signInURL { - log.Printf("expected %s", expected.signInURL) - log.Printf("got %s", p.Data().SignInURL.String()) + t.Errorf("want: %v", expected.signInURL) + t.Errorf("have: %v", p.Data().SignInURL.String()) t.Errorf("unexpected signin url") } if p.Data().RedeemURL.String() != expected.redeemURL { - log.Printf("expected %s", expected.redeemURL) - log.Printf("got %s", p.Data().RedeemURL.String()) + t.Errorf("want: %v", expected.redeemURL) + t.Errorf("have: %v", p.Data().RedeemURL.String()) t.Errorf("unexpected redeem url") } if p.Data().RevokeURL.String() != expected.revokeURL { - log.Printf("expected %s", expected.revokeURL) - log.Printf("got %s", p.Data().RevokeURL.String()) + t.Errorf("want: %v", expected.revokeURL) + t.Errorf("have: %v", p.Data().RevokeURL.String()) t.Errorf("unexpected revoke url") } if p.Data().ValidateURL.String() != expected.validateURL { - log.Printf("expected %s", expected.validateURL) - log.Printf("got %s", p.Data().ValidateURL.String()) + t.Errorf("want: %v", expected.validateURL) + t.Errorf("have: %v", p.Data().ValidateURL.String()) t.Errorf("unexpected validate url") } if p.Data().ProfileURL.String() != expected.profileURL { - log.Printf("expected %s", expected.profileURL) - log.Printf("got %s", p.Data().ProfileURL.String()) + t.Errorf("want: %v", expected.profileURL) + t.Errorf("have: %v", p.Data().ProfileURL.String()) t.Errorf("unexpected profile url") } if p.Data().Scope != expected.scope { - log.Printf("expected %s", expected.scope) - log.Printf("got %s", p.Data().Scope) + t.Errorf("want: %v", expected.scope) + t.Errorf("have: %v", p.Data().Scope) t.Errorf("unexpected scope") } }) From 14f2a962d995bbd4b69da4420df5fe53dc3468c2 Mon Sep 17 00:00:00 2001 From: Justin Hines Date: Wed, 19 Jun 2019 13:45:10 -0400 Subject: [PATCH 2/2] sso: update google rfs --- internal/auth/authenticator_test.go | 2 +- internal/auth/configuration.go | 3 ++ internal/auth/options.go | 18 ++++------- internal/auth/providers/google.go | 34 +++++++++++++-------- internal/auth/providers/google_test.go | 2 +- internal/auth/providers/provider_data.go | 25 ++++++++------- internal/auth/providers/provider_default.go | 1 - 7 files changed, 47 insertions(+), 38 deletions(-) diff --git a/internal/auth/authenticator_test.go b/internal/auth/authenticator_test.go index 75054a67..e07af141 100644 --- a/internal/auth/authenticator_test.go +++ b/internal/auth/authenticator_test.go @@ -1630,5 +1630,5 @@ func TestGoogleGroupInvalidFile(t *testing.T) { }, ) testutil.NotEqual(t, nil, err) - testutil.Equal(t, "invalid Google credentials file: file_doesnt_exist.json", err.Error()) + testutil.Equal(t, "could not read google credentials file", err.Error()) } diff --git a/internal/auth/configuration.go b/internal/auth/configuration.go index 4dcdf050..c36334e5 100644 --- a/internal/auth/configuration.go +++ b/internal/auth/configuration.go @@ -37,6 +37,8 @@ import ( // // PROVIDER_*_GOOGLE_CREDENTIALS // PROVIDER_*_GOOGLE_IMPERSONATE +// PROVIDER_*_GOOGLE_PROMPT +// PROVIDER_*_GOOGLE_DOMAIN // // PROVIDER_*_OKTA_URL // PROVIDER_*_OKTA_SERVER @@ -227,6 +229,7 @@ type GoogleProviderConfig struct { Credentials string `mapstructure:"credentials"` Impersonate string `mapstructure:"impersonate"` ApprovalPrompt string `mapstructure:"prompt"` + HostedDomain string `mapstructure:"domain"` } func (gpc GoogleProviderConfig) Validate() error { diff --git a/internal/auth/options.go b/internal/auth/options.go index 693bba8c..5d15130e 100644 --- a/internal/auth/options.go +++ b/internal/auth/options.go @@ -4,7 +4,6 @@ import ( "encoding/base64" "fmt" "net/url" - "os" "path" "github.com/buzzfeed/sso/internal/auth/providers" @@ -28,16 +27,12 @@ func newProvider(pc ProviderConfig, sc SessionConfig) (providers.Provider, error switch pc.ProviderType { case providers.GoogleProviderName: // Google gpc := pc.GoogleProviderConfig - p.ApprovalPrompt = gpc.ApprovalPrompt - - if gpc.Credentials != "" { - _, err := os.Open(gpc.Credentials) - if err != nil { - return nil, fmt.Errorf("invalid Google credentials file: %s", gpc.Credentials) - } - } - - googleProvider, err := providers.NewGoogleProvider(p, gpc.Impersonate, gpc.Credentials) + googleProvider, err := providers.NewGoogleProvider(p, + gpc.ApprovalPrompt, + gpc.HostedDomain, + gpc.Impersonate, + gpc.Credentials, + ) if err != nil { return nil, err } @@ -48,7 +43,6 @@ func newProvider(pc ProviderConfig, sc SessionConfig) (providers.Provider, error singleFlightProvider = providers.NewSingleFlightProvider(googleProvider) case providers.OktaProviderName: opc := pc.OktaProviderConfig - oktaProvider, err := providers.NewOktaProvider(p, opc.OrgURL, opc.ServerID) if err != nil { return nil, err diff --git a/internal/auth/providers/google.go b/internal/auth/providers/google.go index 1150501f..9a8cf5fa 100644 --- a/internal/auth/providers/google.go +++ b/internal/auth/providers/google.go @@ -28,10 +28,13 @@ type GoogleProvider struct { AdminService AdminService cb *circuit.Breaker GroupsCache groups.MemberSetCache + + Prompt string + HostedDomain string } // NewGoogleProvider returns a new GoogleProvider and sets the provider url endpoints. -func NewGoogleProvider(p *ProviderData, impersonateUser, credsFilePath string) (*GoogleProvider, error) { +func NewGoogleProvider(p *ProviderData, prompt, hd, impersonate, credentials string) (*GoogleProvider, error) { p.ProviderName = "Google" p.SignInURL = &url.URL{Scheme: "https", Host: "accounts.google.com", @@ -49,19 +52,20 @@ func NewGoogleProvider(p *ProviderData, impersonateUser, credsFilePath string) ( Host: "www.googleapis.com", Path: "/oauth2/v3/tokeninfo", } + p.ProfileURL = &url.URL{} if p.Scope == "" { p.Scope = "profile email" } - if p.ApprovalPrompt == "" { - p.ApprovalPrompt = "consent" - } - // not used for google - p.ProfileURL = &url.URL{} + if prompt == "" { + prompt = "consent" + } googleProvider := &GoogleProvider{ ProviderData: p, + Prompt: prompt, + HostedDomain: hd, } googleProvider.cb = circuit.NewBreaker(&circuit.Options{ @@ -74,17 +78,19 @@ func NewGoogleProvider(p *ProviderData, impersonateUser, credsFilePath string) ( time.Duration(200)*time.Second, time.Duration(500)*time.Millisecond, ), }) - if credsFilePath != "" { - credsReader, err := os.Open(credsFilePath) + + if credentials != "" { + credsReader, err := os.Open(credentials) if err != nil { return nil, errors.New("could not read google credentials file") } googleProvider.AdminService = &GoogleAdminService{ - adminService: getAdminService(impersonateUser, credsReader), + adminService: getAdminService(impersonate, credsReader), cb: googleProvider.cb, } } + return googleProvider, nil } @@ -136,9 +142,13 @@ func (p *GoogleProvider) GetSignInURL(redirectURI, state string) string { params.Set("response_type", "code") params.Set("redirect_uri", redirectURI) params.Set("scope", p.Scope) - params.Set("access_token", "offline") - params.Add("state", state) - params.Set("prompt", p.ApprovalPrompt) + params.Set("access_type", "offline") + params.Set("state", state) + params.Set("prompt", p.Prompt) + + if p.HostedDomain != "" { + params.Set("hd", p.HostedDomain) + } a.RawQuery = params.Encode() return a.String() diff --git a/internal/auth/providers/google_test.go b/internal/auth/providers/google_test.go index 044cf21d..596a1ac2 100644 --- a/internal/auth/providers/google_test.go +++ b/internal/auth/providers/google_test.go @@ -37,7 +37,7 @@ func newGoogleProvider(providerData *ProviderData) *GoogleProvider { ValidateURL: &url.URL{}, Scope: ""} } - provider, _ := NewGoogleProvider(providerData, "", "") + provider, _ := NewGoogleProvider(providerData, "", "", "", "") return provider } diff --git a/internal/auth/providers/provider_data.go b/internal/auth/providers/provider_data.go index 707c2867..75601bc9 100644 --- a/internal/auth/providers/provider_data.go +++ b/internal/auth/providers/provider_data.go @@ -8,17 +8,20 @@ import ( // ProviderData holds the fields associated with providers // necessary to implement the Provider interface. type ProviderData struct { - ProviderName string - ProviderSlug string - ClientID string - ClientSecret string - SignInURL *url.URL - RedeemURL *url.URL - RevokeURL *url.URL - ProfileURL *url.URL - ValidateURL *url.URL - Scope string - ApprovalPrompt string + ProviderName string + ProviderSlug string + + ClientID string + ClientSecret string + + SignInURL *url.URL + RedeemURL *url.URL + RevokeURL *url.URL + ValidateURL *url.URL + ProfileURL *url.URL + + Scope string + SessionLifetimeTTL time.Duration } diff --git a/internal/auth/providers/provider_default.go b/internal/auth/providers/provider_default.go index 83fb899e..054ce77d 100644 --- a/internal/auth/providers/provider_default.go +++ b/internal/auth/providers/provider_default.go @@ -100,7 +100,6 @@ func (p *ProviderData) GetSignInURL(redirectURI, state string) string { a = *p.SignInURL params, _ := url.ParseQuery(a.RawQuery) params.Set("redirect_uri", redirectURI) - params.Set("approval_prompt", p.ApprovalPrompt) params.Add("scope", p.Scope) params.Set("client_id", p.ClientID) params.Set("response_type", "code")