Skip to content

Commit c48a024

Browse files
AlevskAlevsk
authored
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
1 parent 3fcf278 commit c48a024

File tree

12 files changed

+13
-47
lines changed

12 files changed

+13
-47
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
}

‎pkg/auth/token_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,14 @@ func TestNewJWTWithClaimsForClient(t *testing.T) {
3636
funcAssert := assert.New(t)
3737
// Test-1 : NewEncryptedTokenForClient() is generated correctly without errors
3838
function := "NewEncryptedTokenForClient()"
39-
token, err := NewEncryptedTokenForClient(creds, "", "", []string{""})
39+
token, err := NewEncryptedTokenForClient(creds, "", []string{""})
4040
if err != nil || token == "" {
4141
t.Errorf("Failed on %s:, error occurred: %s", function, err)
4242
}
4343
// saving token for future tests
4444
goodToken = token
4545
// Test-2 : NewEncryptedTokenForClient() throws error because of empty credentials
46-
if _, err = NewEncryptedTokenForClient(nil, "", "", []string{""}); err != nil {
46+
if _, err = NewEncryptedTokenForClient(nil, "", []string{""}); err != nil {
4747
funcAssert.Equal("provided credentials are empty", err.Error())
4848
}
4949
}

‎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 {

0 commit comments

Comments
 (0)