Update GitLab IDP to use OIDC instead of OAuth2

This change updates the GitLab IDP integration to use OpenID Connect instead of OAuth2. This has the benefit of using a standard that was meant for authentication, which allows us to remove the GitLab specific code from our integration. We can simply reuse the OIDC code with a GitLab specific configuration. From the perspective of the end user, nothing changes. The configuration of the IDP remains exactly the same and the identity/user objects are unchanged. The only difference is that a very recent version of GitLab is required: GitLab version 11.1.0 or later. Signed-off-by: Monis Khan <[email protected]>
openshift · Jun 13, 2018 · e57b69e · e57b69e
1 parent 4839513
commit e57b69e
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 135 deletions.
diff --git a/pkg/oauthserver/oauth/external/gitlab/gitlab.go b/pkg/oauthserver/oauth/external/gitlab/gitlab.go
@@ -1,135 +1,69 @@
 package gitlab
 
 import (
-	"encoding/json"
 	"errors"
-	"fmt"
-	"io/ioutil"
 	"net/http"
 	"net/url"
 	"path"
 
-	"github.com/RangelReale/osincli"
-	"github.com/golang/glog"
-
-	authapi "github.com/openshift/origin/pkg/oauthserver/api"
 	"github.com/openshift/origin/pkg/oauthserver/oauth/external"
+	"github.com/openshift/origin/pkg/oauthserver/oauth/external/openid"
 )
 
 const (
-	// Uses the GitLab User-API (http://doc.gitlab.com/ce/api/users.html#current-user)
-	// 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
+	// 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"
-	gitlabUserAPIPath   = "/api/v3/user"
-	gitlabOAuthScope    = "api"
+	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"
+
+	// An opaque token that uniquely identifies the user
+	// 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"
 )
 
-type provider struct {
-	providerName string
-	transport    http.RoundTripper
-	authorizeURL string
-	tokenURL     string
-	userAPIURL   string
-	clientID     string
-	clientSecret string
-}
-
-type gitlabUser struct {
-	ID       uint64
-	Username string
-	Email    string
-	Name     string
-}
-
-func NewProvider(providerName string, transport http.RoundTripper, URL, clientID, clientSecret string) (external.Provider, error) {
+func NewProvider(providerName, URL, clientID, clientSecret string, transport http.RoundTripper) (external.Provider, error) {
 	// Create service URLs
 	u, err := url.Parse(URL)
 	if err != nil {
-		return nil, errors.New("Host URL is invalid")
+		return nil, errors.New("gitlab host URL is invalid")
 	}
 
-	return &provider{
-		providerName: providerName,
-		transport:    transport,
-		authorizeURL: appendPath(*u, gitlabAuthorizePath),
-		tokenURL:     appendPath(*u, gitlabTokenPath),
-		userAPIURL:   appendPath(*u, gitlabUserAPIPath),
-		clientID:     clientID,
-		clientSecret: clientSecret,
-	}, nil
-}
+	config := openid.Config{
+		ClientID:     clientID,
+		ClientSecret: clientSecret,
 
-func appendPath(u url.URL, subpath string) string {
-	u.Path = path.Join(u.Path, subpath)
-	return u.String()
-}
+		AuthorizeURL: appendPath(*u, gitlabAuthorizePath),
+		TokenURL:     appendPath(*u, gitlabTokenPath),
+		UserInfoURL:  appendPath(*u, gitlabUserInfoPath),
 
-func (p *provider) GetTransport() (http.RoundTripper, error) {
-	return p.transport, nil
-}
+		Scopes: []string{gitlabOIDCScope},
 
-// NewConfig implements external/interfaces/Provider.NewConfig
-func (p *provider) NewConfig() (*osincli.ClientConfig, error) {
-	config := &osincli.ClientConfig{
-		ClientId:                 p.clientID,
-		ClientSecret:             p.clientSecret,
-		ErrorsInStatusCode:       true,
-		SendClientSecretInParams: true,
-		AuthorizeUrl:             p.authorizeURL,
-		TokenUrl:                 p.tokenURL,
-		Scope:                    gitlabOAuthScope,
+		IDClaims:                []string{gitlabIDClaim},
+		PreferredUsernameClaims: []string{gitlabPreferredUsernameClaim},
+		EmailClaims:             []string{gitlabEmailClaim},
+		NameClaims:              []string{gitlabDisplayNameClaim},
 	}
-	return config, nil
-}
 
-// AddCustomParameters implements external/interfaces/Provider.AddCustomParameters
-func (p *provider) AddCustomParameters(req *osincli.AuthorizeRequest) {
+	return openid.NewProvider(providerName, transport, config)
 }
 
-// GetUserIdentity implements external/interfaces/Provider.GetUserIdentity
-func (p *provider) GetUserIdentity(data *osincli.AccessData) (authapi.UserIdentityInfo, bool, error) {
-	req, _ := http.NewRequest("GET", p.userAPIURL, nil)
-	req.Header.Set("Authorization", fmt.Sprintf("bearer %s", data.AccessToken))
-
-	client := http.DefaultClient
-	if p.transport != nil {
-		client = &http.Client{Transport: p.transport}
-	}
-	res, err := client.Do(req)
-	if err != nil {
-		return nil, false, err
-	}
-	defer res.Body.Close()
-
-	body, err := ioutil.ReadAll(res.Body)
-	if err != nil {
-		return nil, false, err
-	}
-
-	userdata := gitlabUser{}
-	err = json.Unmarshal(body, &userdata)
-	if err != nil {
-		return nil, false, err
-	}
-
-	if userdata.ID == 0 {
-		return nil, false, errors.New("Could not retrieve GitLab id")
-	}
-
-	identity := authapi.NewDefaultUserIdentityInfo(p.providerName, fmt.Sprintf("%d", userdata.ID))
-	if len(userdata.Name) > 0 {
-		identity.Extra[authapi.IdentityDisplayNameKey] = userdata.Name
-	}
-	if len(userdata.Username) > 0 {
-		identity.Extra[authapi.IdentityPreferredUsernameKey] = userdata.Username
-	}
-	if len(userdata.Email) > 0 {
-		identity.Extra[authapi.IdentityEmailKey] = userdata.Email
-	}
-	glog.V(4).Infof("Got identity=%#v", identity)
-
-	return identity, true, nil
+func appendPath(u url.URL, subpath string) string {
+	u.Path = path.Join(u.Path, subpath)
+	return u.String()
 }
diff --git a/pkg/oauthserver/oauth/external/gitlab/gitlab_test.go b/pkg/oauthserver/oauth/external/gitlab/gitlab_test.go
diff --git a/pkg/oauthserver/oauthserver/auth.go b/pkg/oauthserver/oauthserver/auth.go
@@ -481,7 +481,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)
 
 	case (*configapi.GoogleIdentityProvider):
 		clientSecret, err := configapi.ResolveStringValue(provider.ClientSecret)