[servicecomb-service-center] branch master updated: RBAC authentication ignores version and health APIs (#704)

[tianxiaoliang]

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 e00f36a  RBAC authentication ignores version and health APIs (#704)
e00f36a is described below

commit e00f36a263dcf2cc2bb765108be89dcc2df02e41
Author: humingcheng <humingcheng1...@163.com>
AuthorDate: Mon Oct 12 21:08:26 2020 +0800

    RBAC authentication ignores version and health APIs (#704)
---
 pkg/rbacframe/api_test.go         |  7 +++++--
 pkg/rbacframe/context.go          |  6 ++++++
 pkg/util/util.go                  |  4 ++++
 pkg/util/util_test.go             | 11 +++++++++++
 server/handler/context/context.go | 13 +------------
 server/service/rbac/rbac.go       |  2 +-
 6 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/pkg/rbacframe/api_test.go b/pkg/rbacframe/api_test.go
index f9be3ef..1835158 100644
--- a/pkg/rbacframe/api_test.go
+++ b/pkg/rbacframe/api_test.go
@@ -48,9 +48,12 @@ func TestFromContext(t *testing.T) {
 func TestMustAuth(t *testing.T) {
        rbacframe.Add2WhiteAPIList("/test")
        assert.False(t, rbacframe.MustAuth("/test"))
+       assert.True(t, rbacframe.MustAuth("/test1"))
        assert.True(t, rbacframe.MustAuth("/auth"))
-       assert.True(t, rbacframe.MustAuth("/version"))
-       assert.True(t, rbacframe.MustAuth("/v4/a/registry/health"))
+       assert.False(t, rbacframe.MustAuth("/version"))
+       assert.False(t, rbacframe.MustAuth("/v4/a/registry/version"))
+       assert.False(t, rbacframe.MustAuth("/health"))
+       assert.False(t, rbacframe.MustAuth("/v4/a/registry/health"))
 }
 
 func TestAuthenticate(t *testing.T) {
diff --git a/pkg/rbacframe/context.go b/pkg/rbacframe/context.go
index 4f13dba..f13b905 100644
--- a/pkg/rbacframe/context.go
+++ b/pkg/rbacframe/context.go
@@ -19,6 +19,9 @@ package rbacframe
 
 import (
        "context"
+
+       "github.com/apache/servicecomb-service-center/pkg/util"
+
        "k8s.io/apimachinery/pkg/util/sets"
 )
 
@@ -49,5 +52,8 @@ func Add2WhiteAPIList(path ...string) {
 }
 
 func MustAuth(pattern string) bool {
+       if util.IsVersionOrHealthPattern(pattern) {
+               return false
+       }
        return !whiteAPIList.Has(pattern)
 }
diff --git a/pkg/util/util.go b/pkg/util/util.go
index a3f271b..e6875bc 100644
--- a/pkg/util/util.go
+++ b/pkg/util/util.go
@@ -168,3 +168,7 @@ func ToDomainProject(domain, project string) (domainProject 
string) {
        domainProject = domain + "/" + project
        return
 }
+
+func IsVersionOrHealthPattern(pattern string) bool {
+       return strings.HasSuffix(pattern, "/version") || 
strings.HasSuffix(pattern, "/health")
+}
diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go
index e0294df..8db19e2 100644
--- a/pkg/util/util_test.go
+++ b/pkg/util/util_test.go
@@ -20,6 +20,8 @@ import (
        "os"
        "testing"
        "time"
+
+       "github.com/stretchr/testify/assert"
 )
 
 func TestInt16ToInt64(t *testing.T) {
@@ -179,3 +181,12 @@ func TestBytesToStringWithNoCopy(t *testing.T) {
                t.Fatal("TestBytesToStringWithNoCopy failed")
        }
 }
+
+func TestIsVersionOrHealthPattern(t *testing.T) {
+       assert.True(t, IsVersionOrHealthPattern("/version"))
+       assert.True(t, IsVersionOrHealthPattern("/v4/a/registry/version"))
+       assert.False(t, IsVersionOrHealthPattern("/version/a"))
+       assert.True(t, IsVersionOrHealthPattern("/health"))
+       assert.True(t, IsVersionOrHealthPattern("/v4/a/registry/health"))
+       assert.False(t, IsVersionOrHealthPattern("/health/a"))
+}
diff --git a/server/handler/context/context.go 
b/server/handler/context/context.go
index ab5217f..87f55d3 100644
--- a/server/handler/context/context.go
+++ b/server/handler/context/context.go
@@ -37,7 +37,7 @@ func (c *Handler) Handle(i *chain.Invocation) {
        )
 
        switch {
-       case IsSkip(pattern):
+       case util.IsVersionOrHealthPattern(pattern):
        case v3.IsMatch(r):
                err = v3.Do(r)
        case v4.IsMatch(r):
@@ -54,17 +54,6 @@ func (c *Handler) Handle(i *chain.Invocation) {
        i.Next()
 }
 
-func IsSkip(url string) bool {
-       l, vl, hl := len(url), len("/version"), len("/health")
-       if l >= vl && url[l-vl:] == "/version" {
-               return true
-       }
-       if l >= hl && url[l-hl:] == "/health" {
-               return true
-       }
-       return false
-}
-
 func RegisterHandlers() {
        chain.RegisterHandler(roa.ServerChainName, &Handler{})
 }
diff --git a/server/service/rbac/rbac.go b/server/service/rbac/rbac.go
index a666e19..e741b7c 100644
--- a/server/service/rbac/rbac.go
+++ b/server/service/rbac/rbac.go
@@ -70,7 +70,7 @@ func Init() {
        }
        readPrivateKey()
        readPublicKey()
-       rbacframe.Add2WhiteAPIList("/health", "/version", "/v4/token")
+       rbacframe.Add2WhiteAPIList("/v4/token")
        log.Info("rbac is enabled")
 }
 func initResourceMap() {