Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update GitLab IDP to support OIDC #19997

Merged
merged 2 commits into from
Jul 6, 2018
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
8 changes: 8 additions & 0 deletions pkg/cmd/server/apis/config/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -1064,6 +1064,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
Expand Down
8 changes: 8 additions & 0 deletions pkg/cmd/server/apis/config/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -1000,6 +1000,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
Expand Down
9 changes: 9 additions & 0 deletions pkg/cmd/server/apis/config/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions pkg/cmd/server/apis/config/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

40 changes: 40 additions & 0 deletions pkg/oauthserver/oauth/external/gitlab/gitlab_mux.go
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"io/ioutil"
"net/http"
"net/url"
"path"

"github.com/RangelReale/osincli"
"github.com/golang/glog"
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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
}
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
97 changes: 97 additions & 0 deletions pkg/oauthserver/oauth/external/gitlab/gitlab_oidc.go
Original file line number Diff line number Diff line change
@@ -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"
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a good reason to prepend all these module variables with "gitlab" ?

Copy link
Contributor Author

@enj enj Jul 3, 2018

Choose a reason for hiding this comment

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

Matches the existing code and the other provider integrations.

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
}
2 changes: 1 addition & 1 deletion pkg/oauthserver/oauthserver/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,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)
Expand Down