Merge pull request #9616 from sgallagher/grants-wip

Merged by openshift-bot

api/swagger-spec/oapi-v1.json

@@ -24584,6 +24584,10 @@
       },
       "description": "RedirectURIs is the valid redirection URIs associated with a client"
      },
+     "grantMethod": {
+      "type": "string",
+      "description": "GrantMethod 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": {

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
+	defaultMethod 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 method in the client
+func NewPerClientGrant(prompt GrantHandler, defaultMethod oauthapi.GrantHandlerType) GrantHandler {
+	return &perClientGrant{
+		auto:          NewAutoGrant(),
+		prompt:        prompt,
+		deny:          NewEmptyGrant(),
+		defaultMethod: defaultMethod,
+	}
 }
 
-// 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")
+	}
+
+	method := client.GrantMethod
+	if len(method) == 0 {
+		// Use the global default
+		method = g.defaultMethod
 	}
 
-	return g.standardGrantHandler.GrantNeeded(user, grant, w, req)
+	switch method {
+	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 method %q unrecognized", method)
+	}
 }

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",
 }
 

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.

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.ServiceAccountMethod))
 
 	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 GrantMethod is present, keep it for compatibility
+		// If it is empty, assign the requested strategy.
+		if len(existing.GrantMethod) == 0 {
+			existing.GrantMethod = client.GrantMethod
+		}
+
 		_, err = clientRegistry.UpdateClient(ctx, existing)
 		return err
 	})
@@ -287,6 +293,7 @@ func CreateOrUpdateDefaultOAuthClients(masterPublicAddr string, assetPublicAddre
 			Secret:                uuid.New(),
 			RespondWithChallenges: false,
 			RedirectURIs:          assetPublicAddresses,
+			GrantMethod:           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)},
+			GrantMethod:           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)},
+			GrantMethod:           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

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"
@@ -536,7 +537,15 @@ 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)
+
+	// If OAuth is disabled, set the strategy to Deny
+	saAccountGrantMethod := oauthapi.GrantHandlerDeny
+	if c.Options.OAuthConfig != nil {
+		// Otherwise, take the value provided in master-config.yaml
+		saAccountGrantMethod = oauthapi.GrantHandlerType(c.Options.OAuthConfig.GrantConfig.ServiceAccountMethod)
+	}
+
+	combinedOAuthClientGetter := saoauth.NewServiceAccountOAuthClientGetter(c.KubeClient(), c.KubeClient(), clientRegistry, saAccountGrantMethod)
 	authorizeTokenStorage, err := authorizetokenetcd.NewREST(c.RESTOptionsGetter, combinedOAuthClientGetter)
 	checkStorageErr(err)
 	accessTokenStorage, err := accesstokenetcd.NewREST(c.RESTOptionsGetter, combinedOAuthClientGetter)

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.GrantMethod = in.GrantMethod
 	if in.ScopeRestrictions != nil {
 		in, out := in.ScopeRestrictions, &out.ScopeRestrictions
 		*out = make([]ScopeRestriction, len(in))

pkg/oauth/api/types.go

@@ -80,12 +80,27 @@ type OAuthClient struct {
 	// RedirectURIs is the valid redirection URIs associated with a client
 	RedirectURIs []string
 
+	// GrantMethod determines how to handle grants for this client. If no method is provided, the
+	// cluster default grant handling method will be used
+	GrantMethod 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

pkg/oauth/api/v1/conversion_generated.go

@@ -258,6 +258,7 @@ func autoConvert_v1_OAuthClient_To_api_OAuthClient(in *OAuthClient, out *oauth_a
 	out.AdditionalSecrets = in.AdditionalSecrets
 	out.RespondWithChallenges = in.RespondWithChallenges
 	out.RedirectURIs = in.RedirectURIs
+	out.GrantMethod = oauth_api.GrantHandlerType(in.GrantMethod)
 	if in.ScopeRestrictions != nil {
 		in, out := &in.ScopeRestrictions, &out.ScopeRestrictions
 		*out = make([]oauth_api.ScopeRestriction, len(*in))
@@ -287,6 +288,7 @@ func autoConvert_api_OAuthClient_To_v1_OAuthClient(in *oauth_api.OAuthClient, ou
 	out.AdditionalSecrets = in.AdditionalSecrets
 	out.RespondWithChallenges = in.RespondWithChallenges
 	out.RedirectURIs = in.RedirectURIs
+	out.GrantMethod = GrantHandlerType(in.GrantMethod)
 	if in.ScopeRestrictions != nil {
 		in, out := &in.ScopeRestrictions, &out.ScopeRestrictions
 		*out = make([]ScopeRestriction, len(*in))

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.GrantMethod = in.GrantMethod
 	if in.ScopeRestrictions != nil {
 		in, out := in.ScopeRestrictions, &out.ScopeRestrictions
 		*out = make([]ScopeRestriction, len(in))

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",
+	"grantMethod":           "GrantMethod 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.",
 }
 

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"`
 
+	// GrantMethod 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
+	GrantMethod GrantHandlerType `json:"grantMethod,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

pkg/serviceaccounts/oauthclient/oauthclientregistry.go

@@ -23,13 +23,14 @@ type saOAuthClientAdapter struct {
 	saClient     kclient.ServiceAccountsNamespacer
 	secretClient kclient.SecretsNamespacer
 
-	delegate oauthclient.Getter
+	delegate    oauthclient.Getter
+	grantMethod 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, grantMethod oauthapi.GrantHandlerType) oauthclient.Getter {
+	return &saOAuthClientAdapter{saClient: saClient, secretClient: secretClient, delegate: delegate, grantMethod: grantMethod}
 }
 
 func (a *saOAuthClientAdapter) GetClient(ctx kapi.Context, name string) (*oauthapi.OAuthClient, error) {
@@ -75,6 +76,7 @@ func (a *saOAuthClientAdapter) GetClient(ctx kapi.Context, name string) (*oautha
 		// 3. route DNS (useful)
 		// 4. loopback? (useful, but maybe a bit weird)
 		RedirectURIs: redirectURIs,
+		GrantMethod:  a.grantMethod,
 	}
 	return saClient, nil
 }