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

Add option to configure an external OAuth server #18969

Merged
merged 2 commits into from
Apr 2, 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
13 changes: 13 additions & 0 deletions pkg/authorization/authorizer/scope/converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}

Expand Down
3 changes: 3 additions & 0 deletions pkg/cmd/server/apis/config/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 6 additions & 0 deletions pkg/cmd/server/apis/config/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions pkg/cmd/server/apis/config/v1/testdata/master-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ auditConfig:
webHookKubeConfig: ""
webHookMode: ""
authConfig:
oauthMetadataFile: ""
requestHeader: null
webhookTokenAuthenticators: null
controllerConfig:
Expand Down
6 changes: 6 additions & 0 deletions pkg/cmd/server/apis/config/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`

Expand Down Expand Up @@ -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
Expand Down
11 changes: 10 additions & 1 deletion pkg/cmd/server/apis/config/validation/master.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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")))
Expand Down Expand Up @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

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

This check has made the oauthmetadatafile format part of our API. Make a TODO to fix the package dependency order and promote the struct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in discovery.go, I have a card to deal with this as a followup

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 {
Expand Down
35 changes: 35 additions & 0 deletions pkg/cmd/server/apis/config/validation/master_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package validation

import (
"fmt"
"io/ioutil"
"os"
"testing"
Expand Down Expand Up @@ -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
}{
{
Expand Down Expand Up @@ -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) {
Expand All @@ -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"]}`)
13 changes: 6 additions & 7 deletions pkg/cmd/server/kubernetes/master/master_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
},
}
}
Expand Down
18 changes: 10 additions & 8 deletions pkg/cmd/server/origin/master.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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
Expand Down
24 changes: 6 additions & 18 deletions pkg/cmd/server/origin/nonapiserver.go
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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)
})
}
69 changes: 68 additions & 1 deletion pkg/oauth/util/discovery.go
Original file line number Diff line number Diff line change
@@ -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"
)
Expand Down Expand Up @@ -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,
Expand All @@ -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
}
Loading