You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by ti...@apache.org on 2023/02/03 06:20:31 UTC
[servicecomb-service-center] branch master updated: [fix]bad performance in login API (#1373)
This is an automated email from the ASF dual-hosted git repository.
tianxiaoliang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-service-center.git
The following commit(s) were added to refs/heads/master by this push:
new d25cb40d [fix]bad performance in login API (#1373)
d25cb40d is described below
commit d25cb40d40eb2c8def891791229f7a377c74f334
Author: little-cui <su...@qq.com>
AuthorDate: Fri Feb 3 14:20:25 2023 +0800
[fix]bad performance in login API (#1373)
---
pkg/privacy/password_test.go | 35 +++++++++++++++++++++---
server/service/rbac/rbac.go | 36 ++++++++++++------------
server/service/rbac/rbac_test.go | 59 ++++++++++++++++++++++++++--------------
3 files changed, 87 insertions(+), 43 deletions(-)
diff --git a/pkg/privacy/password_test.go b/pkg/privacy/password_test.go
index e2a32514..448b023d 100644
--- a/pkg/privacy/password_test.go
+++ b/pkg/privacy/password_test.go
@@ -38,24 +38,51 @@ func (m mockPassword) CheckPassword(hashedPwd, pwd string) bool {
return true
}
-func BenchmarkScrypt(b *testing.B) {
+func BenchmarkSamePassword(b *testing.B) {
h, _ := privacy.ScryptPassword("test")
for i := 0; i < b.N; i++ {
same := privacy.SamePassword(h, "test")
if !same {
- panic("")
+ b.Fatal()
}
}
b.ReportAllocs()
}
-func BenchmarkScryptP(b *testing.B) {
+func BenchmarkSamePasswordP500(b *testing.B) {
h, _ := privacy.ScryptPassword("test")
+ b.SetParallelism(500)
b.RunParallel(func(pb *testing.PB) {
for pb.Next() {
same := privacy.SamePassword(h, "test")
if !same {
- panic("")
+ b.Fatal()
+ }
+ }
+ })
+ b.ReportAllocs()
+}
+func BenchmarkSamePasswordP1000(b *testing.B) {
+ h, _ := privacy.ScryptPassword("test")
+ b.SetParallelism(1000)
+ b.RunParallel(func(pb *testing.PB) {
+ for pb.Next() {
+ same := privacy.SamePassword(h, "test")
+ if !same {
+ b.Fatal()
+ }
+ }
+ })
+ b.ReportAllocs()
+}
+func BenchmarkSamePasswordP5000(b *testing.B) {
+ h, _ := privacy.ScryptPassword("test")
+ b.SetParallelism(5000)
+ b.RunParallel(func(pb *testing.PB) {
+ for pb.Next() {
+ same := privacy.SamePassword(h, "test")
+ if !same {
+ b.Fatal()
}
}
})
diff --git a/server/service/rbac/rbac.go b/server/service/rbac/rbac.go
index cd9e8a68..5d1808ef 100644
--- a/server/service/rbac/rbac.go
+++ b/server/service/rbac/rbac.go
@@ -44,6 +44,9 @@ var (
ErrNoPermChangeAccount = errors.New("can not change other account password")
ErrWrongPassword = errors.New("current pwd is wrong")
ErrSamePassword = errors.New("the password can not be same as old one")
+ ErrNoPrivateKey = errors.New("read private key failed")
+
+ privateKey *rsa.PrivateKey
)
// Init decide whether enable rbac function and save the build-in roles to db
@@ -97,11 +100,22 @@ func readPrivateKey() {
log.Fatal("can not read private key", err)
return
}
- err = archaius.Set("rbac_private_key", string(data))
+ content := string(data)
+ err = archaius.Set("rbac_private_key", content)
if err != nil {
log.Fatal("can not init rbac", err)
return
}
+ privateKeyContent, err := cipher.Decrypt(content)
+ if err != nil {
+ log.Warn("cipher fallback: " + err.Error())
+ privateKeyContent = content
+ }
+ privateKey, err = secret.ParseRSAPrivateKey(privateKeyContent)
+ if err != nil {
+ log.Error("can not parse private key", err)
+ return
+ }
log.Info("read private key success")
}
@@ -167,26 +181,12 @@ func PublicKey() string {
return archaius.GetString("rbac_public_key", "")
}
-// privateKey get decrypted private key to verify a token
-func privateKey() string {
- ep := archaius.GetString("rbac_private_key", "")
- p, err := cipher.Decrypt(ep)
- if err != nil {
- log.Warn("cipher fallback: " + err.Error())
- return ep
- }
- return p
-}
-
// GetPrivateKey return rsa key instance
func GetPrivateKey() (*rsa.PrivateKey, error) {
- sk := privateKey()
- p, err := secret.ParseRSAPrivateKey(sk)
- if err != nil {
- log.Error("can not get key:", err)
- return nil, err
+ if privateKey == nil {
+ return nil, ErrNoPrivateKey
}
- return p, nil
+ return privateKey, nil
}
// MakeBanKey return ban key
diff --git a/server/service/rbac/rbac_test.go b/server/service/rbac/rbac_test.go
index e411a761..b424eaeb 100644
--- a/server/service/rbac/rbac_test.go
+++ b/server/service/rbac/rbac_test.go
@@ -22,10 +22,11 @@ import (
"os"
"testing"
+ _ "github.com/apache/servicecomb-service-center/test"
+
"github.com/apache/servicecomb-service-center/pkg/privacy"
"github.com/apache/servicecomb-service-center/server/config"
rbacsvc "github.com/apache/servicecomb-service-center/server/service/rbac"
- _ "github.com/apache/servicecomb-service-center/test"
beego "github.com/beego/beego/v2/server/web"
"github.com/go-chassis/cari/discovery"
"github.com/go-chassis/cari/pkg/errsvc"
@@ -68,10 +69,12 @@ func init() {
}
func TestInitRBAC(t *testing.T) {
+ ctx := context.Background()
+
t.Run("login and authenticate", func(t *testing.T) {
- token, err := authr.Login(context.Background(), "root", "Complicated_password1")
+ token, err := authr.Login(ctx, "root", "Complicated_password1")
assert.NoError(t, err)
- claims, err := authr.Authenticate(context.Background(), token)
+ claims, err := authr.Authenticate(ctx, token)
assert.NoError(t, err)
assert.Equal(t, "root", claims.(map[string]interface{})[rbac.ClaimsUser])
})
@@ -80,35 +83,43 @@ func TestInitRBAC(t *testing.T) {
rbacsvc.Init()
})
+ t.Run("get private key after init should be not nil", func(t *testing.T) {
+ key, err := rbacsvc.GetPrivateKey()
+ assert.NoError(t, err)
+ assert.NotNil(t, key)
+ })
+
t.Run("change pwd,admin can change any one password", func(t *testing.T) {
- persisted := newAccount("admin_change_other_pwd")
- err := rbacsvc.CreateAccount(context.Background(), persisted)
+ accountName := "admin_change_other_pwd"
+ persisted := newAccount(accountName)
+ err := rbacsvc.CreateAccount(ctx, persisted)
assert.NoError(t, err)
- context.Background()
+ defer rbacsvc.DeleteAccount(ctx, accountName)
claims := map[string]interface{}{
rbac.ClaimsUser: "test",
rbac.ClaimsRoles: []interface{}{rbac.RoleAdmin},
}
- ctx := context.WithValue(context.Background(), rbacsvc.CtxRequestClaims, claims)
+ ctx := context.WithValue(ctx, rbacsvc.CtxRequestClaims, claims)
err = rbacsvc.ChangePassword(ctx, &rbac.Account{Name: persisted.Name, Password: "Complicated_password2"})
assert.NoError(t, err)
- a, err := rbacsvc.GetAccount(context.Background(), persisted.Name)
+ a, err := rbacsvc.GetAccount(ctx, persisted.Name)
assert.NoError(t, err)
assert.True(t, privacy.SamePassword(a.Password, "Complicated_password2"))
})
t.Run("admin change self, must provide current pwd", func(t *testing.T) {
- name := "admin_change_self"
- a := newAccount(name)
+ accountName := "admin_change_self"
+ a := newAccount(accountName)
a.Roles = []string{rbac.RoleAdmin}
err := rbacsvc.CreateAccount(context.TODO(), a)
assert.Nil(t, err)
+ defer rbacsvc.DeleteAccount(ctx, accountName)
claims := map[string]interface{}{
- rbac.ClaimsUser: name,
+ rbac.ClaimsUser: accountName,
rbac.ClaimsRoles: []interface{}{rbac.RoleAdmin},
}
- ctx := context.WithValue(context.Background(), rbacsvc.CtxRequestClaims, claims)
+ ctx := context.WithValue(ctx, rbacsvc.CtxRequestClaims, claims)
err = rbacsvc.ChangePassword(ctx, &rbac.Account{Name: a.Name, CurrentPassword: "", Password: testPwd1})
assert.True(t, errsvc.IsErrEqualCode(err, discovery.ErrInvalidParams))
@@ -116,33 +127,39 @@ func TestInitRBAC(t *testing.T) {
assert.Nil(t, err)
})
t.Run("change self password", func(t *testing.T) {
- a := newAccount("change_self_pwd")
- err := rbacsvc.CreateAccount(context.Background(), a)
+ accountName := "change_self_pwd"
+ a := newAccount(accountName)
+ err := rbacsvc.CreateAccount(ctx, a)
assert.NoError(t, err)
+ defer rbacsvc.DeleteAccount(ctx, accountName)
+
claims := map[string]interface{}{
- rbac.ClaimsUser: "change_self_pwd",
+ rbac.ClaimsUser: accountName,
rbac.ClaimsRoles: []interface{}{rbac.RoleDeveloper},
}
- ctx := context.WithValue(context.Background(), rbacsvc.CtxRequestClaims, claims)
+ ctx := context.WithValue(ctx, rbacsvc.CtxRequestClaims, claims)
err = rbacsvc.ChangePassword(ctx, &rbac.Account{Name: a.Name, CurrentPassword: testPwd0, Password: testPwd1})
assert.NoError(t, err)
- resp, err := rbacsvc.GetAccount(context.Background(), a.Name)
+ resp, err := rbacsvc.GetAccount(ctx, a.Name)
assert.NoError(t, err)
assert.True(t, privacy.SamePassword(resp.Password, testPwd1))
})
t.Run("no admin account change other user password, should return: "+discovery.NewError(discovery.ErrForbidden, "").Error(), func(t *testing.T) {
- a := newAccount("test")
+ accountName := "test"
+ a := newAccount(accountName)
+ defer rbacsvc.DeleteAccount(ctx, accountName)
+
claims := map[string]interface{}{
rbac.ClaimsUser: "change_other_user_password",
rbac.ClaimsRoles: []interface{}{rbac.RoleDeveloper},
}
- ctx := context.WithValue(context.Background(), rbacsvc.CtxRequestClaims, claims)
+ ctx := context.WithValue(ctx, rbacsvc.CtxRequestClaims, claims)
err := rbacsvc.ChangePassword(ctx, &rbac.Account{Name: a.Name, CurrentPassword: testPwd0, Password: testPwd1})
assert.True(t, errsvc.IsErrEqualCode(err, discovery.ErrForbidden))
})
}
-func BenchmarkAuthResource_Login(b *testing.B) {
+func BenchmarkAuthResource_LoginP(b *testing.B) {
b.RunParallel(func(pb *testing.PB) {
for pb.Next() {
_, err := authr.Login(context.TODO(), "root", "Complicated_password1")
@@ -153,7 +170,7 @@ func BenchmarkAuthResource_Login(b *testing.B) {
})
b.ReportAllocs()
}
-func BenchmarkAuthResource_Login2(b *testing.B) {
+func BenchmarkAuthResource_Login(b *testing.B) {
for i := 0; i < b.N; i++ {
_, err := authr.Login(context.TODO(), "root", "Complicated_password1")
if err != nil {