Skip to content

Commit 96f1259

Browse files
simo5simo5
committed
Use helpers to get OAuth related Public URLs
Signed-off-by: Simo Sorce <[email protected]>
1 parent ed7ef90 commit 96f1259

File tree

6 files changed

+163
-122
lines changed

6 files changed

+163
-122
lines changed

Diff for: ‎pkg/cmd/server/apis/config/helpers.go

+20
Original file line numberDiff line numberDiff line change
@@ -642,3 +642,23 @@ func CIDRsOverlap(cidr1, cidr2 string) bool {
642642
}
643643
return ipNet1.Contains(ipNet2.IP) || ipNet2.Contains(ipNet1.IP)
644644
}
645+
646+
func GetMasterPublicURL(config *MasterConfig) string {
647+
master := ""
648+
if config.OAuthConfig != nil {
649+
master = config.OAuthConfig.MasterPublicURL
650+
} else if config.ExternalOAuthConfig != nil {
651+
master = config.ExternalOAuthConfig.MasterPublicURL
652+
}
653+
return master
654+
}
655+
656+
func GetAssetPublicURL(config *MasterConfig) string {
657+
asset := ""
658+
if config.OAuthConfig != nil {
659+
asset = config.OAuthConfig.AssetPublicURL
660+
} else if config.ExternalOAuthConfig != nil {
661+
asset = config.ExternalOAuthConfig.AssetPublicURL
662+
}
663+
return asset
664+
}

Diff for: ‎pkg/cmd/server/origin/master.go

+13-15
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ const (
3232
openShiftOAuthAPIPrefix = "/oauth"
3333
openShiftLoginPrefix = "/login"
3434
openShiftOAuthCallbackPrefix = "/oauth2callback"
35+
OpenShiftWebConsoleClientID = "openshift-web-console"
36+
OpenShiftBrowserClientID = "openshift-browser-client"
37+
OpenShiftCLIClientID = "openshift-challenging-client"
3538
)
3639

3740
func (c *MasterConfig) newOpenshiftAPIConfig(kubeAPIServerConfig apiserver.Config) (*OpenshiftAPIConfig, error) {
@@ -172,26 +175,22 @@ func (c *MasterConfig) newWebConsoleProxy() (http.Handler, error) {
172175
return proxyHandler, nil
173176
}
174177

175-
func (c *MasterConfig) newOAuthServerHandler(genericConfig *apiserver.Config) (http.Handler, map[string]apiserver.PostStartHookFunc, error) {
178+
func (c *MasterConfig) newOAuthServerHandler(genericConfig *apiserver.Config) (http.Handler, error) {
176179
if c.Options.OAuthConfig == nil {
177-
return http.NotFoundHandler(), nil, nil
180+
return http.NotFoundHandler(), nil
178181
}
179182

180183
config, err := NewOAuthServerConfigFromMasterConfig(c, genericConfig.SecureServingInfo.Listener)
181184
if err != nil {
182-
return nil, nil, err
185+
return nil, err
183186
}
184187
config.GenericConfig.AuditBackend = genericConfig.AuditBackend
185188
config.GenericConfig.AuditPolicyChecker = genericConfig.AuditPolicyChecker
186189
oauthServer, err := config.Complete().New(apiserver.EmptyDelegate)
187190
if err != nil {
188-
return nil, nil, err
191+
return nil, err
189192
}
190-
return oauthServer.GenericAPIServer.PrepareRun().GenericAPIServer.Handler.FullHandlerChain,
191-
map[string]apiserver.PostStartHookFunc{
192-
"oauth.openshift.io-StartOAuthClientsBootstrapping": config.StartOAuthClientsBootstrapping,
193-
},
194-
nil
193+
return oauthServer.GenericAPIServer.PrepareRun().GenericAPIServer.Handler.FullHandlerChain, nil
195194
}
196195

197196
func (c *MasterConfig) withAggregator(delegateAPIServer apiserver.DelegationTarget, kubeAPIServerConfig apiserver.Config, apiExtensionsInformers apiextensionsinformers.SharedInformerFactory) (*aggregatorapiserver.APIAggregator, error) {
@@ -367,15 +366,12 @@ func (c *MasterConfig) RunOpenShift(stopCh <-chan struct{}) error {
367366
}
368367

369368
func (c *MasterConfig) buildHandlerChain(genericConfig *apiserver.Config) (func(apiHandler http.Handler, kc *apiserver.Config) http.Handler, map[string]apiserver.PostStartHookFunc, error) {
370-
webconsolePublicURL := ""
371-
if c.Options.OAuthConfig != nil {
372-
webconsolePublicURL = c.Options.OAuthConfig.AssetPublicURL
373-
}
369+
webconsolePublicURL := configapi.GetAssetPublicURL(&c.Options)
374370
webconsoleProxyHandler, err := c.newWebConsoleProxy()
375371
if err != nil {
376372
return nil, nil, err
377373
}
378-
oauthServerHandler, extraPostStartHooks, err := c.newOAuthServerHandler(genericConfig)
374+
oauthServerHandler, err := c.newOAuthServerHandler(genericConfig)
379375
if err != nil {
380376
return nil, nil, err
381377
}
@@ -401,7 +397,9 @@ func (c *MasterConfig) buildHandlerChain(genericConfig *apiserver.Config) (func(
401397

402398
return handler
403399
},
404-
extraPostStartHooks,
400+
map[string]apiserver.PostStartHookFunc{
401+
"oauth.openshift.io-StartOAuthClientsBootstrapping": c.startOAuthClientsBootstrapping,
402+
},
405403
nil
406404
}
407405

Diff for: ‎pkg/cmd/server/origin/oauthclients.go

+127
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
package origin
2+
3+
import (
4+
"time"
5+
6+
"github.com/pborman/uuid"
7+
8+
"k8s.io/apimachinery/pkg/api/errors"
9+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
10+
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
11+
"k8s.io/apimachinery/pkg/util/sets"
12+
"k8s.io/apimachinery/pkg/util/wait"
13+
apiserver "k8s.io/apiserver/pkg/server"
14+
"k8s.io/client-go/util/retry"
15+
16+
oauthapi "github.com/openshift/api/oauth/v1"
17+
oauthclient "github.com/openshift/client-go/oauth/clientset/versioned/typed/oauth/v1"
18+
configapi "github.com/openshift/origin/pkg/cmd/server/apis/config"
19+
oauthutil "github.com/openshift/origin/pkg/oauth/util"
20+
)
21+
22+
func ensureOAuthClient(client oauthapi.OAuthClient, oauthClients oauthclient.OAuthClientInterface, preserveExistingRedirects, preserveExistingSecret bool) error {
23+
_, err := oauthClients.Create(&client)
24+
if err == nil || !errors.IsAlreadyExists(err) {
25+
return err
26+
}
27+
28+
return retry.RetryOnConflict(retry.DefaultRetry, func() error {
29+
existing, err := oauthClients.Get(client.Name, metav1.GetOptions{})
30+
if err != nil {
31+
return err
32+
}
33+
34+
// Ensure the correct challenge setting
35+
existing.RespondWithChallenges = client.RespondWithChallenges
36+
// Preserve an existing client secret
37+
if !preserveExistingSecret || len(existing.Secret) == 0 {
38+
existing.Secret = client.Secret
39+
}
40+
41+
// Preserve redirects for clients other than the CLI client
42+
// The CLI client doesn't care about the redirect URL, just the token or error fragment
43+
if preserveExistingRedirects {
44+
// Add in any redirects from the existing one
45+
// This preserves any additional customized redirects in the default clients
46+
redirects := sets.NewString(client.RedirectURIs...)
47+
for _, redirect := range existing.RedirectURIs {
48+
if !redirects.Has(redirect) {
49+
client.RedirectURIs = append(client.RedirectURIs, redirect)
50+
redirects.Insert(redirect)
51+
}
52+
}
53+
}
54+
existing.RedirectURIs = client.RedirectURIs
55+
56+
// If the GrantMethod is present, keep it for compatibility
57+
// If it is empty, assign the requested strategy.
58+
if len(existing.GrantMethod) == 0 {
59+
existing.GrantMethod = client.GrantMethod
60+
}
61+
62+
_, err = oauthClients.Update(existing)
63+
return err
64+
})
65+
}
66+
67+
// TODO, this moves to the `apiserver.go` when we have it for this group
68+
// TODO TODO, this actually looks a lot like a controller or an add-on manager style thing. Seems like we'd want to do this outside
69+
// EnsureBootstrapOAuthClients creates or updates the bootstrap oauth clients that openshift relies upon.
70+
func (c *MasterConfig) startOAuthClientsBootstrapping(context apiserver.PostStartHookContext) error {
71+
if c.Options.OAuthConfig == nil && c.Options.ExternalOAuthConfig == nil {
72+
return nil
73+
}
74+
assetPublicURL := configapi.GetAssetPublicURL(&c.Options)
75+
masterPublicURL := configapi.GetMasterPublicURL(&c.Options)
76+
oauthClient, err := oauthclient.NewForConfig(&c.PrivilegedLoopbackClientConfig)
77+
if err != nil {
78+
utilruntime.HandleError(err)
79+
return nil
80+
}
81+
oauthClientClient := oauthClient.OAuthClients()
82+
83+
// the TODO above still applies, but this makes it possible for this poststarthook to do its job with a split kubeapiserver and not run forever
84+
go func() {
85+
wait.PollUntil(1*time.Second, func() (done bool, err error) {
86+
webConsoleClient := oauthapi.OAuthClient{
87+
ObjectMeta: metav1.ObjectMeta{Name: OpenShiftWebConsoleClientID},
88+
Secret: "",
89+
RespondWithChallenges: false,
90+
RedirectURIs: []string{assetPublicURL},
91+
GrantMethod: oauthapi.GrantHandlerAuto,
92+
}
93+
if err := ensureOAuthClient(webConsoleClient, oauthClientClient, true, false); err != nil {
94+
utilruntime.HandleError(err)
95+
return false, nil
96+
}
97+
98+
browserClient := oauthapi.OAuthClient{
99+
ObjectMeta: metav1.ObjectMeta{Name: OpenShiftBrowserClientID},
100+
Secret: uuid.New(),
101+
RespondWithChallenges: false,
102+
RedirectURIs: []string{oauthutil.OpenShiftOAuthTokenDisplayURL(masterPublicURL)},
103+
GrantMethod: oauthapi.GrantHandlerAuto,
104+
}
105+
if err := ensureOAuthClient(browserClient, oauthClientClient, true, true); err != nil {
106+
utilruntime.HandleError(err)
107+
return false, nil
108+
}
109+
110+
cliClient := oauthapi.OAuthClient{
111+
ObjectMeta: metav1.ObjectMeta{Name: OpenShiftCLIClientID},
112+
Secret: "",
113+
RespondWithChallenges: true,
114+
RedirectURIs: []string{oauthutil.OpenShiftOAuthTokenImplicitURL(masterPublicURL)},
115+
GrantMethod: oauthapi.GrantHandlerAuto,
116+
}
117+
if err := ensureOAuthClient(cliClient, oauthClientClient, false, false); err != nil {
118+
utilruntime.HandleError(err)
119+
return false, nil
120+
}
121+
122+
return true, nil
123+
}, context.StopCh)
124+
}()
125+
126+
return nil
127+
}

Diff for: ‎pkg/oauthserver/oauthserver/auth.go

-48
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414
"github.com/RangelReale/osincli"
1515
"github.com/golang/glog"
1616

17-
kerrs "k8s.io/apimachinery/pkg/api/errors"
1817
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1918
knet "k8s.io/apimachinery/pkg/util/net"
2019
"k8s.io/apimachinery/pkg/util/sets"
@@ -24,7 +23,6 @@ import (
2423
kuser "k8s.io/apiserver/pkg/authentication/user"
2524
"k8s.io/apiserver/pkg/endpoints/request"
2625
"k8s.io/client-go/util/cert"
27-
"k8s.io/client-go/util/retry"
2826

2927
oauthapi "github.com/openshift/api/oauth/v1"
3028
oauthclient "github.com/openshift/client-go/oauth/clientset/versioned/typed/oauth/v1"
@@ -66,7 +64,6 @@ const (
6664
openShiftLoginPrefix = "/login"
6765
openShiftApproveSubpath = "approve"
6866
OpenShiftOAuthCallbackPrefix = "/oauth2callback"
69-
OpenShiftWebConsoleClientID = "openshift-web-console"
7067
OpenShiftBrowserClientID = "openshift-browser-client"
7168
OpenShiftCLIClientID = "openshift-challenging-client"
7269
)
@@ -218,51 +215,6 @@ func newOpenShiftOAuthClientConfig(clientId, clientSecret, masterPublicURL, mast
218215
return config
219216
}
220217

221-
func ensureOAuthClient(client oauthapi.OAuthClient, oauthClients oauthclient.OAuthClientInterface, preserveExistingRedirects, preserveExistingSecret bool) error {
222-
_, err := oauthClients.Create(&client)
223-
if err == nil || !kerrs.IsAlreadyExists(err) {
224-
return err
225-
}
226-
227-
return retry.RetryOnConflict(retry.DefaultRetry, func() error {
228-
existing, err := oauthClients.Get(client.Name, metav1.GetOptions{})
229-
if err != nil {
230-
return err
231-
}
232-
233-
// Ensure the correct challenge setting
234-
existing.RespondWithChallenges = client.RespondWithChallenges
235-
// Preserve an existing client secret
236-
if !preserveExistingSecret || len(existing.Secret) == 0 {
237-
existing.Secret = client.Secret
238-
}
239-
240-
// Preserve redirects for clients other than the CLI client
241-
// The CLI client doesn't care about the redirect URL, just the token or error fragment
242-
if preserveExistingRedirects {
243-
// Add in any redirects from the existing one
244-
// This preserves any additional customized redirects in the default clients
245-
redirects := sets.NewString(client.RedirectURIs...)
246-
for _, redirect := range existing.RedirectURIs {
247-
if !redirects.Has(redirect) {
248-
client.RedirectURIs = append(client.RedirectURIs, redirect)
249-
redirects.Insert(redirect)
250-
}
251-
}
252-
}
253-
existing.RedirectURIs = client.RedirectURIs
254-
255-
// If the GrantMethod is present, keep it for compatibility
256-
// If it is empty, assign the requested strategy.
257-
if len(existing.GrantMethod) == 0 {
258-
existing.GrantMethod = client.GrantMethod
259-
}
260-
261-
_, err = oauthClients.Update(existing)
262-
return err
263-
})
264-
}
265-
266218
// getCSRF returns the object responsible for generating and checking CSRF tokens
267219
func (c *OAuthServerConfig) getCSRF() csrf.CSRF {
268220
secure := isHTTPS(c.ExtraOAuthConfig.Options.MasterPublicURL)

Diff for: ‎pkg/oauthserver/oauthserver/oauth_apiserver.go

-56
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,11 @@ import (
55
"fmt"
66
"net/http"
77
"net/url"
8-
"time"
98

109
"github.com/pborman/uuid"
1110

12-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1311
"k8s.io/apimachinery/pkg/runtime"
1412
"k8s.io/apimachinery/pkg/runtime/serializer"
15-
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
16-
"k8s.io/apimachinery/pkg/util/wait"
1713
genericapifilters "k8s.io/apiserver/pkg/endpoints/filters"
1814
apirequest "k8s.io/apiserver/pkg/endpoints/request"
1915
"k8s.io/apiserver/pkg/features"
@@ -24,13 +20,11 @@ import (
2420
corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
2521
"k8s.io/client-go/rest"
2622

27-
oauthapi "github.com/openshift/api/oauth/v1"
2823
oauthclient "github.com/openshift/client-go/oauth/clientset/versioned/typed/oauth/v1"
2924
routeclient "github.com/openshift/client-go/route/clientset/versioned/typed/route/v1"
3025
userclient "github.com/openshift/client-go/user/clientset/versioned/typed/user/v1"
3126
configapi "github.com/openshift/origin/pkg/cmd/server/apis/config"
3227
"github.com/openshift/origin/pkg/cmd/server/apis/config/latest"
33-
oauthutil "github.com/openshift/origin/pkg/oauth/util"
3428
"github.com/openshift/origin/pkg/oauthserver/server/session"
3529
)
3630

@@ -224,53 +218,3 @@ func (c *OAuthServerConfig) buildHandlerChainForOAuth(startingHandler http.Handl
224218
handler = genericfilters.WithPanicRecovery(handler)
225219
return handler
226220
}
227-
228-
// TODO, this moves to the `apiserver.go` when we have it for this group
229-
// TODO TODO, this actually looks a lot like a controller or an add-on manager style thing. Seems like we'd want to do this outside
230-
// EnsureBootstrapOAuthClients creates or updates the bootstrap oauth clients that openshift relies upon.
231-
func (c *OAuthServerConfig) StartOAuthClientsBootstrapping(context genericapiserver.PostStartHookContext) error {
232-
// the TODO above still applies, but this makes it possible for this poststarthook to do its job with a split kubeapiserver and not run forever
233-
go func() {
234-
wait.PollUntil(1*time.Second, func() (done bool, err error) {
235-
webConsoleClient := oauthapi.OAuthClient{
236-
ObjectMeta: metav1.ObjectMeta{Name: OpenShiftWebConsoleClientID},
237-
Secret: "",
238-
RespondWithChallenges: false,
239-
RedirectURIs: c.ExtraOAuthConfig.AssetPublicAddresses,
240-
GrantMethod: oauthapi.GrantHandlerAuto,
241-
}
242-
if err := ensureOAuthClient(webConsoleClient, c.ExtraOAuthConfig.OAuthClientClient, true, false); err != nil {
243-
utilruntime.HandleError(err)
244-
return false, nil
245-
}
246-
247-
browserClient := oauthapi.OAuthClient{
248-
ObjectMeta: metav1.ObjectMeta{Name: OpenShiftBrowserClientID},
249-
Secret: uuid.New(),
250-
RespondWithChallenges: false,
251-
RedirectURIs: []string{oauthutil.OpenShiftOAuthTokenDisplayURL(c.ExtraOAuthConfig.Options.MasterPublicURL)},
252-
GrantMethod: oauthapi.GrantHandlerAuto,
253-
}
254-
if err := ensureOAuthClient(browserClient, c.ExtraOAuthConfig.OAuthClientClient, true, true); err != nil {
255-
utilruntime.HandleError(err)
256-
return false, nil
257-
}
258-
259-
cliClient := oauthapi.OAuthClient{
260-
ObjectMeta: metav1.ObjectMeta{Name: OpenShiftCLIClientID},
261-
Secret: "",
262-
RespondWithChallenges: true,
263-
RedirectURIs: []string{oauthutil.OpenShiftOAuthTokenImplicitURL(c.ExtraOAuthConfig.Options.MasterPublicURL)},
264-
GrantMethod: oauthapi.GrantHandlerAuto,
265-
}
266-
if err := ensureOAuthClient(cliClient, c.ExtraOAuthConfig.OAuthClientClient, false, false); err != nil {
267-
utilruntime.HandleError(err)
268-
return false, nil
269-
}
270-
271-
return true, nil
272-
}, context.StopCh)
273-
}()
274-
275-
return nil
276-
}

0 commit comments

Comments
 (0)