Skip to content

Commit 8aa0ec1

Browse files
harshavardhanaharshavardhana
authored
simplify the provider config init, loading and allow reachable IDPs (#3168)
fixes #3018
1 parent a8c5b53 commit 8aa0ec1

File tree

4 files changed

+158
-110
lines changed

4 files changed

+158
-110
lines changed

‎Makefile

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -254,19 +254,19 @@ test-initialize-minio-nginx: test-start-docker-minio-w-redirect-url test-start-d
254254
cleanup-minio-nginx:
255255
@(docker stop minio test-nginx & docker network rm test-network)
256256

257+
# https://stackoverflow.com/questions/19200235/golang-tests-in-sub-directory
258+
# Note: go test ./... will run tests on the current folder and all subfolders.
259+
# This is needed because tests can be in the folder or sub-folder(s), let's include them all please!.
257260
test:
258261
@echo "execute test and get coverage"
259-
# https://stackoverflow.com/questions/19200235/golang-tests-in-sub-directory
260-
# Note: go test ./... will run tests on the current folder and all subfolders.
261-
# This is needed because tests can be in the folder or sub-folder(s), let's include them all please!.
262262
@(cd restapi && mkdir -p coverage && GO111MODULE=on go test ./... -test.v -coverprofile=coverage/coverage.out)
263263

264264

265+
# https://stackoverflow.com/questions/19200235/golang-tests-in-sub-directory
266+
# Note: go test ./... will run tests on the current folder and all subfolders.
267+
# This is since tests in pkg folder are in subfolders and were not executed.
265268
test-pkg:
266269
@echo "execute test and get coverage"
267-
# https://stackoverflow.com/questions/19200235/golang-tests-in-sub-directory
268-
# Note: go test ./... will run tests on the current folder and all subfolders.
269-
# This is since tests in pkg folder are in subfolders and were not executed.
270270
@(cd pkg && mkdir -p coverage && GO111MODULE=on go test ./... -test.v -coverprofile=coverage/coverage-pkg.out)
271271

272272
coverage:

‎pkg/auth/idp/oauth2/config.go

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,16 @@ package oauth2
2020

2121
import (
2222
"crypto/sha1"
23+
"fmt"
24+
"net/http"
2325
"strings"
2426

2527
"github.com/minio/console/pkg/auth/token"
28+
"github.com/minio/minio-go/v7/pkg/set"
2629
"github.com/minio/pkg/v2/env"
2730
"golang.org/x/crypto/pbkdf2"
31+
"golang.org/x/oauth2"
32+
xoauth2 "golang.org/x/oauth2"
2833
)
2934

3035
// ProviderConfig - OpenID IDP Configuration for console.
@@ -41,8 +46,82 @@ type ProviderConfig struct {
4146
RoleArn string // can be empty
4247
}
4348

49+
// GetOauth2Provider instantiates a new oauth2 client using the configured credentials
50+
// it returns a *Provider object that contains the necessary configuration to initiate an
51+
// oauth2 authentication flow.
52+
//
53+
// We only support Authentication with the Authorization Code Flow - spec:
54+
// https://openid.net/specs/openid-connect-core-1_0.html#CodeFlowAuth
55+
func (pc ProviderConfig) GetOauth2Provider(name string, scopes []string, r *http.Request, idpClient, stsClient *http.Client) (provider *Provider, err error) {
56+
var ddoc DiscoveryDoc
57+
ddoc, err = parseDiscoveryDoc(r.Context(), pc.URL, idpClient)
58+
if err != nil {
59+
return nil, err
60+
}
61+
62+
supportedResponseTypes := set.NewStringSet()
63+
for _, responseType := range ddoc.ResponseTypesSupported {
64+
// FIXME: ResponseTypesSupported is a JSON array of strings - it
65+
// may not actually have strings with spaces inside them -
66+
// making the following code unnecessary.
67+
for _, s := range strings.Fields(responseType) {
68+
supportedResponseTypes.Add(s)
69+
}
70+
}
71+
72+
isSupported := requiredResponseTypes.Difference(supportedResponseTypes).IsEmpty()
73+
if !isSupported {
74+
return nil, fmt.Errorf("expected 'code' response type - got %s, login not allowed", ddoc.ResponseTypesSupported)
75+
}
76+
77+
// If provided scopes are empty we use the user configured list or a default list.
78+
if len(scopes) == 0 {
79+
for _, s := range strings.Split(pc.Scopes, ",") {
80+
w := strings.TrimSpace(s)
81+
if w == "" {
82+
continue
83+
}
84+
scopes = append(scopes, w)
85+
}
86+
if len(scopes) == 0 {
87+
scopes = defaultScopes
88+
}
89+
}
90+
91+
redirectURL := pc.RedirectCallback
92+
if pc.RedirectCallbackDynamic {
93+
// dynamic redirect if set, will generate redirect URLs
94+
// dynamically based on incoming requests.
95+
redirectURL = getLoginCallbackURL(r)
96+
}
97+
98+
// add "openid" scope always.
99+
scopes = append(scopes, "openid")
100+
101+
client := new(Provider)
102+
client.oauth2Config = &xoauth2.Config{
103+
ClientID: pc.ClientID,
104+
ClientSecret: pc.ClientSecret,
105+
RedirectURL: redirectURL,
106+
Endpoint: oauth2.Endpoint{
107+
AuthURL: ddoc.AuthEndpoint,
108+
TokenURL: ddoc.TokenEndpoint,
109+
},
110+
Scopes: scopes,
111+
}
112+
113+
client.IDPName = name
114+
client.UserInfo = pc.Userinfo
115+
116+
client.provHTTPClient = idpClient
117+
client.stsHTTPClient = stsClient
118+
119+
return client, nil
120+
}
121+
44122
// GetStateKeyFunc - return the key function used to generate the authorization
45123
// code flow state parameter.
124+
46125
func (pc ProviderConfig) GetStateKeyFunc() StateKeyFunc {
47126
return func() []byte {
48127
return pbkdf2.Key([]byte(pc.HMACPassphrase), []byte(pc.HMACSalt), 4096, 32, sha1.New)

‎pkg/auth/idp/oauth2/provider.go

Lines changed: 22 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ var requiredResponseTypes = set.CreateStringSet("code")
154154
// We only support Authentication with the Authorization Code Flow - spec:
155155
// https://openid.net/specs/openid-connect-core-1_0.html#CodeFlowAuth
156156
func NewOauth2ProviderClient(scopes []string, r *http.Request, httpClient *http.Client) (*Provider, error) {
157-
ddoc, err := parseDiscoveryDoc(GetIDPURL(), httpClient)
157+
ddoc, err := parseDiscoveryDoc(r.Context(), GetIDPURL(), httpClient)
158158
if err != nil {
159159
return nil, err
160160
}
@@ -211,77 +211,34 @@ func NewOauth2ProviderClient(scopes []string, r *http.Request, httpClient *http.
211211

212212
var defaultScopes = []string{"openid", "profile", "email"}
213213

214+
// NewOauth2ProviderClientByName returns a provider if present specified by the input name of the provider.
215+
func (ois OpenIDPCfg) NewOauth2ProviderClientByName(name string, scopes []string, r *http.Request, idpClient, stsClient *http.Client) (provider *Provider, err error) {
216+
oi, ok := ois[name]
217+
if !ok {
218+
return nil, fmt.Errorf("%s IDP provider does not exist", name)
219+
}
220+
return oi.GetOauth2Provider(name, scopes, r, idpClient, stsClient)
221+
}
222+
214223
// NewOauth2ProviderClient instantiates a new oauth2 client using the
215224
// `OpenIDPCfg` configuration struct. It returns a *Provider object that
216225
// contains the necessary configuration to initiate an oauth2 authentication
217226
// flow.
218227
//
219228
// We only support Authentication with the Authorization Code Flow - spec:
220229
// https://openid.net/specs/openid-connect-core-1_0.html#CodeFlowAuth
221-
func (o OpenIDPCfg) NewOauth2ProviderClient(name string, scopes []string, r *http.Request, idpClient, stsClient *http.Client) (*Provider, error) {
222-
ddoc, err := parseDiscoveryDoc(o[name].URL, idpClient)
223-
if err != nil {
224-
return nil, err
225-
}
226-
227-
supportedResponseTypes := set.NewStringSet()
228-
for _, responseType := range ddoc.ResponseTypesSupported {
229-
// FIXME: ResponseTypesSupported is a JSON array of strings - it
230-
// may not actually have strings with spaces inside them -
231-
// making the following code unnecessary.
232-
for _, s := range strings.Fields(responseType) {
233-
supportedResponseTypes.Add(s)
234-
}
235-
}
236-
isSupported := requiredResponseTypes.Difference(supportedResponseTypes).IsEmpty()
237-
238-
if !isSupported {
239-
return nil, fmt.Errorf("expected 'code' response type - got %s, login not allowed", ddoc.ResponseTypesSupported)
240-
}
241-
242-
// If provided scopes are empty we use the user configured list or a default
243-
// list.
244-
if len(scopes) == 0 {
245-
scopesTmp := strings.Split(o[name].Scopes, ",")
246-
for _, s := range scopesTmp {
247-
w := strings.TrimSpace(s)
248-
if w != "" {
249-
scopes = append(scopes, w)
250-
}
251-
}
252-
if len(scopes) == 0 {
253-
scopes = defaultScopes
230+
func (ois OpenIDPCfg) NewOauth2ProviderClient(scopes []string, r *http.Request, idpClient, stsClient *http.Client) (provider *Provider, providerCfg ProviderConfig, err error) {
231+
for name, oi := range ois {
232+
provider, err = oi.GetOauth2Provider(name, scopes, r, idpClient, stsClient)
233+
if err != nil {
234+
// Upon error look for the next IDP.
235+
continue
254236
}
237+
// Upon success return right away.
238+
providerCfg = oi
239+
break
255240
}
256-
257-
redirectURL := o[name].RedirectCallback
258-
if o[name].RedirectCallbackDynamic {
259-
// dynamic redirect if set, will generate redirect URLs
260-
// dynamically based on incoming requests.
261-
redirectURL = getLoginCallbackURL(r)
262-
}
263-
264-
// add "openid" scope always.
265-
scopes = append(scopes, "openid")
266-
267-
client := new(Provider)
268-
client.oauth2Config = &xoauth2.Config{
269-
ClientID: o[name].ClientID,
270-
ClientSecret: o[name].ClientSecret,
271-
RedirectURL: redirectURL,
272-
Endpoint: oauth2.Endpoint{
273-
AuthURL: ddoc.AuthEndpoint,
274-
TokenURL: ddoc.TokenEndpoint,
275-
},
276-
Scopes: scopes,
277-
}
278-
279-
client.IDPName = name
280-
client.UserInfo = o[name].Userinfo
281-
282-
client.provHTTPClient = idpClient
283-
client.stsHTTPClient = stsClient
284-
return client, nil
241+
return provider, providerCfg, err
285242
}
286243

287244
type User struct {
@@ -427,9 +384,9 @@ func validateOauth2State(state string, keyFunc StateKeyFunc) error {
427384

428385
// parseDiscoveryDoc parses a discovery doc from an OAuth provider
429386
// into a DiscoveryDoc struct that have the correct endpoints
430-
func parseDiscoveryDoc(ustr string, httpClient *http.Client) (DiscoveryDoc, error) {
387+
func parseDiscoveryDoc(ctx context.Context, ustr string, httpClient *http.Client) (DiscoveryDoc, error) {
431388
d := DiscoveryDoc{}
432-
req, err := http.NewRequest(http.MethodGet, ustr, nil)
389+
req, err := http.NewRequestWithContext(ctx, http.MethodGet, ustr, nil)
433390
if err != nil {
434391
return d, err
435392
}

‎restapi/user_login.go

Lines changed: 51 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -177,58 +177,68 @@ func isKubernetes() bool {
177177
}
178178

179179
// getLoginDetailsResponse returns information regarding the Console authentication mechanism.
180-
func getLoginDetailsResponse(params authApi.LoginDetailParams, openIDProviders oauth2.OpenIDPCfg) (*models.LoginDetails, *CodedAPIError) {
181-
ctx, cancel := context.WithCancel(params.HTTPRequest.Context())
182-
defer cancel()
180+
func getLoginDetailsResponse(params authApi.LoginDetailParams, openIDProviders oauth2.OpenIDPCfg) (ld *models.LoginDetails, apiErr *CodedAPIError) {
183181
loginStrategy := models.LoginDetailsLoginStrategyForm
184182
var redirectRules []*models.RedirectRule
185183

186184
r := params.HTTPRequest
185+
187186
var loginDetails *models.LoginDetails
188-
if len(openIDProviders) >= 1 {
187+
if len(openIDProviders) > 0 {
189188
loginStrategy = models.LoginDetailsLoginStrategyRedirect
190-
for name, provider := range openIDProviders {
191-
// initialize new oauth2 client
192-
oauth2Client, err := openIDProviders.NewOauth2ProviderClient(name, nil, r, GetConsoleHTTPClient("", getClientIP(params.HTTPRequest)), GetConsoleHTTPClient(getMinIOServer(), getClientIP(params.HTTPRequest)))
193-
if err != nil {
194-
return nil, ErrorWithContext(ctx, err, ErrOauth2Provider)
195-
}
196-
// Validate user against IDP
197-
identityProvider := &auth.IdentityProvider{
198-
KeyFunc: provider.GetStateKeyFunc(),
199-
Client: oauth2Client,
200-
}
189+
}
201190

202-
displayName := fmt.Sprintf("Login with SSO (%s)", name)
203-
serviceType := ""
191+
for name, provider := range openIDProviders {
192+
// initialize new oauth2 client
204193

205-
if provider.DisplayName != "" {
206-
displayName = provider.DisplayName
207-
}
194+
oauth2Client, err := provider.GetOauth2Provider(name, nil, r, GetConsoleHTTPClient("", getClientIP(params.HTTPRequest)),
195+
GetConsoleHTTPClient(getMinIOServer(), getClientIP(params.HTTPRequest)))
196+
if err != nil {
197+
continue
198+
}
208199

209-
if provider.RoleArn != "" {
210-
splitRoleArn := strings.Split(provider.RoleArn, ":")
200+
// Validate user against IDP
201+
identityProvider := &auth.IdentityProvider{
202+
KeyFunc: provider.GetStateKeyFunc(),
203+
Client: oauth2Client,
204+
}
211205

212-
if len(splitRoleArn) > 2 {
213-
serviceType = splitRoleArn[2]
214-
}
215-
}
206+
displayName := fmt.Sprintf("Login with SSO (%s)", name)
207+
serviceType := ""
216208

217-
redirectRule := models.RedirectRule{
218-
Redirect: identityProvider.GenerateLoginURL(),
219-
DisplayName: displayName,
220-
ServiceType: serviceType,
209+
if provider.DisplayName != "" {
210+
displayName = provider.DisplayName
211+
}
212+
213+
if provider.RoleArn != "" {
214+
splitRoleArn := strings.Split(provider.RoleArn, ":")
215+
216+
if len(splitRoleArn) > 2 {
217+
serviceType = splitRoleArn[2]
221218
}
219+
}
222220

223-
redirectRules = append(redirectRules, &redirectRule)
221+
redirectRule := models.RedirectRule{
222+
Redirect: identityProvider.GenerateLoginURL(),
223+
DisplayName: displayName,
224+
ServiceType: serviceType,
224225
}
226+
227+
redirectRules = append(redirectRules, &redirectRule)
225228
}
229+
230+
if len(openIDProviders) > 0 && len(redirectRules) == 0 {
231+
loginStrategy = models.LoginDetailsLoginStrategyForm
232+
// No IDP configured fallback to username/password
233+
}
234+
226235
loginDetails = &models.LoginDetails{
227236
LoginStrategy: loginStrategy,
228237
RedirectRules: redirectRules,
229238
IsK8S: isKubernetes(),
230239
AnimatedLogin: getConsoleAnimatedLogin(),
231240
}
241+
232242
return loginDetails, nil
233243
}
234244

@@ -248,7 +258,7 @@ func getLoginOauth2AuthResponse(params authApi.LoginOauth2AuthParams, openIDProv
248258
r := params.HTTPRequest
249259
lr := params.Body
250260

251-
if openIDProviders != nil {
261+
if len(openIDProviders) > 0 {
252262
// we read state
253263
rState := *lr.State
254264

@@ -258,22 +268,24 @@ func getLoginOauth2AuthResponse(params authApi.LoginOauth2AuthParams, openIDProv
258268
}
259269

260270
var requestItems oauth2.LoginURLParams
261-
262-
err = json.Unmarshal(decodedRState, &requestItems)
263-
264-
if err != nil {
271+
if err = json.Unmarshal(decodedRState, &requestItems); err != nil {
265272
return nil, ErrorWithContext(ctx, err)
266273
}
267274

268275
IDPName := requestItems.IDPName
269276
state := requestItems.State
270-
providerCfg := openIDProviders[IDPName]
271277

272-
oauth2Client, err := openIDProviders.NewOauth2ProviderClient(IDPName, nil, r, GetConsoleHTTPClient("", getClientIP(params.HTTPRequest)), GetConsoleHTTPClient(getMinIOServer(), getClientIP(params.HTTPRequest)))
278+
providerCfg, ok := openIDProviders[IDPName]
279+
if !ok {
280+
return nil, ErrorWithContext(ctx, fmt.Errorf("selected IDP %s does not exist", IDPName))
281+
}
282+
283+
// Initialize new identity provider with new oauth2Client per IDPName
284+
oauth2Client, err := providerCfg.GetOauth2Provider(IDPName, nil, r, GetConsoleHTTPClient("", getClientIP(params.HTTPRequest)),
285+
GetConsoleHTTPClient(getMinIOServer(), getClientIP(params.HTTPRequest)))
273286
if err != nil {
274287
return nil, ErrorWithContext(ctx, err)
275288
}
276-
// initialize new identity provider
277289

278290
identityProvider := auth.IdentityProvider{
279291
KeyFunc: providerCfg.GetStateKeyFunc(),

0 commit comments

Comments
 (0)