Remove use of Privileged Credentials (#535)

- Leverage on MinIO Oauth integration instead of the 
  current Console implementation
- Refactor pkg/idp
- Added tests to login
This commit is contained in:
Lenin Alevski
2021-01-07 13:49:56 -06:00
committed by GitHub
parent 5b98bb8fd6
commit 58903e8337
14 changed files with 186 additions and 303 deletions

View File

@@ -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),
},
}

View File

@@ -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
}

View File

@@ -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"))
}

View File

@@ -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"

View File

@@ -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)
}

View File

@@ -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)
}

View File

@@ -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)
}
})
}