Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 28 additions & 6 deletions pkg/auth/oauth/external/google/google.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package google

import (
"errors"
"fmt"

"github.com/openshift/origin/pkg/auth/oauth/external"
"github.com/openshift/origin/pkg/auth/oauth/external/openid"
)
Expand All @@ -9,15 +12,14 @@ const (
googleAuthorizeURL = "https://accounts.google.com/o/oauth2/auth"
googleTokenURL = "https://www.googleapis.com/oauth2/v3/token"
googleUserInfoURL = "https://www.googleapis.com/oauth2/v3/userinfo"

// https://developers.google.com/identity/protocols/OpenIDConnect#hd-param
googleHostedDomain = "hd"
)

var googleOAuthScopes = []string{"openid", "email", "profile"}

type provider struct {
providerName, clientID, clientSecret string
}

func NewProvider(providerName, clientID, clientSecret string) (external.Provider, error) {
func NewProvider(providerName, clientID, clientSecret, hostedDomain string) (external.Provider, error) {
config := openid.Config{
ClientID: clientID,
ClientSecret: clientSecret,
Expand All @@ -29,9 +31,29 @@ func NewProvider(providerName, clientID, clientSecret string) (external.Provider
Scopes: googleOAuthScopes,

IDClaims: []string{"sub"},
PreferredUsernameClaims: []string{"email"},
PreferredUsernameClaims: []string{"preferred_username", "email"},
EmailClaims: []string{"email"},
NameClaims: []string{"name", "email"},
}

if len(hostedDomain) > 0 {
// Request a specific hosted domain during authorization
config.ExtraAuthorizeParameters = map[string]string{
googleHostedDomain: hostedDomain,
}

// Validate the returned id_token is from that hosted domain
config.IDTokenValidator = func(idToken map[string]interface{}) error {
hdClaim, ok := idToken[googleHostedDomain].(string)
if !ok {
return errors.New("id_token did not contain a hd claim")
}
if hdClaim != hostedDomain {
return fmt.Errorf("id_token hd claim (%s) did not match hostedDomain (%s)", hdClaim, hostedDomain)
}
return nil
}
}

return openid.NewProvider(providerName, nil, config)
}
2 changes: 1 addition & 1 deletion pkg/auth/oauth/external/google/google_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
)

func TestGoogle(t *testing.T) {
p, err := NewProvider("google", "clientid", "clientsecret")
p, err := NewProvider("google", "clientid", "clientsecret", "")
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
Expand Down
15 changes: 15 additions & 0 deletions pkg/auth/oauth/external/openid/openid.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,16 @@ const (
NameClaim = "name"
)

type TokenValidator func(map[string]interface{}) error

type Config struct {
ClientID string
ClientSecret string

Scopes []string

ExtraAuthorizeParameters map[string]string

AuthorizeURL string
TokenURL string
UserInfoURL string
Expand All @@ -40,6 +44,8 @@ type Config struct {
PreferredUsernameClaims []string
EmailClaims []string
NameClaims []string

IDTokenValidator TokenValidator
}

type provider struct {
Expand Down Expand Up @@ -121,6 +127,9 @@ func (p provider) GetTransport() (http.RoundTripper, error) {

// AddCustomParameters implements external/interfaces/Provider.AddCustomParameters
func (p provider) AddCustomParameters(req *osincli.AuthorizeRequest) {
for k, v := range p.ExtraAuthorizeParameters {
req.CustomParameters[k] = v
}
}

// GetUserIdentity implements external/interfaces/Provider.GetUserIdentity
Expand All @@ -138,6 +147,12 @@ func (p provider) GetUserIdentity(data *osincli.AccessData) (authapi.UserIdentit
return nil, false, err
}

if p.IDTokenValidator != nil {
if err := p.IDTokenValidator(idTokenClaims); err != nil {
return nil, false, err
}
}

// TODO: validate JWT
// http://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation

Expand Down
6 changes: 6 additions & 0 deletions pkg/cmd/server/api/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,9 @@ type GoogleIdentityProvider struct {
ClientID string
// ClientSecret is the oauth client secret
ClientSecret string

// HostedDomain is the optional Google App domain (e.g. "mycompany.com") to restrict logins to
HostedDomain string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the format well known enough to validate well or would we be risking cutting off valid cases we don't anticipate?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find a description of the format, so I didn't want to assume standard subdomain format

}

type OpenIDIdentityProvider struct {
Expand All @@ -301,6 +304,9 @@ type OpenIDIdentityProvider struct {
// ExtraScopes are any scopes to request in addition to the standard "openid" scope.
ExtraScopes []string

// ExtraAuthorizeParameters are any custom parameters to add to the authorize request.
ExtraAuthorizeParameters map[string]string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What utility does this have for someone writing config? I don't remember us stuffing all the claims into .Extra or anything, though I'm not necessarily opposed. I would worry about staleness though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nm, parameters, not claims.


// URLs to use to authenticate
URLs OpenIDURLs

Expand Down
6 changes: 6 additions & 0 deletions pkg/cmd/server/api/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,9 @@ type GoogleIdentityProvider struct {
ClientID string `json:"clientID"`
// ClientSecret is the oauth client secret
ClientSecret string `json:"clientSecret"`

// HostedDomain is the optional Google App domain (e.g. "mycompany.com") to restrict logins to
HostedDomain string `json:"hostedDomain"`
}

type OpenIDIdentityProvider struct {
Expand All @@ -292,6 +295,9 @@ type OpenIDIdentityProvider struct {
// ExtraScopes are any scopes to request in addition to the standard "openid" scope.
ExtraScopes []string `json:"extraScopes"`

// ExtraAuthorizeParameters are any custom parameters to add to the authorize request.
ExtraAuthorizeParameters map[string]string `json:"extraAuthorizeParameters"`

// URLs to use to authenticate
URLs OpenIDURLs `json:"urls"`

Expand Down
4 changes: 3 additions & 1 deletion pkg/cmd/server/origin/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ func (c *AuthConfig) getOAuthProvider(identityProvider configapi.IdentityProvide
return github.NewProvider(identityProvider.Name, provider.ClientID, provider.ClientSecret), nil

case (*configapi.GoogleIdentityProvider):
return google.NewProvider(identityProvider.Name, provider.ClientID, provider.ClientSecret)
return google.NewProvider(identityProvider.Name, provider.ClientID, provider.ClientSecret, provider.HostedDomain)

case (*configapi.OpenIDIdentityProvider):
transport, err := cmdutil.TransportFor(provider.CA, "", "")
Expand All @@ -410,6 +410,8 @@ func (c *AuthConfig) getOAuthProvider(identityProvider configapi.IdentityProvide

Scopes: scopes.List(),

ExtraAuthorizeParameters: provider.ExtraAuthorizeParameters,

AuthorizeURL: provider.URLs.Authorize,
TokenURL: provider.URLs.Token,
UserInfoURL: provider.URLs.UserInfo,
Expand Down