Merge pull request #18969 from simo5/noauthsrv

Add option to configure an external OAuth server

Author: openshift-merge-robot

pkg/authorization/authorizer/scope/converter.go

@@ -150,6 +150,19 @@ func DefaultSupportedScopesMap() map[string]string {
 	return defaultSupportedScopesMap
 }
 
+func DescribeScopes(scopes []string) map[string]string {
+	ret := map[string]string{}
+	for _, s := range scopes {
+		val, ok := defaultSupportedScopesMap[s]
+		if ok {
+			ret[s] = val
+		} else {
+			ret[s] = ""
+		}
+	}
+	return ret
+}
+
 // user:<scope name>
 type userEvaluator struct{}
 

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

@@ -167,6 +167,9 @@ func GetMasterFileReferences(config *MasterConfig) []*string {
 	for k := range config.AuthConfig.WebhookTokenAuthenticators {
 		refs = append(refs, &config.AuthConfig.WebhookTokenAuthenticators[k].ConfigFile)
 	}
+	if len(config.AuthConfig.OAuthMetadataFile) > 0 {
+		refs = append(refs, &config.AuthConfig.OAuthMetadataFile)
+	}
 
 	refs = append(refs, &config.AggregatorConfig.ProxyClientInfo.CertFile)
 	refs = append(refs, &config.AggregatorConfig.ProxyClientInfo.KeyFile)

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

@@ -381,6 +381,7 @@ type MasterConfig struct {
 	EtcdConfig *EtcdConfig
 	// OAuthConfig, if present start the /oauth endpoint in this process
 	OAuthConfig *OAuthConfig
+
 	// DNSConfig, if present start the DNS server in this process
 	DNSConfig *DNSConfig
 
@@ -430,6 +431,11 @@ type MasterAuthConfig struct {
 	RequestHeader *RequestHeaderAuthenticationOptions
 	// WebhookTokenAuthnConfig, if present configures remote token reviewers
 	WebhookTokenAuthenticators []WebhookTokenAuthenticator
+	// OAuthMetadataFile is a path to a file containing the discovery endpoint for OAuth 2.0 Authorization
+	// Server Metadata for an external OAuth server.
+	// See IETF Draft: // https://tools.ietf.org/html/draft-ietf-oauth-discovery-04#section-2
+	// This option is mutually exclusive with OAuthConfig
+	OAuthMetadataFile string
 }
 
 // RequestHeaderAuthenticationOptions provides options for setting up a front proxy against the entire

pkg/cmd/server/apis/config/v1/testdata/master-config.yaml

@@ -27,6 +27,7 @@ auditConfig:
   webHookKubeConfig: ""
   webHookMode: ""
 authConfig:
+  oauthMetadataFile: ""
   requestHeader: null
   webhookTokenAuthenticators: null
 controllerConfig:

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

@@ -244,6 +244,7 @@ type MasterConfig struct {
 	EtcdConfig *EtcdConfig `json:"etcdConfig"`
 	// OAuthConfig, if present start the /oauth endpoint in this process
 	OAuthConfig *OAuthConfig `json:"oauthConfig"`
+
 	// DNSConfig, if present start the DNS server in this process
 	DNSConfig *DNSConfig `json:"dnsConfig"`
 
@@ -293,6 +294,11 @@ type MasterAuthConfig struct {
 	RequestHeader *RequestHeaderAuthenticationOptions `json:"requestHeader"`
 	// WebhookTokenAuthnConfig, if present configures remote token reviewers
 	WebhookTokenAuthenticators []WebhookTokenAuthenticator `json:"webhookTokenAuthenticators"`
+	// OAuthMetadataFile is a path to a file containing the discovery endpoint for OAuth 2.0 Authorization
+	// Server Metadata for an external OAuth server.
+	// See IETF Draft: // https://tools.ietf.org/html/draft-ietf-oauth-discovery-04#section-2
+	// This option is mutually exclusive with OAuthConfig
+	OAuthMetadataFile string `json:"oauthMetadataFile"`
 }
 
 // RequestHeaderAuthenticationOptions provides options for setting up a front proxy against the entire

pkg/cmd/server/apis/config/validation/master.go

@@ -25,6 +25,7 @@ import (
 	configapi "github.com/openshift/origin/pkg/cmd/server/apis/config"
 	"github.com/openshift/origin/pkg/cmd/server/bootstrappolicy"
 	"github.com/openshift/origin/pkg/cmd/server/cm"
+	oauthutil "github.com/openshift/origin/pkg/oauth/util"
 	"github.com/openshift/origin/pkg/security/mcs"
 	"github.com/openshift/origin/pkg/security/uid"
 	"github.com/openshift/origin/pkg/util/labelselector"
@@ -141,8 +142,10 @@ func ValidateMasterConfig(config *configapi.MasterConfig, fldPath *field.Path) V
 	validationResults.AddErrors(ValidatePolicyConfig(config.PolicyConfig, fldPath.Child("policyConfig"))...)
 	if config.OAuthConfig != nil {
 		validationResults.Append(ValidateOAuthConfig(config.OAuthConfig, fldPath.Child("oauthConfig")))
+		if len(config.AuthConfig.OAuthMetadataFile) > 0 {
+			validationResults.AddErrors(field.Invalid(fldPath.Child("authConfig", "oauthMetadataFile"), config.AuthConfig.OAuthMetadataFile, "Cannot specify external OAuth Metadata when the internal Oauth Server is configured"))
+		}
 	}
-
 	validationResults.Append(ValidateServiceAccountConfig(config.ServiceAccountConfig, builtInKubernetes, fldPath.Child("serviceAccountConfig")))
 
 	validationResults.Append(ValidateHTTPServingInfo(config.ServingInfo, fldPath.Child("servingInfo")))
@@ -171,6 +174,12 @@ func ValidateMasterConfig(config *configapi.MasterConfig, fldPath *field.Path) V
 func ValidateMasterAuthConfig(config configapi.MasterAuthConfig, fldPath *field.Path) ValidationResults {
 	validationResults := ValidationResults{}
 
+	if len(config.OAuthMetadataFile) > 0 {
+		if _, _, err := oauthutil.LoadOAuthMetadataFile(config.OAuthMetadataFile); err != nil {
+			validationResults.AddErrors(field.Invalid(fldPath.Child("oauthMetadataFile"), config.OAuthMetadataFile, fmt.Sprintf("Metadata validation failed: %v", err)))
+		}
+	}
+
 	for _, wta := range config.WebhookTokenAuthenticators {
 		configFile := fldPath.Child("webhookTokenAuthenticators", "ConfigFile")
 		if len(wta.ConfigFile) == 0 {

pkg/cmd/server/apis/config/validation/master_test.go

@@ -1,6 +1,7 @@
 package validation
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
 	"testing"
@@ -535,10 +536,24 @@ func TestValidateMasterAuthConfig(t *testing.T) {
 	}
 	defer os.Remove(testConfigFile.Name())
 
+	metadataFile, err := ioutil.TempFile("", "oauth.metadata")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	defer os.Remove(metadataFile.Name())
+	ioutil.WriteFile(metadataFile.Name(), testMetadataContent, os.FileMode(0644))
+	badMetadataFile, err := ioutil.TempFile("", "badoauth.metadata")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	defer os.Remove(badMetadataFile.Name())
+	ioutil.WriteFile(badMetadataFile.Name(), []byte("bad file"), os.FileMode(0644))
+
 	testCases := []struct {
 		testName                   string
 		RequestHeader              *configapi.RequestHeaderAuthenticationOptions
 		WebhookTokenAuthenticators []configapi.WebhookTokenAuthenticator
+		OAuthMetadataFile          string
 		expectedErrors             []string
 	}{
 		{
@@ -593,11 +608,22 @@ func TestValidateMasterAuthConfig(t *testing.T) {
 				"webhookTokenAuthenticators.cacheTTL: Required value",
 			},
 		},
+		{
+			testName:          "No OAuth Metadata file",
+			OAuthMetadataFile: "NoFile",
+			expectedErrors:    []string{`oauthMetadataFile: Invalid value: "NoFile": Metadata validation failed: Unable to read External OAuth Metadata file: open NoFile: no such file or directory`},
+		},
+		{
+			testName:          "Bad Metadata file",
+			OAuthMetadataFile: badMetadataFile.Name(),
+			expectedErrors:    []string{fmt.Sprintf(`oauthMetadataFile: Invalid value: %q: Metadata validation failed: Unable to decode External OAuth Metadata file: invalid character 'b' looking for beginning of value`, badMetadataFile.Name())},
+		},
 	}
 	for _, test := range testCases {
 		config := configapi.MasterAuthConfig{
 			RequestHeader:              test.RequestHeader,
 			WebhookTokenAuthenticators: test.WebhookTokenAuthenticators,
+			OAuthMetadataFile:          test.OAuthMetadataFile,
 		}
 		errors := ValidateMasterAuthConfig(config, nil)
 		if len(test.expectedErrors) != len(errors.Errors) {
@@ -611,3 +637,12 @@ func TestValidateMasterAuthConfig(t *testing.T) {
 		}
 	}
 }
+
+var testMetadataContent = []byte(`{
+	"issuer": "https://127.0.0.1/",
+	"authorization_endpoint": "https://127.0.0.1/",
+	"token_endpoint": "https://127.0.0.1/",
+	"scopes_supported": ["openid", "profile", "email", "address", "phone", "offline_access"],
+	"response_types_supported": ["code", "code token"],
+	"grant_types_supported": ["authorization_code", "implicit"],
+	"code_challenge_methods_supported": ["plain", "S256"]}`)

pkg/cmd/server/kubernetes/master/master_config.go

@@ -598,23 +598,22 @@ func defaultOpenAPIConfig(config configapi.MasterConfig) *openapicommon.Config {
 			},
 		}
 	}
-	if config.OAuthConfig != nil {
-		baseUrl := config.OAuthConfig.MasterPublicURL
+	if _, oauthMetadata, _ := oauthutil.PrepOauthMetadata(config); oauthMetadata != nil {
 		securityDefinitions["Oauth2Implicit"] = &spec.SecurityScheme{
 			SecuritySchemeProps: spec.SecuritySchemeProps{
 				Type:             "oauth2",
 				Flow:             "implicit",
-				AuthorizationURL: oauthutil.OpenShiftOAuthAuthorizeURL(baseUrl),
-				Scopes:           scope.DefaultSupportedScopesMap(),
+				AuthorizationURL: oauthMetadata.AuthorizationEndpoint,
+				Scopes:           scope.DescribeScopes(oauthMetadata.ScopesSupported),
 			},
 		}
 		securityDefinitions["Oauth2AccessToken"] = &spec.SecurityScheme{
 			SecuritySchemeProps: spec.SecuritySchemeProps{
 				Type:             "oauth2",
 				Flow:             "accessCode",
-				AuthorizationURL: oauthutil.OpenShiftOAuthAuthorizeURL(baseUrl),
-				TokenURL:         oauthutil.OpenShiftOAuthTokenURL(baseUrl),
-				Scopes:           scope.DefaultSupportedScopesMap(),
+				AuthorizationURL: oauthMetadata.AuthorizationEndpoint,
+				TokenURL:         oauthMetadata.TokenEndpoint,
+				Scopes:           scope.DescribeScopes(oauthMetadata.ScopesSupported),
 			},
 		}
 	}

pkg/cmd/server/origin/master.go

@@ -75,21 +75,20 @@ func (c *MasterConfig) newOpenshiftAPIConfig(kubeAPIServerConfig apiserver.Confi
 	return ret, ret.ExtraConfig.Validate()
 }
 
-func (c *MasterConfig) newOpenshiftNonAPIConfig(kubeAPIServerConfig apiserver.Config) *OpenshiftNonAPIConfig {
+func (c *MasterConfig) newOpenshiftNonAPIConfig(kubeAPIServerConfig apiserver.Config) (*OpenshiftNonAPIConfig, error) {
+	var err error
 	ret := &OpenshiftNonAPIConfig{
 		GenericConfig: &apiserver.RecommendedConfig{
 			Config:                kubeAPIServerConfig,
 			SharedInformerFactory: c.ClientGoKubeInformers,
 		},
-		ExtraConfig: NonAPIExtraConfig{
-			EnableOAuth: c.Options.OAuthConfig != nil,
-		},
 	}
-	if c.Options.OAuthConfig != nil {
-		ret.ExtraConfig.MasterPublicURL = c.Options.OAuthConfig.MasterPublicURL
+	ret.ExtraConfig.OAuthMetadata, _, err = oauthutil.PrepOauthMetadata(c.Options)
+	if err != nil {
+		return nil, err
 	}
 
-	return ret
+	return ret, nil
 }
 
 func (c *MasterConfig) withAPIExtensions(delegateAPIServer apiserver.DelegationTarget, kubeAPIServerConfig apiserver.Config) (apiserver.DelegationTarget, apiextensionsinformers.SharedInformerFactory, error) {
@@ -110,7 +109,10 @@ func (c *MasterConfig) withAPIExtensions(delegateAPIServer apiserver.DelegationT
 }
 
 func (c *MasterConfig) withNonAPIRoutes(delegateAPIServer apiserver.DelegationTarget, kubeAPIServerConfig apiserver.Config) (apiserver.DelegationTarget, error) {
-	openshiftNonAPIConfig := c.newOpenshiftNonAPIConfig(kubeAPIServerConfig)
+	openshiftNonAPIConfig, err := c.newOpenshiftNonAPIConfig(kubeAPIServerConfig)
+	if err != nil {
+		return nil, err
+	}
 	openshiftNonAPIServer, err := openshiftNonAPIConfig.Complete().New(delegateAPIServer)
 	if err != nil {
 		return nil, err

pkg/cmd/server/origin/nonapiserver.go

@@ -1,20 +1,15 @@
 package origin
 
 import (
-	"encoding/json"
 	"net/http"
 
-	"github.com/golang/glog"
-
 	genericmux "k8s.io/apiserver/pkg/server/mux"
 
-	oauthutil "github.com/openshift/origin/pkg/oauth/util"
 	genericapiserver "k8s.io/apiserver/pkg/server"
 )
 
 type NonAPIExtraConfig struct {
-	MasterPublicURL string
-	EnableOAuth     bool
+	OAuthMetadata []byte
 }
 
 type OpenshiftNonAPIConfig struct {
@@ -59,8 +54,8 @@ func (c completedOpenshiftNonAPIConfig) New(delegationTarget genericapiserver.De
 
 	// TODO move this up to the spot where we wire the oauth endpoint
 	// Set up OAuth metadata only if we are configured to use OAuth
-	if c.ExtraConfig.EnableOAuth {
-		initOAuthAuthorizationServerMetadataRoute(s.GenericAPIServer.Handler.NonGoRestfulMux, oauthMetadataEndpoint, c.ExtraConfig.MasterPublicURL)
+	if len(c.ExtraConfig.OAuthMetadata) > 0 {
+		initOAuthAuthorizationServerMetadataRoute(s.GenericAPIServer.Handler.NonGoRestfulMux, c.ExtraConfig)
 	}
 
 	return s, nil
@@ -76,17 +71,10 @@ const (
 // initOAuthAuthorizationServerMetadataRoute initializes an HTTP endpoint for OAuth 2.0 Authorization Server Metadata discovery
 // https://tools.ietf.org/id/draft-ietf-oauth-discovery-04.html#rfc.section.2
 // masterPublicURL should be internally and externally routable to allow all users to discover this information
-func initOAuthAuthorizationServerMetadataRoute(mux *genericmux.PathRecorderMux, path, masterPublicURL string) {
-	// Build OAuth metadata once
-	metadata, err := json.MarshalIndent(oauthutil.GetOauthMetadata(masterPublicURL), "", "  ")
-	if err != nil {
-		glog.Errorf("Unable to initialize OAuth authorization server metadata route: %v", err)
-		return
-	}
-
-	mux.UnlistedHandleFunc(path, func(w http.ResponseWriter, req *http.Request) {
+func initOAuthAuthorizationServerMetadataRoute(mux *genericmux.PathRecorderMux, ExtraConfig *NonAPIExtraConfig) {
+	mux.UnlistedHandleFunc(oauthMetadataEndpoint, func(w http.ResponseWriter, req *http.Request) {
 		w.Header().Set("Content-Type", "application/json")
 		w.WriteHeader(http.StatusOK)
-		w.Write(metadata)
+		w.Write(ExtraConfig.OAuthMetadata)
 	})
 }

pkg/oauth/util/discovery.go

@@ -1,8 +1,16 @@
 package util
 
 import (
+	"encoding/json"
+	"fmt"
+	"io/ioutil"
+	"net/url"
+
+	"github.com/golang/glog"
+
 	"github.com/RangelReale/osin"
 	"github.com/openshift/origin/pkg/authorization/authorizer/scope"
+	configapi "github.com/openshift/origin/pkg/cmd/server/apis/config"
 	"github.com/openshift/origin/pkg/oauth/apis/oauth/validation"
 	"github.com/openshift/origin/pkg/oauthserver/osinserver"
 )
@@ -38,7 +46,10 @@ type OauthAuthorizationServerMetadata struct {
 	CodeChallengeMethodsSupported []string `json:"code_challenge_methods_supported"`
 }
 
-func GetOauthMetadata(masterPublicURL string) OauthAuthorizationServerMetadata {
+// TODO: promote this struct as it is not effectively part of our API, since we
+// validate configuration using LoadOAuthMetadataFile
+
+func getOauthMetadata(masterPublicURL string) OauthAuthorizationServerMetadata {
 	config := osinserver.NewDefaultServerConfig()
 	return OauthAuthorizationServerMetadata{
 		Issuer:                masterPublicURL,
@@ -51,3 +62,59 @@ func GetOauthMetadata(masterPublicURL string) OauthAuthorizationServerMetadata {
 		CodeChallengeMethodsSupported: validation.CodeChallengeMethodsSupported,
 	}
 }
+
+func validateURL(urlString string) error {
+	urlObj, err := url.Parse(urlString)
+	if err != nil {
+		return fmt.Errorf("%q is an invalid URL: %v", urlString, err)
+	}
+	if len(urlObj.Scheme) == 0 {
+		return fmt.Errorf("must contain a valid scheme")
+	}
+	if len(urlObj.Host) == 0 {
+		return fmt.Errorf("must contain a valid host")
+	}
+	return nil
+}
+
+func LoadOAuthMetadataFile(metadataFile string) ([]byte, *OauthAuthorizationServerMetadata, error) {
+	data, err := ioutil.ReadFile(metadataFile)
+	if err != nil {
+		return nil, nil, fmt.Errorf("Unable to read External OAuth Metadata file: %v", err)
+	}
+
+	oauthMetadata := &OauthAuthorizationServerMetadata{}
+	if err := json.Unmarshal(data, oauthMetadata); err != nil {
+		return nil, nil, fmt.Errorf("Unable to decode External OAuth Metadata file: %v", err)
+	}
+
+	if err := validateURL(oauthMetadata.Issuer); err != nil {
+		return nil, nil, fmt.Errorf("Error validating External OAuth Metadata Issuer field: %v", err)
+	}
+
+	if err := validateURL(oauthMetadata.AuthorizationEndpoint); err != nil {
+		return nil, nil, fmt.Errorf("Error validating External OAuth Metadata AuthorizationEndpoint field: %v", err)
+	}
+
+	if err := validateURL(oauthMetadata.TokenEndpoint); err != nil {
+		return nil, nil, fmt.Errorf("Error validating External OAuth Metadata TokenEndpoint field: %v", err)
+	}
+
+	return data, oauthMetadata, nil
+}
+
+func PrepOauthMetadata(config configapi.MasterConfig) ([]byte, *OauthAuthorizationServerMetadata, error) {
+	if config.OAuthConfig != nil {
+		metadataStruct := getOauthMetadata(config.OAuthConfig.MasterPublicURL)
+		metadata, err := json.MarshalIndent(metadataStruct, "", "  ")
+		if err != nil {
+			glog.Errorf("Unable to initialize OAuth authorization server metadata route: %v", err)
+			return nil, nil, err
+		}
+		return metadata, &metadataStruct, nil
+	}
+	if len(config.AuthConfig.OAuthMetadataFile) > 0 {
+		return LoadOAuthMetadataFile(config.AuthConfig.OAuthMetadataFile)
+	}
+	return nil, nil, nil
+}