WIP: move around oauth metadata creation

simo5 · simo5 · commit 0bf2e51867b3 · 2018-03-29T20:03:01.000-04:00
Signed-off-by: Simo Sorce &lt;simo@redhat.com&gt;
diff --git a/pkg/authorization/authorizer/scope/converter.go b/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{}
 
diff --git a/pkg/cmd/server/apis/config/validation/master.go b/pkg/cmd/server/apis/config/validation/master.go
@@ -769,7 +769,7 @@ func ValidateDeprecatedClusterNetworkConfig(config *configapi.MasterConfig, fldP
 func ValidateExternalOAuthConfig(config *configapi.ExternalOAuthConfig, fldPath *field.Path) ValidationResults {
 	validationResults := ValidationResults{}
 
-	_, err := oauthutil.LoadOAuthMetadataFile(config.MetadataFile)
+	_, _, err := oauthutil.LoadOAuthMetadataFile(config.MetadataFile)
 	if err != nil {
 		validationResults.AddErrors(field.Invalid(fldPath.Child("metadataFile"), config.MetadataFile, fmt.Sprintf("Metadata validation failed: %v", err)))
 	}
diff --git a/pkg/cmd/server/kubernetes/master/master_config.go b/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),
 			},
 		}
 	}
diff --git a/pkg/cmd/server/origin/master.go b/pkg/cmd/server/origin/master.go
@@ -75,24 +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 || c.Options.ExternalOAuthConfig != nil,
-		},
 	}
-	if c.Options.OAuthConfig != nil {
-		ret.ExtraConfig.MasterPublicURL = c.Options.OAuthConfig.MasterPublicURL
-	}
-	if c.Options.ExternalOAuthConfig != nil {
-		ret.ExtraConfig.OAuthMetadataFile = c.Options.ExternalOAuthConfig.MetadataFile
+	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) {
@@ -113,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
diff --git a/pkg/cmd/server/origin/nonapiserver.go b/pkg/cmd/server/origin/nonapiserver.go
@@ -1,21 +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
-	OAuthMetadataFile string
+	OAuthMetadata []byte
 }
 
 type OpenshiftNonAPIConfig struct {
@@ -60,7 +54,7 @@ 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 {
+	if len(c.ExtraConfig.OAuthMetadata) > 0 {
 		initOAuthAuthorizationServerMetadataRoute(s.GenericAPIServer.Handler.NonGoRestfulMux, c.ExtraConfig)
 	}
 
@@ -78,27 +72,9 @@ const (
 // 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, ExtraConfig *NonAPIExtraConfig) {
-	// Build OAuth metadata once
-	var metadata []byte
-	var err error
-
-	if len(ExtraConfig.OAuthMetadataFile) > 0 {
-		metadata, err = oauthutil.LoadOAuthMetadataFile(ExtraConfig.OAuthMetadataFile)
-		if err != nil {
-			glog.Error(err)
-			return
-		}
-	} else {
-		metadata, err = json.MarshalIndent(oauthutil.GetOauthMetadata(ExtraConfig.MasterPublicURL), "", "  ")
-		if err != nil {
-			glog.Errorf("Unable to initialize OAuth authorization server metadata route: %v", err)
-			return
-		}
-	}
-
 	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)
 	})
 }
diff --git a/pkg/oauth/util/discovery.go b/pkg/oauth/util/discovery.go
@@ -6,8 +6,11 @@ import (
 	"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"
 )
@@ -43,7 +46,7 @@ type OauthAuthorizationServerMetadata struct {
 	CodeChallengeMethodsSupported []string `json:"code_challenge_methods_supported"`
 }
 
-func GetOauthMetadata(masterPublicURL string) OauthAuthorizationServerMetadata {
+func getOauthMetadata(masterPublicURL string) OauthAuthorizationServerMetadata {
 	config := osinserver.NewDefaultServerConfig()
 	return OauthAuthorizationServerMetadata{
 		Issuer:                masterPublicURL,
@@ -71,28 +74,44 @@ func validateURL(urlString string) error {
 	return nil
 }
 
-func LoadOAuthMetadataFile(metadataFile string) ([]byte, error) {
+func LoadOAuthMetadataFile(metadataFile string) ([]byte, *OauthAuthorizationServerMetadata, error) {
 	data, err := ioutil.ReadFile(metadataFile)
 	if err != nil {
-		return nil, fmt.Errorf("Unable to read External OAuth Metadata file: %v", err)
+		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, fmt.Errorf("Unable to decode 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, fmt.Errorf("Error validating External OAuth Metadata Issuer field: %v", err)
+		return nil, nil, fmt.Errorf("Error validating External OAuth Metadata Issuer field: %v", err)
 	}
 
 	if err := validateURL(oauthMetadata.AuthorizationEndpoint); err != nil {
-		return nil, fmt.Errorf("Error validating External OAuth Metadata AuthorizationEndpoint field: %v", err)
+		return nil, nil, fmt.Errorf("Error validating External OAuth Metadata AuthorizationEndpoint field: %v", err)
 	}
 
 	if err := validateURL(oauthMetadata.TokenEndpoint); err != nil {
-		return nil, fmt.Errorf("Error validating External OAuth Metadata TokenEndpoint field: %v", err)
+		return nil, nil, fmt.Errorf("Error validating External OAuth Metadata TokenEndpoint field: %v", err)
 	}
 
-	return data, nil
+	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 config.ExternalOAuthConfig != nil {
+		return LoadOAuthMetadataFile(config.ExternalOAuthConfig.MetadataFile)
+	}
+	return nil, nil, nil
 }
diff --git a/pkg/oauth/util/discovery_test.go b/pkg/oauth/util/discovery_test.go
@@ -8,7 +8,7 @@ import (
 )
 
 func TestGetOauthMetadata(t *testing.T) {
-	actual := GetOauthMetadata("https://localhost:8443")
+	actual := getOauthMetadata("https://localhost:8443")
 	expected := OauthAuthorizationServerMetadata{
 		Issuer:                "https://localhost:8443",
 		AuthorizationEndpoint: "https://localhost:8443/oauth/authorize",

Original file line number	Diff line number	Diff line change
`@@ -769,7 +769,7 @@ func ValidateDeprecatedClusterNetworkConfig(config *configapi.MasterConfig, fldP`
`769`	`769`	`func ValidateExternalOAuthConfig(config configapi.ExternalOAuthConfig, fldPath field.Path) ValidationResults {`
`770`	`770`	`validationResults := ValidationResults{}`
`771`	`771`
`772`		`- _, err := oauthutil.LoadOAuthMetadataFile(config.MetadataFile)`
	`772`	`+ _, _, err := oauthutil.LoadOAuthMetadataFile(config.MetadataFile)`
`773`	`773`	`if err != nil {`
`774`	`774`	`validationResults.AddErrors(field.Invalid(fldPath.Child("metadataFile"), config.MetadataFile, fmt.Sprintf("Metadata validation failed: %v", err)))`
`775`	`775`	`}`
Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,7 @@ import (`
`8`	`8`	`)`
`9`	`9`
`10`	`10`	`func TestGetOauthMetadata(t *testing.T) {`
`11`		`- actual := GetOauthMetadata("https://localhost:8443")`
	`11`	`+ actual := getOauthMetadata("https://localhost:8443")`
`12`	`12`	`expected := OauthAuthorizationServerMetadata{`
`13`	`13`	`Issuer: "https://localhost:8443",`
`14`	`14`	`AuthorizationEndpoint: "https://localhost:8443/oauth/authorize",`