From 58903e8337e87eac5a43bc9bf0049251c6b3ad34 Mon Sep 17 00:00:00 2001 From: Lenin Alevski Date: Thu, 7 Jan 2021 13:49:56 -0600 Subject: [PATCH] Remove use of Privileged Credentials (#535) - Leverage on MinIO Oauth integration instead of the current Console implementation - Refactor pkg/idp - Added tests to login --- DEVELOPMENT.md | 3 - README.md | 5 +- pkg/auth/idp.go | 16 ++- pkg/auth/idp/oauth2/config.go | 15 +-- pkg/auth/idp/oauth2/const.go | 3 +- pkg/auth/idp/oauth2/provider.go | 54 ++++---- pkg/auth/idp/oauth2/provider_test.go | 27 ---- restapi/admin_tenants.go | 2 - restapi/client-admin.go | 40 ------ restapi/config.go | 8 -- restapi/consts.go | 2 - restapi/user_account.go | 2 +- restapi/user_login.go | 118 ++++++++-------- restapi/user_login_test.go | 194 ++++++++++++--------------- 14 files changed, 186 insertions(+), 303 deletions(-) diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 3bed0675d..e340218b8 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -86,9 +86,6 @@ export MINIO_IDENTITY_LDAP_SERVER_INSECURE=on ## Run Console ``` -export CONSOLE_ACCESS_KEY=minio -export CONSOLE_SECRET_KEY=minio123 -... export CONSOLE_LDAP_ENABLED=on ./console server ``` diff --git a/README.md b/README.md index 85501656f..26ee5ecd8 100644 --- a/README.md +++ b/README.md @@ -101,14 +101,13 @@ Additionally, you can create policies to limit the privileges for `console` user To run the server: ```bash -#required to encrypt jwet payload +# Salt to encrypt JWT payload export CONSOLE_PBKDF_PASSPHRASE=SECRET #required to encrypt jwet payload export CONSOLE_PBKDF_SALT=SECRET -export CONSOLE_ACCESS_KEY=console -export CONSOLE_SECRET_KEY=YOURCONSOLESECRET +# MinIO endpoint export CONSOLE_MINIO_SERVER=http://localhost:9000 ./console server ``` diff --git a/pkg/auth/idp.go b/pkg/auth/idp.go index f667cdd8e..bb4827f85 100644 --- a/pkg/auth/idp.go +++ b/pkg/auth/idp.go @@ -20,26 +20,28 @@ import ( "context" "github.com/minio/console/pkg/auth/idp/oauth2" + + "github.com/minio/minio-go/v7/pkg/credentials" ) -// IdentityProviderClient interface with all functions to be implemented -// by mock when testing, it should include all IdentityProviderClient respective api calls +// IdentityProviderI interface with all functions to be implemented +// by mock when testing, it should include all IdentityProvider respective api calls // that are used within this project. -type IdentityProviderClient interface { - VerifyIdentity(ctx context.Context, code, state string) (*oauth2.User, error) +type IdentityProviderI interface { + VerifyIdentity(ctx context.Context, code, state string) (*credentials.Credentials, error) GenerateLoginURL() string } // Interface implementation // -// Define the structure of a IdentityProvider Client and define the functions that are actually used +// Define the structure of a IdentityProvider with Client inside and define the functions that are used // during the authentication flow. type IdentityProvider struct { - Client IdentityProviderClient + Client *oauth2.Provider } // VerifyIdentity will verify the user identity against the idp using the authorization code flow -func (c IdentityProvider) VerifyIdentity(ctx context.Context, code, state string) (*oauth2.User, error) { +func (c IdentityProvider) VerifyIdentity(ctx context.Context, code, state string) (*credentials.Credentials, error) { return c.Client.VerifyIdentity(ctx, code, state) } diff --git a/pkg/auth/idp/oauth2/config.go b/pkg/auth/idp/oauth2/config.go index 795f58976..6410bf6d2 100644 --- a/pkg/auth/idp/oauth2/config.go +++ b/pkg/auth/idp/oauth2/config.go @@ -19,10 +19,16 @@ package oauth2 import ( + "strings" + "github.com/minio/console/pkg/auth/utils" "github.com/minio/minio/pkg/env" ) +func GetSTSEndpoint() string { + return strings.TrimSpace(env.Get(ConsoleMinIOServer, "http://localhost:9000")) +} + func GetIdpURL() string { return env.Get(ConsoleIdpURL, "") } @@ -40,10 +46,6 @@ func GetIdpCallbackURL() string { return env.Get(ConsoleIdpCallbackURL, "") } -func GetIdpAdminRoles() string { - return env.Get(ConsoleIdpAdminRoles, "") -} - func IsIdpEnabled() bool { return GetIdpURL() != "" && GetIdpClientID() != "" && @@ -64,8 +66,3 @@ var defaultSaltForIdpHmac = utils.RandomCharString(64) func getSaltForIdpHmac() string { return env.Get(ConsoleIdpHmacSalt, defaultSaltForIdpHmac) } - -// GetSaltForIdpHmac returns the policy to be assigned to the users authenticating via an IDP -func GetIDPPolicyForUser() string { - return env.Get(ConsoleIdpPolicyUser, "consoleAdmin") -} diff --git a/pkg/auth/idp/oauth2/const.go b/pkg/auth/idp/oauth2/const.go index 289226a51..29b34b271 100644 --- a/pkg/auth/idp/oauth2/const.go +++ b/pkg/auth/idp/oauth2/const.go @@ -18,12 +18,11 @@ package oauth2 const ( // const for idp configuration + ConsoleMinIOServer = "CONSOLE_MINIO_SERVER" ConsoleIdpURL = "CONSOLE_IDP_URL" ConsoleIdpClientID = "CONSOLE_IDP_CLIENT_ID" ConsoleIdpSecret = "CONSOLE_IDP_SECRET" ConsoleIdpCallbackURL = "CONSOLE_IDP_CALLBACK" - ConsoleIdpAdminRoles = "CONSOLE_IDP_ADMIN_ROLES" ConsoleIdpHmacPassphrase = "CONSOLE_IDP_HMAC_PASSPHRASE" ConsoleIdpHmacSalt = "CONSOLE_IDP_HMAC_SALT" - ConsoleIdpPolicyUser = "CONSOLE_IDP_POLICY_USER" ) diff --git a/pkg/auth/idp/oauth2/provider.go b/pkg/auth/idp/oauth2/provider.go index 59064c10a..a9d0ffbc4 100644 --- a/pkg/auth/idp/oauth2/provider.go +++ b/pkg/auth/idp/oauth2/provider.go @@ -26,6 +26,9 @@ import ( "net/http" "net/url" "strings" + "time" + + "github.com/minio/minio-go/v7/pkg/credentials" "github.com/coreos/go-oidc" "github.com/minio/console/pkg/auth/utils" @@ -110,14 +113,13 @@ func NewOauth2ProviderClient(ctx context.Context, scopes []string) (*Provider, e scopes = []string{oidc.ScopeOpenID, "profile", "app_metadata", "user_metadata", "email"} } client := new(Provider) - config := xoauth2.Config{ + client.oauth2Config = &xoauth2.Config{ ClientID: GetIdpClientID(), ClientSecret: GetIdpSecret(), RedirectURL: GetIdpCallbackURL(), Endpoint: provider.Endpoint(), Scopes: scopes, } - client.oauth2Config = &config client.oidcProvider = provider client.ClientID = GetIdpClientID() @@ -137,7 +139,7 @@ type User struct { LastLogin string `json:"last_login"` LastPasswordReset string `json:"last_password_reset"` LoginsCount int `json:"logins_count"` - Mltifactor string `json:"multifactor"` + MultiFactor string `json:"multifactor"` Name string `json:"name"` Nickname string `json:"nickname"` PhoneNumber string `json:"phone_number"` @@ -150,39 +152,31 @@ type User struct { } // VerifyIdentity will contact the configured IDP and validate the user identity based on the authorization code -func (client *Provider) VerifyIdentity(ctx context.Context, code, state string) (*User, error) { +func (client *Provider) VerifyIdentity(ctx context.Context, code, state string) (*credentials.Credentials, error) { // verify the provided state is valid (prevents CSRF attacks) if !validateOauth2State(state) { return nil, errGeneric } - // verify the authorization code against the identity oidcProvider - // idp will return a token in exchange - token, err := client.oauth2Config.Exchange(ctx, code) + getWebTokenExpiry := func() (*credentials.WebIdentityToken, error) { + oauth2Token, err := client.oauth2Config.Exchange(ctx, code) + if err != nil { + return nil, err + } + if !oauth2Token.Valid() { + return nil, errors.New("invalid token") + } + + return &credentials.WebIdentityToken{ + Token: oauth2Token.Extra("id_token").(string), + Expiry: int(oauth2Token.Expiry.Sub(time.Now().UTC()).Seconds()), + }, nil + } + stsEndpoint := GetSTSEndpoint() + sts, err := credentials.NewSTSWebIdentity(stsEndpoint, getWebTokenExpiry) if err != nil { - log.Println("Failed to verify authorization code", err) - return nil, errGeneric + return nil, err } - // extract and check id_token field is provided in the response - rawIDToken, ok := token.Extra("id_token").(string) - if !ok { - log.Println("No id_token field in oauth2 token") - return nil, errGeneric - } - config := &oidc.Config{ - ClientID: client.ClientID, - } - idToken, err := client.oidcProvider.Verifier(config).Verify(ctx, rawIDToken) - if err != nil { - log.Println("Failed to verify ID token", err) - return nil, errGeneric - } - var profile User - // Populate the profile object using the claims included in the token - if err := idToken.Claims(&profile); err != nil { - log.Println("Failed to read profile information", err) - return nil, errGeneric - } - return &profile, nil + return sts, nil } // validateOauth2State validates the provided state was originated using the same diff --git a/pkg/auth/idp/oauth2/provider_test.go b/pkg/auth/idp/oauth2/provider_test.go index ce512e3c4..376cf14c7 100644 --- a/pkg/auth/idp/oauth2/provider_test.go +++ b/pkg/auth/idp/oauth2/provider_test.go @@ -69,30 +69,3 @@ func TestGenerateLoginURL(t *testing.T) { url := oauth2Provider.GenerateLoginURL() funcAssert.NotEqual("", url) } - -func TestVerifyIdentity(t *testing.T) { - ctx := context.Background() - funcAssert := assert.New(t) - // mock data - oauth2Provider := Provider{ - oauth2Config: Oauth2configMock{}, - oidcProvider: &oidc.Provider{}, - } - // Test-1 : VerifyIdentity() should fail because of bad state token - _, err := oauth2Provider.VerifyIdentity(ctx, "AAABBBCCCDDDEEEFFF", "badtoken") - funcAssert.NotNil(err) - // Test-2 : VerifyIdentity() should fail because no id_token is provided by the idp - oauth2ConfigExchangeMock = func(ctx context.Context, code string, opts ...oauth2.AuthCodeOption) (*oauth2.Token, error) { - return &oauth2.Token{}, nil - } - state := GetRandomStateWithHMAC(32) - code := "AAABBBCCCDDDEEEFFF" - _, err = oauth2Provider.VerifyIdentity(ctx, code, state) - funcAssert.NotNil(err) - // Test-3 : VerifyIdentity() should fail because no id_token is provided by the idp - // TODO - // Test-4 : VerifyIdentity() should fail because oidcProvider.Verifier returned an error - // TODO - // Test-5 : VerifyIdentity() should fail because idToken.Claims contains invalid fields - // TODO -} diff --git a/restapi/admin_tenants.go b/restapi/admin_tenants.go index 258ff3a21..a7e63c1c0 100644 --- a/restapi/admin_tenants.go +++ b/restapi/admin_tenants.go @@ -665,8 +665,6 @@ func getTenantCreatedResponse(session *models.Principal, params admin_api.Create Data: map[string][]byte{ "CONSOLE_PBKDF_PASSPHRASE": []byte(RandomCharString(16)), "CONSOLE_PBKDF_SALT": []byte(RandomCharString(8)), - "CONSOLE_ACCESS_KEY": []byte(consoleAccess), - "CONSOLE_SECRET_KEY": []byte(consoleSecret), }, } diff --git a/restapi/client-admin.go b/restapi/client-admin.go index 8b6fc5e0b..43b0d854f 100644 --- a/restapi/client-admin.go +++ b/restapi/client-admin.go @@ -24,8 +24,6 @@ import ( "runtime" "github.com/minio/console/models" - "github.com/minio/console/pkg/auth" - "github.com/minio/console/pkg/auth/ldap" mcCmd "github.com/minio/mc/cmd" "github.com/minio/mc/pkg/probe" "github.com/minio/minio-go/v7/pkg/credentials" @@ -317,11 +315,6 @@ func newAdminFromClaims(claims *models.Principal) (*madmin.AdminClient, error) { return adminClient, nil } -var ( - consoleAccessKey = getAccessKey() - consoleSecretKey = getSecretKey() -) - // stsClient is a custom http client, this client should not be called directly and instead be // called using GetConsoleSTSClient() to ensure is initialized and the certificates are loaded correctly var stsClient *http.Client @@ -333,36 +326,3 @@ func GetConsoleSTSClient() *http.Client { } return stsClient } - -var consoleLDAPAdminCreds consoleCredentials - -func newSuperMAdminClient() (*madmin.AdminClient, error) { - accessKey := consoleAccessKey - secretKey := consoleSecretKey - sessionToken := "" - // If LDAP is enabled (External IDP) in minio, then obtain the session tokens associated with the super admin credentials - // configured in console - if ldap.GetLDAPEnabled() { - // initialize LDAP super Admin Credentials once - if consoleLDAPAdminCreds.consoleCredentials == nil { - consoleCredentialsFromLDAP, err := auth.GetCredentialsFromLDAP(GetConsoleSTSClient(), MinioEndpoint, consoleAccessKey, consoleSecretKey) - if err != nil { - return nil, err - } - consoleLDAPAdminCreds = consoleCredentials{consoleCredentials: consoleCredentialsFromLDAP} - } - tokens, err := consoleLDAPAdminCreds.Get() - if err != nil { - return nil, err - } - accessKey = tokens.AccessKeyID - secretKey = tokens.SecretAccessKey - sessionToken = tokens.SessionToken - } - - adminClient, pErr := NewAdminClient(MinioEndpoint, accessKey, secretKey, sessionToken) - if pErr != nil { - return nil, pErr.Cause - } - return adminClient, nil -} diff --git a/restapi/config.go b/restapi/config.go index 14d5090f3..fa2f0dae3 100644 --- a/restapi/config.go +++ b/restapi/config.go @@ -44,14 +44,6 @@ var TLSRedirect = "off" var SessionDuration = 45 * time.Minute -func getAccessKey() string { - return env.Get(ConsoleAccessKey, "minioadmin") -} - -func getSecretKey() string { - return env.Get(ConsoleSecretKey, "minioadmin") -} - func getMinIOServer() string { return strings.TrimSpace(env.Get(ConsoleMinIOServer, "http://localhost:9000")) } diff --git a/restapi/consts.go b/restapi/consts.go index 1937a459d..9f715469b 100644 --- a/restapi/consts.go +++ b/restapi/consts.go @@ -19,8 +19,6 @@ package restapi const ( // Constants for common configuration ConsoleVersion = `0.2.0` - ConsoleAccessKey = "CONSOLE_ACCESS_KEY" - ConsoleSecretKey = "CONSOLE_SECRET_KEY" ConsoleMinIOServer = "CONSOLE_MINIO_SERVER" ConsoleMinIORegion = "CONSOLE_MINIO_REGION" ConsoleProductionMode = "CONSOLE_PRODUCTION_MODE" diff --git a/restapi/user_account.go b/restapi/user_account.go index dc965b13b..893611b3d 100644 --- a/restapi/user_account.go +++ b/restapi/user_account.go @@ -80,7 +80,7 @@ func getChangePasswordResponse(session *models.Principal, params user_api.Accoun } // user credentials are updated at this point, we need to generate a new admin client and authenticate using // the new credentials - credentials, err := getConsoleCredentials(ctx, accessKey, newSecretKey) + credentials, err := getConsoleCredentials(ctx, accessKey, newSecretKey, "") if err != nil { return nil, prepareError(errInvalidCredentials, nil, err) } diff --git a/restapi/user_login.go b/restapi/user_login.go index eeaecded7..6072b0400 100644 --- a/restapi/user_login.go +++ b/restapi/user_login.go @@ -22,13 +22,16 @@ import ( "net/http" "time" + "github.com/minio/minio-go/v7/pkg/credentials" + + iampolicy "github.com/minio/minio/pkg/iam/policy" + "github.com/go-openapi/runtime" "github.com/go-openapi/runtime/middleware" "github.com/minio/console/models" "github.com/minio/console/pkg/acl" "github.com/minio/console/pkg/auth" "github.com/minio/console/pkg/auth/idp/oauth2" - "github.com/minio/console/pkg/auth/utils" "github.com/minio/console/restapi/operations" "github.com/minio/console/restapi/operations/user_api" ) @@ -98,36 +101,34 @@ func login(credentials ConsoleCredentialsI) (*string, error) { return &token, nil } -func getConfiguredRegionForLogin(ctx context.Context, client MinioAdmin) (string, error) { - location := "" - configuration, err := getConfig(ctx, client, "region") +// getAccountPolicy will return the associated policy of the current account +func getAccountPolicy(ctx context.Context, client MinioAdmin) (*iampolicy.Policy, error) { + // Obtain the current policy assigned to this user + // necessary for generating the list of allowed endpoints + accountInfo, err := client.accountInfo(ctx) if err != nil { - log.Println("error obtaining MinIO region:", err) - return location, errorGeneric + return nil, err } - // region is an array of 1 element - if len(configuration) > 0 { - location = configuration[0].Value - } - return location, nil + return &accountInfo.Policy, err } -func getConsoleCredentials(ctx context.Context, accessKey, secretKey string) (*consoleCredentials, error) { +// getConsoleCredentials will return consoleCredentials interface including the associated policy of the current account +func getConsoleCredentials(ctx context.Context, accessKey, secretKey, sessionToken string) (*consoleCredentials, error) { mAdminClient, err := newMAdminClient(&models.Principal{ STSAccessKeyID: accessKey, STSSecretAccessKey: secretKey, + STSSessionToken: sessionToken, }) if err != nil { return nil, err } userAdminClient := adminClient{client: mAdminClient} - // obtain the current policy assigned to this user + // Obtain the current policy assigned to this user // necessary for generating the list of allowed endpoints - userInfo, err := userAdminClient.getUserInfo(ctx, accessKey) + policy, err := getAccountPolicy(ctx, userAdminClient) if err != nil { return nil, err } - policy, _ := userAdminClient.getPolicy(ctx, userInfo.PolicyName) // by default every user starts with an empty array of available actions // therefore we would have access only to pages that doesn't require any privilege // ie: service-account page @@ -136,13 +137,13 @@ func getConsoleCredentials(ctx context.Context, accessKey, secretKey string) (*c if policy != nil { actions = acl.GetActionsStringFromPolicy(policy) } - creds, err := newConsoleCredentials(accessKey, secretKey, MinioRegion) + credentials, err := newConsoleCredentials(accessKey, secretKey, MinioRegion) if err != nil { return nil, err } // consoleCredentials will be sts credentials, account credentials will be need it in the scenario the user wish return &consoleCredentials{ - consoleCredentials: creds, + consoleCredentials: credentials, accountAccessKey: accessKey, accountSecretKey: secretKey, actions: actions, @@ -154,11 +155,11 @@ func getLoginResponse(lr *models.LoginRequest) (*models.LoginResponse, *models.E ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) defer cancel() // prepare console credentials - credentials, err := getConsoleCredentials(ctx, *lr.AccessKey, *lr.SecretKey) + consolCreds, err := getConsoleCredentials(ctx, *lr.AccessKey, *lr.SecretKey, "") if err != nil { return nil, prepareError(errInvalidCredentials, nil, err) } - sessionID, err := login(credentials) + sessionID, err := login(consolCreds) if err != nil { return nil, prepareError(errInvalidCredentials, nil, err) } @@ -195,13 +196,14 @@ func getLoginDetailsResponse() (*models.LoginDetails, *models.Error) { return loginDetails, nil } -func loginOauth2Auth(ctx context.Context, provider *auth.IdentityProvider, code, state string) (*oauth2.User, error) { - userIdentity, err := provider.VerifyIdentity(ctx, code, state) +// verifyUserAgainstIDP will verify user identity against the configured IDP and return MinIO credentials +func verifyUserAgainstIDP(ctx context.Context, provider auth.IdentityProviderI, code, state string) (*credentials.Credentials, error) { + userCredentials, err := provider.VerifyIdentity(ctx, code, state) if err != nil { log.Println("error validating user identity against idp:", err) return nil, errInvalidCredentials } - return userIdentity, nil + return userCredentials, nil } func getLoginOauth2AuthResponse(lr *models.LoginOauth2AuthRequest) (*models.LoginResponse, *models.Error) { @@ -214,55 +216,47 @@ func getLoginOauth2AuthResponse(lr *models.LoginOauth2AuthRequest) (*models.Logi return nil, prepareError(err) } // initialize new identity provider - identityProvider := &auth.IdentityProvider{Client: oauth2Client} + identityProvider := auth.IdentityProvider{Client: oauth2Client} // Validate user against IDP - identity, err := loginOauth2Auth(ctx, identityProvider, *lr.Code, *lr.State) + userCredentials, err := verifyUserAgainstIDP(ctx, identityProvider, *lr.Code, *lr.State) if err != nil { return nil, prepareError(errInvalidCredentials, nil, err) } - mAdmin, err := newSuperMAdminClient() + creds, err := userCredentials.Get() if err != nil { - return nil, prepareError(err) + return nil, prepareError(errInvalidCredentials, nil, err) } - adminClient := adminClient{client: mAdmin} - accessKey := identity.Email - secretKey := utils.RandomCharString(32) - // obtain the configured MinIO region - // need it for user authentication - location, err := getConfiguredRegionForLogin(ctx, adminClient) + // initialize admin client + mAdminClient, err := newMAdminClient(&models.Principal{ + STSAccessKeyID: creds.AccessKeyID, + STSSecretAccessKey: creds.SecretAccessKey, + STSSessionToken: creds.SessionToken, + }) if err != nil { - return nil, prepareError(err) + return nil, prepareError(errInvalidCredentials, nil, err) } - // create user in MinIO - if _, err := addUser(ctx, adminClient, &accessKey, &secretKey, []string{}); err != nil { - return nil, prepareError(err) - } - // rollback user if there's an error after this point - defer func() { - if err != nil { - if errRemove := removeUser(ctx, adminClient, accessKey); errRemove != nil { - log.Println("error removing user:", errRemove) - } - } - }() - // assign the "consoleAdmin" policy to this user - policyName := oauth2.GetIDPPolicyForUser() - if err := setPolicy(ctx, adminClient, policyName, accessKey, models.PolicyEntityUser); err != nil { - return nil, prepareError(err) - } - // obtain the current policy details, necessary for generating the list of allowed endpoints - policy, err := adminClient.getPolicy(ctx, policyName) + userAdminClient := adminClient{client: mAdminClient} + // Obtain the current policy assigned to this user + // necessary for generating the list of allowed endpoints + policy, err := getAccountPolicy(ctx, userAdminClient) if err != nil { - return nil, prepareError(err) + return nil, prepareError(errorGeneric, nil, err) } - actions := acl.GetActionsStringFromPolicy(policy) - // User was created correctly, create a new session - creds, err := newConsoleCredentials(accessKey, secretKey, location) - if err != nil { - return nil, prepareError(err) + // by default every user starts with an empty array of available actions + // therefore we would have access only to pages that doesn't require any privilege + // ie: service-account page + var actions []string + // if a policy is assigned to this user we parse the actions from there + if policy != nil { + actions = acl.GetActionsStringFromPolicy(policy) } - credentials := consoleCredentials{consoleCredentials: creds, actions: actions} - token, err := login(credentials) + // login user against console and generate session token + token, err := login(&consoleCredentials{ + consoleCredentials: userCredentials, + accountAccessKey: "", + accountSecretKey: "", + actions: actions, + }) if err != nil { return nil, prepareError(errInvalidCredentials, nil, err) } @@ -281,8 +275,8 @@ func getLoginOperatorResponse(lmr *models.LoginOperatorRequest) (*models.LoginRe if err != nil { return nil, prepareError(err) } - credentials := consoleCredentials{consoleCredentials: creds, actions: []string{}} - token, err := login(credentials) + consoleCreds := consoleCredentials{consoleCredentials: creds, actions: []string{}} + token, err := login(consoleCreds) if err != nil { return nil, prepareError(errInvalidCredentials, nil, err) } diff --git a/restapi/user_login_test.go b/restapi/user_login_test.go index c0aef34bd..9e7fc5683 100644 --- a/restapi/user_login_test.go +++ b/restapi/user_login_test.go @@ -19,13 +19,16 @@ package restapi import ( "context" "errors" + "reflect" "testing" - "github.com/minio/console/pkg/auth" - "github.com/minio/console/pkg/auth/idp/oauth2" - "github.com/minio/minio-go/v7/pkg/credentials" - "github.com/minio/minio/cmd/config" "github.com/minio/minio/pkg/madmin" + + iampolicy "github.com/minio/minio/pkg/iam/policy" + + "github.com/minio/console/pkg/auth" + + "github.com/minio/minio-go/v7/pkg/credentials" "github.com/stretchr/testify/assert" ) @@ -76,133 +79,110 @@ func TestLogin(t *testing.T) { funcAssert.NotNil(err, "not error returned creating a session") } -type IdentityProviderClientMock struct{} +type IdentityProviderMock struct{} -var idpVerifyIdentityMock func(ctx context.Context, code, state string) (*oauth2.User, error) +var idpVerifyIdentityMock func(ctx context.Context, code, state string) (*credentials.Credentials, error) var idpGenerateLoginURLMock func() string -func (ac IdentityProviderClientMock) VerifyIdentity(ctx context.Context, code, state string) (*oauth2.User, error) { +func (ac IdentityProviderMock) VerifyIdentity(ctx context.Context, code, state string) (*credentials.Credentials, error) { return idpVerifyIdentityMock(ctx, code, state) } -func (ac IdentityProviderClientMock) GenerateLoginURL() string { +func (ac IdentityProviderMock) GenerateLoginURL() string { return idpGenerateLoginURLMock() } -// TestLoginOauth2Auth is the main function that test the Oauth2 Authentication -func TestLoginOauth2Auth(t *testing.T) { - ctx := context.Background() - funcAssert := assert.New(t) - // mock data +func Test_validateUserAgainstIDP(t *testing.T) { + provider := IdentityProviderMock{} mockCode := "EAEAEAE" mockState := "HUEHUEHUE" - idpClientMock := IdentityProviderClientMock{} - identityProvider := &auth.IdentityProvider{Client: idpClientMock} - // Test-1 : loginOauth2Auth() correctly authenticates the user - idpVerifyIdentityMock = func(ctx context.Context, code, state string) (*oauth2.User, error) { - return &oauth2.User{}, nil - } - function := "loginOauth2Auth()" - _, err := loginOauth2Auth(ctx, identityProvider, mockCode, mockState) - if err != nil { - t.Errorf("Failed on %s:, error occurred: %s", function, err.Error()) - } - // Test-2 : loginOauth2Auth() returns an error - idpVerifyIdentityMock = func(ctx context.Context, code, state string) (*oauth2.User, error) { - return nil, errors.New("error") - } - if _, err := loginOauth2Auth(ctx, identityProvider, mockCode, mockState); funcAssert.Error(err) { - funcAssert.Equal(errInvalidCredentials.Error(), err.Error()) - } -} - -func Test_getConfiguredRegion(t *testing.T) { - client := adminClientMock{} type args struct { - client adminClientMock + ctx context.Context + provider auth.IdentityProviderI + code string + state string } - tests := []struct { - name string - args args - want string - mock func() + name string + args args + want *credentials.Credentials + wantErr bool + mockFunc func() }{ - // If MinIO returns an error, we return empty region name { - name: "region", + name: "failed to verify user identity with idp", args: args{ - client: client, + ctx: context.Background(), + provider: provider, + code: mockCode, + state: mockState, }, - want: "", - mock: func() { - // mock function response from getConfig() - minioGetConfigKVMock = func(key string) ([]byte, error) { - return nil, errors.New("invalid config") - } - // mock function response from listConfig() - minioHelpConfigKVMock = func(subSys, key string, envOnly bool) (madmin.Help, error) { - return madmin.Help{}, errors.New("no help") - } - }, - }, - // MinIO returns an empty region name - { - name: "region", - args: args{ - client: client, - }, - want: "", - mock: func() { - // mock function response from getConfig() - minioGetConfigKVMock = func(key string) ([]byte, error) { - return []byte("region name= "), nil - } - // mock function response from listConfig() - minioHelpConfigKVMock = func(subSys, key string, envOnly bool) (madmin.Help, error) { - return madmin.Help{ - SubSys: config.RegionSubSys, - Description: "label the location of the server", - MultipleTargets: false, - KeysHelp: []madmin.HelpKV{ - { - Key: "name", - Description: "name of the location of the server e.g. \"us-west-rack2\"", - Optional: true, - Type: "string", - MultipleTargets: false, - }, - { - Key: "comment", - Description: "optionally add a comment to this setting", - Optional: true, - Type: "sentence", - MultipleTargets: false, - }, - }, - }, nil - } - }, - }, - // MinIO returns the asia region - { - name: "region", - args: args{ - client: client, - }, - want: "asia", - mock: func() { - minioGetConfigKVMock = func(key string) ([]byte, error) { - return []byte("region name=asia "), nil + want: nil, + wantErr: true, + mockFunc: func() { + idpVerifyIdentityMock = func(ctx context.Context, code, state string) (*credentials.Credentials, error) { + return nil, errors.New("something went wrong") } }, }, } for _, tt := range tests { - tt.mock() t.Run(tt.name, func(t *testing.T) { - if got, _ := getConfiguredRegionForLogin(context.Background(), tt.args.client); got != tt.want { - t.Errorf("getConfiguredRegionForLogin() = %v, want %v", got, tt.want) + if tt.mockFunc != nil { + tt.mockFunc() + } + got, err := verifyUserAgainstIDP(tt.args.ctx, tt.args.provider, tt.args.code, tt.args.state) + if (err != nil) != tt.wantErr { + t.Errorf("verifyUserAgainstIDP() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("verifyUserAgainstIDP() got = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_getAccountPolicy(t *testing.T) { + client := adminClientMock{} + type args struct { + ctx context.Context + client MinioAdmin + } + tests := []struct { + name string + args args + want *iampolicy.Policy + wantErr bool + mockFunc func() + }{ + { + name: "error getting account policy", + args: args{ + ctx: context.Background(), + client: client, + }, + want: nil, + wantErr: true, + mockFunc: func() { + minioAccountInfoMock = func(ctx context.Context) (madmin.AccountInfo, error) { + return madmin.AccountInfo{}, errors.New("something went wrong") + } + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.mockFunc != nil { + tt.mockFunc() + } + got, err := getAccountPolicy(tt.args.ctx, tt.args.client) + if (err != nil) != tt.wantErr { + t.Errorf("getAccountPolicy() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("getAccountPolicy() got = %v, want %v", got, tt.want) } }) }