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 2021/05/30 09:57:18 UTC
[servicecomb-service-center] branch master updated: SCB-2176 Fix:
should not update or delete your own account info (#1026)
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 c42b61d SCB-2176 Fix: should not update or delete your own account info (#1026)
c42b61d is described below
commit c42b61d29d6ea40faf9bb651d38c3eb3727911c6
Author: little-cui <su...@qq.com>
AuthorDate: Sun May 30 17:56:17 2021 +0800
SCB-2176 Fix: should not update or delete your own account info (#1026)
---
pkg/errors/text.go | 1 +
server/handler/auth/auth.go | 1 -
server/plugin/auth/buildin/buildin.go | 9 +--
server/resource/v4/auth_resource.go | 27 ++++++--
server/resource/v4/auth_resource_test.go | 51 +++++++++++---
.../text.go => server/service/rbac/context.go | 40 +++++++----
server/service/rbac/context_test.go | 78 ++++++++++++++++++++++
7 files changed, 171 insertions(+), 36 deletions(-)
diff --git a/pkg/errors/text.go b/pkg/errors/text.go
index d5696d5..5b7e5ea 100644
--- a/pkg/errors/text.go
+++ b/pkg/errors/text.go
@@ -23,6 +23,7 @@ const (
MsgOperateAccountFailed = "operate account failed"
MsgCantOperateRoot = "root can not be operated"
MsgCantOperateRole = "build-in role can not be operated"
+ MsgCantOperateYour = "your account can not be operated"
MsgOperateRoleFailed = "operate role failed"
MsgGetAccountFailed = "get account failed"
MsgGetRoleFailed = "get role failed"
diff --git a/server/handler/auth/auth.go b/server/handler/auth/auth.go
index 1966b02..b67c465 100644
--- a/server/handler/auth/auth.go
+++ b/server/handler/auth/auth.go
@@ -31,7 +31,6 @@ import (
const (
CtxResourceLabels util.CtxKey = "_resource_labels"
CtxResourceScopes util.CtxKey = "_resource_scopes"
- CtxRequestClaims util.CtxKey = "_request_claims"
)
type Handler struct {
diff --git a/server/plugin/auth/buildin/buildin.go b/server/plugin/auth/buildin/buildin.go
index 3117e30..9b4a616 100644
--- a/server/plugin/auth/buildin/buildin.go
+++ b/server/plugin/auth/buildin/buildin.go
@@ -78,7 +78,7 @@ func (ba *TokenAuthenticator) Identify(req *http.Request) error {
log.Error("get account failed", err)
return err
}
- util.SetRequestContext(req, authHandler.CtxRequestClaims, m)
+ util.SetRequestContext(req, rbacsvc.CtxRequestClaims, m)
// user can change self password
if isChangeSelfPassword(pattern, account, req) {
return nil
@@ -164,10 +164,3 @@ func (ba *TokenAuthenticator) ResourceScopes(r *http.Request) *auth.ResourceScop
}
return FromRequest(r)
}
-func AccountFromContext(ctx context.Context) (*rbac.Account, error) {
- m, ok := ctx.Value(authHandler.CtxRequestClaims).(map[string]interface{})
- if !ok {
- return nil, errors.New("no claims from request context")
- }
- return rbac.GetAccount(m)
-}
diff --git a/server/resource/v4/auth_resource.go b/server/resource/v4/auth_resource.go
index 2cbaab4..98a5ca4 100644
--- a/server/resource/v4/auth_resource.go
+++ b/server/resource/v4/auth_resource.go
@@ -29,7 +29,6 @@ import (
"github.com/apache/servicecomb-service-center/pkg/log"
"github.com/apache/servicecomb-service-center/pkg/rest"
"github.com/apache/servicecomb-service-center/pkg/util"
- "github.com/apache/servicecomb-service-center/server/plugin/auth/buildin"
rbacsvc "github.com/apache/servicecomb-service-center/server/service/rbac"
"github.com/apache/servicecomb-service-center/server/service/rbac/dao"
"github.com/apache/servicecomb-service-center/server/service/validator"
@@ -54,6 +53,7 @@ func (ar *AuthResource) URLPatterns() []rest.Route {
{Method: http.MethodPost, Path: "/v4/accounts/:name/password", Func: ar.ChangePassword},
}
}
+
func (ar *AuthResource) CreateAccount(w http.ResponseWriter, req *http.Request) {
body, err := ioutil.ReadAll(req.Body)
if err != nil {
@@ -89,10 +89,10 @@ func (ar *AuthResource) CreateAccount(w http.ResponseWriter, req *http.Request)
}
rest.WriteSuccess(w, req)
}
+
func (ar *AuthResource) DeleteAccount(w http.ResponseWriter, req *http.Request) {
name := req.URL.Query().Get(":name")
- if name == rbacsvc.RootName {
- rest.WriteError(w, discovery.ErrInvalidParams, errorsEx.MsgCantOperateRoot)
+ if ar.illegalCheck(w, req, name) {
return
}
_, err := dao.DeleteAccount(context.TODO(), name)
@@ -103,10 +103,10 @@ func (ar *AuthResource) DeleteAccount(w http.ResponseWriter, req *http.Request)
}
rest.WriteSuccess(w, req)
}
+
func (ar *AuthResource) UpdateAccount(w http.ResponseWriter, req *http.Request) {
name := req.URL.Query().Get(":name")
- if name == rbacsvc.RootName {
- rest.WriteError(w, discovery.ErrInvalidParams, errorsEx.MsgCantOperateRoot)
+ if ar.illegalCheck(w, req, name) {
return
}
body, err := ioutil.ReadAll(req.Body)
@@ -130,6 +130,7 @@ func (ar *AuthResource) UpdateAccount(w http.ResponseWriter, req *http.Request)
}
rest.WriteSuccess(w, req)
}
+
func (ar *AuthResource) ListAccount(w http.ResponseWriter, r *http.Request) {
as, n, err := dao.ListAccount(context.TODO())
if err != nil {
@@ -143,6 +144,7 @@ func (ar *AuthResource) ListAccount(w http.ResponseWriter, r *http.Request) {
}
rest.WriteResponse(w, r, nil, resp)
}
+
func (ar *AuthResource) GetAccount(w http.ResponseWriter, r *http.Request) {
a, err := dao.GetAccount(context.TODO(), r.URL.Query().Get(":name"))
if err != nil {
@@ -154,6 +156,19 @@ func (ar *AuthResource) GetAccount(w http.ResponseWriter, r *http.Request) {
rest.WriteResponse(w, r, nil, a)
}
+func (ar *AuthResource) illegalCheck(w http.ResponseWriter, req *http.Request, name string) bool {
+ if name == rbacsvc.RootName {
+ rest.WriteError(w, discovery.ErrInvalidParams, errorsEx.MsgCantOperateRoot)
+ return true
+ }
+ user := rbacsvc.UserFromContext(req.Context())
+ if name == user {
+ rest.WriteError(w, discovery.ErrInvalidParams, errorsEx.MsgCantOperateYour)
+ return true
+ }
+ return false
+}
+
func (ar *AuthResource) ChangePassword(w http.ResponseWriter, req *http.Request) {
name := req.URL.Query().Get(":name")
ip := util.GetRealIP(req)
@@ -181,7 +196,7 @@ func (ar *AuthResource) ChangePassword(w http.ResponseWriter, req *http.Request)
return
}
- changer, err := buildin.AccountFromContext(req.Context())
+ changer, err := rbacsvc.AccountFromContext(req.Context())
if err != nil {
rest.WriteError(w, discovery.ErrInternal, "can not parse account info")
return
diff --git a/server/resource/v4/auth_resource_test.go b/server/resource/v4/auth_resource_test.go
index 624440a..6e0e233 100644
--- a/server/resource/v4/auth_resource_test.go
+++ b/server/resource/v4/auth_resource_test.go
@@ -94,7 +94,7 @@ func TestAuthResource_Login(t *testing.T) {
})
// root account token
- var to = &rbacmodel.Token{}
+ var rootToken = &rbacmodel.Token{}
t.Run("root login", func(t *testing.T) {
b, _ := json.Marshal(&rbacmodel.Account{Name: "root", Password: pwd, Roles: []string{"admin"}})
@@ -102,7 +102,7 @@ func TestAuthResource_Login(t *testing.T) {
w := httptest.NewRecorder()
rest.GetRouter().ServeHTTP(w, r)
assert.Equal(t, http.StatusOK, w.Code)
- json.Unmarshal(w.Body.Bytes(), to)
+ json.Unmarshal(w.Body.Bytes(), rootToken)
})
t.Run("invalid password", func(t *testing.T) {
@@ -118,7 +118,7 @@ func TestAuthResource_Login(t *testing.T) {
b, _ := json.Marshal(&rbacmodel.Account{Name: "dev_account", Password: "Complicated_password1", Roles: []string{"developer"}})
r, _ := http.NewRequest(http.MethodPost, "/v4/accounts", bytes.NewBuffer(b))
- r.Header.Set(restful.HeaderAuth, "Bearer "+to.TokenStr)
+ r.Header.Set(restful.HeaderAuth, "Bearer "+rootToken.TokenStr)
w := httptest.NewRecorder()
rest.GetRouter().ServeHTTP(w, r)
assert.Equal(t, http.StatusOK, w.Code)
@@ -126,7 +126,7 @@ func TestAuthResource_Login(t *testing.T) {
t.Run("given a valid token and deleted account, auth should fail", func(t *testing.T) {
b, _ := json.Marshal(&rbacmodel.Account{Name: "admin_account", Password: "Complicated_password1", Roles: []string{"admin"}})
r, _ := http.NewRequest(http.MethodPost, "/v4/accounts", bytes.NewBuffer(b))
- r.Header.Set(restful.HeaderAuth, "Bearer "+to.TokenStr)
+ r.Header.Set(restful.HeaderAuth, "Bearer "+rootToken.TokenStr)
w := httptest.NewRecorder()
rest.GetRouter().ServeHTTP(w, r)
assert.Equal(t, http.StatusOK, w.Code)
@@ -140,7 +140,7 @@ func TestAuthResource_Login(t *testing.T) {
json.Unmarshal(w2.Body.Bytes(), deletedToken)
r3, _ := http.NewRequest(http.MethodDelete, "/v4/accounts/admin_account", nil)
- r3.Header.Set(restful.HeaderAuth, "Bearer "+deletedToken.TokenStr)
+ r3.Header.Set(restful.HeaderAuth, "Bearer "+rootToken.TokenStr)
w3 := httptest.NewRecorder()
rest.GetRouter().ServeHTTP(w3, r3)
assert.Equal(t, http.StatusOK, w3.Code)
@@ -161,7 +161,7 @@ func TestAuthResource_Login(t *testing.T) {
b2, _ := json.Marshal(&rbacmodel.Account{Name: "dev_account", CurrentPassword: "Complicated_password1", Password: "Complicated_password2"})
r2, _ := http.NewRequest(http.MethodPost, "/v4/accounts/dev_account/password", bytes.NewBuffer(b2))
- r2.Header.Set(restful.HeaderAuth, "Bearer "+to.TokenStr)
+ r2.Header.Set(restful.HeaderAuth, "Bearer "+rootToken.TokenStr)
w2 := httptest.NewRecorder()
rest.GetRouter().ServeHTTP(w2, r2)
assert.Equal(t, http.StatusOK, w2.Code)
@@ -175,6 +175,8 @@ func TestAuthResource_Login(t *testing.T) {
}
func TestAuthResource_DeleteAccount(t *testing.T) {
+ rootToken := &rbacmodel.Token{}
+
t.Run("given root, try to delete it, should fail ", func(t *testing.T) {
b, _ := json.Marshal(&rbacmodel.Account{Name: "root", Password: "Complicated_password1"})
@@ -182,11 +184,10 @@ func TestAuthResource_DeleteAccount(t *testing.T) {
w := httptest.NewRecorder()
rest.GetRouter().ServeHTTP(w, r)
assert.Equal(t, http.StatusOK, w.Code)
- rt := &rbacmodel.Token{}
- json.Unmarshal(w.Body.Bytes(), rt)
+ json.Unmarshal(w.Body.Bytes(), rootToken)
r2, _ := http.NewRequest(http.MethodDelete, "/v4/accounts/root", nil)
- r2.Header.Set(restful.HeaderAuth, "Bearer "+rt.TokenStr)
+ r2.Header.Set(restful.HeaderAuth, "Bearer "+rootToken.TokenStr)
w2 := httptest.NewRecorder()
rest.GetRouter().ServeHTTP(w2, r2)
assert.Equal(t, http.StatusBadRequest, w2.Code)
@@ -207,6 +208,38 @@ func TestAuthResource_DeleteAccount(t *testing.T) {
rest.GetRouter().ServeHTTP(w2, r2)
assert.Equal(t, http.StatusUnauthorized, w2.Code)
})
+
+ t.Run("create your_account with admin role should be pass", func(t *testing.T) {
+ b, _ := json.Marshal(&rbacmodel.Account{Name: "your_account", Password: "Complicated_password3", Roles: []string{"admin"}})
+
+ r, _ := http.NewRequest(http.MethodPost, "/v4/accounts", bytes.NewBuffer(b))
+ r.Header.Set(restful.HeaderAuth, "Bearer "+rootToken.TokenStr)
+ w := httptest.NewRecorder()
+ rest.GetRouter().ServeHTTP(w, r)
+ assert.Equal(t, http.StatusOK, w.Code)
+ })
+ t.Run("account can not even delete him self", func(t *testing.T) {
+ b, _ := json.Marshal(&rbacmodel.Account{Name: "your_account", Password: "Complicated_password3", Roles: []string{"admin"}})
+
+ r, _ := http.NewRequest(http.MethodPost, "/v4/token", bytes.NewBuffer(b))
+ w := httptest.NewRecorder()
+ rest.GetRouter().ServeHTTP(w, r)
+ assert.Equal(t, http.StatusOK, w.Code)
+ yourToken := &rbacmodel.Token{}
+ json.Unmarshal(w.Body.Bytes(), yourToken)
+
+ r2, _ := http.NewRequest(http.MethodDelete, "/v4/accounts/your_account", nil)
+ r2.Header.Set(restful.HeaderAuth, "Bearer "+yourToken.TokenStr)
+ w2 := httptest.NewRecorder()
+ rest.GetRouter().ServeHTTP(w2, r2)
+ assert.Equal(t, http.StatusBadRequest, w2.Code)
+
+ r3, _ := http.NewRequest(http.MethodDelete, "/v4/accounts/your_account", nil)
+ r3.Header.Set(restful.HeaderAuth, "Bearer "+rootToken.TokenStr)
+ w3 := httptest.NewRecorder()
+ rest.GetRouter().ServeHTTP(w3, r3)
+ assert.Equal(t, http.StatusOK, w3.Code)
+ })
t.Run("root can delete account", func(t *testing.T) {
var err error
b, err := json.Marshal(&rbacmodel.Account{Name: "root", Password: "Complicated_password1"})
diff --git a/pkg/errors/text.go b/server/service/rbac/context.go
similarity index 54%
copy from pkg/errors/text.go
copy to server/service/rbac/context.go
index d5696d5..f00e51a 100644
--- a/pkg/errors/text.go
+++ b/server/service/rbac/context.go
@@ -15,17 +15,33 @@
* limitations under the License.
*/
-package errors
+package rbac
-const (
- MsgJSON = "json is invalid"
-
- MsgOperateAccountFailed = "operate account failed"
- MsgCantOperateRoot = "root can not be operated"
- MsgCantOperateRole = "build-in role can not be operated"
- MsgOperateRoleFailed = "operate role failed"
- MsgGetAccountFailed = "get account failed"
- MsgGetRoleFailed = "get role failed"
- MsgRolePerm = "check role permissions failed"
- MsgNoPerm = "no permission to operate"
+import (
+ "context"
+ "errors"
+ "github.com/apache/servicecomb-service-center/pkg/util"
+ rbacmodel "github.com/go-chassis/cari/rbac"
)
+
+const CtxRequestClaims util.CtxKey = "_request_claims"
+
+func UserFromContext(ctx context.Context) string {
+ m, ok := ctx.Value(CtxRequestClaims).(map[string]interface{})
+ if !ok {
+ return ""
+ }
+ user, ok := m[rbacmodel.ClaimsUser].(string)
+ if !ok {
+ return ""
+ }
+ return user
+}
+
+func AccountFromContext(ctx context.Context) (*rbacmodel.Account, error) {
+ m, ok := ctx.Value(CtxRequestClaims).(map[string]interface{})
+ if !ok {
+ return nil, errors.New("no claims from request context")
+ }
+ return rbacmodel.GetAccount(m)
+}
diff --git a/server/service/rbac/context_test.go b/server/service/rbac/context_test.go
new file mode 100644
index 0000000..0616b9c
--- /dev/null
+++ b/server/service/rbac/context_test.go
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package rbac_test
+
+import (
+ "context"
+ rbacsvc "github.com/apache/servicecomb-service-center/server/service/rbac"
+ "testing"
+)
+
+func TestUserFromContext(t *testing.T) {
+ type args struct {
+ ctx context.Context
+ }
+ tests := []struct {
+ name string
+ args args
+ want string
+ }{
+ {
+ "have a claims with user A, should return A",
+ args{
+ ctx: context.WithValue(context.Background(), rbacsvc.CtxRequestClaims, map[string]interface{}{"account": "A"}),
+ },
+ "A",
+ },
+ {
+ "no claims, should return empty",
+ args{
+ ctx: context.Background(),
+ },
+ "",
+ },
+ {
+ "have invalid claims, should return empty",
+ args{
+ ctx: context.WithValue(context.Background(), rbacsvc.CtxRequestClaims, nil),
+ },
+ "",
+ },
+ {
+ "have a claims with no user, should return empty",
+ args{
+ ctx: context.WithValue(context.Background(), rbacsvc.CtxRequestClaims, map[string]interface{}{}),
+ },
+ "",
+ },
+ {
+ "have a claims with invalid user, should return empty",
+ args{
+ ctx: context.WithValue(context.Background(), rbacsvc.CtxRequestClaims, map[string]interface{}{"account": nil}),
+ },
+ "",
+ },
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ if got := rbacsvc.UserFromContext(tt.args.ctx); got != tt.want {
+ t.Errorf("UserFromContext() = %v, want %v", got, tt.want)
+ }
+ })
+ }
+}