Skip to content

Commit d4c2891

Browse files
author
OpenShift Bot
committed
Merge pull request #1532 from liggitt/unnest_idp_usage
Merged by openshift-bot
2 parents 93e693a + 0fd3525 commit d4c2891

File tree

13 files changed

+87
-89
lines changed

13 files changed

+87
-89
lines changed

‎pkg/cmd/server/api/register.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ func init() {
1111
&MasterConfig{},
1212
&NodeConfig{},
1313

14-
&IdentityProviderUsage{},
1514
&IdentityProvider{},
1615
&BasicAuthPasswordIdentityProvider{},
1716
&AllowAllPasswordIdentityProvider{},
@@ -25,7 +24,6 @@ func init() {
2524
)
2625
}
2726

28-
func (*IdentityProviderUsage) IsAnAPIObject() {}
2927
func (*IdentityProvider) IsAnAPIObject() {}
3028
func (*BasicAuthPasswordIdentityProvider) IsAnAPIObject() {}
3129
func (*AllowAllPasswordIdentityProvider) IsAnAPIObject() {}

‎pkg/cmd/server/api/types.go

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -146,9 +146,9 @@ type AssetConfig struct {
146146
// PublicURL is where you can find the asset server (TODO do we really need this?)
147147
PublicURL string
148148

149-
// LogoutURI is an optional, absolute URI to redirect web browsers to after logging out of the web console.
149+
// LogoutURL is an optional, absolute URL to redirect web browsers to after logging out of the web console.
150150
// If not specified, the built-in logout page is shown.
151-
LogoutURI string
151+
LogoutURL string
152152

153153
// MasterPublicURL is how the web console can access the OpenShift api server
154154
MasterPublicURL string
@@ -159,7 +159,7 @@ type AssetConfig struct {
159159
}
160160

161161
type OAuthConfig struct {
162-
// MasterURL is used for building valid client redirect URLs for external access
162+
// MasterURL is used for building valid client redirect URLs for internal access
163163
MasterURL string
164164

165165
// MasterPublicURL is used for building valid client redirect URLs for external access
@@ -196,20 +196,13 @@ type SessionConfig struct {
196196
SessionName string
197197
}
198198

199-
type IdentityProviderUsage struct {
200-
// ProviderName is used to qualify the identities returned by this provider
201-
ProviderName string
202-
199+
type IdentityProvider struct {
200+
// Name is used to qualify the identities returned by this provider
201+
Name string
203202
// UseAsChallenger indicates whether to issue WWW-Authenticate challenges for this provider
204203
UseAsChallenger bool
205204
// UseAsLogin indicates whether to use this identity provider for unauthenticated browsers to login against
206205
UseAsLogin bool
207-
}
208-
209-
type IdentityProvider struct {
210-
// Usage contains metadata about how to use this provider
211-
Usage IdentityProviderUsage
212-
213206
// Provider contains the information about how to set up a specific identity provider
214207
Provider runtime.EmbeddedObject
215208
}
@@ -242,7 +235,7 @@ type RequestHeaderIdentityProvider struct {
242235
// ClientCA is a file with the trusted signer certs. If empty, no request verification is done, and any direct request to the OAuth server can impersonate any identity from this provider, merely by setting a request header.
243236
ClientCA string
244237
// Headers is the set of headers to check for identity information
245-
Headers util.StringSet
238+
Headers []string
246239
}
247240

248241
type OAuthRedirectingIdentityProvider struct {

‎pkg/cmd/server/api/v1/conversions.go

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package v1
22

33
import (
44
"github.com/GoogleCloudPlatform/kubernetes/pkg/conversion"
5-
"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
65
newer "github.com/openshift/origin/pkg/cmd/server/api"
76
)
87

@@ -22,6 +21,20 @@ func init() {
2221
out.KeyFile = in.ServerCert.KeyFile
2322
return nil
2423
},
24+
func(in *RemoteConnectionInfo, out *newer.RemoteConnectionInfo, s conversion.Scope) error {
25+
out.URL = in.URL
26+
out.CA = in.CA
27+
out.ClientCert.CertFile = in.CertFile
28+
out.ClientCert.KeyFile = in.KeyFile
29+
return nil
30+
},
31+
func(in *newer.RemoteConnectionInfo, out *RemoteConnectionInfo, s conversion.Scope) error {
32+
out.URL = in.URL
33+
out.CA = in.CA
34+
out.CertFile = in.ClientCert.CertFile
35+
out.KeyFile = in.ClientCert.KeyFile
36+
return nil
37+
},
2538
func(in *EtcdConnectionInfo, out *newer.EtcdConnectionInfo, s conversion.Scope) error {
2639
out.URLs = in.URLs
2740
out.CA = in.CA
@@ -50,20 +63,6 @@ func init() {
5063
out.KeyFile = in.ClientCert.KeyFile
5164
return nil
5265
},
53-
func(in *RequestHeaderIdentityProvider, out *newer.RequestHeaderIdentityProvider, s conversion.Scope) error {
54-
if err := s.DefaultConvert(in, out, conversion.IgnoreMissingFields); err != nil {
55-
return err
56-
}
57-
out.Headers = util.NewStringSet(in.HeadersSlice...)
58-
return nil
59-
},
60-
func(in *newer.RequestHeaderIdentityProvider, out *RequestHeaderIdentityProvider, s conversion.Scope) error {
61-
if err := s.DefaultConvert(in, out, conversion.IgnoreMissingFields); err != nil {
62-
return err
63-
}
64-
out.HeadersSlice = in.Headers.List()
65-
return nil
66-
},
6766
)
6867
if err != nil {
6968
// If one of the conversion functions is malformed, detect it immediately.

‎pkg/cmd/server/api/v1/register.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ func init() {
1212
&MasterConfig{},
1313
&NodeConfig{},
1414

15-
&IdentityProviderUsage{},
1615
&IdentityProvider{},
1716
&BasicAuthPasswordIdentityProvider{},
1817
&AllowAllPasswordIdentityProvider{},
@@ -26,7 +25,6 @@ func init() {
2625
)
2726
}
2827

29-
func (*IdentityProviderUsage) IsAnAPIObject() {}
3028
func (*IdentityProvider) IsAnAPIObject() {}
3129
func (*BasicAuthPasswordIdentityProvider) IsAnAPIObject() {}
3230
func (*AllowAllPasswordIdentityProvider) IsAnAPIObject() {}

‎pkg/cmd/server/api/v1/types.go

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -145,9 +145,9 @@ type AssetConfig struct {
145145
// PublicURL is where you can find the asset server (TODO do we really need this?)
146146
PublicURL string `json:"publicURL"`
147147

148-
// LogoutURI is an optional, absolute URI to redirect web browsers to after logging out of the web console.
148+
// LogoutURL is an optional, absolute URL to redirect web browsers to after logging out of the web console.
149149
// If not specified, the built-in logout page is shown.
150-
LogoutURI string `json:"logoutURI"`
150+
LogoutURL string `json:"logoutURL"`
151151

152152
// MasterPublicURL is how the web console can access the OpenShift v1beta3 server
153153
MasterPublicURL string `json:"masterPublicURL"`
@@ -192,16 +192,14 @@ type SessionConfig struct {
192192
SessionName string `json:"sessionName"`
193193
}
194194

195-
type IdentityProviderUsage struct {
196-
ProviderName string `json:"providerName"`
197-
198-
UseAsChallenger bool `json:"challenge"`
199-
UseAsLogin bool `json:"login"`
200-
}
201-
202195
type IdentityProvider struct {
203-
Usage IdentityProviderUsage `json:"usage"`
204-
196+
// Name is used to qualify the identities returned by this provider
197+
Name string `json:"name"`
198+
// UseAsChallenger indicates whether to issue WWW-Authenticate challenges for this provider
199+
UseAsChallenger bool `json:"challenge"`
200+
// UseAsLogin indicates whether to use this identity provider for unauthenticated browsers to login against
201+
UseAsLogin bool `json:"login"`
202+
// Provider contains the information about how to set up a specific identity provider
205203
Provider runtime.RawExtension `json:"provider"`
206204
}
207205

@@ -228,8 +226,8 @@ type HTPasswdPasswordIdentityProvider struct {
228226
type RequestHeaderIdentityProvider struct {
229227
v1beta3.TypeMeta `json:",inline"`
230228

231-
ClientCA string `json:"clientCA"`
232-
HeadersSlice []string `json:"headers"`
229+
ClientCA string `json:"clientCA"`
230+
Headers []string `json:"headers"`
233231
}
234232

235233
type OAuthRedirectingIdentityProvider struct {

‎pkg/cmd/server/api/validation/master.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,13 @@ func ValidateAssetConfig(config *api.AssetConfig) fielderrors.ValidationErrorLis
8383

8484
allErrs = append(allErrs, ValidateServingInfo(config.ServingInfo).Prefix("servingInfo")...)
8585

86+
if len(config.LogoutURL) > 0 {
87+
_, urlErrs := ValidateURL(config.LogoutURL, "logoutURL")
88+
if len(urlErrs) > 0 {
89+
allErrs = append(allErrs, urlErrs...)
90+
}
91+
}
92+
8693
urlObj, urlErrs := ValidateURL(config.PublicURL, "publicURL")
8794
if len(urlErrs) > 0 {
8895
allErrs = append(allErrs, urlErrs...)

‎pkg/cmd/server/api/validation/oauth.go

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package validation
33
import (
44
"fmt"
55

6+
"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
67
"github.com/GoogleCloudPlatform/kubernetes/pkg/util/fielderrors"
78
"github.com/openshift/origin/pkg/cmd/server/api"
89
)
@@ -28,10 +29,11 @@ func ValidateOAuthConfig(config *api.OAuthConfig) fielderrors.ValidationErrorLis
2829

2930
allErrs = append(allErrs, ValidateGrantConfig(config.GrantConfig).Prefix("grantConfig")...)
3031

31-
redirectingIdentityProviders := []int{}
32+
providerNames := util.NewStringSet()
33+
redirectingIdentityProviders := []string{}
3234
for i, identityProvider := range config.IdentityProviders {
33-
if identityProvider.Usage.UseAsLogin {
34-
redirectingIdentityProviders = append(redirectingIdentityProviders, i)
35+
if identityProvider.UseAsLogin {
36+
redirectingIdentityProviders = append(redirectingIdentityProviders, identityProvider.Name)
3537

3638
if api.IsPasswordAuthenticator(identityProvider) {
3739
if config.SessionConfig == nil {
@@ -41,6 +43,13 @@ func ValidateOAuthConfig(config *api.OAuthConfig) fielderrors.ValidationErrorLis
4143
}
4244

4345
allErrs = append(allErrs, ValidateIdentityProvider(identityProvider).Prefix(fmt.Sprintf("identityProvider[%d]", i))...)
46+
47+
if len(identityProvider.Name) > 0 {
48+
if providerNames.Has(identityProvider.Name) {
49+
allErrs = append(allErrs, fielderrors.NewFieldInvalid(fmt.Sprintf("identityProvider[%d].name", i), identityProvider.Name, "must have a unique name"))
50+
}
51+
providerNames.Insert(identityProvider.Name)
52+
}
4453
}
4554

4655
if len(redirectingIdentityProviders) > 1 {
@@ -53,8 +62,8 @@ func ValidateOAuthConfig(config *api.OAuthConfig) fielderrors.ValidationErrorLis
5362
func ValidateIdentityProvider(identityProvider api.IdentityProvider) fielderrors.ValidationErrorList {
5463
allErrs := fielderrors.ValidationErrorList{}
5564

56-
if len(identityProvider.Usage.ProviderName) == 0 {
57-
allErrs = append(allErrs, fielderrors.NewFieldRequired("usage.providerName"))
65+
if len(identityProvider.Name) == 0 {
66+
allErrs = append(allErrs, fielderrors.NewFieldRequired("name"))
5867
}
5968

6069
if !api.IsIdentityProviderType(identityProvider.Provider) {
@@ -68,11 +77,11 @@ func ValidateIdentityProvider(identityProvider api.IdentityProvider) fielderrors
6877
if len(provider.Headers) == 0 {
6978
allErrs = append(allErrs, fielderrors.NewFieldRequired("provider.headers"))
7079
}
71-
if identityProvider.Usage.UseAsChallenger {
72-
allErrs = append(allErrs, fielderrors.NewFieldInvalid("provider.useAsChallenger", identityProvider.Usage.UseAsChallenger, "request header providers cannot be used for challenges"))
80+
if identityProvider.UseAsChallenger {
81+
allErrs = append(allErrs, fielderrors.NewFieldInvalid("challenge", identityProvider.UseAsChallenger, "request header providers cannot be used for challenges"))
7382
}
74-
if identityProvider.Usage.UseAsLogin {
75-
allErrs = append(allErrs, fielderrors.NewFieldInvalid("provider.useAsLogin", identityProvider.Usage.UseAsChallenger, "request header providers cannot be used for browser login"))
83+
if identityProvider.UseAsLogin {
84+
allErrs = append(allErrs, fielderrors.NewFieldInvalid("login", identityProvider.UseAsChallenger, "request header providers cannot be used for browser login"))
7685
}
7786

7887
case (*api.BasicAuthPasswordIdentityProvider):
@@ -91,8 +100,8 @@ func ValidateIdentityProvider(identityProvider api.IdentityProvider) fielderrors
91100
if !api.IsOAuthProviderType(provider.Provider) {
92101
allErrs = append(allErrs, fielderrors.NewFieldInvalid("provider.provider", provider.Provider, fmt.Sprintf("%v is invalid in this context", identityProvider.Provider)))
93102
}
94-
if identityProvider.Usage.UseAsChallenger {
95-
allErrs = append(allErrs, fielderrors.NewFieldInvalid("provider.useAsChallenger", identityProvider.Usage.UseAsChallenger, "oauth providers cannot be used for challenges"))
103+
if identityProvider.UseAsChallenger {
104+
allErrs = append(allErrs, fielderrors.NewFieldInvalid("challenge", identityProvider.UseAsChallenger, "oauth providers cannot be used for challenges"))
96105
}
97106
}
98107

‎pkg/cmd/server/api/validation/validation.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,9 @@ func ValidateRemoteConnectionInfo(remoteConnectionInfo api.RemoteConnectionInfo)
7979

8080
if len(remoteConnectionInfo.URL) == 0 {
8181
allErrs = append(allErrs, fielderrors.NewFieldRequired("url"))
82+
} else {
83+
_, urlErrs := ValidateURL(remoteConnectionInfo.URL, "url")
84+
allErrs = append(allErrs, urlErrs...)
8285
}
8386

8487
if len(remoteConnectionInfo.CA) > 0 {

‎pkg/cmd/server/origin/asset.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ func (c *AssetConfig) buildHandler() (http.Handler, error) {
114114
OAuthAuthorizeURI: OpenShiftOAuthAuthorizeURL(masterURL.String()),
115115
OAuthRedirectBase: c.Options.PublicURL,
116116
OAuthClientID: OpenShiftWebConsoleClientID,
117-
LogoutURI: c.Options.LogoutURI,
117+
LogoutURI: c.Options.LogoutURL,
118118
}
119119

120120
handler := http.FileServer(

‎pkg/cmd/server/origin/auth.go

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -307,12 +307,12 @@ func (c *AuthConfig) getAuthenticationHandler(mux cmdutil.Mux, errorHandler hand
307307
return nil, err
308308
}
309309

310-
if identityProvider.Usage.UseAsLogin {
310+
if identityProvider.UseAsLogin {
311311
redirectors["login"] = &redirector{RedirectURL: OpenShiftLoginPrefix, ThenParam: "then"}
312312
login := login.NewLogin(getCSRF(), &callbackPasswordAuthenticator{passwordAuth, successHandler}, login.DefaultLoginFormRenderer)
313313
login.Install(mux, OpenShiftLoginPrefix)
314314
}
315-
if identityProvider.Usage.UseAsChallenger {
315+
if identityProvider.UseAsChallenger {
316316
challengers["login"] = passwordchallenger.NewBasicAuthChallenger("openshift")
317317
}
318318

@@ -324,10 +324,10 @@ func (c *AuthConfig) getAuthenticationHandler(mux cmdutil.Mux, errorHandler hand
324324
switch provider.Provider.Object.(type) {
325325
case (*configapi.GoogleOAuthProvider):
326326
callbackPath = path.Join(OpenShiftOAuthCallbackPrefix, "google")
327-
oauthProvider = google.NewProvider(identityProvider.Usage.ProviderName, provider.ClientID, provider.ClientSecret)
327+
oauthProvider = google.NewProvider(identityProvider.Name, provider.ClientID, provider.ClientSecret)
328328
case (*configapi.GitHubOAuthProvider):
329329
callbackPath = path.Join(OpenShiftOAuthCallbackPrefix, "github")
330-
oauthProvider = github.NewProvider(identityProvider.Usage.ProviderName, provider.ClientID, provider.ClientSecret)
330+
oauthProvider = github.NewProvider(identityProvider.Name, provider.ClientID, provider.ClientSecret)
331331
default:
332332
return nil, fmt.Errorf("unexpected oauth provider %#v", provider)
333333
}
@@ -339,10 +339,10 @@ func (c *AuthConfig) getAuthenticationHandler(mux cmdutil.Mux, errorHandler hand
339339
}
340340

341341
mux.Handle(callbackPath, oauthHandler)
342-
if identityProvider.Usage.UseAsLogin {
343-
redirectors[identityProvider.Usage.ProviderName] = oauthHandler
342+
if identityProvider.UseAsLogin {
343+
redirectors[identityProvider.Name] = oauthHandler
344344
}
345-
if identityProvider.Usage.UseAsChallenger {
345+
if identityProvider.UseAsChallenger {
346346
return nil, errors.New("oauth identity providers cannot issue challenges")
347347
}
348348
}
@@ -358,7 +358,7 @@ func (c *AuthConfig) getPasswordAuthenticator(identityProvider configapi.Identit
358358

359359
switch provider := identityProvider.Provider.Object.(type) {
360360
case (*configapi.AllowAllPasswordIdentityProvider):
361-
return allowanypassword.New(identityProvider.Usage.ProviderName, identityMapper), nil
361+
return allowanypassword.New(identityProvider.Name, identityMapper), nil
362362

363363
case (*configapi.DenyAllPasswordIdentityProvider):
364364
return denypassword.New(), nil
@@ -368,7 +368,7 @@ func (c *AuthConfig) getPasswordAuthenticator(identityProvider configapi.Identit
368368
if len(htpasswdFile) == 0 {
369369
return nil, fmt.Errorf("HTPasswdFile is required to support htpasswd auth")
370370
}
371-
if htpasswordAuth, err := htpasswd.New(identityProvider.Usage.ProviderName, htpasswdFile, identityMapper); err != nil {
371+
if htpasswordAuth, err := htpasswd.New(identityProvider.Name, htpasswdFile, identityMapper); err != nil {
372372
return nil, fmt.Errorf("Error loading htpasswd file %s: %v", htpasswdFile, err)
373373
} else {
374374
return htpasswordAuth, nil
@@ -379,7 +379,7 @@ func (c *AuthConfig) getPasswordAuthenticator(identityProvider configapi.Identit
379379
if len(basicAuthURL) == 0 {
380380
return nil, fmt.Errorf("BasicAuthURL is required to support basic password auth")
381381
}
382-
return basicauthpassword.New(identityProvider.Usage.ProviderName, basicAuthURL, identityMapper), nil
382+
return basicauthpassword.New(identityProvider.Name, basicAuthURL, identityMapper), nil
383383

384384
default:
385385
return nil, fmt.Errorf("No password auth found that matches %v. The oauth server cannot start!", identityProvider)
@@ -396,7 +396,7 @@ func (c *AuthConfig) getAuthenticationSuccessHandler() handlers.AuthenticationSu
396396

397397
addedRedirectSuccessHandler := false
398398
for _, identityProvider := range c.Options.IdentityProviders {
399-
if !identityProvider.Usage.UseAsLogin {
399+
if !identityProvider.UseAsLogin {
400400
continue
401401
}
402402

@@ -437,9 +437,9 @@ func (c *AuthConfig) getAuthenticationRequestHandler() (authenticator.Request, e
437437
var authRequestHandler authenticator.Request
438438

439439
authRequestConfig := &headerrequest.Config{
440-
UserNameHeaders: provider.Headers.List(),
440+
UserNameHeaders: provider.Headers,
441441
}
442-
authRequestHandler = headerrequest.NewAuthenticator(identityProvider.Usage.ProviderName, authRequestConfig, identityMapper)
442+
authRequestHandler = headerrequest.NewAuthenticator(identityProvider.Name, authRequestConfig, identityMapper)
443443

444444
// Wrap with an x509 verifier
445445
if len(provider.ClientCA) > 0 {

0 commit comments

Comments
 (0)