Skip to content

fix(core): isolate token cache by app secret #218

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Zhang-986 wants to merge 1 commit into
base: v3_main
Choose a base branch
from
Open

fix(core): isolate token cache by app secret #218

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 17 additions & 10 deletions core/tokenmanager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ package larkcore

import (
"context"
"crypto/sha256"
"encoding/hex"
"encoding/json"
"errors"
"fmt"
Expand All @@ -33,7 +35,7 @@ func (m *TokenManager) getAppAccessToken(ctx context.Context, config *Config, ap
return "", &CodeError{Code: ErrCodeClientAssertionProviderNotConfigured, Msg: "AppAccessToken is not available in ClientAssertion mode"}
}

token, err := m.get(ctx, appAccessTokenKey(config.AppId))
token, err := m.get(ctx, appAccessTokenKey(config.AppId, config.AppSecret))
if err != nil {
return "", err
}
Expand Down Expand Up @@ -62,7 +64,7 @@ func (m *TokenManager) getTenantAccessToken(ctx context.Context, config *Config,
return m.getTenantTokenByClientAssertion(ctx, config, tenantKey)
}

token, err := m.get(ctx, tenantAccessTokenKey(config.AppId, tenantKey))
token, err := m.get(ctx, tenantAccessTokenKey(config.AppId, config.AppSecret, tenantKey))
if err != nil {
return "", err
}
Expand Down Expand Up @@ -155,18 +157,23 @@ type MarketplaceTenantAccessTokenReq struct {
TenantKey string `json:"tenant_key"`
}

func appAccessTokenKey(appID string) string {
return fmt.Sprintf("%s-%s", appAccessTokenKeyPrefix, appID)
func appAccessTokenKey(appID, appSecret string) string {
return fmt.Sprintf("%s:app_secret:%s:%s", appAccessTokenKeyPrefix, appID, appSecretFingerprint(appSecret))
}

func tenantAccessTokenKey(appID, tenantKey string) string {
return fmt.Sprintf("%s:app_secret:%s:%s", tenantAccessTokenKeyPrefix, appID, tenantKey)
func tenantAccessTokenKey(appID, appSecret, tenantKey string) string {
return fmt.Sprintf("%s:app_secret:%s:%s:%s", tenantAccessTokenKeyPrefix, appID, appSecretFingerprint(appSecret), tenantKey)
}

func clientAssertionTenantAccessTokenKey(appID, tenantKey, aud string) string {
return fmt.Sprintf("%s:client_assertion:%s:%s:%s", tenantAccessTokenKeyPrefix, appID, tenantKey, aud)
}

func appSecretFingerprint(appSecret string) string {
sum := sha256.Sum256([]byte(appSecret))
return hex.EncodeToString(sum[:])
}

func (m *TokenManager) getTenantTokenByClientAssertion(ctx context.Context, config *Config, tenantKey string) (string, error) {
oauthBaseUrl, err := ResolveOAuthBaseUrl(config)
if err != nil {
Expand Down Expand Up @@ -282,7 +289,7 @@ func (m *TokenManager) getCustomAppAccessTokenThenCache(ctx context.Context, con
return "", resp.CodeError
}
expire := time.Duration(resp.Expire)*time.Second - expiryDelta
err = m.set(ctx, appAccessTokenKey(config.AppId), resp.AppAccessToken, expire)
err = m.set(ctx, appAccessTokenKey(config.AppId, config.AppSecret), resp.AppAccessToken, expire)
if err != nil {
config.Logger.Warn(ctx, fmt.Sprintf("custom app appAccessToken save cache, err:%v", err))
}
Expand Down Expand Up @@ -313,7 +320,7 @@ func (m *TokenManager) getCustomTenantAccessTokenThenCache(ctx context.Context,
return "", tenantAccessTokenResp.CodeError
}
expire := time.Duration(tenantAccessTokenResp.Expire)*time.Second - expiryDelta
err = m.cache.Set(ctx, tenantAccessTokenKey(config.AppId, tenantKey), tenantAccessTokenResp.TenantAccessToken, expire)
err = m.cache.Set(ctx, tenantAccessTokenKey(config.AppId, config.AppSecret, tenantKey), tenantAccessTokenResp.TenantAccessToken, expire)
if err != nil {
config.Logger.Warn(ctx, fmt.Sprintf("custom app tenantAccessToken save cache, err:%v", err))
}
Expand Down Expand Up @@ -357,7 +364,7 @@ func (m *TokenManager) getMarketplaceAppAccessTokenThenCache(ctx context.Context
return "", appAccessTokenResp.CodeError
}
expire := time.Duration(appAccessTokenResp.Expire)*time.Second - expiryDelta
err = m.set(ctx, appAccessTokenKey(config.AppId), appAccessTokenResp.AppAccessToken, expire)
err = m.set(ctx, appAccessTokenKey(config.AppId, config.AppSecret), appAccessTokenResp.AppAccessToken, expire)
if err != nil {
config.Logger.Warn(ctx, fmt.Sprintf("marketplace app appAccessToken save cache, err:%v", err))
}
Expand Down Expand Up @@ -393,7 +400,7 @@ func (m *TokenManager) getMarketplaceTenantAccessTokenThenCache(ctx context.Cont
return "", tenantAccessTokenResp.CodeError
}
expire := time.Duration(tenantAccessTokenResp.Expire)*time.Second - expiryDelta
err = m.set(ctx, tenantAccessTokenKey(config.AppId, tenantKey), tenantAccessTokenResp.TenantAccessToken, expire)
err = m.set(ctx, tenantAccessTokenKey(config.AppId, config.AppSecret, tenantKey), tenantAccessTokenResp.TenantAccessToken, expire)
if err != nil {
config.Logger.Warn(ctx, fmt.Sprintf("market app tenantAccessToken save cache, err:%v", err))
}
Expand Down
125 changes: 121 additions & 4 deletions core/tokenmanager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"encoding/json"
"net/http"
"net/http/httptest"
"strings"
"testing"
"time"
)
Expand All @@ -39,7 +40,7 @@ func TestTokenManagerSetAndGet(t *testing.T) {
cache := &localCache{}
tokenManager := TokenManager{cache: cache}

err := tokenManager.set(context.Background(), tenantAccessTokenKey(config.AppId, "tenantKey"), "tokenValue", time.Minute)
err := tokenManager.set(context.Background(), tenantAccessTokenKey(config.AppId, config.AppSecret, "tenantKey"), "tokenValue", time.Minute)
if err != nil {
t.Errorf("set key failed ,%v", err)
}
Expand All @@ -56,14 +57,130 @@ func TestTokenManagerSetAndGet(t *testing.T) {
}

func TestTenantAccessTokenCacheKeysIncludeCredentialMode(t *testing.T) {
if got, want := tenantAccessTokenKey("cli_a", "tenantKey"), "tenant_access_token:app_secret:cli_a:tenantKey"; got != want {
t.Fatalf("unexpected app secret tenant token key: %s", got)
appKey := appAccessTokenKey("cli_a", "secret-a")
if appKey == appAccessTokenKey("cli_a", "secret-b") {
t.Fatalf("app token cache key must change when app secret changes")
}
if strings.Contains(appKey, "secret-a") {
t.Fatalf("app token cache key must not contain raw app secret: %s", appKey)
}

tenantKey := tenantAccessTokenKey("cli_a", "secret-a", "tenantKey")
if tenantKey == tenantAccessTokenKey("cli_a", "secret-b", "tenantKey") {
t.Fatalf("tenant token cache key must change when app secret changes")
}
if strings.Contains(tenantKey, "secret-a") {
t.Fatalf("tenant token cache key must not contain raw app secret: %s", tenantKey)
}
if got, want := clientAssertionTenantAccessTokenKey("cli_a", "tenantKey", "accounts.feishu.cn"), "tenant_access_token:client_assertion:cli_a:tenantKey:accounts.feishu.cn"; got != want {
t.Fatalf("unexpected client assertion tenant token key: %s", got)
}
}

func TestGetAppAccessTokenDoesNotReuseCacheAcrossAppSecrets(t *testing.T) {
var calls int
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
calls++
if r.URL.Path != AppAccessTokenInternalUrlPath {
t.Fatalf("unexpected path: %s", r.URL.Path)
}
var req SelfBuiltAppAccessTokenReq
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
t.Fatalf("decode body failed: %v", err)
}
if req.AppSecret != "correct-secret" {
_ = json.NewEncoder(w).Encode(&AppAccessTokenResp{
CodeError: CodeError{Code: 19002, Msg: "invalid app secret"},
})
return
}
_ = json.NewEncoder(w).Encode(&AppAccessTokenResp{
CodeError: CodeError{Code: 0},
Expire: 7200,
AppAccessToken: "app-token",
})
}))
defer server.Close()

config := mockConfig()
config.BaseUrl = server.URL
config.HttpClient = server.Client()
config.EnableTokenCache = true
config.AppId = "cli_a"
config.AppSecret = "correct-secret"

manager := TokenManager{cache: &localCache{}}
token, err := manager.getAppAccessToken(context.Background(), config, "")
if err != nil {
t.Fatalf("get app access token failed: %v", err)
}
if token != "app-token" {
t.Fatalf("unexpected token: %s", token)
}

wrongSecretConfig := *config
wrongSecretConfig.AppSecret = "wrong-secret"
_, err = manager.getAppAccessToken(context.Background(), &wrongSecretConfig, "")
if err == nil {
t.Fatalf("expected wrong app secret to miss cache and return an auth error")
}
if calls != 2 {
t.Fatalf("expected wrong app secret to trigger a second token request, got %d calls", calls)
}
}

func TestGetTenantAccessTokenDoesNotReuseCacheAcrossAppSecrets(t *testing.T) {
var calls int
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
calls++
if r.URL.Path != TenantAccessTokenInternalUrlPath {
t.Fatalf("unexpected path: %s", r.URL.Path)
}
var req SelfBuiltTenantAccessTokenReq
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
t.Fatalf("decode body failed: %v", err)
}
if req.AppSecret != "correct-secret" {
_ = json.NewEncoder(w).Encode(&TenantAccessTokenResp{
CodeError: CodeError{Code: 19002, Msg: "invalid app secret"},
})
return
}
_ = json.NewEncoder(w).Encode(&TenantAccessTokenResp{
CodeError: CodeError{Code: 0},
Expire: 7200,
TenantAccessToken: "tenant-token",
})
}))
defer server.Close()

config := mockConfig()
config.BaseUrl = server.URL
config.HttpClient = server.Client()
config.EnableTokenCache = true
config.AppId = "cli_a"
config.AppSecret = "correct-secret"

manager := TokenManager{cache: &localCache{}}
token, err := manager.getTenantAccessToken(context.Background(), config, "tenantKey", "")
if err != nil {
t.Fatalf("get tenant access token failed: %v", err)
}
if token != "tenant-token" {
t.Fatalf("unexpected token: %s", token)
}

wrongSecretConfig := *config
wrongSecretConfig.AppSecret = "wrong-secret"
_, err = manager.getTenantAccessToken(context.Background(), &wrongSecretConfig, "tenantKey", "")
if err == nil {
t.Fatalf("expected wrong app secret to miss cache and return an auth error")
}
if calls != 2 {
t.Fatalf("expected wrong app secret to trigger a second token request, got %d calls", calls)
}
}

func TestGetTenantAccessTokenByClientAssertionIgnoresAppSecretCache(t *testing.T) {
provider := &mockClientAssertionProvider{token: &Token{Value: "client-assertion"}}
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
Expand All @@ -83,7 +200,7 @@ func TestGetTenantAccessTokenByClientAssertionIgnoresAppSecretCache(t *testing.T
config.ClientAssertionProvider = provider

cache := &localCache{}
if err := cache.Set(context.Background(), tenantAccessTokenKey(config.AppId, "tenantKey"), "cached-appsecret-token", time.Hour); err != nil {
if err := cache.Set(context.Background(), tenantAccessTokenKey(config.AppId, config.AppSecret, "tenantKey"), "cached-appsecret-token", time.Hour); err != nil {
t.Fatalf("seed app secret cache failed: %v", err)
}

Expand Down