diff --git a/api/swagger-spec/oapi-v1.json b/api/swagger-spec/oapi-v1.json index c7697cae5567..e5ad41f16c78 100644 --- a/api/swagger-spec/oapi-v1.json +++ b/api/swagger-spec/oapi-v1.json @@ -24254,6 +24254,10 @@ }, "description": "RedirectURIs is the valid redirection URIs associated with a client" }, + "grantStrategy": { + "type": "string", + "description": "GrantStrategy determines how to handle grants for this client. If no method is provided, the cluster default grant handling method will be used. Valid grant handling methods are:\n - auto: always approves grant requests, useful for trusted clients\n - prompt: prompts the end user for approval of grant requests, useful for third-party clients\n - deny: always denies grant requests, useful for black-listed clients" + }, "scopeRestrictions": { "type": "array", "items": { diff --git a/pkg/auth/oauth/handlers/grant.go b/pkg/auth/oauth/handlers/grant.go index 513e5aed710b..89813ad11492 100644 --- a/pkg/auth/oauth/handlers/grant.go +++ b/pkg/auth/oauth/handlers/grant.go @@ -2,15 +2,16 @@ package handlers import ( "errors" + "fmt" "net/http" "net/url" "github.com/RangelReale/osin" "k8s.io/kubernetes/pkg/auth/user" - "k8s.io/kubernetes/pkg/serviceaccount" "github.com/openshift/origin/pkg/auth/api" + oauthapi "github.com/openshift/origin/pkg/oauth/api" ) // GrantCheck implements osinserver.AuthorizeHandler to ensure requested scopes have been authorized @@ -126,23 +127,46 @@ func (g *redirectGrant) GrantNeeded(user user.Info, grant *api.Grant, w http.Res return false, true, nil } -type serviceAccountAwareGrant struct { - standardGrantHandler GrantHandler - // saClientGrantHandler allows an autogrant handler to do something else when the client is a service account. - // TODO: I think this can be removed once we can set granthandler overrides per-client, but we need something for safety now. - saClientGrantHandler GrantHandler +type perClientGrant struct { + auto GrantHandler + prompt GrantHandler + deny GrantHandler + defaultStrategy oauthapi.GrantHandlerType } -// NewAutoGrant returns a grant handler that automatically approves client authorizations -func NewServiceAccountAwareGrant(standardGrantHandler, saClientGrantHandler GrantHandler) GrantHandler { - return &serviceAccountAwareGrant{standardGrantHandler: standardGrantHandler, saClientGrantHandler: saClientGrantHandler} +// NewPerClientGrant returns a grant handler that determines what to do based on the grant strategy in the client +func NewPerClientGrant(prompt GrantHandler, defaultStrategy oauthapi.GrantHandlerType) GrantHandler { + return &perClientGrant{ + auto: NewAutoGrant(), + prompt: prompt, + deny: NewEmptyGrant(), + defaultStrategy: defaultStrategy, + } } -// GrantNeeded implements the GrantHandler interface -func (g *serviceAccountAwareGrant) GrantNeeded(user user.Info, grant *api.Grant, w http.ResponseWriter, req *http.Request) (bool, bool, error) { - if _, _, err := serviceaccount.SplitUsername(grant.Client.GetId()); err == nil { - return g.saClientGrantHandler.GrantNeeded(user, grant, w, req) +func (g *perClientGrant) GrantNeeded(user user.Info, grant *api.Grant, w http.ResponseWriter, req *http.Request) (bool, bool, error) { + client, ok := grant.Client.GetUserData().(*oauthapi.OAuthClient) + if !ok { + return false, false, errors.New("unrecognized OAuth client type") + } + + strategy := client.GrantStrategy + if len(strategy) == 0 { + // Use the global default + strategy = g.defaultStrategy } - return g.standardGrantHandler.GrantNeeded(user, grant, w, req) + switch strategy { + case oauthapi.GrantHandlerAuto: + return g.auto.GrantNeeded(user, grant, w, req) + + case oauthapi.GrantHandlerPrompt: + return g.prompt.GrantNeeded(user, grant, w, req) + + case oauthapi.GrantHandlerDeny: + return g.deny.GrantNeeded(user, grant, w, req) + + default: + return false, false, fmt.Errorf("OAuth client grant strategy %q unrecognized", strategy) + } } diff --git a/pkg/cmd/server/api/v1/swagger_doc.go b/pkg/cmd/server/api/v1/swagger_doc.go index 9f8cb49ee644..fabc475aeae9 100644 --- a/pkg/cmd/server/api/v1/swagger_doc.go +++ b/pkg/cmd/server/api/v1/swagger_doc.go @@ -232,7 +232,7 @@ func (GoogleIdentityProvider) SwaggerDoc() map[string]string { var map_GrantConfig = map[string]string{ "": "GrantConfig holds the necessary configuration options for grant handlers", - "method": "Method: allow, deny, prompt", + "method": "Method determines the default strategy to use when an OAuth client requests a grant. This method will be used only if the specific OAuth client doesn't provide a strategy of their own. Valid grant handling methods are:\n - auto: always approves grant requests, useful for trusted clients\n - prompt: prompts the end user for approval of grant requests, useful for third-party clients\n - deny: always denies grant requests, useful for black-listed clients", "serviceAccountMethod": "ServiceAccountMethod is used for determining client authorization for service account oauth client. It must be either: deny, prompt", } diff --git a/pkg/cmd/server/api/v1/types.go b/pkg/cmd/server/api/v1/types.go index dd7a97a69d67..0676293d971e 100644 --- a/pkg/cmd/server/api/v1/types.go +++ b/pkg/cmd/server/api/v1/types.go @@ -890,7 +890,12 @@ type OpenIDClaims struct { // GrantConfig holds the necessary configuration options for grant handlers type GrantConfig struct { - // Method: allow, deny, prompt + // Method determines the default strategy to use when an OAuth client requests a grant. + // This method will be used only if the specific OAuth client doesn't provide a strategy + // of their own. Valid grant handling methods are: + // - auto: always approves grant requests, useful for trusted clients + // - prompt: prompts the end user for approval of grant requests, useful for third-party clients + // - deny: always denies grant requests, useful for black-listed clients Method GrantHandlerType `json:"method"` // ServiceAccountMethod is used for determining client authorization for service account oauth client. diff --git a/pkg/cmd/server/origin/auth.go b/pkg/cmd/server/origin/auth.go index c2a12ae04b0a..5c96ddf58fd7 100644 --- a/pkg/cmd/server/origin/auth.go +++ b/pkg/cmd/server/origin/auth.go @@ -12,7 +12,7 @@ import ( "github.com/RangelReale/osin" "github.com/RangelReale/osincli" - "github.com/emicklei/go-restful" + restful "github.com/emicklei/go-restful" "github.com/golang/glog" "github.com/pborman/uuid" @@ -89,7 +89,7 @@ func (c *AuthConfig) InstallAPI(container *restful.Container) ([]string, error) return nil, err } clientRegistry := clientregistry.NewRegistry(clientStorage) - combinedOAuthClientGetter := saoauth.NewServiceAccountOAuthClientGetter(c.KubeClient, c.KubeClient, clientRegistry) + combinedOAuthClientGetter := saoauth.NewServiceAccountOAuthClientGetter(c.KubeClient, c.KubeClient, clientRegistry, oauthapi.GrantHandlerType(c.Options.GrantConfig.Method)) accessTokenStorage, err := accesstokenetcd.NewREST(c.RESTOptionsGetter, combinedOAuthClientGetter, c.EtcdBackends...) if err != nil { @@ -275,6 +275,12 @@ func ensureOAuthClient(client oauthapi.OAuthClient, clientRegistry clientregistr } existing.RedirectURIs = client.RedirectURIs + // If the GrantStrategy is present, keep it for compatibility + // If it is empty, assign the requested strategy. + if len(existing.GrantStrategy) == 0 { + existing.GrantStrategy = client.GrantStrategy + } + _, err = clientRegistry.UpdateClient(ctx, existing) return err }) @@ -287,6 +293,7 @@ func CreateOrUpdateDefaultOAuthClients(masterPublicAddr string, assetPublicAddre Secret: uuid.New(), RespondWithChallenges: false, RedirectURIs: assetPublicAddresses, + GrantStrategy: oauthapi.GrantHandlerAuto, } if err := ensureOAuthClient(webConsoleClient, clientRegistry, true); err != nil { return err @@ -299,6 +306,7 @@ func CreateOrUpdateDefaultOAuthClients(masterPublicAddr string, assetPublicAddre Secret: uuid.New(), RespondWithChallenges: false, RedirectURIs: []string{masterPublicAddr + path.Join(OpenShiftOAuthAPIPrefix, tokenrequest.DisplayTokenEndpoint)}, + GrantStrategy: oauthapi.GrantHandlerAuto, } if err := ensureOAuthClient(browserClient, clientRegistry, true); err != nil { return err @@ -311,6 +319,7 @@ func CreateOrUpdateDefaultOAuthClients(masterPublicAddr string, assetPublicAddre Secret: uuid.New(), RespondWithChallenges: true, RedirectURIs: []string{masterPublicAddr + path.Join(OpenShiftOAuthAPIPrefix, tokenrequest.ImplicitTokenEndpoint)}, + GrantStrategy: oauthapi.GrantHandlerAuto, } if err := ensureOAuthClient(cliClient, clientRegistry, false); err != nil { return err @@ -342,38 +351,19 @@ func (c *AuthConfig) getAuthorizeAuthenticationHandlers(mux cmdutil.Mux, errorHa // getGrantHandler returns the object that handles approving or rejecting grant requests func (c *AuthConfig) getGrantHandler(mux cmdutil.Mux, auth authenticator.Request, clientregistry clientregistry.Getter, authregistry clientauthregistry.Registry) handlers.GrantHandler { - startGrantServer := false - - var saGrantHandler handlers.GrantHandler - switch c.Options.GrantConfig.ServiceAccountMethod { - case configapi.GrantHandlerDeny: - saGrantHandler = handlers.NewEmptyGrant() - case configapi.GrantHandlerPrompt: - startGrantServer = true - saGrantHandler = handlers.NewRedirectGrant(OpenShiftApprovePrefix) - default: - glog.Fatalf("No grant handler found that matches %v. The oauth server cannot start!", c.Options.GrantConfig.ServiceAccountMethod) - } - - var standardGrantHandler handlers.GrantHandler - switch c.Options.GrantConfig.Method { - case configapi.GrantHandlerDeny: - standardGrantHandler = handlers.NewEmptyGrant() - case configapi.GrantHandlerAuto: - standardGrantHandler = handlers.NewAutoGrant() - case configapi.GrantHandlerPrompt: - startGrantServer = true - standardGrantHandler = handlers.NewRedirectGrant(OpenShiftApprovePrefix) - default: - glog.Fatalf("No grant handler found that matches %v. The oauth server cannot start!", c.Options.GrantConfig.Method) + // check that the global default strategy is something we honor + if !configapi.ValidGrantHandlerTypes.Has(string(c.Options.GrantConfig.Method)) { + glog.Fatalf("No grant handler found that matches %v. The OAuth server cannot start!", c.Options.GrantConfig.Method) } - if startGrantServer { - grantServer := grant.NewGrant(c.getCSRF(), auth, grant.DefaultFormRenderer, clientregistry, authregistry) - grantServer.Install(mux, OpenShiftApprovePrefix) - } + // Since any OAuth client could require prompting, we will unconditionally + // start the GrantServer here. + grantServer := grant.NewGrant(c.getCSRF(), auth, grant.DefaultFormRenderer, clientregistry, authregistry) + grantServer.Install(mux, OpenShiftApprovePrefix) - return handlers.NewServiceAccountAwareGrant(standardGrantHandler, saGrantHandler) + // Set defaults for standard clients. These can be overridden. + return handlers.NewPerClientGrant(handlers.NewRedirectGrant(OpenShiftApprovePrefix), + oauthapi.GrantHandlerType(c.Options.GrantConfig.Method)) } // getAuthenticationFinalizer returns an authentication finalizer which is called just prior to writing a response to an authorization request diff --git a/pkg/cmd/server/origin/master.go b/pkg/cmd/server/origin/master.go index e11c55936f86..9f7bcef131ed 100644 --- a/pkg/cmd/server/origin/master.go +++ b/pkg/cmd/server/origin/master.go @@ -64,6 +64,7 @@ import ( "github.com/openshift/origin/pkg/image/registry/imagestreamimport" "github.com/openshift/origin/pkg/image/registry/imagestreammapping" "github.com/openshift/origin/pkg/image/registry/imagestreamtag" + oauthapi "github.com/openshift/origin/pkg/oauth/api" accesstokenetcd "github.com/openshift/origin/pkg/oauth/registry/oauthaccesstoken/etcd" authorizetokenetcd "github.com/openshift/origin/pkg/oauth/registry/oauthauthorizetoken/etcd" clientregistry "github.com/openshift/origin/pkg/oauth/registry/oauthclient" @@ -540,7 +541,7 @@ func (c *MasterConfig) GetRestStorage() map[string]rest.Storage { clientStorage, err := clientetcd.NewREST(c.RESTOptionsGetter) checkStorageErr(err) clientRegistry := clientregistry.NewRegistry(clientStorage) - combinedOAuthClientGetter := saoauth.NewServiceAccountOAuthClientGetter(c.KubeClient(), c.KubeClient(), clientRegistry) + combinedOAuthClientGetter := saoauth.NewServiceAccountOAuthClientGetter(c.KubeClient(), c.KubeClient(), clientRegistry, oauthapi.GrantHandlerType(c.Options.OAuthConfig.GrantConfig.Method)) authorizeTokenStorage, err := authorizetokenetcd.NewREST(c.RESTOptionsGetter, combinedOAuthClientGetter) checkStorageErr(err) accessTokenStorage, err := accesstokenetcd.NewREST(c.RESTOptionsGetter, combinedOAuthClientGetter) diff --git a/pkg/oauth/api/deep_copy_generated.go b/pkg/oauth/api/deep_copy_generated.go index 87d627baf16a..c9872771688b 100644 --- a/pkg/oauth/api/deep_copy_generated.go +++ b/pkg/oauth/api/deep_copy_generated.go @@ -159,6 +159,7 @@ func DeepCopy_api_OAuthClient(in OAuthClient, out *OAuthClient, c *conversion.Cl } else { out.RedirectURIs = nil } + out.GrantStrategy = in.GrantStrategy if in.ScopeRestrictions != nil { in, out := in.ScopeRestrictions, &out.ScopeRestrictions *out = make([]ScopeRestriction, len(in)) diff --git a/pkg/oauth/api/types.go b/pkg/oauth/api/types.go index e5604f3c3050..1af1cadea153 100644 --- a/pkg/oauth/api/types.go +++ b/pkg/oauth/api/types.go @@ -80,12 +80,27 @@ type OAuthClient struct { // RedirectURIs is the valid redirection URIs associated with a client RedirectURIs []string + // GrantStrategy determines how to handle grants for this client. If no method is provided, the + // cluster default grant handling method will be used + GrantStrategy GrantHandlerType + // ScopeRestrictions describes which scopes this client can request. Each requested scope // is checked against each restriction. If any restriction matches, then the scope is allowed. // If no restriction matches, then the scope is denied. ScopeRestrictions []ScopeRestriction } +type GrantHandlerType string + +const ( + // GrantHandlerAuto auto-approves client authorization grant requests + GrantHandlerAuto GrantHandlerType = "auto" + // GrantHandlerPrompt prompts the user to approve new client authorization grant requests + GrantHandlerPrompt GrantHandlerType = "prompt" + // GrantHandlerDeny auto-denies client authorization grant requests + GrantHandlerDeny GrantHandlerType = "deny" +) + // ScopeRestriction describe one restriction on scopes. Exactly one option must be non-nil. type ScopeRestriction struct { // ExactValues means the scope has to match a particular set of strings exactly diff --git a/pkg/oauth/api/v1/conversion_generated.go b/pkg/oauth/api/v1/conversion_generated.go index c84e43eef648..8fc8c003c1d6 100644 --- a/pkg/oauth/api/v1/conversion_generated.go +++ b/pkg/oauth/api/v1/conversion_generated.go @@ -322,6 +322,7 @@ func autoConvert_v1_OAuthClient_To_api_OAuthClient(in *OAuthClient, out *oauth_a } else { out.RedirectURIs = nil } + out.GrantStrategy = oauth_api.GrantHandlerType(in.GrantStrategy) if in.ScopeRestrictions != nil { in, out := &in.ScopeRestrictions, &out.ScopeRestrictions *out = make([]oauth_api.ScopeRestriction, len(*in)) @@ -364,6 +365,7 @@ func autoConvert_api_OAuthClient_To_v1_OAuthClient(in *oauth_api.OAuthClient, ou } else { out.RedirectURIs = nil } + out.GrantStrategy = GrantHandlerType(in.GrantStrategy) if in.ScopeRestrictions != nil { in, out := &in.ScopeRestrictions, &out.ScopeRestrictions *out = make([]ScopeRestriction, len(*in)) diff --git a/pkg/oauth/api/v1/deep_copy_generated.go b/pkg/oauth/api/v1/deep_copy_generated.go index d2a427ae77a3..c227f4f750c8 100644 --- a/pkg/oauth/api/v1/deep_copy_generated.go +++ b/pkg/oauth/api/v1/deep_copy_generated.go @@ -160,6 +160,7 @@ func DeepCopy_v1_OAuthClient(in OAuthClient, out *OAuthClient, c *conversion.Clo } else { out.RedirectURIs = nil } + out.GrantStrategy = in.GrantStrategy if in.ScopeRestrictions != nil { in, out := in.ScopeRestrictions, &out.ScopeRestrictions *out = make([]ScopeRestriction, len(in)) diff --git a/pkg/oauth/api/v1/swagger_doc.go b/pkg/oauth/api/v1/swagger_doc.go index fd7c8c556022..ccde1d68e2ee 100644 --- a/pkg/oauth/api/v1/swagger_doc.go +++ b/pkg/oauth/api/v1/swagger_doc.go @@ -76,6 +76,7 @@ var map_OAuthClient = map[string]string{ "additionalSecrets": "AdditionalSecrets holds other secrets that may be used to identify the client. This is useful for rotation and for service account token validation", "respondWithChallenges": "RespondWithChallenges indicates whether the client wants authentication needed responses made in the form of challenges instead of redirects", "redirectURIs": "RedirectURIs is the valid redirection URIs associated with a client", + "grantStrategy": "GrantStrategy determines how to handle grants for this client. If no method is provided, the cluster default grant handling method will be used. Valid grant handling methods are:\n - auto: always approves grant requests, useful for trusted clients\n - prompt: prompts the end user for approval of grant requests, useful for third-party clients\n - deny: always denies grant requests, useful for black-listed clients", "scopeRestrictions": "ScopeRestrictions describes which scopes this client can request. Each requested scope is checked against each restriction. If any restriction matches, then the scope is allowed. If no restriction matches, then the scope is denied.", } diff --git a/pkg/oauth/api/v1/types.go b/pkg/oauth/api/v1/types.go index bed8fe4eae1b..9bf19fb0afee 100644 --- a/pkg/oauth/api/v1/types.go +++ b/pkg/oauth/api/v1/types.go @@ -86,12 +86,30 @@ type OAuthClient struct { // RedirectURIs is the valid redirection URIs associated with a client RedirectURIs []string `json:"redirectURIs,omitempty"` + // GrantStrategy determines how to handle grants for this client. If no method is provided, the + // cluster default grant handling method will be used. Valid grant handling methods are: + // - auto: always approves grant requests, useful for trusted clients + // - prompt: prompts the end user for approval of grant requests, useful for third-party clients + // - deny: always denies grant requests, useful for black-listed clients + GrantStrategy GrantHandlerType `json:"grantStrategy,omitempty"` + // ScopeRestrictions describes which scopes this client can request. Each requested scope // is checked against each restriction. If any restriction matches, then the scope is allowed. // If no restriction matches, then the scope is denied. ScopeRestrictions []ScopeRestriction `json:"scopeRestrictions,omitempty"` } +type GrantHandlerType string + +const ( + // GrantHandlerAuto auto-approves client authorization grant requests + GrantHandlerAuto GrantHandlerType = "auto" + // GrantHandlerPrompt prompts the user to approve new client authorization grant requests + GrantHandlerPrompt GrantHandlerType = "prompt" + // GrantHandlerDeny auto-denies client authorization grant requests + GrantHandlerDeny GrantHandlerType = "deny" +) + // ScopeRestriction describe one restriction on scopes. Exactly one option must be non-nil. type ScopeRestriction struct { // ExactValues means the scope has to match a particular set of strings exactly diff --git a/pkg/serviceaccounts/oauthclient/oauthclientregistry.go b/pkg/serviceaccounts/oauthclient/oauthclientregistry.go index 926ebec52c9b..53b1a6e197b5 100644 --- a/pkg/serviceaccounts/oauthclient/oauthclientregistry.go +++ b/pkg/serviceaccounts/oauthclient/oauthclientregistry.go @@ -23,13 +23,14 @@ type saOAuthClientAdapter struct { saClient kclient.ServiceAccountsNamespacer secretClient kclient.SecretsNamespacer - delegate oauthclient.Getter + delegate oauthclient.Getter + grantStrategy oauthapi.GrantHandlerType } var _ oauthclient.Getter = &saOAuthClientAdapter{} -func NewServiceAccountOAuthClientGetter(saClient kclient.ServiceAccountsNamespacer, secretClient kclient.SecretsNamespacer, delegate oauthclient.Getter) oauthclient.Getter { - return &saOAuthClientAdapter{saClient: saClient, secretClient: secretClient, delegate: delegate} +func NewServiceAccountOAuthClientGetter(saClient kclient.ServiceAccountsNamespacer, secretClient kclient.SecretsNamespacer, delegate oauthclient.Getter, grantStrategy oauthapi.GrantHandlerType) oauthclient.Getter { + return &saOAuthClientAdapter{saClient: saClient, secretClient: secretClient, delegate: delegate, grantStrategy: grantStrategy} } func (a *saOAuthClientAdapter) GetClient(ctx kapi.Context, name string) (*oauthapi.OAuthClient, error) { @@ -74,7 +75,8 @@ func (a *saOAuthClientAdapter) GetClient(ctx kapi.Context, name string) (*oautha // 2. service DNS (useless in general) // 3. route DNS (useful) // 4. loopback? (useful, but maybe a bit weird) - RedirectURIs: redirectURIs, + RedirectURIs: redirectURIs, + GrantStrategy: a.grantStrategy, } return saClient, nil }