From 140614548608f2f79694499fe18c20263d768bb6 Mon Sep 17 00:00:00 2001 From: dencoded <33698537+dencoded@users.noreply.github.com> Date: Tue, 27 Aug 2019 15:53:10 -0400 Subject: [PATCH 1/2] support of imported keys with new format added --- gateway/api.go | 5 +++-- gateway/api_definition.go | 11 ++++++++--- gateway/api_test.go | 18 ++++++++++-------- gateway/auth_manager.go | 25 ++++++++++++++++++++++--- gateway/cert_test.go | 1 + gateway/ldap_auth_handler.go | 6 ++++++ gateway/mw_auth_key_test.go | 3 +-- gateway/mw_jwt_test.go | 9 +++++++-- gateway/policy_test.go | 4 ++-- gateway/rpc_storage_handler.go | 6 ++++++ gateway/testutil.go | 7 +++++-- storage/redis_cluster.go | 31 +++++++++++++++++++++++++------ storage/storage.go | 1 + 13 files changed, 97 insertions(+), 30 deletions(-) diff --git a/gateway/api.go b/gateway/api.go index adb00538f98..23b8769050b 100644 --- a/gateway/api.go +++ b/gateway/api.go @@ -330,6 +330,7 @@ func handleAddOrUpdate(keyName string, r *http.Request, isHashed bool) (interfac // get original session in case of update and preserve fields that SHOULD NOT be updated originalKey := user.SessionState{} + originalKeyName := keyName if r.Method == http.MethodPut { found := false for apiID := range newSession.AccessRights { @@ -367,6 +368,7 @@ func handleAddOrUpdate(keyName string, r *http.Request, isHashed bool) (interfac } } else { newSession.DateCreated = time.Now() + keyName = generateToken(newSession.OrgID, keyName) } // Update our session object (create it) @@ -375,7 +377,6 @@ func handleAddOrUpdate(keyName string, r *http.Request, isHashed bool) (interfac // Only if it's NEW switch r.Method { case http.MethodPost: - keyName = generateToken(newSession.OrgID, keyName) // It's a create, so lets hash the password setSessionPassword(&newSession) case http.MethodPut: @@ -406,7 +407,7 @@ func handleAddOrUpdate(keyName string, r *http.Request, isHashed bool) (interfac }) response := apiModifyKeySuccess{ - Key: keyName, + Key: originalKeyName, Status: "ok", Action: action, } diff --git a/gateway/api_definition.go b/gateway/api_definition.go index e12b345e360..9dd23b97e86 100644 --- a/gateway/api_definition.go +++ b/gateway/api_definition.go @@ -19,9 +19,10 @@ import ( sprig "gopkg.in/Masterminds/sprig.v2" + "github.com/gorilla/mux" + "github.com/TykTechnologies/tyk/headers" "github.com/TykTechnologies/tyk/rpc" - "github.com/gorilla/mux" circuit "github.com/rubyist/circuitbreaker" "github.com/sirupsen/logrus" @@ -248,8 +249,12 @@ func (a APIDefinitionLoader) MakeSpec(def *apidef.APIDefinition, logger *logrus. // Add any new session managers or auth handlers here spec.AuthManager = &DefaultAuthorisationManager{} - spec.SessionManager = &DefaultSessionManager{} - spec.OrgSessionManager = &DefaultSessionManager{} + spec.SessionManager = &DefaultSessionManager{ + orgID: spec.OrgID, + } + spec.OrgSessionManager = &DefaultSessionManager{ + orgID: spec.OrgID, + } spec.GlobalConfig = config.Global() diff --git a/gateway/api_test.go b/gateway/api_test.go index 839b82566c0..ccf621209be 100644 --- a/gateway/api_test.go +++ b/gateway/api_test.go @@ -158,6 +158,7 @@ func TestKeyHandler(t *testing.T) { AccessRights: map[string]user.AccessDefinition{"test": { APIID: "test", Versions: []string{"v1"}, }}, + OrgID: "default", } policiesMu.Unlock() withPolicy := CreateStandardSession() @@ -281,6 +282,7 @@ func TestKeyHandler_UpdateKey(t *testing.T) { spec.APIID = testAPIID spec.UseKeylessAccess = false spec.Auth.UseParam = true + spec.OrgID = "default" }) pID := CreatePolicy(func(p *user.Policy) { @@ -429,8 +431,8 @@ func testHashKeyHandlerHelper(t *testing.T, expectedHashSize int) { }} withAccessJSON, _ := json.Marshal(withAccess) - myKey := generateToken("", "") - myKeyHash := storage.HashKey(myKey) + myKey := "my_key_id" + myKeyHash := storage.HashKey(generateToken("default", myKey)) if len(myKeyHash) != expectedHashSize { t.Errorf("Expected hash size: %d, got %d. Hash: %s. Key: %s", expectedHashSize, len(myKeyHash), myKeyHash, myKey) @@ -473,10 +475,10 @@ func testHashKeyHandlerHelper(t *testing.T, expectedHashSize int) { Code: 200, BodyMatch: fmt.Sprintf(`"key":"%s"`, myKeyHash), }, - // get one key by key name + // get one key by key name (API specified) { Method: "GET", - Path: "/tyk/keys/" + myKey, + Path: "/tyk/keys/" + myKey + "?api_id=test", Data: string(withAccessJSON), AdminAuth: true, Code: 200, @@ -595,7 +597,7 @@ func TestHashKeyListingDisabled(t *testing.T) { withAccessJSON, _ := json.Marshal(withAccess) myKey := "my_key_id" - myKeyHash := storage.HashKey(myKey) + myKeyHash := storage.HashKey(generateToken("default", myKey)) t.Run("Create, get and delete key with key hashing", func(t *testing.T) { ts.Run(t, []test.TestCase{ @@ -625,10 +627,10 @@ func TestHashKeyListingDisabled(t *testing.T) { Code: 200, BodyMatch: fmt.Sprintf(`"key_hash":"%s"`, myKeyHash), }, - // get one key by key name + // get one key by key name (API specified) { Method: "GET", - Path: "/tyk/keys/" + myKey, + Path: "/tyk/keys/" + myKey + "?api_id=test", Data: string(withAccessJSON), AdminAuth: true, Code: 200, @@ -713,7 +715,7 @@ func TestHashKeyHandlerHashingDisabled(t *testing.T) { withAccessJSON, _ := json.Marshal(withAccess) myKey := "my_key_id" - myKeyHash := storage.HashKey(myKey) + myKeyHash := storage.HashKey(generateToken("default", myKey)) t.Run("Create, get and delete key with key hashing", func(t *testing.T) { ts.Run(t, []test.TestCase{ diff --git a/gateway/auth_manager.go b/gateway/auth_manager.go index b55f5422194..64aba95102d 100644 --- a/gateway/auth_manager.go +++ b/gateway/auth_manager.go @@ -7,10 +7,11 @@ import ( "sync" "time" + uuid "github.com/satori/go.uuid" + "github.com/TykTechnologies/tyk/config" "github.com/TykTechnologies/tyk/storage" "github.com/TykTechnologies/tyk/user" - uuid "github.com/satori/go.uuid" "github.com/sirupsen/logrus" ) @@ -117,6 +118,7 @@ type DefaultSessionManager struct { store storage.Handler asyncWrites bool disableCacheSessionState bool + orgID string } type SessionUpdate struct { @@ -267,7 +269,10 @@ func (b *DefaultSessionManager) RemoveSession(keyName string, hashed bool) bool if hashed { return b.store.DeleteRawKey(b.store.GetKeyPrefix() + keyName) } else { - return b.store.DeleteKey(keyName) + // support both old and new key hashing + res1 := b.store.DeleteKey(keyName) + res2 := b.store.DeleteKey(generateToken(b.orgID, keyName)) + return res1 || res2 } } @@ -281,7 +286,21 @@ func (b *DefaultSessionManager) SessionDetail(keyName string, hashed bool) (user if hashed { jsonKeyVal, err = b.store.GetRawKey(b.store.GetKeyPrefix() + keyName) } else { - jsonKeyVal, err = b.store.GetKey(keyName) + // try to get legacy and new format key at once + var jsonKeyValList []string + jsonKeyValList, err = b.store.GetMultiKey( + []string{ + keyName, + generateToken(b.orgID, keyName), + }, + ) + // pick the 1st non empty from the returned list + for _, val := range jsonKeyValList { + if val != "" { + jsonKeyVal = val + break + } + } } if err != nil { diff --git a/gateway/cert_test.go b/gateway/cert_test.go index ee4c2c22349..ffa0123c1cf 100644 --- a/gateway/cert_test.go +++ b/gateway/cert_test.go @@ -493,6 +493,7 @@ func TestKeyWithCertificateTLS(t *testing.T) { spec.BaseIdentityProvidedBy = apidef.AuthToken spec.Auth.UseCertificate = true spec.Proxy.ListenPath = "/" + spec.OrgID = "default" }) client := getTLSClient(&clientCert, nil) diff --git a/gateway/ldap_auth_handler.go b/gateway/ldap_auth_handler.go index 4b8dee6cd75..2f90e5b2c4f 100644 --- a/gateway/ldap_auth_handler.go +++ b/gateway/ldap_auth_handler.go @@ -89,6 +89,12 @@ func (l *LDAPStorageHandler) GetKey(filter string) (string, error) { return "", nil } +func (r *LDAPStorageHandler) GetMultiKey(keyNames []string) ([]string, error) { + log.Warning("Not implementated") + + return nil, nil +} + func (l *LDAPStorageHandler) GetRawKey(filter string) (string, error) { log.Warning("Not implementated") diff --git a/gateway/mw_auth_key_test.go b/gateway/mw_auth_key_test.go index a34dee4834e..24ececd1e93 100644 --- a/gateway/mw_auth_key_test.go +++ b/gateway/mw_auth_key_test.go @@ -60,8 +60,7 @@ func TestMurmur3CharBug(t *testing.T) { ts.Run(t, []test.TestCase{ genTestCase("wrong", 403), - // Should reject instead, just to show bug - genTestCase(key+"abc", 200), + genTestCase(key+"abc", 403), genTestCase(key, 200), }...) }) diff --git a/gateway/mw_jwt_test.go b/gateway/mw_jwt_test.go index a2edd1edaec..b864961cffa 100644 --- a/gateway/mw_jwt_test.go +++ b/gateway/mw_jwt_test.go @@ -903,6 +903,7 @@ func TestJWTScopeToPolicyMapping(t *testing.T) { spec.JWTIdentityBaseField = "user_id" spec.JWTPolicyFieldName = "policy_id" spec.Proxy.ListenPath = "/api1" + spec.OrgID = "default" })[0] p1ID := CreatePolicy(func(p *user.Policy) { @@ -929,6 +930,7 @@ func TestJWTScopeToPolicyMapping(t *testing.T) { spec.JWTIdentityBaseField = "user_id" spec.JWTPolicyFieldName = "policy_id" spec.Proxy.ListenPath = "/api2" + spec.OrgID = "default" })[0] p2ID := CreatePolicy(func(p *user.Policy) { @@ -955,6 +957,7 @@ func TestJWTScopeToPolicyMapping(t *testing.T) { spec.JWTIdentityBaseField = "user_id" spec.JWTPolicyFieldName = "policy_id" spec.Proxy.ListenPath = "/api3" + spec.OrgID = "default" })[0] spec := BuildAPI(func(spec *APISpec) { @@ -970,6 +973,7 @@ func TestJWTScopeToPolicyMapping(t *testing.T) { "user:read": p1ID, "user:write": p2ID, } + spec.OrgID = "default" })[0] LoadAPI(spec, spec1, spec2, spec3) @@ -996,7 +1000,7 @@ func TestJWTScopeToPolicyMapping(t *testing.T) { }) // check that key has right set of policies assigned - there should be all three - base one and two from scope - sessionID := generateToken("", fmt.Sprintf("%x", md5.Sum([]byte(userID)))) + sessionID := generateToken("default", fmt.Sprintf("%x", md5.Sum([]byte(userID)))) t.Run("Request to check that session has got correct apply_policies value", func(t *testing.T) { ts.Run( t, @@ -1077,6 +1081,7 @@ func TestJWTExistingSessionRSAWithRawSourcePolicyIDChanged(t *testing.T) { spec.JWTIdentityBaseField = "user_id" spec.JWTPolicyFieldName = "policy_id" spec.Proxy.ListenPath = "/" + spec.OrgID = "default" })[0] LoadAPI(spec) @@ -1100,7 +1105,7 @@ func TestJWTExistingSessionRSAWithRawSourcePolicyIDChanged(t *testing.T) { t.Claims.(jwt.MapClaims)["exp"] = time.Now().Add(time.Hour * 72).Unix() }) - sessionID := generateToken("", fmt.Sprintf("%x", md5.Sum([]byte(user_id)))) + sessionID := generateToken("default", fmt.Sprintf("%x", md5.Sum([]byte("user")))) authHeaders := map[string]string{"authorization": jwtToken} t.Run("Initial request with 1st policy", func(t *testing.T) { diff --git a/gateway/policy_test.go b/gateway/policy_test.go index 2add0f76247..fe8b2bc0394 100644 --- a/gateway/policy_test.go +++ b/gateway/policy_test.go @@ -764,7 +764,7 @@ func TestPerAPIPolicyUpdate(t *testing.T) { ts.Run(t, []test.TestCase{ { Method: http.MethodGet, - Path: "/tyk/keys/" + key, + Path: "/tyk/keys/" + key + "?api_id=api1", AdminAuth: true, Code: http.StatusOK, BodyMatchFunc: func(data []byte) bool { @@ -815,7 +815,7 @@ func TestPerAPIPolicyUpdate(t *testing.T) { ts.Run(t, []test.TestCase{ { Method: http.MethodGet, - Path: "/tyk/keys/" + key, + Path: "/tyk/keys/" + key + "?api_id=api1", AdminAuth: true, Code: http.StatusOK, BodyMatchFunc: func(data []byte) bool { diff --git a/gateway/rpc_storage_handler.go b/gateway/rpc_storage_handler.go index 5977bb0b360..825688dcd22 100644 --- a/gateway/rpc_storage_handler.go +++ b/gateway/rpc_storage_handler.go @@ -230,6 +230,12 @@ func (r *RPCStorageHandler) GetRawKey(keyName string) (string, error) { return value.(string), nil } +func (r *RPCStorageHandler) GetMultiKey(keyNames []string) ([]string, error) { + log.Warning("RPCStorageHandler.GetMultiKey - Not implemented") + + return nil, nil +} + func (r *RPCStorageHandler) GetExp(keyName string) (int64, error) { log.Debug("GetExp called") value, err := rpc.FuncClientSingleton("GetExp", r.fixKey(keyName)) diff --git a/gateway/testutil.go b/gateway/testutil.go index dc6fda8987a..b35ac748cc3 100644 --- a/gateway/testutil.go +++ b/gateway/testutil.go @@ -362,13 +362,13 @@ func withAuth(r *http.Request) *http.Request { // Deprecated: Use Test.CreateSession instead. func CreateSession(sGen ...func(s *user.SessionState)) string { - key := generateToken("", "") + key := generateToken("default", "") session := CreateStandardSession() if len(sGen) > 0 { sGen[0](session) } if session.Certificate != "" { - key = generateToken("", session.Certificate) + key = generateToken("default", session.Certificate) } FallbackKeySesionManager.UpdateSession(storage.HashKey(key), session, 60, config.Global().HashKeys) @@ -388,6 +388,7 @@ func CreateStandardSession() *user.SessionState { session.QuotaMax = -1 session.Tags = []string{} session.MetaData = make(map[string]interface{}) + session.OrgID = "default" return session } @@ -407,6 +408,7 @@ func CreatePolicy(pGen ...func(p *user.Policy)) string { pID := keyGen.GenerateAuthKey("") pol := CreateStandardPolicy() pol.ID = pID + pol.OrgID = "default" if len(pGen) > 0 { pGen[0](pol) @@ -680,6 +682,7 @@ func StartTest(config ...TestConfig) *Test { const sampleAPI = `{ "api_id": "test", + "org_id": "default", "use_keyless": true, "definition": { "location": "header", diff --git a/storage/redis_cluster.go b/storage/redis_cluster.go index 20bee721d43..8613ef74705 100644 --- a/storage/redis_cluster.go +++ b/storage/redis_cluster.go @@ -235,6 +235,25 @@ func (r *RedisCluster) GetKey(keyName string) (string, error) { return value, nil } +// GetMultiKey gets multiple keys from the database +func (r *RedisCluster) GetMultiKey(keyNames []string) ([]string, error) { + r.ensureConnection() + cluster := r.singleton() + + fixedKeyNames := make([]interface{}, len(keyNames)) + for index, val := range keyNames { + fixedKeyNames[index] = r.fixKey(val) + } + + value, err := redis.Strings(cluster.Do("MGET", fixedKeyNames...)) + if err != nil { + log.WithError(err).Debug("Error trying to get value") + return nil, ErrKeyNotFound + } + + return value, nil +} + func (r *RedisCluster) GetKeyTTL(keyName string) (ttl int64, err error) { r.ensureConnection() return redis.Int64(r.singleton().Do("TTL", r.fixKey(keyName))) @@ -419,23 +438,23 @@ func (r *RedisCluster) DeleteKey(keyName string) bool { r.ensureConnection() log.Debug("DEL Key was: ", keyName) log.Debug("DEL Key became: ", r.fixKey(keyName)) - _, err := r.singleton().Do("DEL", r.fixKey(keyName)) + n, err := r.singleton().Do("DEL", r.fixKey(keyName)) if err != nil { - log.Error("Error trying to delete key: ", err) + log.WithError(err).Error("Error trying to delete key") } - return true + return n.(int64) > 0 } // DeleteKey will remove a key from the database without prefixing, assumes user knows what they are doing func (r *RedisCluster) DeleteRawKey(keyName string) bool { r.ensureConnection() - _, err := r.singleton().Do("DEL", keyName) + n, err := r.singleton().Do("DEL", keyName) if err != nil { - log.Error("Error trying to delete key: ", err) + log.WithError(err).Error("Error trying to delete key") } - return true + return n.(int64) > 0 } // DeleteKeys will remove a group of keys in bulk diff --git a/storage/storage.go b/storage/storage.go index 7841aeb277b..3c2a60c4f46 100644 --- a/storage/storage.go +++ b/storage/storage.go @@ -26,6 +26,7 @@ var ErrKeyNotFound = errors.New("key not found") // AuthorisationManager to read and write key values to the backend type Handler interface { GetKey(string) (string, error) // Returned string is expected to be a JSON object (user.SessionState) + GetMultiKey([]string) ([]string, error) GetRawKey(string) (string, error) SetKey(string, string, int64) error // Second input string is expected to be a JSON object (user.SessionState) SetRawKey(string, string, int64) error From f1dc5cf5834bb6cb723052b05d3568567d2dcbca Mon Sep 17 00:00:00 2001 From: dencoded <33698537+dencoded@users.noreply.github.com> Date: Thu, 29 Aug 2019 12:34:07 -0400 Subject: [PATCH 2/2] addressing PR comments --- gateway/auth_manager.go | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/gateway/auth_manager.go b/gateway/auth_manager.go index 64aba95102d..216e0eb2f7f 100644 --- a/gateway/auth_manager.go +++ b/gateway/auth_manager.go @@ -286,20 +286,25 @@ func (b *DefaultSessionManager) SessionDetail(keyName string, hashed bool) (user if hashed { jsonKeyVal, err = b.store.GetRawKey(b.store.GetKeyPrefix() + keyName) } else { - // try to get legacy and new format key at once - var jsonKeyValList []string - jsonKeyValList, err = b.store.GetMultiKey( - []string{ - keyName, - generateToken(b.orgID, keyName), - }, - ) - // pick the 1st non empty from the returned list - for _, val := range jsonKeyValList { - if val != "" { - jsonKeyVal = val - break + if storage.TokenOrg(keyName) != b.orgID { + // try to get legacy and new format key at once + var jsonKeyValList []string + jsonKeyValList, err = b.store.GetMultiKey( + []string{ + keyName, + generateToken(b.orgID, keyName), + }, + ) + // pick the 1st non empty from the returned list + for _, val := range jsonKeyValList { + if val != "" { + jsonKeyVal = val + break + } } + } else { + // key is not an imported one + jsonKeyVal, err = b.store.GetKey(keyName) } }