Merge pull request #85 from wklken/ft_appcode_appsecret_check

verify app_code app_secret from paas only keep cache for 1min if invalid

.gitignore

@@ -42,3 +42,4 @@ init_test.sh
 .Archive/
 .md_configs.data
 .me_configs.data
+.codecc

pkg/api/model/handler/system_test.go

@@ -99,10 +99,7 @@ func TestCreateSystem(t *testing.T) {
 	appSecret := "123"
 
 	cacheimpls.InitCaches(false)
-	cacheimpls.LocalAppCodeAppSecretCache.Set(cacheimpls.AppCodeAppSecretCacheKey{
-		AppCode:   appCode,
-		AppSecret: appSecret,
-	}, true)
+	cacheimpls.LocalAppCodeAppSecretCache.Set(appCode+":"+appSecret, true, 0)
 
 	// for mock
 	var ctl *gomock.Controller
@@ -296,10 +293,7 @@ func TestUpdateSystem(t *testing.T) {
 	appSecret := "123"
 
 	cacheimpls.InitCaches(false)
-	cacheimpls.LocalAppCodeAppSecretCache.Set(cacheimpls.AppCodeAppSecretCacheKey{
-		AppCode:   appCode,
-		AppSecret: appSecret,
-	}, true)
+	cacheimpls.LocalAppCodeAppSecretCache.Set(appCode+":"+appSecret, true, 0)
 
 	// for mock
 	var ctl *gomock.Controller

pkg/cacheimpls/init.go

@@ -29,7 +29,7 @@ const CacheLayer = "Cache"
 
 // LocalAppCodeAppSecretCache ...
 var (
-	LocalAppCodeAppSecretCache      memory.Cache
+	LocalAppCodeAppSecretCache      *gocache.Cache
 	LocalAuthAppAccessKeyCache      *gocache.Cache
 	LocalSubjectCache               memory.Cache
 	LocalSubjectRoleCache           memory.Cache
@@ -74,13 +74,7 @@ func newRandomDuration(seconds int) backend.RandomExtraExpirationDurationFunc {
 // Cache should only know about get/retrieve data
 // ! DO NOT CARE ABOUT WHAT THE DATA WILL BE USED FOR
 func InitCaches(disabled bool) {
-	LocalAppCodeAppSecretCache = memory.NewCache(
-		"app_code_app_secret",
-		disabled,
-		retrieveAppCodeAppSecret,
-		12*time.Hour,
-		nil,
-	)
+	LocalAppCodeAppSecretCache = gocache.New(12*time.Hour, 5*time.Minute)
 
 	// auth app_code/app_secret cache
 	LocalAuthAppAccessKeyCache = gocache.New(12*time.Hour, 5*time.Minute)

pkg/cacheimpls/init_test.go

@@ -18,8 +18,8 @@ import (
 
 func TestInitCaches(t *testing.T) {
 	InitCaches(true)
-	assert.True(t, LocalAppCodeAppSecretCache.Disabled())
+	assert.True(t, LocalSubjectCache.Disabled())
 
 	InitCaches(false)
-	assert.False(t, LocalAppCodeAppSecretCache.Disabled())
+	assert.False(t, LocalSubjectCache.Disabled())
 }

pkg/cacheimpls/local_app_code_secret.go

@@ -13,7 +13,6 @@ package cacheimpls
 import (
 	"time"
 
-	"github.com/TencentBlueKing/gopkg/cache"
 	"github.com/TencentBlueKing/gopkg/stringx"
 	gocache "github.com/patrickmn/go-cache"
 	log "github.com/sirupsen/logrus"
@@ -22,39 +21,35 @@ import (
 	"iam/pkg/database/edao"
 )
 
-// AppCodeAppSecretCacheKey ...
-type AppCodeAppSecretCacheKey struct {
-	AppCode   string
-	AppSecret string
-}
-
-// Key ...
-func (k AppCodeAppSecretCacheKey) Key() string {
-	return k.AppCode + ":" + k.AppSecret
-}
-
-func retrieveAppCodeAppSecret(key cache.Key) (interface{}, error) {
-	k := key.(AppCodeAppSecretCacheKey)
-
-	manager := edao.NewAppSecretManager()
-	return manager.Exists(k.AppCode, k.AppSecret)
-}
-
 // VerifyAppCodeAppSecret ...
 func VerifyAppCodeAppSecret(appCode, appSecret string) bool {
-	key := AppCodeAppSecretCacheKey{
-		AppCode:   appCode,
-		AppSecret: appSecret,
+	// 1. get from cache
+	key := appCode + ":" + appSecret
+
+	value, found := LocalAppCodeAppSecretCache.Get(key)
+	if found {
+		return value.(bool)
 	}
-	exists, err := LocalAppCodeAppSecretCache.GetBool(key)
+
+	// 2. get from database
+	manager := edao.NewAppSecretManager()
+	valid, err := manager.Exists(appCode, appSecret)
 	if err != nil {
-		log.Errorf("get app_code_app_secret from memory cache fail, app_code=%s, app_secret=%s, err=%s",
+		log.Errorf("verify app_code_app_secret from bk_paas+esb_app_account fail, app_code=%s, app_secret=%s, err=%s",
 			appCode,
 			stringx.Truncate(appSecret, 6)+"******",
 			err)
 		return false
 	}
-	return exists
+
+	// 3. set to cache, default 12 hours, if not valid, only keep in cache for 1 minutes
+	//    in case of auth server down, we can still get the valid matched accessKeys from cache
+	ttl := gocache.DefaultExpiration
+	if !valid {
+		ttl = 1 * time.Minute
+	}
+	LocalAppCodeAppSecretCache.Set(key, valid, ttl)
+	return valid
 }
 
 func VerifyAppCodeAppSecretFromAuth(appCode, appSecret string) bool {

pkg/cacheimpls/local_app_code_secret_test.go

@@ -1,53 +1,127 @@
-/*
- * TencentBlueKing is pleased to support the open source community by making 蓝鲸智云-权限中心(BlueKing-IAM) available.
- * Copyright (C) 2017-2021 THL A29 Limited, a Tencent company. All rights reserved.
- * Licensed under the MIT License (the "License"); you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at http://opensource.org/licenses/MIT
- * 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 cacheimpls
+package cacheimpls_test
 
 import (
 	"errors"
-	"testing"
 	"time"
 
-	"github.com/TencentBlueKing/gopkg/cache"
-	"github.com/TencentBlueKing/gopkg/cache/memory"
+	"github.com/agiledragon/gomonkey/v2"
+	"github.com/golang/mock/gomock"
+	. "github.com/onsi/ginkgo/v2"
+	gocache "github.com/patrickmn/go-cache"
 	"github.com/stretchr/testify/assert"
+
+	"iam/pkg/cacheimpls"
+	"iam/pkg/component"
+	mock2 "iam/pkg/component/mock"
+	"iam/pkg/database/edao"
+	"iam/pkg/database/edao/mock"
 )
 
-func TestAppCodeAppSecretCacheKey_Key(t *testing.T) {
-	k := AppCodeAppSecretCacheKey{
-		AppCode:   "hello",
-		AppSecret: "123",
-	}
-	assert.Equal(t, "hello:123", k.Key())
-}
-
-func TestVerifyAppCodeAppSecret(t *testing.T) {
-	var (
-		expiration = 5 * time.Minute
-	)
-
-	// valid
-	retrieveFunc := func(key cache.Key) (interface{}, error) {
-		return true, nil
-	}
-	mockCache := memory.NewCache(
-		"mockCache", false, retrieveFunc, expiration, nil)
-	LocalAppCodeAppSecretCache = mockCache
-	assert.True(t, VerifyAppCodeAppSecret("test", "123"))
-
-	// error
-	retrieveFunc = func(key cache.Key) (interface{}, error) {
-		return false, errors.New("error here")
-	}
-	mockCache = memory.NewCache(
-		"mockCache", false, retrieveFunc, expiration, nil)
-	LocalAppCodeAppSecretCache = mockCache
-	assert.False(t, VerifyAppCodeAppSecret("test", "123"))
-}
+var _ = Describe("LocalAppCodeSecret", func() {
+
+	Describe("VerifyAppCodeAppSecret", func() {
+		var ctl *gomock.Controller
+		var mockManager *mock.MockAppSecretManager
+		var patches *gomonkey.Patches
+		BeforeEach(func() {
+			cacheimpls.LocalAppCodeAppSecretCache = gocache.New(12*time.Hour, 5*time.Minute)
+
+			ctl = gomock.NewController(GinkgoT())
+			mockManager = mock.NewMockAppSecretManager(ctl)
+
+		})
+
+		AfterEach(func() {
+			ctl.Finish()
+			if patches != nil {
+				patches.Reset()
+			}
+		})
+
+		It("hit", func() {
+			cacheimpls.LocalAppCodeAppSecretCache.Set("app:123", true, 12*time.Hour)
+			ok := cacheimpls.VerifyAppCodeAppSecret("app", "123")
+			assert.True(GinkgoT(), ok)
+		})
+
+		It("miss, get from database error", func() {
+			mockManager.EXPECT().Exists(gomock.Any(), gomock.Any()).Return(false, errors.New("errror happend")).AnyTimes()
+			patches = gomonkey.ApplyFunc(edao.NewAppSecretManager,
+				func() edao.AppSecretManager {
+					return mockManager
+				})
+
+			ok := cacheimpls.VerifyAppCodeAppSecret("app", "123")
+			assert.False(GinkgoT(), ok)
+		})
+
+		It("miss, get from database valid", func() {
+			mockManager.EXPECT().Exists(gomock.Any(), gomock.Any()).Return(false, nil).AnyTimes()
+			patches = gomonkey.ApplyFunc(edao.NewAppSecretManager,
+				func() edao.AppSecretManager {
+					return mockManager
+				})
+
+			ok := cacheimpls.VerifyAppCodeAppSecret("app", "123")
+			assert.False(GinkgoT(), ok)
+
+		})
+
+		It("miss, get from database invalid", func() {
+			mockManager.EXPECT().Exists(gomock.Any(), gomock.Any()).Return(true, nil).AnyTimes()
+			patches = gomonkey.ApplyFunc(edao.NewAppSecretManager,
+				func() edao.AppSecretManager {
+					return mockManager
+				})
+
+			ok := cacheimpls.VerifyAppCodeAppSecret("app", "123")
+			assert.True(GinkgoT(), ok)
+		})
+	})
+
+	Describe("VerifyAppCodeAppSecretFromAuth", func() {
+		var ctl *gomock.Controller
+		var mockCli *mock2.MockAuthClient
+		BeforeEach(func() {
+			cacheimpls.LocalAuthAppAccessKeyCache = gocache.New(12*time.Hour, 5*time.Minute)
+
+			ctl = gomock.NewController(GinkgoT())
+			mockCli = mock2.NewMockAuthClient(ctl)
+		})
+
+		AfterEach(func() {
+			ctl.Finish()
+		})
+
+		It("hit", func() {
+			cacheimpls.LocalAuthAppAccessKeyCache.Set("app:123", true, 12*time.Hour)
+			ok := cacheimpls.VerifyAppCodeAppSecretFromAuth("app", "123")
+			assert.True(GinkgoT(), ok)
+		})
+
+		It("miss, get from bkauth error", func() {
+			mockCli.EXPECT().Verify(gomock.Any(), gomock.Any()).Return(false, errors.New("errror happend")).AnyTimes()
+			component.BkAuth = mockCli
+
+			ok := cacheimpls.VerifyAppCodeAppSecretFromAuth("app", "123")
+			assert.False(GinkgoT(), ok)
+		})
+
+		It("miss, get from bkauth valid", func() {
+			mockCli.EXPECT().Verify(gomock.Any(), gomock.Any()).Return(false, nil).AnyTimes()
+			component.BkAuth = mockCli
+
+			ok := cacheimpls.VerifyAppCodeAppSecretFromAuth("app", "123")
+			assert.False(GinkgoT(), ok)
+
+		})
+
+		It("miss, get from bkauth invalid", func() {
+			mockCli.EXPECT().Verify(gomock.Any(), gomock.Any()).Return(true, nil).AnyTimes()
+			component.BkAuth = mockCli
+
+			ok := cacheimpls.VerifyAppCodeAppSecretFromAuth("app", "123")
+			assert.True(GinkgoT(), ok)
+		})
+	})
+})

pkg/component/auth.go

@@ -24,6 +24,8 @@ import (
 	"iam/pkg/logging"
 )
 
+//go:generate mockgen -source=$GOFILE -destination=./mock/$GOFILE -package=mock
+
 // AuthResponse is the struct of iam backend response
 type AuthResponse struct {
 	Code    int                    `json:"code"`