Remove user secret key from encrypted session token (#652)

User secret key is not really need it to be stored inside the encrypted
session key, since the `change-password` endpoint requires the user to
provide the current `secret key` that password will be used to
initialize a new minio client then we will leverage on the
`SetUser` operation, this api only works with actual user credentials
and not sts credentials

pkg/auth/token.go

@@ -65,7 +65,6 @@ type TokenClaims struct {
 	STSSecretAccessKey string   `json:"stsSecretAccessKey,omitempty"`
 	STSSessionToken    string   `json:"stsSessionToken,omitempty"`
 	AccountAccessKey   string   `json:"accountAccessKey,omitempty"`
-	AccountSecretKey   string   `json:"accountSecretKey,omitempty"`
 	Actions            []string `json:"actions,omitempty"`
 }
 
@@ -79,7 +78,6 @@ type TokenClaims struct {
 //		STSSecretAccessKey
 //		STSSessionToken
 //		AccountAccessKey
-//		AccountSecretKey
 //		Actions
 //	}
 func SessionTokenAuthenticate(token string) (*TokenClaims, error) {
@@ -100,14 +98,13 @@ func SessionTokenAuthenticate(token string) (*TokenClaims, error) {
 
 // NewEncryptedTokenForClient generates a new session token with claims based on the provided STS credentials, first
 // encrypts the claims and the sign them
-func NewEncryptedTokenForClient(credentials *credentials.Value, accountAccessKey, accountSecretKey string, actions []string) (string, error) {
+func NewEncryptedTokenForClient(credentials *credentials.Value, accountAccessKey string, actions []string) (string, error) {
 	if credentials != nil {
 		encryptedClaims, err := encryptClaims(&TokenClaims{
 			STSAccessKeyID:     credentials.AccessKeyID,
 			STSSecretAccessKey: credentials.SecretAccessKey,
 			STSSessionToken:    credentials.SessionToken,
 			AccountAccessKey:   accountAccessKey,
-			AccountSecretKey:   accountSecretKey,
 			Actions:            actions,
 		})
 		if err != nil {
@@ -330,6 +327,5 @@ func GetClaimsFromTokenInRequest(req *http.Request) (*models.Principal, error) {
 		STSSecretAccessKey: claims.STSSecretAccessKey,
 		STSSessionToken:    claims.STSSessionToken,
 		AccountAccessKey:   claims.AccountAccessKey,
-		AccountSecretKey:   claims.AccountSecretKey,
 	}, nil
 }

pkg/auth/token_test.go

@@ -36,14 +36,14 @@ func TestNewJWTWithClaimsForClient(t *testing.T) {
 	funcAssert := assert.New(t)
 	// Test-1 : NewEncryptedTokenForClient() is generated correctly without errors
 	function := "NewEncryptedTokenForClient()"
-	token, err := NewEncryptedTokenForClient(creds, "", "", []string{""})
+	token, err := NewEncryptedTokenForClient(creds, "", []string{""})
 	if err != nil || token == "" {
 		t.Errorf("Failed on %s:, error occurred: %s", function, err)
 	}
 	// saving token for future tests
 	goodToken = token
 	// Test-2 : NewEncryptedTokenForClient() throws error because of empty credentials
-	if _, err = NewEncryptedTokenForClient(nil, "", "", []string{""}); err != nil {
+	if _, err = NewEncryptedTokenForClient(nil, "", []string{""}); err != nil {
 		funcAssert.Equal("provided credentials are empty", err.Error())
 	}
 }

restapi/client.go

@@ -242,15 +242,13 @@ type ConsoleCredentialsI interface {
 	Get() (credentials.Value, error)
 	Expire()
 	GetAccountAccessKey() string
-	GetAccountSecretKey() string
 	GetActions() []string
 }
 
 // Interface implementation
 type consoleCredentials struct {
 	consoleCredentials *credentials.Credentials
 	accountAccessKey   string
-	accountSecretKey   string
 	actions            []string
 }
 
@@ -262,10 +260,6 @@ func (c consoleCredentials) GetAccountAccessKey() string {
 	return c.accountAccessKey
 }
 
-func (c consoleCredentials) GetAccountSecretKey() string {
-	return c.accountSecretKey
-}
-
 // implements *Login.Get()
 func (c consoleCredentials) Get() (credentials.Value, error) {
 	return c.consoleCredentials.Get()

restapi/configure_console.go

@@ -84,7 +84,6 @@ func configureAPI(api *operations.ConsoleAPI) http.Handler {
 			STSSecretAccessKey: claims.STSSecretAccessKey,
 			STSSessionToken:    claims.STSSessionToken,
 			AccountAccessKey:   claims.AccountAccessKey,
-			AccountSecretKey:   claims.AccountSecretKey,
 		}, nil
 	}
 

restapi/error.go

@@ -31,7 +31,7 @@ var (
 	errPolicyBodyNotInRequest             = errors.New("error policy body not in request")
 	errInvalidEncryptionAlgorithm         = errors.New("error invalid encryption algorithm")
 	errSSENotConfigured                   = errors.New("error server side encryption configuration was not found")
-	errChangePassword                     = errors.New("unable to update password, please check your current password")
+	errChangePassword                     = errors.New("error please check your current password")
 	errInvalidLicense                     = errors.New("invalid license key")
 	errLicenseNotFound                    = errors.New("license not found")
 	errAvoidSelfAccountDelete             = errors.New("logged in user cannot be deleted by itself")
@@ -95,7 +95,7 @@ func prepareError(err ...error) *models.Error {
 			errorMessage = errorGenericInvalidSession.Error()
 		}
 		// account change password
-		if errors.Is(err[0], errChangePassword) {
+		if madmin.ToErrorResponse(err[0]).Code == "SignatureDoesNotMatch" {
 			errorCode = 403
 			errorMessage = errChangePassword.Error()
 		}

restapi/user_account.go

@@ -45,10 +45,7 @@ func registerAccountHandlers(api *operations.ConsoleAPI) {
 }
 
 // changePassword validate current current user password and if it's correct set the new password
-func changePassword(ctx context.Context, client MinioAdmin, session *models.Principal, currentSecretKey, newSecretKey string) error {
-	if session.AccountSecretKey != currentSecretKey {
-		return errChangePassword
-	}
+func changePassword(ctx context.Context, client MinioAdmin, session *models.Principal, newSecretKey string) error {
 	if err := client.changePassword(ctx, session.AccountAccessKey, newSecretKey); err != nil {
 		return err
 	}
@@ -60,22 +57,22 @@ func changePassword(ctx context.Context, client MinioAdmin, session *models.Prin
 func getChangePasswordResponse(session *models.Principal, params user_api.AccountChangePasswordParams) (*models.LoginResponse, *models.Error) {
 	ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
 	defer cancel()
+	accessKey := session.AccountAccessKey
+	currentSecretKey := *params.Body.CurrentSecretKey
+	newSecretKey := *params.Body.NewSecretKey
 	// changePassword operations requires an AdminClient initialized with parent account credentials not
 	// STS credentials
 	parentAccountClient, err := newMAdminClient(&models.Principal{
 		STSAccessKeyID:     session.AccountAccessKey,
-		STSSecretAccessKey: session.AccountSecretKey,
+		STSSecretAccessKey: currentSecretKey,
 	})
 	if err != nil {
 		return nil, prepareError(err)
 	}
 	// parentAccountClient will contain access and secret key credentials for the user
 	userClient := adminClient{client: parentAccountClient}
-	accessKey := session.AccountAccessKey
-	currentSecretKey := *params.Body.CurrentSecretKey
-	newSecretKey := *params.Body.NewSecretKey
 	// currentSecretKey will compare currentSecretKey against the stored secret key inside the encrypted session
-	if err := changePassword(ctx, userClient, session, currentSecretKey, newSecretKey); err != nil {
+	if err := changePassword(ctx, userClient, session, newSecretKey); err != nil {
 		return nil, prepareError(err)
 	}
 	// user credentials are updated at this point, we need to generate a new admin client and authenticate using

restapi/user_account_test.go

@@ -48,7 +48,6 @@ func Test_changePassword(t *testing.T) {
 				ctx:    context.Background(),
 				session: &models.Principal{
 					AccountAccessKey: "TESTTEST",
-					AccountSecretKey: "TESTTEST",
 				},
 				currentSecretKey: "TESTTEST",
 				newSecretKey:     "TESTTEST2",
@@ -66,7 +65,6 @@ func Test_changePassword(t *testing.T) {
 				ctx:    context.Background(),
 				session: &models.Principal{
 					AccountAccessKey: "TESTTEST",
-					AccountSecretKey: "TESTTEST",
 				},
 				currentSecretKey: "TESTTEST",
 				newSecretKey:     "TESTTEST2",
@@ -85,7 +83,6 @@ func Test_changePassword(t *testing.T) {
 				ctx:    context.Background(),
 				session: &models.Principal{
 					AccountAccessKey: "TESTTEST",
-					AccountSecretKey: "TESTTEST123",
 				},
 				currentSecretKey: "TESTTEST",
 				newSecretKey:     "TESTTEST2",
@@ -103,7 +100,7 @@ func Test_changePassword(t *testing.T) {
 			if tt.mock != nil {
 				tt.mock()
 			}
-			if err := changePassword(tt.args.ctx, tt.args.client, tt.args.session, tt.args.currentSecretKey, tt.args.newSecretKey); (err != nil) != tt.wantErr {
+			if err := changePassword(tt.args.ctx, tt.args.client, tt.args.session, tt.args.newSecretKey); (err != nil) != tt.wantErr {
 				t.Errorf("changePassword() error = %v, wantErr %v", err, tt.wantErr)
 			}
 		})

restapi/user_login.go

@@ -93,7 +93,7 @@ func login(credentials ConsoleCredentialsI) (*string, error) {
 		return nil, err
 	}
 	// if we made it here, the consoleCredentials work, generate a jwt with claims
-	token, err := auth.NewEncryptedTokenForClient(&tokens, credentials.GetAccountAccessKey(), credentials.GetAccountSecretKey(), credentials.GetActions())
+	token, err := auth.NewEncryptedTokenForClient(&tokens, credentials.GetAccountAccessKey(), credentials.GetActions())
 	if err != nil {
 		log.Println("error authenticating user", err)
 		return nil, errInvalidCredentials
@@ -123,7 +123,6 @@ func getConsoleCredentials(ctx context.Context, accessKey, secretKey string) (*c
 	cCredentials := &consoleCredentials{
 		consoleCredentials: creds,
 		accountAccessKey:   accessKey,
-		accountSecretKey:   secretKey,
 	}
 	tokens, err := cCredentials.Get()
 	if err != nil {
@@ -278,7 +277,6 @@ func getLoginOauth2AuthResponse(lr *models.LoginOauth2AuthRequest) (*models.Logi
 		token, err := login(&consoleCredentials{
 			consoleCredentials: userCredentials,
 			accountAccessKey:   "",
-			accountSecretKey:   "",
 			actions:            actions,
 		})
 		if err != nil {

restapi/user_login_test.go

@@ -43,10 +43,6 @@ func (ac consoleCredentialsMock) GetAccountAccessKey() string {
 	return ""
 }
 
-func (ac consoleCredentialsMock) GetAccountSecretKey() string {
-	return ""
-}
-
 // Common mocks
 var consoleCredentialsGetMock func() (credentials.Value, error)
 

swagger.yml

@@ -2612,8 +2612,6 @@ definitions:
           type: string
       accountAccessKey:
         type: string
-      accountSecretKey:
-        type: string
   startProfilingItem:
     type: object
     properties: