From f88bf8ae5887d459edcc121a604ea93e5a7fd957 Mon Sep 17 00:00:00 2001 From: Monis Khan Date: Mon, 25 Mar 2019 16:17:16 -0400 Subject: [PATCH 1/3] Infer challenge and login based on IDP These fields offer a level of flexibility that is not particularly useful to admins. In reality all IDPs have a correct configuration for these fields based on the feature set that the IDP supports. We should avoid asking users questions that we already know the answer to. Signed-off-by: Monis Khan --- config/v1/types_oauth.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/config/v1/types_oauth.go b/config/v1/types_oauth.go index 6e4ed1ca1a6..cf2d48412a6 100644 --- a/config/v1/types_oauth.go +++ b/config/v1/types_oauth.go @@ -121,12 +121,6 @@ type IdentityProvider struct { // Ref: https://godoc.org/github.com/openshift/origin/pkg/user/apis/user/validation#ValidateIdentityProviderName Name string `json:"name"` - // challenge indicates whether to issue WWW-Authenticate challenges for this provider - UseAsChallenger bool `json:"challenge"` - - // login indicates whether to use this identity provider for unauthenticated browsers to login against - UseAsLogin bool `json:"login"` - // mappingMethod determines how identities from this provider are mapped to users // Defaults to "claim" // +optional From 3924b1da5216ade6d77951fffedf23c811e5be94 Mon Sep 17 00:00:00 2001 From: Monis Khan Date: Mon, 25 Mar 2019 16:18:00 -0400 Subject: [PATCH 2/3] Add issuer to OpenIDIdentityProvider This change adds an issuer field which can be used to perform discovery via the /.well-known/openid-configuration endpoint. This makes the OpenIDURLs struct obsolete. Signed-off-by: Monis Khan --- config/v1/types_oauth.go | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/config/v1/types_oauth.go b/config/v1/types_oauth.go index cf2d48412a6..7aa0133e732 100644 --- a/config/v1/types_oauth.go +++ b/config/v1/types_oauth.go @@ -511,28 +511,14 @@ type OpenIDIdentityProvider struct { // +optional ExtraAuthorizeParameters map[string]string `json:"extraAuthorizeParameters,omitempty"` - // urls to use to authenticate - URLs OpenIDURLs `json:"urls"` + // issuer is the URL that the OpenID Provider asserts as its Issuer Identifier. + // It must use the https scheme with no query or fragment component. + Issuer string `json:"issuer"` // claims mappings Claims OpenIDClaims `json:"claims"` } -// OpenIDURLs are URLs to use when authenticating with an OpenID identity provider -type OpenIDURLs struct { - // authorize is the oauth authorization URL - Authorize string `json:"authorize"` - - // token is the oauth token granting URL - Token string `json:"token"` - - // userInfo is the optional userinfo URL. - // If present, a granted access_token is used to request claims - // If empty, a granted id_token is parsed for claims - // +optional - UserInfo string `json:"userInfo"` -} - // UserIDClaim is the claim used to provide a stable identifier for OIDC identities. // Per http://openid.net/specs/openid-connect-core-1_0.html#ClaimStability // "The sub (subject) and iss (issuer) Claims, used together, are the only Claims that an RP can From 5d161524c7741f22a66c7462135c95e21bcb5f10 Mon Sep 17 00:00:00 2001 From: Monis Khan Date: Mon, 25 Mar 2019 16:12:09 -0400 Subject: [PATCH 3/3] Generated Signed-off-by: Monis Khan --- config/v1/zz_generated.deepcopy.go | 17 ----------------- config/v1/zz_generated.swagger_doc_generated.go | 17 ++--------------- 2 files changed, 2 insertions(+), 32 deletions(-) diff --git a/config/v1/zz_generated.deepcopy.go b/config/v1/zz_generated.deepcopy.go index 91ecabf3ddb..06ae741c706 100644 --- a/config/v1/zz_generated.deepcopy.go +++ b/config/v1/zz_generated.deepcopy.go @@ -2298,7 +2298,6 @@ func (in *OpenIDIdentityProvider) DeepCopyInto(out *OpenIDIdentityProvider) { (*out)[key] = val } } - out.URLs = in.URLs in.Claims.DeepCopyInto(&out.Claims) return } @@ -2313,22 +2312,6 @@ func (in *OpenIDIdentityProvider) DeepCopy() *OpenIDIdentityProvider { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *OpenIDURLs) DeepCopyInto(out *OpenIDURLs) { - *out = *in - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OpenIDURLs. -func (in *OpenIDURLs) DeepCopy() *OpenIDURLs { - if in == nil { - return nil - } - out := new(OpenIDURLs) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OperandVersion) DeepCopyInto(out *OperandVersion) { *out = *in diff --git a/config/v1/zz_generated.swagger_doc_generated.go b/config/v1/zz_generated.swagger_doc_generated.go index b55376c149d..f6d03f9442b 100644 --- a/config/v1/zz_generated.swagger_doc_generated.go +++ b/config/v1/zz_generated.swagger_doc_generated.go @@ -872,8 +872,6 @@ func (HTPasswdIdentityProvider) SwaggerDoc() map[string]string { var map_IdentityProvider = map[string]string{ "": "IdentityProvider provides identities for users authenticating using credentials", "name": "name is used to qualify the identities returned by this provider. - It MUST be unique and not shared by any other identity provider used - It MUST be a valid path segment: name cannot equal \".\" or \"..\" or contain \"/\" or \"%\" or \":\"\n Ref: https://godoc.org/github.com/openshift/origin/pkg/user/apis/user/validation#ValidateIdentityProviderName", - "challenge": "challenge indicates whether to issue WWW-Authenticate challenges for this provider", - "login": "login indicates whether to use this identity provider for unauthenticated browsers to login against", "mappingMethod": "mappingMethod determines how identities from this provider are mapped to users Defaults to \"claim\"", } @@ -1002,25 +1000,14 @@ var map_OpenIDIdentityProvider = map[string]string{ "ca": "ca is an optional reference to a config map by name containing the PEM-encoded CA bundle. It is used as a trust anchor to validate the TLS certificate presented by the remote server. The key \"ca.crt\" is used to locate the data. If specified and the config map or expected key is not found, the identity provider is not honored. If the specified ca data is not valid, the identity provider is not honored. If empty, the default system roots are used. The namespace for this config map is openshift-config.", "extraScopes": "extraScopes are any scopes to request in addition to the standard \"openid\" scope.", "extraAuthorizeParameters": "extraAuthorizeParameters are any custom parameters to add to the authorize request.", - "urls": "urls to use to authenticate", - "claims": "claims mappings", + "issuer": "issuer is the URL that the OpenID Provider asserts as its Issuer Identifier. It must use the https scheme with no query or fragment component.", + "claims": "claims mappings", } func (OpenIDIdentityProvider) SwaggerDoc() map[string]string { return map_OpenIDIdentityProvider } -var map_OpenIDURLs = map[string]string{ - "": "OpenIDURLs are URLs to use when authenticating with an OpenID identity provider", - "authorize": "authorize is the oauth authorization URL", - "token": "token is the oauth token granting URL", - "userInfo": "userInfo is the optional userinfo URL. If present, a granted access_token is used to request claims If empty, a granted id_token is parsed for claims", -} - -func (OpenIDURLs) SwaggerDoc() map[string]string { - return map_OpenIDURLs -} - var map_RequestHeaderIdentityProvider = map[string]string{ "": "RequestHeaderIdentityProvider provides identities for users authenticating using request header credentials", "loginURL": "loginURL is a URL to redirect unauthenticated /authorize requests to Unauthenticated requests from OAuth clients which expect interactive logins will be redirected here ${url} is replaced with the current URL, escaped to be safe in a query parameter\n https://www.example.com/sso-login?then=${url}\n${query} is replaced with the current query string\n https://www.example.com/auth-proxy/oauth/authorize?${query}\nRequired when login is set to true.",