Skip to content

Commit ef05054

Browse files
committed
Remove user secret key from encrypted session token
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 credentiails
1 parent 3fcf278 commit ef05054

File tree

11 files changed

+11
-45
lines changed

11 files changed

+11
-45
lines changed

models/principal.go

Lines changed: 0 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/auth/token.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ type TokenClaims struct {
6565
STSSecretAccessKey string `json:"stsSecretAccessKey,omitempty"`
6666
STSSessionToken string `json:"stsSessionToken,omitempty"`
6767
AccountAccessKey string `json:"accountAccessKey,omitempty"`
68-
AccountSecretKey string `json:"accountSecretKey,omitempty"`
6968
Actions []string `json:"actions,omitempty"`
7069
}
7170

@@ -79,7 +78,6 @@ type TokenClaims struct {
7978
// STSSecretAccessKey
8079
// STSSessionToken
8180
// AccountAccessKey
82-
// AccountSecretKey
8381
// Actions
8482
// }
8583
func SessionTokenAuthenticate(token string) (*TokenClaims, error) {
@@ -100,14 +98,13 @@ func SessionTokenAuthenticate(token string) (*TokenClaims, error) {
10098

10199
// NewEncryptedTokenForClient generates a new session token with claims based on the provided STS credentials, first
102100
// encrypts the claims and the sign them
103-
func NewEncryptedTokenForClient(credentials *credentials.Value, accountAccessKey, accountSecretKey string, actions []string) (string, error) {
101+
func NewEncryptedTokenForClient(credentials *credentials.Value, accountAccessKey string, actions []string) (string, error) {
104102
if credentials != nil {
105103
encryptedClaims, err := encryptClaims(&TokenClaims{
106104
STSAccessKeyID: credentials.AccessKeyID,
107105
STSSecretAccessKey: credentials.SecretAccessKey,
108106
STSSessionToken: credentials.SessionToken,
109107
AccountAccessKey: accountAccessKey,
110-
AccountSecretKey: accountSecretKey,
111108
Actions: actions,
112109
})
113110
if err != nil {
@@ -330,6 +327,5 @@ func GetClaimsFromTokenInRequest(req *http.Request) (*models.Principal, error) {
330327
STSSecretAccessKey: claims.STSSecretAccessKey,
331328
STSSessionToken: claims.STSSessionToken,
332329
AccountAccessKey: claims.AccountAccessKey,
333-
AccountSecretKey: claims.AccountSecretKey,
334330
}, nil
335331
}

restapi/client.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -242,15 +242,13 @@ type ConsoleCredentialsI interface {
242242
Get() (credentials.Value, error)
243243
Expire()
244244
GetAccountAccessKey() string
245-
GetAccountSecretKey() string
246245
GetActions() []string
247246
}
248247

249248
// Interface implementation
250249
type consoleCredentials struct {
251250
consoleCredentials *credentials.Credentials
252251
accountAccessKey string
253-
accountSecretKey string
254252
actions []string
255253
}
256254

@@ -262,10 +260,6 @@ func (c consoleCredentials) GetAccountAccessKey() string {
262260
return c.accountAccessKey
263261
}
264262

265-
func (c consoleCredentials) GetAccountSecretKey() string {
266-
return c.accountSecretKey
267-
}
268-
269263
// implements *Login.Get()
270264
func (c consoleCredentials) Get() (credentials.Value, error) {
271265
return c.consoleCredentials.Get()

restapi/configure_console.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,6 @@ func configureAPI(api *operations.ConsoleAPI) http.Handler {
8484
STSSecretAccessKey: claims.STSSecretAccessKey,
8585
STSSessionToken: claims.STSSessionToken,
8686
AccountAccessKey: claims.AccountAccessKey,
87-
AccountSecretKey: claims.AccountSecretKey,
8887
}, nil
8988
}
9089

restapi/embedded_spec.go

Lines changed: 0 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

restapi/error.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ var (
3131
errPolicyBodyNotInRequest = errors.New("error policy body not in request")
3232
errInvalidEncryptionAlgorithm = errors.New("error invalid encryption algorithm")
3333
errSSENotConfigured = errors.New("error server side encryption configuration was not found")
34-
errChangePassword = errors.New("unable to update password, please check your current password")
34+
errChangePassword = errors.New("error please check your current password")
3535
errInvalidLicense = errors.New("invalid license key")
3636
errLicenseNotFound = errors.New("license not found")
3737
errAvoidSelfAccountDelete = errors.New("logged in user cannot be deleted by itself")
@@ -95,7 +95,7 @@ func prepareError(err ...error) *models.Error {
9595
errorMessage = errorGenericInvalidSession.Error()
9696
}
9797
// account change password
98-
if errors.Is(err[0], errChangePassword) {
98+
if madmin.ToErrorResponse(err[0]).Code == "SignatureDoesNotMatch" {
9999
errorCode = 403
100100
errorMessage = errChangePassword.Error()
101101
}

restapi/user_account.go

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,7 @@ func registerAccountHandlers(api *operations.ConsoleAPI) {
4545
}
4646

4747
// changePassword validate current current user password and if it's correct set the new password
48-
func changePassword(ctx context.Context, client MinioAdmin, session *models.Principal, currentSecretKey, newSecretKey string) error {
49-
if session.AccountSecretKey != currentSecretKey {
50-
return errChangePassword
51-
}
48+
func changePassword(ctx context.Context, client MinioAdmin, session *models.Principal, newSecretKey string) error {
5249
if err := client.changePassword(ctx, session.AccountAccessKey, newSecretKey); err != nil {
5350
return err
5451
}
@@ -60,22 +57,22 @@ func changePassword(ctx context.Context, client MinioAdmin, session *models.Prin
6057
func getChangePasswordResponse(session *models.Principal, params user_api.AccountChangePasswordParams) (*models.LoginResponse, *models.Error) {
6158
ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
6259
defer cancel()
60+
accessKey := session.AccountAccessKey
61+
currentSecretKey := *params.Body.CurrentSecretKey
62+
newSecretKey := *params.Body.NewSecretKey
6363
// changePassword operations requires an AdminClient initialized with parent account credentials not
6464
// STS credentials
6565
parentAccountClient, err := newMAdminClient(&models.Principal{
6666
STSAccessKeyID: session.AccountAccessKey,
67-
STSSecretAccessKey: session.AccountSecretKey,
67+
STSSecretAccessKey: currentSecretKey,
6868
})
6969
if err != nil {
7070
return nil, prepareError(err)
7171
}
7272
// parentAccountClient will contain access and secret key credentials for the user
7373
userClient := adminClient{client: parentAccountClient}
74-
accessKey := session.AccountAccessKey
75-
currentSecretKey := *params.Body.CurrentSecretKey
76-
newSecretKey := *params.Body.NewSecretKey
7774
// currentSecretKey will compare currentSecretKey against the stored secret key inside the encrypted session
78-
if err := changePassword(ctx, userClient, session, currentSecretKey, newSecretKey); err != nil {
75+
if err := changePassword(ctx, userClient, session, newSecretKey); err != nil {
7976
return nil, prepareError(err)
8077
}
8178
// user credentials are updated at this point, we need to generate a new admin client and authenticate using

restapi/user_account_test.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ func Test_changePassword(t *testing.T) {
4848
ctx: context.Background(),
4949
session: &models.Principal{
5050
AccountAccessKey: "TESTTEST",
51-
AccountSecretKey: "TESTTEST",
5251
},
5352
currentSecretKey: "TESTTEST",
5453
newSecretKey: "TESTTEST2",
@@ -66,7 +65,6 @@ func Test_changePassword(t *testing.T) {
6665
ctx: context.Background(),
6766
session: &models.Principal{
6867
AccountAccessKey: "TESTTEST",
69-
AccountSecretKey: "TESTTEST",
7068
},
7169
currentSecretKey: "TESTTEST",
7270
newSecretKey: "TESTTEST2",
@@ -85,7 +83,6 @@ func Test_changePassword(t *testing.T) {
8583
ctx: context.Background(),
8684
session: &models.Principal{
8785
AccountAccessKey: "TESTTEST",
88-
AccountSecretKey: "TESTTEST123",
8986
},
9087
currentSecretKey: "TESTTEST",
9188
newSecretKey: "TESTTEST2",
@@ -103,7 +100,7 @@ func Test_changePassword(t *testing.T) {
103100
if tt.mock != nil {
104101
tt.mock()
105102
}
106-
if err := changePassword(tt.args.ctx, tt.args.client, tt.args.session, tt.args.currentSecretKey, tt.args.newSecretKey); (err != nil) != tt.wantErr {
103+
if err := changePassword(tt.args.ctx, tt.args.client, tt.args.session, tt.args.newSecretKey); (err != nil) != tt.wantErr {
107104
t.Errorf("changePassword() error = %v, wantErr %v", err, tt.wantErr)
108105
}
109106
})

restapi/user_login.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ func login(credentials ConsoleCredentialsI) (*string, error) {
9393
return nil, err
9494
}
9595
// if we made it here, the consoleCredentials work, generate a jwt with claims
96-
token, err := auth.NewEncryptedTokenForClient(&tokens, credentials.GetAccountAccessKey(), credentials.GetAccountSecretKey(), credentials.GetActions())
96+
token, err := auth.NewEncryptedTokenForClient(&tokens, credentials.GetAccountAccessKey(), credentials.GetActions())
9797
if err != nil {
9898
log.Println("error authenticating user", err)
9999
return nil, errInvalidCredentials
@@ -123,7 +123,6 @@ func getConsoleCredentials(ctx context.Context, accessKey, secretKey string) (*c
123123
cCredentials := &consoleCredentials{
124124
consoleCredentials: creds,
125125
accountAccessKey: accessKey,
126-
accountSecretKey: secretKey,
127126
}
128127
tokens, err := cCredentials.Get()
129128
if err != nil {
@@ -278,7 +277,6 @@ func getLoginOauth2AuthResponse(lr *models.LoginOauth2AuthRequest) (*models.Logi
278277
token, err := login(&consoleCredentials{
279278
consoleCredentials: userCredentials,
280279
accountAccessKey: "",
281-
accountSecretKey: "",
282280
actions: actions,
283281
})
284282
if err != nil {

restapi/user_login_test.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,6 @@ func (ac consoleCredentialsMock) GetAccountAccessKey() string {
4343
return ""
4444
}
4545

46-
func (ac consoleCredentialsMock) GetAccountSecretKey() string {
47-
return ""
48-
}
49-
5046
// Common mocks
5147
var consoleCredentialsGetMock func() (credentials.Value, error)
5248

0 commit comments

Comments
 (0)