From dab4eb76646abb5af9bdc51c61f22182b2522c72 Mon Sep 17 00:00:00 2001 From: Alex <33497058+bexsoft@users.noreply.github.com> Date: Thu, 20 Oct 2022 20:08:54 -0500 Subject: [PATCH] Fixes to Multiple IDP support in console (#2392) Signed-off-by: Benjamin Perez --- models/login_details.go | 69 ++++++++++++++++-- models/redirect_rule.go | 70 +++++++++++++++++++ operatorapi/embedded_spec.go | 42 ++++++----- operatorapi/login.go | 13 +++- pkg/auth/idp.go | 5 +- pkg/auth/idp/oauth2/config.go | 6 +- pkg/auth/idp/oauth2/const.go | 1 + pkg/auth/idp/oauth2/provider.go | 44 +++++++++--- pkg/auth/idp/oauth2/provider_test.go | 2 +- portal-ui/src/screens/LoginPage/LoginPage.tsx | 37 ++++++---- portal-ui/src/screens/LoginPage/loginSlice.ts | 3 +- portal-ui/src/screens/LoginPage/types.ts | 8 ++- portal-ui/src/utils/sortFunctions.ts | 12 ++++ restapi/embedded_spec.go | 42 ++++++----- restapi/policy/policies.go | 1 - restapi/user_login.go | 68 ++++++++++++------ sso-integration/sso_test.go | 37 +++++++--- swagger-console.yml | 16 +++-- swagger-operator.yml | 16 +++-- 19 files changed, 380 insertions(+), 112 deletions(-) create mode 100644 models/redirect_rule.go diff --git a/models/login_details.go b/models/login_details.go index 53b319889..b703f8a2e 100644 --- a/models/login_details.go +++ b/models/login_details.go @@ -25,6 +25,7 @@ package models import ( "context" "encoding/json" + "strconv" "github.com/go-openapi/errors" "github.com/go-openapi/strfmt" @@ -37,9 +38,6 @@ import ( // swagger:model loginDetails type LoginDetails struct { - // display names - DisplayNames []string `json:"displayNames"` - // is direct p v IsDirectPV bool `json:"isDirectPV,omitempty"` @@ -47,8 +45,8 @@ type LoginDetails struct { // Enum: [form redirect service-account redirect-service-account] LoginStrategy string `json:"loginStrategy,omitempty"` - // redirect - Redirect []string `json:"redirect"` + // redirect rules + RedirectRules []*RedirectRule `json:"redirectRules"` } // Validate validates this login details @@ -59,6 +57,10 @@ func (m *LoginDetails) Validate(formats strfmt.Registry) error { res = append(res, err) } + if err := m.validateRedirectRules(formats); err != nil { + res = append(res, err) + } + if len(res) > 0 { return errors.CompositeValidationError(res...) } @@ -113,8 +115,63 @@ func (m *LoginDetails) validateLoginStrategy(formats strfmt.Registry) error { return nil } -// ContextValidate validates this login details based on context it is used +func (m *LoginDetails) validateRedirectRules(formats strfmt.Registry) error { + if swag.IsZero(m.RedirectRules) { // not required + return nil + } + + for i := 0; i < len(m.RedirectRules); i++ { + if swag.IsZero(m.RedirectRules[i]) { // not required + continue + } + + if m.RedirectRules[i] != nil { + if err := m.RedirectRules[i].Validate(formats); err != nil { + if ve, ok := err.(*errors.Validation); ok { + return ve.ValidateName("redirectRules" + "." + strconv.Itoa(i)) + } else if ce, ok := err.(*errors.CompositeError); ok { + return ce.ValidateName("redirectRules" + "." + strconv.Itoa(i)) + } + return err + } + } + + } + + return nil +} + +// ContextValidate validate this login details based on the context it is used func (m *LoginDetails) ContextValidate(ctx context.Context, formats strfmt.Registry) error { + var res []error + + if err := m.contextValidateRedirectRules(ctx, formats); err != nil { + res = append(res, err) + } + + if len(res) > 0 { + return errors.CompositeValidationError(res...) + } + return nil +} + +func (m *LoginDetails) contextValidateRedirectRules(ctx context.Context, formats strfmt.Registry) error { + + for i := 0; i < len(m.RedirectRules); i++ { + + if m.RedirectRules[i] != nil { + if err := m.RedirectRules[i].ContextValidate(ctx, formats); err != nil { + if ve, ok := err.(*errors.Validation); ok { + return ve.ValidateName("redirectRules" + "." + strconv.Itoa(i)) + } else if ce, ok := err.(*errors.CompositeError); ok { + return ce.ValidateName("redirectRules" + "." + strconv.Itoa(i)) + } + return err + } + } + + } + return nil } diff --git a/models/redirect_rule.go b/models/redirect_rule.go new file mode 100644 index 000000000..436474f14 --- /dev/null +++ b/models/redirect_rule.go @@ -0,0 +1,70 @@ +// Code generated by go-swagger; DO NOT EDIT. + +// This file is part of MinIO Console Server +// Copyright (c) 2022 MinIO, Inc. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . +// + +package models + +// This file was generated by the swagger tool. +// Editing this file might prove futile when you re-run the swagger generate command + +import ( + "context" + + "github.com/go-openapi/strfmt" + "github.com/go-openapi/swag" +) + +// RedirectRule redirect rule +// +// swagger:model redirectRule +type RedirectRule struct { + + // display name + DisplayName string `json:"displayName,omitempty"` + + // redirect + Redirect string `json:"redirect,omitempty"` +} + +// Validate validates this redirect rule +func (m *RedirectRule) Validate(formats strfmt.Registry) error { + return nil +} + +// ContextValidate validates this redirect rule based on context it is used +func (m *RedirectRule) ContextValidate(ctx context.Context, formats strfmt.Registry) error { + return nil +} + +// MarshalBinary interface implementation +func (m *RedirectRule) MarshalBinary() ([]byte, error) { + if m == nil { + return nil, nil + } + return swag.WriteJSON(m) +} + +// UnmarshalBinary interface implementation +func (m *RedirectRule) UnmarshalBinary(b []byte) error { + var res RedirectRule + if err := swag.ReadJSON(b, &res); err != nil { + return err + } + *m = res + return nil +} diff --git a/operatorapi/embedded_spec.go b/operatorapi/embedded_spec.go index 7dd8a56da..975b6bdbe 100644 --- a/operatorapi/embedded_spec.go +++ b/operatorapi/embedded_spec.go @@ -3623,12 +3623,6 @@ func init() { "loginDetails": { "type": "object", "properties": { - "displayNames": { - "type": "array", - "items": { - "type": "string" - } - }, "isDirectPV": { "type": "boolean" }, @@ -3641,10 +3635,10 @@ func init() { "redirect-service-account" ] }, - "redirect": { + "redirectRules": { "type": "array", "items": { - "type": "string" + "$ref": "#/definitions/redirectRule" } } } @@ -4396,6 +4390,17 @@ func init() { } } }, + "redirectRule": { + "type": "object", + "properties": { + "displayName": { + "type": "string" + }, + "redirect": { + "type": "string" + } + } + }, "resourceQuota": { "type": "object", "properties": { @@ -9614,12 +9619,6 @@ func init() { "loginDetails": { "type": "object", "properties": { - "displayNames": { - "type": "array", - "items": { - "type": "string" - } - }, "isDirectPV": { "type": "boolean" }, @@ -9632,10 +9631,10 @@ func init() { "redirect-service-account" ] }, - "redirect": { + "redirectRules": { "type": "array", "items": { - "type": "string" + "$ref": "#/definitions/redirectRule" } } } @@ -10252,6 +10251,17 @@ func init() { } } }, + "redirectRule": { + "type": "object", + "properties": { + "displayName": { + "type": "string" + }, + "redirect": { + "type": "string" + } + } + }, "resourceQuota": { "type": "object", "properties": { diff --git a/operatorapi/login.go b/operatorapi/login.go index 1a65255be..669e51393 100644 --- a/operatorapi/login.go +++ b/operatorapi/login.go @@ -101,7 +101,8 @@ func getLoginDetailsResponse(params authApi.LoginDetailParams) (*models.LoginDet r := params.HTTPRequest loginStrategy := models.LoginDetailsLoginStrategyServiceDashAccount - redirectURL := []string{} + + var redirectRules []*models.RedirectRule if oauth2.IsIDPEnabled() { loginStrategy = models.LoginDetailsLoginStrategyRedirectDashServiceDashAccount @@ -115,12 +116,18 @@ func getLoginDetailsResponse(params authApi.LoginDetailParams) (*models.LoginDet KeyFunc: oauth2.DefaultDerivedKey, Client: oauth2Client, } - redirectURL = append(redirectURL, identityProvider.GenerateLoginURL()) + + newRedirectRule := &models.RedirectRule{ + Redirect: identityProvider.GenerateLoginURL(), + DisplayName: "Login with SSO", + } + + redirectRules = append(redirectRules, newRedirectRule) } loginDetails := &models.LoginDetails{ LoginStrategy: loginStrategy, - Redirect: redirectURL, + RedirectRules: redirectRules, IsDirectPV: getDirectPVEnabled(), } return loginDetails, nil diff --git a/pkg/auth/idp.go b/pkg/auth/idp.go index cc47c407d..8c39f18e0 100644 --- a/pkg/auth/idp.go +++ b/pkg/auth/idp.go @@ -40,11 +40,12 @@ type IdentityProviderI interface { type IdentityProvider struct { KeyFunc oauth2.StateKeyFunc Client *oauth2.Provider + RoleARN string } // VerifyIdentity will verify the user identity against the idp using the authorization code flow func (c IdentityProvider) VerifyIdentity(ctx context.Context, code, state string) (*credentials.Credentials, error) { - return c.Client.VerifyIdentity(ctx, code, state, c.KeyFunc) + return c.Client.VerifyIdentity(ctx, code, state, c.RoleARN, c.KeyFunc) } // VerifyIdentityForOperator will verify the user identity against the idp using the authorization code flow @@ -54,5 +55,5 @@ func (c IdentityProvider) VerifyIdentityForOperator(ctx context.Context, code, s // GenerateLoginURL returns a new URL used by the user to login against the idp func (c IdentityProvider) GenerateLoginURL() string { - return c.Client.GenerateLoginURL(c.KeyFunc) + return c.Client.GenerateLoginURL(c.KeyFunc, c.Client.IDPName) } diff --git a/pkg/auth/idp/oauth2/config.go b/pkg/auth/idp/oauth2/config.go index b90bdb3c1..5409a88ba 100644 --- a/pkg/auth/idp/oauth2/config.go +++ b/pkg/auth/idp/oauth2/config.go @@ -48,9 +48,11 @@ func (pc ProviderConfig) GetStateKeyFunc() StateKeyFunc { } } -type OpenIDPCfg map[string]ProviderConfig +func (pc ProviderConfig) GetARNInf() string { + return pc.RoleArn +} -var DefaultIDPConfig = "_" +type OpenIDPCfg map[string]ProviderConfig func GetSTSEndpoint() string { return strings.TrimSpace(env.Get(ConsoleMinIOServer, "http://localhost:9000")) diff --git a/pkg/auth/idp/oauth2/const.go b/pkg/auth/idp/oauth2/const.go index 6fe1971a2..a4d57d4ad 100644 --- a/pkg/auth/idp/oauth2/const.go +++ b/pkg/auth/idp/oauth2/const.go @@ -29,4 +29,5 @@ const ( ConsoleIDPScopes = "CONSOLE_IDP_SCOPES" ConsoleIDPUserInfo = "CONSOLE_IDP_USERINFO" ConsoleIDPTokenExpiration = "CONSOLE_IDP_TOKEN_EXPIRATION" + ConsoleIDPRoleARN = "CONSOLE_IDP_ROLE_ARN" ) diff --git a/pkg/auth/idp/oauth2/provider.go b/pkg/auth/idp/oauth2/provider.go index d41767a89..54ddf1fcb 100644 --- a/pkg/auth/idp/oauth2/provider.go +++ b/pkg/auth/idp/oauth2/provider.go @@ -92,13 +92,13 @@ func (ac Config) TokenSource(ctx context.Context, t *xoauth2.Token) xoauth2.Toke type Provider struct { // oauth2Config is an interface configuration that contains the following fields // Config{ - // ClientID string + // IDPName string // ClientSecret string // RedirectURL string // Endpoint oauth2.Endpoint // Scopes []string // } - // - ClientID is the public identifier for this application + // - IDPName is the public identifier for this application // - ClientSecret is a shared secret between this application and the authorization server // - RedirectURL is the URL to redirect users going through // the OAuth flow, after the resource owner's URLs. @@ -107,7 +107,7 @@ type Provider struct { // often available via site-specific packages, such as // google.Endpoint or github.Endpoint. // - Scopes specifies optional requested permissions. - ClientID string + IDPName string // if enabled means that we need extrace access_token as well UserInfo bool oauth2Config Configuration @@ -178,6 +178,7 @@ func NewOauth2ProviderClient(scopes []string, r *http.Request, httpClient *http. } redirectURL := GetIDPCallbackURL() + if GetIDPCallbackURLDynamic() { // dynamic redirect if set, will generate redirect URLs // dynamically based on incoming requests. @@ -199,7 +200,7 @@ func NewOauth2ProviderClient(scopes []string, r *http.Request, httpClient *http. Scopes: scopes, } - client.ClientID = GetIDPClientID() + client.IDPName = GetIDPClientID() client.UserInfo = GetIDPUserInfo() client.provHTTPClient = httpClient @@ -273,7 +274,7 @@ func (o OpenIDPCfg) NewOauth2ProviderClient(name string, scopes []string, r *htt Scopes: scopes, } - client.ClientID = o[name].ClientID + client.IDPName = name client.UserInfo = o[name].Userinfo client.provHTTPClient = httpClient return client, nil @@ -310,9 +311,10 @@ type StateKeyFunc func() []byte // VerifyIdentity will contact the configured IDP to the user identity based on the authorization code and state // if the user is valid, then it will contact MinIO to get valid sts credentials based on the identity provided by the IDP -func (client *Provider) VerifyIdentity(ctx context.Context, code, state string, keyFunc StateKeyFunc) (*credentials.Credentials, error) { +func (client *Provider) VerifyIdentity(ctx context.Context, code, state, roleARN string, keyFunc StateKeyFunc) (*credentials.Credentials, error) { // verify the provided state is valid (prevents CSRF attacks) if err := validateOauth2State(state, keyFunc); err != nil { + fmt.Println("err1", err) return nil, err } getWebTokenExpiry := func() (*credentials.WebIdentityToken, error) { @@ -352,10 +354,12 @@ func (client *Provider) VerifyIdentity(ctx context.Context, code, state string, return token, nil } stsEndpoint := GetSTSEndpoint() + sts := credentials.New(&credentials.STSWebIdentity{ Client: client.provHTTPClient, STSEndpoint: stsEndpoint, GetWebIDTokenExpiry: getWebTokenExpiry, + RoleARN: roleARN, }) return sts, nil } @@ -439,10 +443,34 @@ func GetRandomStateWithHMAC(length int, keyFunc StateKeyFunc) string { return base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf("%s:%s", state, hmac))) } +type LoginURLParams struct { + State string `json:"state"` + IDPName string `json:"idp_name"` +} + // GenerateLoginURL returns a new login URL based on the configured IDP -func (client *Provider) GenerateLoginURL(keyFunc StateKeyFunc) string { +func (client *Provider) GenerateLoginURL(keyFunc StateKeyFunc, iDPName string) string { // generates random state and sign it using HMAC256 state := GetRandomStateWithHMAC(25, keyFunc) - loginURL := client.oauth2Config.AuthCodeURL(state) + + configureID := "_" + + if iDPName != "" { + configureID = iDPName + } + + lgParams := LoginURLParams{ + State: state, + IDPName: configureID, + } + + jsonEnc, err := json.Marshal(lgParams) + if err != nil { + return "" + } + + stEncode := base64.StdEncoding.EncodeToString(jsonEnc) + loginURL := client.oauth2Config.AuthCodeURL(stEncode) + return strings.TrimSpace(loginURL) } diff --git a/pkg/auth/idp/oauth2/provider_test.go b/pkg/auth/idp/oauth2/provider_test.go index db7f3d1c8..bc3cc519d 100644 --- a/pkg/auth/idp/oauth2/provider_test.go +++ b/pkg/auth/idp/oauth2/provider_test.go @@ -66,6 +66,6 @@ func TestGenerateLoginURL(t *testing.T) { // a non-empty string return state } - url := oauth2Provider.GenerateLoginURL(DefaultDerivedKey) + url := oauth2Provider.GenerateLoginURL(DefaultDerivedKey, "testIDP") funcAssert.NotEqual("", url) } diff --git a/portal-ui/src/screens/LoginPage/LoginPage.tsx b/portal-ui/src/screens/LoginPage/LoginPage.tsx index f16c80cab..0e3fe7277 100644 --- a/portal-ui/src/screens/LoginPage/LoginPage.tsx +++ b/portal-ui/src/screens/LoginPage/LoginPage.tsx @@ -15,21 +15,20 @@ // along with this program. If not, see . import React, { useEffect } from "react"; - import { useNavigate } from "react-router-dom"; import { Box, InputAdornment, LinearProgress, - Select, MenuItem, + Select, } from "@mui/material"; import { Button } from "mds"; import { Theme, useTheme } from "@mui/material/styles"; import createStyles from "@mui/styles/createStyles"; import makeStyles from "@mui/styles/makeStyles"; import Grid from "@mui/material/Grid"; -import { loginStrategyType } from "./types"; +import { loginStrategyType, redirectRule } from "./types"; import LogoutIcon from "../../icons/LogoutIcon"; import RefreshIcon from "../../icons/RefreshIcon"; import MainError from "../Console/Common/MainError/MainError"; @@ -58,6 +57,7 @@ import { resetForm, setJwt } from "./loginSlice"; import StrategyForm from "./StrategyForm"; import { LoginField } from "./LoginField"; import DirectPVLogo from "../../icons/DirectPVLogo"; +import { redirectRules } from "../../utils/sortFunctions"; const useStyles = makeStyles((theme: Theme) => createStyles({ @@ -344,7 +344,19 @@ const Login = () => { } case loginStrategyType.redirect: case loginStrategyType.redirectServiceAccount: { - if (loginStrategy.redirect.length > 1) { + let redirectItems: redirectRule[] = []; + + if ( + loginStrategy.redirectRules && + loginStrategy.redirectRules.length > 0 + ) { + redirectItems = [...loginStrategy.redirectRules].sort(redirectRules); + } + + if ( + loginStrategy.redirectRules && + loginStrategy.redirectRules.length > 1 + ) { loginComponent = (
Login with SSO:
@@ -361,21 +373,21 @@ const Login = () => { className={classes.ssoSelect} renderValue={() => "Select Provider"} > - {loginStrategy.redirect.map((r, idx) => ( + {redirectItems.map((r, idx) => ( - {loginStrategy.displayNames[idx]} + {r.displayName} ))} ); - } else if (loginStrategy.redirect.length === 1) { + } else if (redirectItems.length === 1) { loginComponent = (
diff --git a/portal-ui/src/screens/LoginPage/loginSlice.ts b/portal-ui/src/screens/LoginPage/loginSlice.ts index 13641fc40..b11803fa1 100644 --- a/portal-ui/src/screens/LoginPage/loginSlice.ts +++ b/portal-ui/src/screens/LoginPage/loginSlice.ts @@ -50,8 +50,7 @@ const initialState: LoginState = { jwt: "", loginStrategy: { loginStrategy: loginStrategyType.unknown, - redirect: [], - displayNames: [], + redirectRules: [], }, loginSending: false, loadingFetchConfiguration: true, diff --git a/portal-ui/src/screens/LoginPage/types.ts b/portal-ui/src/screens/LoginPage/types.ts index 5a8301741..1e6e93716 100644 --- a/portal-ui/src/screens/LoginPage/types.ts +++ b/portal-ui/src/screens/LoginPage/types.ts @@ -16,11 +16,15 @@ export interface ILoginDetails { loginStrategy: loginStrategyType; - redirect: string[]; - displayNames: string[]; + redirectRules: redirectRule[]; isDirectPV?: boolean; } +export interface redirectRule { + redirect: string; + displayName: string; +} + export enum loginStrategyType { unknown = "unknown", form = "form", diff --git a/portal-ui/src/utils/sortFunctions.ts b/portal-ui/src/utils/sortFunctions.ts index e74c1520d..ee5cb5d4f 100644 --- a/portal-ui/src/utils/sortFunctions.ts +++ b/portal-ui/src/utils/sortFunctions.ts @@ -14,6 +14,8 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . +import { redirectRule } from "../screens/LoginPage/types"; + interface userInterface { accessKey: string; } @@ -72,3 +74,13 @@ export const policyDetailsSort = ( // a must be equal to b return 0; }; + +export const redirectRules = (a: redirectRule, b: redirectRule) => { + if (a.displayName > b.displayName) { + return 1; + } + if (a.displayName < b.displayName) { + return -1; + } + return 0; +}; diff --git a/restapi/embedded_spec.go b/restapi/embedded_spec.go index 59ee422d7..d18efff1d 100644 --- a/restapi/embedded_spec.go +++ b/restapi/embedded_spec.go @@ -6349,12 +6349,6 @@ func init() { "loginDetails": { "type": "object", "properties": { - "displayNames": { - "type": "array", - "items": { - "type": "string" - } - }, "isDirectPV": { "type": "boolean" }, @@ -6367,10 +6361,10 @@ func init() { "redirect-service-account" ] }, - "redirect": { + "redirectRules": { "type": "array", "items": { - "type": "string" + "$ref": "#/definitions/redirectRule" } } } @@ -7038,6 +7032,17 @@ func init() { } } }, + "redirectRule": { + "type": "object", + "properties": { + "displayName": { + "type": "string" + }, + "redirect": { + "type": "string" + } + } + }, "remoteBucket": { "type": "object", "required": [ @@ -14605,12 +14610,6 @@ func init() { "loginDetails": { "type": "object", "properties": { - "displayNames": { - "type": "array", - "items": { - "type": "string" - } - }, "isDirectPV": { "type": "boolean" }, @@ -14623,10 +14622,10 @@ func init() { "redirect-service-account" ] }, - "redirect": { + "redirectRules": { "type": "array", "items": { - "type": "string" + "$ref": "#/definitions/redirectRule" } } } @@ -15294,6 +15293,17 @@ func init() { } } }, + "redirectRule": { + "type": "object", + "properties": { + "displayName": { + "type": "string" + }, + "redirect": { + "type": "string" + } + } + }, "remoteBucket": { "type": "object", "required": [ diff --git a/restapi/policy/policies.go b/restapi/policy/policies.go index bc42e5aaa..396c4ea88 100644 --- a/restapi/policy/policies.go +++ b/restapi/policy/policies.go @@ -66,7 +66,6 @@ func replaceJwtVariables(rawPolicy []byte, claims map[string]interface{}) json.R for _, field := range jwtFields { if val, ok := claims[field]; ok { variable := fmt.Sprintf("${jwt:%s}", field) - fmt.Println("found", variable) rawPolicy = bytes.ReplaceAll(rawPolicy, []byte(variable), []byte(fmt.Sprintf("%v", val))) } } diff --git a/restapi/user_login.go b/restapi/user_login.go index 94347145b..ec844637f 100644 --- a/restapi/user_login.go +++ b/restapi/user_login.go @@ -18,11 +18,10 @@ package restapi import ( "context" + "encoding/base64" + "encoding/json" "net/http" - "github.com/minio/madmin-go" - "github.com/minio/minio-go/v7/pkg/credentials" - "github.com/go-openapi/runtime" "github.com/go-openapi/runtime/middleware" "github.com/minio/console/models" @@ -30,12 +29,14 @@ import ( "github.com/minio/console/pkg/auth/idp/oauth2" "github.com/minio/console/restapi/operations" authApi "github.com/minio/console/restapi/operations/auth" + "github.com/minio/madmin-go" + "github.com/minio/minio-go/v7/pkg/credentials" ) func registerLoginHandlers(api *operations.ConsoleAPI) { // GET login strategy api.AuthLoginDetailHandler = authApi.LoginDetailHandlerFunc(func(params authApi.LoginDetailParams) middleware.Responder { - loginDetails, err := getLoginDetailsResponse(params, GlobalMinIOConfig.OpenIDProviders, oauth2.DefaultIDPConfig) + loginDetails, err := getLoginDetailsResponse(params, GlobalMinIOConfig.OpenIDProviders) if err != nil { return authApi.NewLoginDetailDefault(int(err.Code)).WithPayload(err) } @@ -56,7 +57,7 @@ func registerLoginHandlers(api *operations.ConsoleAPI) { }) // POST login using external IDP api.AuthLoginOauth2AuthHandler = authApi.LoginOauth2AuthHandlerFunc(func(params authApi.LoginOauth2AuthParams) middleware.Responder { - loginResponse, err := getLoginOauth2AuthResponse(params, GlobalMinIOConfig.OpenIDProviders, oauth2.DefaultIDPConfig) + loginResponse, err := getLoginOauth2AuthResponse(params, GlobalMinIOConfig.OpenIDProviders) if err != nil { return authApi.NewLoginOauth2AuthDefault(int(err.Code)).WithPayload(err) } @@ -145,12 +146,12 @@ func getLoginResponse(params authApi.LoginParams) (*models.LoginResponse, *model } // getLoginDetailsResponse returns information regarding the Console authentication mechanism. -func getLoginDetailsResponse(params authApi.LoginDetailParams, openIDProviders oauth2.OpenIDPCfg, idpName string) (*models.LoginDetails, *models.Error) { +func getLoginDetailsResponse(params authApi.LoginDetailParams, openIDProviders oauth2.OpenIDPCfg) (*models.LoginDetails, *models.Error) { ctx, cancel := context.WithCancel(params.HTTPRequest.Context()) defer cancel() loginStrategy := models.LoginDetailsLoginStrategyForm - redirectURL := []string{} - displayNames := []string{} + var redirectRules []*models.RedirectRule + r := params.HTTPRequest var loginDetails *models.LoginDetails if len(openIDProviders) >= 1 { @@ -166,18 +167,24 @@ func getLoginDetailsResponse(params authApi.LoginDetailParams, openIDProviders o KeyFunc: provider.GetStateKeyFunc(), Client: oauth2Client, } - redirectURL = append(redirectURL, identityProvider.GenerateLoginURL()) + + displayName := "Login with SSO" + if provider.DisplayName != "" { - displayNames = append(displayNames, provider.DisplayName) - } else { - displayNames = append(displayNames, "Login with SSO") + displayName = provider.DisplayName } + + redirectRule := models.RedirectRule{ + Redirect: identityProvider.GenerateLoginURL(), + DisplayName: displayName, + } + + redirectRules = append(redirectRules, &redirectRule) } } loginDetails = &models.LoginDetails{ LoginStrategy: loginStrategy, - Redirect: redirectURL, - DisplayNames: displayNames, + RedirectRules: redirectRules, } return loginDetails, nil } @@ -187,29 +194,50 @@ func verifyUserAgainstIDP(ctx context.Context, provider auth.IdentityProviderI, userCredentials, err := provider.VerifyIdentity(ctx, code, state) if err != nil { LogError("error validating user identity against idp: %v", err) - return nil, ErrInvalidLogin + return nil, err } return userCredentials, nil } -func getLoginOauth2AuthResponse(params authApi.LoginOauth2AuthParams, openIDProviders oauth2.OpenIDPCfg, idpName string) (*models.LoginResponse, *models.Error) { +func getLoginOauth2AuthResponse(params authApi.LoginOauth2AuthParams, openIDProviders oauth2.OpenIDPCfg) (*models.LoginResponse, *models.Error) { ctx, cancel := context.WithCancel(params.HTTPRequest.Context()) defer cancel() r := params.HTTPRequest lr := params.Body + if openIDProviders != nil { - // initialize new oauth2 client - oauth2Client, err := openIDProviders.NewOauth2ProviderClient(idpName, nil, r, GetConsoleHTTPClient("")) + // we read state + rState := *lr.State + + decodedRState, err := base64.StdEncoding.DecodeString(rState) + if err != nil { + return nil, ErrorWithContext(ctx, err) + } + + var requestItems oauth2.LoginURLParams + + err = json.Unmarshal(decodedRState, &requestItems) + + if err != nil { + return nil, ErrorWithContext(ctx, err) + } + + IDPName := requestItems.IDPName + state := requestItems.State + providerCfg := openIDProviders[IDPName] + oauth2Client, err := openIDProviders.NewOauth2ProviderClient(IDPName, nil, r, GetConsoleHTTPClient("")) if err != nil { return nil, ErrorWithContext(ctx, err) } // initialize new identity provider + identityProvider := auth.IdentityProvider{ - KeyFunc: openIDProviders[idpName].GetStateKeyFunc(), + KeyFunc: providerCfg.GetStateKeyFunc(), Client: oauth2Client, + RoleARN: providerCfg.RoleArn, } // Validate user against IDP - userCredentials, err := verifyUserAgainstIDP(ctx, identityProvider, *lr.Code, *lr.State) + userCredentials, err := verifyUserAgainstIDP(ctx, identityProvider, *lr.Code, state) if err != nil { return nil, ErrorWithContext(ctx, err) } diff --git a/sso-integration/sso_test.go b/sso-integration/sso_test.go index bfee20fbc..ae555d6ea 100644 --- a/sso-integration/sso_test.go +++ b/sso-integration/sso_test.go @@ -18,6 +18,7 @@ package ssointegration import ( "bytes" + "encoding/base64" "encoding/json" "fmt" "io" @@ -44,7 +45,7 @@ var token string func initConsoleServer(consoleIDPURL string) (*restapi.Server, error) { // Configure Console Server with vars to get the idp config from the container pcfg := map[string]consoleoauth2.ProviderConfig{ - consoleoauth2.DefaultIDPConfig: { + "_": { URL: consoleIDPURL, ClientID: "minio-client-app", ClientSecret: "minio-client-app-secret", @@ -130,11 +131,18 @@ func TestMain(t *testing.T) { if err != nil { log.Fatal(err) } - var jsonMap map[string][]interface{} - json.Unmarshal(body, &jsonMap) - fmt.Println(jsonMap["redirect"][0]) - redirect := jsonMap["redirect"][0] - redirectAsString := fmt.Sprint(redirect) + var jsonMap models.LoginDetails + + fmt.Println(body) + + err = json.Unmarshal(body, &jsonMap) + + if err != nil { + fmt.Printf("error JSON Unmarshal %s\n", err) + } + + redirectRule := jsonMap.RedirectRules[0] + redirectAsString := fmt.Sprint(redirectRule.Redirect) fmt.Println(redirectAsString) // execute script to get the code and state @@ -238,12 +246,25 @@ func TestBadLogin(t *testing.T) { Timeout: 2 * time.Second, } + encodeItem := consoleoauth2.LoginURLParams{ + State: "invalidState", + IDPName: "_", + } + + jsonState, err := json.Marshal(encodeItem) + if err != nil { + log.Println(err) + assert.Nil(err) + } + // get login credentials + stateVarIable := base64.StdEncoding.EncodeToString(jsonState) + codeVarIable := "invalidCode" - stateVarIabl := "invalidState" + requestData := map[string]string{ "code": codeVarIable, - "state": stateVarIabl, + "state": stateVarIable, } requestDataJSON, _ := json.Marshal(requestData) diff --git a/swagger-console.yml b/swagger-console.yml index 3ca0da32a..af6028d6d 100644 --- a/swagger-console.yml +++ b/swagger-console.yml @@ -4045,14 +4045,10 @@ definitions: loginStrategy: type: string enum: [ form, redirect, service-account, redirect-service-account ] - redirect: + redirectRules: type: array items: - type: string - displayNames: - type: array - items: - type: string + $ref: "#/definitions/redirectRule" isDirectPV: type: boolean loginOauth2AuthRequest: @@ -5567,3 +5563,11 @@ definitions: type: integer maxConcurrentDownloads: type: integer + + redirectRule: + type: object + properties: + redirect: + type: string + displayName: + type: string diff --git a/swagger-operator.yml b/swagger-operator.yml index 766967fd5..727020e53 100644 --- a/swagger-operator.yml +++ b/swagger-operator.yml @@ -1639,14 +1639,10 @@ definitions: loginStrategy: type: string enum: [form, redirect, service-account, redirect-service-account] - redirect: + redirectRules: type: array items: - type: string - displayNames: - type: array - items: - type: string + $ref: "#/definitions/redirectRule" isDirectPV: type: boolean loginRequest: @@ -3745,3 +3741,11 @@ definitions: properties: registered: type: boolean + + redirectRule: + type: object + properties: + redirect: + type: string + displayName: + type: string