Merge pull request #19997 from enj/enj/f/gitlab_oidc

Update GitLab IDP to support OIDC

Author: openshift-merge-robot

pkg/cmd/server/apis/config/types.go

@@ -1070,6 +1070,14 @@ type GitLabIdentityProvider struct {
 	ClientID string
 	// ClientSecret is the oauth client secret
 	ClientSecret StringSource
+	// Legacy determines if OAuth2 or OIDC should be used
+	// If true, OAuth2 is used
+	// If false, OIDC is used
+	// If nil and the URL's host is gitlab.com, OIDC is used
+	// Otherwise, OAuth2 is used
+	// In a future release, nil will default to using OIDC
+	// Eventually this flag will be removed and only OIDC will be used
+	Legacy *bool
 }
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

pkg/cmd/server/apis/config/v1/types.go

@@ -1006,6 +1006,14 @@ type GitLabIdentityProvider struct {
 	ClientID string `json:"clientID"`
 	// ClientSecret is the oauth client secret
 	ClientSecret StringSource `json:"clientSecret"`
+	// Legacy determines if OAuth2 or OIDC should be used
+	// If true, OAuth2 is used
+	// If false, OIDC is used
+	// If nil and the URL's host is gitlab.com, OIDC is used
+	// Otherwise, OAuth2 is used
+	// In a future release, nil will default to using OIDC
+	// Eventually this flag will be removed and only OIDC will be used
+	Legacy *bool `json:"legacy,omitempty"`
 }
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

pkg/cmd/server/apis/config/v1/zz_generated.deepcopy.go

@@ -0,0 +1,40 @@
+package gitlab
+
+import (
+	"net/http"
+	"net/url"
+	"strings"
+
+	"github.com/openshift/origin/pkg/oauthserver/oauth/external"
+
+	"github.com/golang/glog"
+)
+
+// The hosted version of GitLab is guaranteed to be using the latest stable version
+// meaning that we can count on it having OIDC support (and no sub claim bug)
+const gitlabHostedDomain = "gitlab.com"
+
+func NewProvider(providerName, URL, clientID, clientSecret string, transport http.RoundTripper, legacy *bool) (external.Provider, error) {
+	if isLegacy(legacy, URL) {
+		glog.Infof("Using legacy OAuth2 for GitLab identity provider %s url=%s clientID=%s", providerName, URL, clientID)
+		return NewOAuthProvider(providerName, URL, clientID, clientSecret, transport)
+	}
+	glog.Infof("Using OIDC for GitLab identity provider %s url=%s clientID=%s", providerName, URL, clientID)
+	return NewOIDCProvider(providerName, URL, clientID, clientSecret, transport)
+}
+
+func isLegacy(legacy *bool, URL string) bool {
+	// if a value is specified, honor it
+	if legacy != nil {
+		return *legacy
+	}
+
+	// use OIDC if we know it will work since the hosted version is being used
+	// validation handles URL parsing errors so we can ignore them here
+	if u, err := url.Parse(URL); err == nil && strings.EqualFold(u.Hostname(), gitlabHostedDomain) {
+		return false
+	}
+
+	// otherwise use OAuth2 (to be safe for now)
+	return true
+}

pkg/cmd/server/apis/config/zz_generated.deepcopy.go

@@ -7,7 +7,6 @@ import (
 	"io/ioutil"
 	"net/http"
 	"net/url"
-	"path"
 
 	"github.com/RangelReale/osincli"
 	"github.com/golang/glog"
@@ -21,10 +20,8 @@ const (
 	// and OAuth-Provider (http://doc.gitlab.com/ce/integration/oauth_provider.html)
 	// with default OAuth scope (http://doc.gitlab.com/ce/api/users.html#current-user)
 	// Requires GitLab 7.7.0 or higher
-	gitlabAuthorizePath = "/oauth/authorize"
-	gitlabTokenPath     = "/oauth/token"
-	gitlabUserAPIPath   = "/api/v3/user"
-	gitlabOAuthScope    = "api"
+	gitlabUserAPIPath = "/api/v3/user"
+	gitlabOAuthScope  = "api"
 )
 
 type provider struct {
@@ -44,7 +41,7 @@ type gitlabUser struct {
 	Name     string
 }
 
-func NewProvider(providerName string, transport http.RoundTripper, URL, clientID, clientSecret string) (external.Provider, error) {
+func NewOAuthProvider(providerName, URL, clientID, clientSecret string, transport http.RoundTripper) (external.Provider, error) {
 	// Create service URLs
 	u, err := url.Parse(URL)
 	if err != nil {
@@ -62,11 +59,6 @@ func NewProvider(providerName string, transport http.RoundTripper, URL, clientID
 	}, nil
 }
 
-func appendPath(u url.URL, subpath string) string {
-	u.Path = path.Join(u.Path, subpath)
-	return u.String()
-}
-
 func (p *provider) GetTransport() (http.RoundTripper, error) {
 	return p.transport, nil
 }
@@ -86,8 +78,7 @@ func (p *provider) NewConfig() (*osincli.ClientConfig, error) {
 }
 
 // AddCustomParameters implements external/interfaces/Provider.AddCustomParameters
-func (p *provider) AddCustomParameters(req *osincli.AuthorizeRequest) {
-}
+func (p *provider) AddCustomParameters(req *osincli.AuthorizeRequest) {}
 
 // GetUserIdentity implements external/interfaces/Provider.GetUserIdentity
 func (p *provider) GetUserIdentity(data *osincli.AccessData) (authapi.UserIdentityInfo, bool, error) {

pkg/oauthserver/oauth/external/gitlab/gitlab_mux.go

@@ -8,7 +8,7 @@ import (
 )
 
 func TestGitLab(t *testing.T) {
-	p, err := NewProvider("gitlab", nil, "https://gitlab.com/", "clientid", "clientsecret")
+	p, err := NewOAuthProvider("gitlab", "https://gitlab.com/", "clientid", "clientsecret", nil)
 	if err != nil {
 		t.Fatalf("Unexpected error: %v", err)
 	}

pkg/oauthserver/oauth/external/gitlab/gitlab.go renamed to pkg/oauthserver/oauth/external/gitlab/gitlab_oauth.go

@@ -0,0 +1,97 @@
+package gitlab
+
+import (
+	"fmt"
+	"net/http"
+	"net/url"
+	"path"
+	"regexp"
+	"strconv"
+
+	"github.com/openshift/origin/pkg/oauthserver/oauth/external"
+	"github.com/openshift/origin/pkg/oauthserver/oauth/external/openid"
+)
+
+const (
+	// https://gitlab.com/help/integration/openid_connect_provider.md
+	// Uses GitLab OIDC, requires GitLab 11.1.0 or higher
+	// Earlier versions do not work: https://gitlab.com/gitlab-org/gitlab-ce/issues/47791#note_81269161
+	gitlabAuthorizePath = "/oauth/authorize"
+	gitlabTokenPath     = "/oauth/token"
+	gitlabUserInfoPath  = "/oauth/userinfo"
+
+	// https://gitlab.com/gitlab-org/gitlab-ce/blob/master/config/locales/doorkeeper.en.yml
+	// Authenticate using OpenID Connect
+	// The ability to authenticate using GitLab, and read-only access to the user's profile information and group memberships
+	gitlabOIDCScope = "openid"
+
+	// The ID of the user
+	// See above comment about GitLab 11.1.0 and the custom IDTokenValidator below
+	// Along with providerName, builds the identity object's Name field (see Identity.ProviderUserName)
+	gitlabIDClaim = "sub"
+	// The user's GitLab username
+	// Used as the Name field of the user object (stored in Identity.Extra, see IdentityPreferredUsernameKey)
+	gitlabPreferredUsernameClaim = "nickname"
+	// The user's public email address
+	// The value can optionally be used during manual provisioning (stored in Identity.Extra, see IdentityEmailKey)
+	gitlabEmailClaim = "email"
+	// The user's full name
+	// Used as the FullName field of the user object (stored in Identity.Extra, see IdentityDisplayNameKey)
+	gitlabDisplayNameClaim = "name"
+)
+
+func NewOIDCProvider(providerName, URL, clientID, clientSecret string, transport http.RoundTripper) (external.Provider, error) {
+	// Create service URLs
+	u, err := url.Parse(URL)
+	if err != nil {
+		return nil, fmt.Errorf("gitlab host URL %q is invalid", URL)
+	}
+
+	config := openid.Config{
+		ClientID:     clientID,
+		ClientSecret: clientSecret,
+
+		AuthorizeURL: appendPath(*u, gitlabAuthorizePath),
+		TokenURL:     appendPath(*u, gitlabTokenPath),
+		UserInfoURL:  appendPath(*u, gitlabUserInfoPath),
+
+		Scopes: []string{gitlabOIDCScope},
+
+		IDClaims:                []string{gitlabIDClaim},
+		PreferredUsernameClaims: []string{gitlabPreferredUsernameClaim},
+		EmailClaims:             []string{gitlabEmailClaim},
+		NameClaims:              []string{gitlabDisplayNameClaim},
+
+		// make sure that gitlabIDClaim is a valid uint64, see above comment about GitLab 11.1.0
+		IDTokenValidator: func(idTokenClaims map[string]interface{}) error {
+			gitlabID, ok := idTokenClaims[gitlabIDClaim].(string)
+			if !ok {
+				return nil // this is an OIDC spec violation which is handled by the default code path
+			}
+			if reSHA256HexDigest.MatchString(gitlabID) {
+				return fmt.Errorf("incompatible gitlab IDP, ID claim is SHA256 hex digest instead of digit, claims=%#v", idTokenClaims)
+			}
+			if !isValidUint64(gitlabID) {
+				return fmt.Errorf("invalid gitlab IDP, ID claim is not a digit, claims=%#v", idTokenClaims)
+			}
+			return nil
+		},
+	}
+
+	return openid.NewProvider(providerName, transport, config)
+}
+
+func appendPath(u url.URL, subpath string) string {
+	u.Path = path.Join(u.Path, subpath)
+	return u.String()
+}
+
+// Have 256 bits from hex digest
+// In hexadecimal each digit encodes 4 bits
+// Thus we need 64 digits to represent 256 bits
+var reSHA256HexDigest = regexp.MustCompile(`^[[:xdigit:]]{64}$`)
+
+func isValidUint64(s string) bool {
+	_, err := strconv.ParseUint(s, 10, 64)
+	return err == nil
+}

pkg/oauthserver/oauth/external/gitlab/gitlab_test.go renamed to pkg/oauthserver/oauth/external/gitlab/gitlab_oauth_test.go

@@ -484,7 +484,7 @@ func (c *OAuthServerConfig) getOAuthProvider(identityProvider configapi.Identity
 		if err != nil {
 			return nil, err
 		}
-		return gitlab.NewProvider(identityProvider.Name, transport, provider.URL, provider.ClientID, clientSecret)
+		return gitlab.NewProvider(identityProvider.Name, provider.URL, provider.ClientID, clientSecret, transport, provider.Legacy)
 
 	case (*configapi.GoogleIdentityProvider):
 		clientSecret, err := configapi.ResolveStringValue(provider.ClientSecret)