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 securityDefinitions to the generated OpenAPI spec #14745

Merged
merged 4 commits into from
Jun 30, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
5 changes: 3 additions & 2 deletions pkg/cmd/server/origin/asset.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
configapi "github.com/openshift/origin/pkg/cmd/server/api"
"github.com/openshift/origin/pkg/cmd/server/crypto"
cmdutil "github.com/openshift/origin/pkg/cmd/util"
oauthutil "github.com/openshift/origin/pkg/oauth/util"
Copy link
Contributor

Choose a reason for hiding this comment

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

This package dependency was previously excluded intentionally. The asset server should not depend on the oauth package at all. The two are logically independent.

oversion "github.com/openshift/origin/pkg/version"
)

Expand Down Expand Up @@ -217,8 +218,8 @@ func (c *AssetConfig) addHandlers(handler http.Handler) (http.Handler, error) {
KubernetesAddr: masterURL.Host,
KubernetesPrefix: server.DefaultLegacyAPIPrefix,
KubernetesResources: k8sResources.List(),
OAuthAuthorizeURI: OpenShiftOAuthAuthorizeURL(masterURL.String()),
OAuthTokenURI: OpenShiftOAuthTokenURL(masterURL.String()),
OAuthAuthorizeURI: oauthutil.OpenShiftOAuthAuthorizeURL(masterURL.String()),
OAuthTokenURI: oauthutil.OpenShiftOAuthTokenURL(masterURL.String()),
OAuthRedirectBase: c.Options.PublicURL,
OAuthClientID: OpenShiftWebConsoleClientID,
LogoutURI: c.Options.LogoutURL,
Expand Down
32 changes: 11 additions & 21 deletions pkg/cmd/server/origin/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,11 @@ import (
clientauthetcd "github.com/openshift/origin/pkg/oauth/registry/oauthclientauthorization/etcd"
"github.com/openshift/origin/pkg/oauth/server/osinserver"
"github.com/openshift/origin/pkg/oauth/server/osinserver/registrystorage"
oauthutil "github.com/openshift/origin/pkg/oauth/util"
saoauth "github.com/openshift/origin/pkg/serviceaccounts/oauthclient"
)

const (
OpenShiftOAuthAPIPrefix = "/oauth"
openShiftLoginPrefix = "/login"
openShiftApproveSubpath = "approve"
OpenShiftOAuthCallbackPrefix = "/oauth2callback"
Expand Down Expand Up @@ -155,7 +155,7 @@ func (c *AuthConfig) WithOAuth(handler http.Handler) (http.Handler, error) {
},
osinserver.NewDefaultErrorHandler(),
)
server.Install(mux, OpenShiftOAuthAPIPrefix)
server.Install(mux, oauthutil.OpenShiftOAuthAPIPrefix)

if err := CreateOrUpdateDefaultOAuthClients(c.Options.MasterPublicURL, c.AssetPublicAddresses, clientRegistry); err != nil {
glog.Fatal(err)
Expand All @@ -165,7 +165,7 @@ func (c *AuthConfig) WithOAuth(handler http.Handler) (http.Handler, error) {
glog.Fatal(err)
}
osOAuthClientConfig := c.NewOpenShiftOAuthClientConfig(browserClient)
osOAuthClientConfig.RedirectUrl = c.Options.MasterPublicURL + path.Join(OpenShiftOAuthAPIPrefix, tokenrequest.DisplayTokenEndpoint)
osOAuthClientConfig.RedirectUrl = c.Options.MasterPublicURL + path.Join(oauthutil.OpenShiftOAuthAPIPrefix, tokenrequest.DisplayTokenEndpoint)

osOAuthClient, _ := osincli.NewClient(osOAuthClientConfig)
if len(*c.Options.MasterCA) > 0 {
Expand All @@ -180,7 +180,7 @@ func (c *AuthConfig) WithOAuth(handler http.Handler) (http.Handler, error) {
}

tokenRequestEndpoints := tokenrequest.NewEndpoints(c.Options.MasterPublicURL, osOAuthClient)
tokenRequestEndpoints.Install(mux, OpenShiftOAuthAPIPrefix)
tokenRequestEndpoints.Install(mux, oauthutil.OpenShiftOAuthAPIPrefix)

// glog.Infof("oauth server configured as: %#v", server)
// glog.Infof("auth handler: %#v", authHandler)
Expand Down Expand Up @@ -224,23 +224,13 @@ func (c *AuthConfig) NewOpenShiftOAuthClientConfig(client *oauthapi.OAuthClient)
ClientSecret: client.Secret,
ErrorsInStatusCode: true,
SendClientSecretInParams: true,
AuthorizeUrl: OpenShiftOAuthAuthorizeURL(c.Options.MasterPublicURL),
TokenUrl: OpenShiftOAuthTokenURL(c.Options.MasterURL),
AuthorizeUrl: oauthutil.OpenShiftOAuthAuthorizeURL(c.Options.MasterPublicURL),
TokenUrl: oauthutil.OpenShiftOAuthTokenURL(c.Options.MasterURL),
Scope: "",
}
return config
}

func OpenShiftOAuthAuthorizeURL(masterAddr string) string {
return masterAddr + path.Join(OpenShiftOAuthAPIPrefix, osinserver.AuthorizePath)
}
func OpenShiftOAuthTokenURL(masterAddr string) string {
return masterAddr + path.Join(OpenShiftOAuthAPIPrefix, osinserver.TokenPath)
}
func OpenShiftOAuthTokenRequestURL(masterAddr string) string {
return masterAddr + path.Join(OpenShiftOAuthAPIPrefix, tokenrequest.RequestTokenEndpoint)
}

func ensureOAuthClient(client oauthapi.OAuthClient, clientRegistry clientregistry.Registry, preserveExistingRedirects, preserveExistingSecret bool) error {
ctx := apirequest.NewContext()
_, err := clientRegistry.CreateClient(ctx, &client)
Expand Down Expand Up @@ -306,7 +296,7 @@ func CreateOrUpdateDefaultOAuthClients(masterPublicAddr string, assetPublicAddre
ObjectMeta: metav1.ObjectMeta{Name: OpenShiftBrowserClientID},
Secret: uuid.New(),
RespondWithChallenges: false,
RedirectURIs: []string{masterPublicAddr + path.Join(OpenShiftOAuthAPIPrefix, tokenrequest.DisplayTokenEndpoint)},
RedirectURIs: []string{masterPublicAddr + path.Join(oauthutil.OpenShiftOAuthAPIPrefix, tokenrequest.DisplayTokenEndpoint)},
GrantMethod: oauthapi.GrantHandlerAuto,
}
if err := ensureOAuthClient(browserClient, clientRegistry, true, true); err != nil {
Expand All @@ -319,7 +309,7 @@ func CreateOrUpdateDefaultOAuthClients(masterPublicAddr string, assetPublicAddre
ObjectMeta: metav1.ObjectMeta{Name: OpenShiftCLIClientID},
Secret: "",
RespondWithChallenges: true,
RedirectURIs: []string{masterPublicAddr + path.Join(OpenShiftOAuthAPIPrefix, tokenrequest.ImplicitTokenEndpoint)},
RedirectURIs: []string{masterPublicAddr + path.Join(oauthutil.OpenShiftOAuthAPIPrefix, tokenrequest.ImplicitTokenEndpoint)},
GrantMethod: oauthapi.GrantHandlerAuto,
}
if err := ensureOAuthClient(cliClient, clientRegistry, false, false); err != nil {
Expand Down Expand Up @@ -360,7 +350,7 @@ func (c *AuthConfig) getGrantHandler(mux cmdutil.Mux, auth authenticator.Request
// 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, path.Join(OpenShiftOAuthAPIPrefix, osinserver.AuthorizePath, openShiftApproveSubpath))
grantServer.Install(mux, path.Join(oauthutil.OpenShiftOAuthAPIPrefix, osinserver.AuthorizePath, openShiftApproveSubpath))

// Set defaults for standard clients. These can be overridden.
return handlers.NewPerClientGrant(
Expand Down Expand Up @@ -497,7 +487,7 @@ func (c *AuthConfig) getAuthenticationHandler(mux cmdutil.Mux, errorHandler hand
}
} else if requestHeaderProvider, isRequestHeader := identityProvider.Provider.(*configapi.RequestHeaderIdentityProvider); isRequestHeader {
// We might be redirecting to an external site, we need to fully resolve the request URL to the public master
baseRequestURL, err := url.Parse(c.Options.MasterPublicURL + OpenShiftOAuthAPIPrefix + osinserver.AuthorizePath)
baseRequestURL, err := url.Parse(c.Options.MasterPublicURL + oauthutil.OpenShiftOAuthAPIPrefix + osinserver.AuthorizePath)
if err != nil {
return nil, err
}
Expand All @@ -512,7 +502,7 @@ func (c *AuthConfig) getAuthenticationHandler(mux cmdutil.Mux, errorHandler hand

if redirectors.Count() > 0 && len(challengers) == 0 {
// Add a default challenger that will warn and give a link to the web browser token-granting location
challengers["placeholder"] = placeholderchallenger.New(OpenShiftOAuthTokenRequestURL(c.Options.MasterPublicURL))
challengers["placeholder"] = placeholderchallenger.New(oauthutil.OpenShiftOAuthTokenRequestURL(c.Options.MasterPublicURL))
}

var selectProviderTemplateFile string
Expand Down
3 changes: 2 additions & 1 deletion pkg/cmd/server/origin/master.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
serverhandlers "github.com/openshift/origin/pkg/cmd/server/handlers"
cmdutil "github.com/openshift/origin/pkg/cmd/util"
"github.com/openshift/origin/pkg/cmd/util/plug"
oauthutil "github.com/openshift/origin/pkg/oauth/util"
routeplugin "github.com/openshift/origin/pkg/route/allocation/simple"
routeallocationcontroller "github.com/openshift/origin/pkg/route/controller/allocation"
sccstorage "github.com/openshift/origin/pkg/security/registry/securitycontextconstraints/etcd"
Expand Down Expand Up @@ -179,7 +180,7 @@ func (c *MasterConfig) Run(kubeAPIServerConfig *kubeapiserver.Config, assetConfi

func (c *MasterConfig) buildHandlerChain(assetConfig *AssetConfig) (func(http.Handler, *apiserver.Config) (secure http.Handler), error) {
if c.Options.OAuthConfig != nil {
glog.Infof("Starting OAuth2 API at %s", OpenShiftOAuthAPIPrefix)
glog.Infof("Starting OAuth2 API at %s", oauthutil.OpenShiftOAuthAPIPrefix)
}

if assetConfig != nil {
Expand Down
4 changes: 2 additions & 2 deletions pkg/cmd/server/origin/nonapiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (

"github.com/openshift/origin/pkg/cmd/server/bootstrappolicy"
"github.com/openshift/origin/pkg/cmd/util/plug"
"github.com/openshift/origin/pkg/oauth/discovery"
oauthutil "github.com/openshift/origin/pkg/oauth/util"
openservicebrokerserver "github.com/openshift/origin/pkg/openservicebroker/server"
templateapi "github.com/openshift/origin/pkg/template/apis/template"
templateinformer "github.com/openshift/origin/pkg/template/generated/informers/internalversion"
Expand Down Expand Up @@ -106,7 +106,7 @@ const (
// 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(discovery.Get(masterPublicURL, OpenShiftOAuthAuthorizeURL(masterPublicURL), OpenShiftOAuthTokenURL(masterPublicURL)), "", " ")
metadata, err := json.MarshalIndent(oauthutil.GetOauthMetadata(masterPublicURL), "", " ")
if err != nil {
glog.Errorf("Unable to initialize OAuth authorization server metadata route: %v", err)
return
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package discovery
package util

import (
"github.com/RangelReale/osin"
Expand Down Expand Up @@ -38,12 +38,12 @@ type OauthAuthorizationServerMetadata struct {
CodeChallengeMethodsSupported []string `json:"code_challenge_methods_supported"`
}

func Get(masterPublicURL, authorizeURL, tokenURL string) OauthAuthorizationServerMetadata {
func GetOauthMetadata(masterPublicURL string) OauthAuthorizationServerMetadata {
config := osinserver.NewDefaultServerConfig()
return OauthAuthorizationServerMetadata{
Issuer: masterPublicURL,
AuthorizationEndpoint: authorizeURL,
TokenEndpoint: tokenURL,
AuthorizationEndpoint: OpenShiftOAuthAuthorizeURL(masterPublicURL),
TokenEndpoint: OpenShiftOAuthTokenURL(masterPublicURL),
// Note: this list is incomplete, which is allowed per the draft spec
ScopesSupported: scope.DefaultSupportedScopes(),
ResponseTypesSupported: config.AllowedAuthorizeTypes,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package discovery
package util

import (
"reflect"
Expand All @@ -7,18 +7,18 @@ import (
"github.com/RangelReale/osin"
)

func TestGet(t *testing.T) {
actual := Get("https://localhost:8443", "https://localhost:8443/oauth/authorize", "https://localhost:8443/oauth/token")
func TestGetOauthMetadata(t *testing.T) {
actual := GetOauthMetadata("https://localhost:8443")
Copy link
Contributor

Choose a reason for hiding this comment

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

Capitalize the A in OAuth: GetOauthMetadata -> GetOAuthMetadata (sorry I missed this earlier)

Copy link
Contributor

Choose a reason for hiding this comment

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

nvm, this is consistent with the containing file and the whole code base is really inconsistent on this anyway.

expected := OauthAuthorizationServerMetadata{
Issuer: "https://localhost:8443",
AuthorizationEndpoint: "https://localhost:8443/oauth/authorize",
TokenEndpoint: "https://localhost:8443/oauth/token",
ScopesSupported: []string{
"user:check-access",
"user:full",
"user:info",
"user:check-access",
"user:list-scoped-projects",
"user:list-projects",
"user:list-scoped-projects",
},
ResponseTypesSupported: osin.AllowedAuthorizeType{
"code",
Expand Down
22 changes: 22 additions & 0 deletions pkg/oauth/util/url.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package util

import (
"path"

"github.com/openshift/origin/pkg/auth/server/tokenrequest"
"github.com/openshift/origin/pkg/oauth/server/osinserver"
)

const (
OpenShiftOAuthAPIPrefix = "/oauth"
)

func OpenShiftOAuthAuthorizeURL(masterAddr string) string {
return masterAddr + path.Join(OpenShiftOAuthAPIPrefix, osinserver.AuthorizePath)
}
func OpenShiftOAuthTokenURL(masterAddr string) string {
return masterAddr + path.Join(OpenShiftOAuthAPIPrefix, osinserver.TokenPath)
}
func OpenShiftOAuthTokenRequestURL(masterAddr string) string {
return masterAddr + path.Join(OpenShiftOAuthAPIPrefix, tokenrequest.RequestTokenEndpoint)
}
4 changes: 2 additions & 2 deletions test/integration/auth_proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import (
ktransport "k8s.io/client-go/transport"

configapi "github.com/openshift/origin/pkg/cmd/server/api"
"github.com/openshift/origin/pkg/cmd/server/origin"
oauthapi "github.com/openshift/origin/pkg/oauth/apis/oauth"
oauthutil "github.com/openshift/origin/pkg/oauth/util"
testutil "github.com/openshift/origin/test/util"
testserver "github.com/openshift/origin/test/util/server"
)
Expand Down Expand Up @@ -66,7 +66,7 @@ func TestAuthProxyOnAuthorize(t *testing.T) {
}

// our simple URL to get back a code. We want to go through the front proxy
rawAuthorizeRequest := proxyServer.URL + origin.OpenShiftOAuthAPIPrefix + "/authorize?response_type=code&client_id=test"
rawAuthorizeRequest := proxyServer.URL + oauthutil.OpenShiftOAuthAPIPrefix + "/authorize?response_type=code&client_id=test"

// the first request we make to the front proxy should challenge us for authentication info
shouldBeAChallengeResponse, err := http.Get(rawAuthorizeRequest)
Expand Down