Skip to content

Commit

Permalink
Replicate changes of custom keys in slave nodes (deletion/updates) (T…
Browse files Browse the repository at this point in the history
…ykTechnologies#3616)

<!-- Provide a general summary of your changes in the Title above -->

## Description
When create custom key in slave node, we must ensure that the hash is the same as in the master node, otherwise we could face issues updating and deleting tokens. Also in this PR was solved an issue where we would end up having 2 keys in redis when we used the key (when we used the key then a new key and quota key were registered)

## Related Issue
- https://tyktech.atlassian.net/browse/TT-2820
- https://tyktech.atlassian.net/browse/TT-2639

## Motivation and Context
Fix CRUD of custom keys in slave nodes. Prevent the creation of 2 keys 

## How This Has Been Tested
Replicating the steps to reproduce the bug

## Screenshots (if appropriate)

## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] Refactoring or add test (improvements in base code or adds test coverage to functionality)

## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes that apply -->
<!-- If you're unsure about any of these, don't hesitate to ask; we're here to help! -->
- [x] Make sure you are requesting to **pull a topic/feature/bugfix branch** (right side). If pulling from your own
      fork, don't request your `master`!
- [x] Make sure you are making a pull request against the **`master` branch** (left side). Also, you should start
      *your branch* off *our latest `master`*.
- [ ] My change requires a change to the documentation.
  - [ ] If you've changed APIs, describe what needs to be updated in the documentation.
  - [ ] If new config option added, ensure that it can be set via ENV variable
- [ ] I have updated the documentation accordingly.
- [ ] Modules and vendor dependencies have been updated; run `go mod tidy && go mod vendor`
- [ ] When updating library version must provide reason/explanation for this update.
- [ ] I have added tests to cover my changes.
- [x] All new and existing tests passed.
- [x] Check your code additions will not fail linting checks:
  - [x] `go fmt -s`
  - [x] `go vet`
  • Loading branch information
sredxny authored Jul 23, 2021
1 parent 7fe43eb commit 529518d
Show file tree
Hide file tree
Showing 22 changed files with 382 additions and 294 deletions.
15 changes: 8 additions & 7 deletions ctx/ctx.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,23 +46,24 @@ func setContext(r *http.Request, ctx context.Context) {
*r = *r2
}

func ctxSetSession(r *http.Request, s *user.SessionState, token string, scheduleUpdate bool) {
// ctxSetSession stores in ctx the s.KeyID
func ctxSetSession(r *http.Request, s *user.SessionState, scheduleUpdate bool) {
if s == nil {
panic("setting a nil context SessionData")
}

if token == "" {
token = GetAuthToken(r)
if s.KeyID == "" {
s.KeyID = GetAuthToken(r)
}

if s.KeyHashEmpty() {
s.SetKeyHash(storage.HashKey(token))
s.SetKeyHash(storage.HashKey(s.KeyID))
}

ctx := r.Context()
ctx = context.WithValue(ctx, SessionData, s)
ctx = context.WithValue(ctx, AuthToken, token)

ctx = context.WithValue(ctx, AuthToken, s.KeyID)
if scheduleUpdate {
ctx = context.WithValue(ctx, UpdateSession, true)
}
Expand All @@ -84,8 +85,8 @@ func GetSession(r *http.Request) *user.SessionState {
return nil
}

func SetSession(r *http.Request, s *user.SessionState, token string, scheduleUpdate bool) {
ctxSetSession(r, s, token, scheduleUpdate)
func SetSession(r *http.Request, s *user.SessionState, scheduleUpdate bool) {
ctxSetSession(r, s, scheduleUpdate)
}

func SetDefinition(r *http.Request, s *apidef.APIDefinition) {
Expand Down
18 changes: 11 additions & 7 deletions gateway/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ func applyPoliciesAndSave(keyName string, session *user.SessionState, spec *APIS
mw := BaseMiddleware{
Spec: spec,
}

if err := mw.ApplyPolicies(session); err != nil {
return err
}
Expand Down Expand Up @@ -337,13 +338,13 @@ func handleAddOrUpdate(keyName string, r *http.Request, isHashed bool) (interfac
mw := BaseMiddleware{}
// TODO: handle apply policies error
mw.ApplyPolicies(newSession)

// DO ADD OR UPDATE

// get original session in case of update and preserve fields that SHOULD NOT be updated
originalKey := user.SessionState{}
if r.Method == http.MethodPut {
key, found := GlobalSessionManager.SessionDetail(newSession.OrgID, keyName, isHashed)
keyName = key.KeyID
if !found {
log.Error("Could not find key when updating")
return apiError("Key is not found"), http.StatusNotFound
Expand Down Expand Up @@ -475,18 +476,18 @@ func handleAddOrUpdate(keyName string, r *http.Request, isHashed bool) (interfac
return response, http.StatusOK
}

func handleGetDetail(sessionKey, apiID string, byHash bool) (interface{}, int) {
func handleGetDetail(sessionKey, apiID, orgID string, byHash bool) (interface{}, int) {
if byHash && !config.Global().HashKeys {
return apiError("Key requested by hash but key hashing is not enabled"), http.StatusBadRequest
}

spec := getApiSpec(apiID)
orgID := ""
if spec != nil {
orgID = spec.OrgID
}

session, ok := GlobalSessionManager.SessionDetail(orgID, sessionKey, byHash)
sessionKey = session.KeyID

if !ok {
return apiError("Key not found"), http.StatusNotFound
Expand Down Expand Up @@ -636,6 +637,7 @@ func handleAddKey(keyName, hashedName, sessionString, apiID string) {

func handleDeleteKey(keyName, orgID, apiID string, resetQuota bool) (interface{}, int) {
session, ok := GlobalSessionManager.SessionDetail(orgID, keyName, false)
keyName = session.KeyID
if !ok {
return apiError("There is no such key found"), http.StatusNotFound
}
Expand Down Expand Up @@ -724,6 +726,7 @@ func handleDeleteHashedKeyWithLogs(keyName, orgID, apiID string, resetQuota bool
func handleDeleteHashedKey(keyName, orgID, apiID string, resetQuota bool) (interface{}, int) {

session, ok := GlobalSessionManager.SessionDetail(orgID, keyName, true)
keyName = session.KeyID
if !ok {
return apiError("There is no such key found"), http.StatusNotFound
}
Expand Down Expand Up @@ -944,10 +947,10 @@ func keyHandler(w http.ResponseWriter, r *http.Request) {
case http.MethodGet:
if keyName != "" {
// Return single key detail
obj, code = handleGetDetail(keyName, apiID, isHashed)
obj, code = handleGetDetail(keyName, apiID, orgID, isHashed)
if code != http.StatusOK && hashKeyFunction != "" {
// try to use legacy key format
obj, code = handleGetDetail(origKeyName, apiID, isHashed)
obj, code = handleGetDetail(origKeyName, apiID, orgID, isHashed)
}
} else {
// Return list of keys
Expand Down Expand Up @@ -1023,6 +1026,7 @@ func handleUpdateHashedKey(keyName string, applyPolicies []string) (interface{},
}

sess, ok := GlobalSessionManager.SessionDetail(orgID, keyName, true)
keyName = sess.KeyID
if !ok {
log.WithFields(logrus.Fields{
"prefix": "api",
Expand Down Expand Up @@ -2327,8 +2331,8 @@ func ctxGetSession(r *http.Request) *user.SessionState {
return ctx.GetSession(r)
}

func ctxSetSession(r *http.Request, s *user.SessionState, token string, scheduleUpdate bool) {
ctx.SetSession(r, s, token, scheduleUpdate)
func ctxSetSession(r *http.Request, s *user.SessionState, scheduleUpdate bool) {
ctx.SetSession(r, s, scheduleUpdate)
}

func ctxScheduleSessionUpdate(r *http.Request) {
Expand Down
4 changes: 2 additions & 2 deletions gateway/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1479,7 +1479,7 @@ func TestContextSession(t *testing.T) {
if ctxGetSession(r) != nil {
t.Fatal("expected ctxGetSession to return nil")
}
ctxSetSession(r, &user.SessionState{}, "", false)
ctxSetSession(r, &user.SessionState{}, false)
if ctxGetSession(r) == nil {
t.Fatal("expected ctxGetSession to return non-nil")
}
Expand All @@ -1488,7 +1488,7 @@ func TestContextSession(t *testing.T) {
t.Fatal("expected ctxSetSession of zero val to panic")
}
}()
ctxSetSession(r, nil, "", false)
ctxSetSession(r, nil, false)
}

func TestApiLoaderLongestPathFirst(t *testing.T) {
Expand Down
6 changes: 4 additions & 2 deletions gateway/auth_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ func (b *DefaultSessionManager) RemoveSession(orgID string, keyName string, hash
func (b *DefaultSessionManager) SessionDetail(orgID string, keyName string, hashed bool) (user.SessionState, bool) {
var jsonKeyVal string
var err error
keyId := keyName

// get session by key
if hashed {
Expand All @@ -148,9 +149,10 @@ func (b *DefaultSessionManager) SessionDetail(orgID string, keyName string, hash
jsonKeyValList, err = b.store.GetMultiKey(toSearchList)

// pick the 1st non empty from the returned list
for _, val := range jsonKeyValList {
for idx, val := range jsonKeyValList {
if val != "" {
jsonKeyVal = val
keyId = toSearchList[idx]
break
}
}
Expand All @@ -173,7 +175,7 @@ func (b *DefaultSessionManager) SessionDetail(orgID string, keyName string, hash
log.Error("Couldn't unmarshal session object (may be cache miss): ", err)
return user.SessionState{}, false
}

session.KeyID = keyId
return session.Clone(), true
}

Expand Down
8 changes: 4 additions & 4 deletions gateway/coprocess.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,6 @@ func (m *CoProcessMiddleware) ProcessRequest(w http.ResponseWriter, r *http.Requ

if m.HookType == coprocess.HookType_CustomKeyCheck && extractor != nil {
sessionID, returnOverrides = extractor.ExtractAndCheck(r)

if returnOverrides.ResponseCode != 0 {
if returnOverrides.ResponseError == "" {
return nil, returnOverrides.ResponseCode
Expand Down Expand Up @@ -454,12 +453,13 @@ func (m *CoProcessMiddleware) ProcessRequest(w http.ResponseWriter, r *http.Requ
}

returnedSession.OrgID = m.Spec.OrgID
// set a Key ID as default
returnedSession.KeyID = token

if err := m.ApplyPolicies(returnedSession); err != nil {
AuthFailed(m, r, authToken)
return errors.New(http.StatusText(http.StatusForbidden)), http.StatusForbidden
}

existingSession, found := GlobalSessionManager.SessionDetail(m.Spec.OrgID, sessionID, false)
if found {
returnedSession.QuotaRenews = existingSession.QuotaRenews
Expand All @@ -480,8 +480,8 @@ func (m *CoProcessMiddleware) ProcessRequest(w http.ResponseWriter, r *http.Requ
AuthFailed(m, r, authToken)
return errors.New(http.StatusText(http.StatusForbidden)), http.StatusForbidden
}

ctxSetSession(r, returnedSession, sessionID, true)
returnedSession.KeyID = sessionID
ctxSetSession(r, returnedSession, true)
}

return nil, http.StatusOK
Expand Down
15 changes: 9 additions & 6 deletions gateway/coprocess_id_extractor.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,12 @@ func (e *ValueExtractor) ExtractAndCheck(r *http.Request) (sessionID string, ret

sessionID = e.GenerateSessionID(extractorOutput, e.BaseMid)

previousSession, keyExists := e.BaseMid.CheckSessionAndIdentityForValidKey(&sessionID, r)
previousSession, keyExists := e.BaseMid.CheckSessionAndIdentityForValidKey(sessionID, r)
sessionID = previousSession.KeyID

if keyExists {
if previousSession.IdExtractorDeadline > time.Now().Unix() {
ctxSetSession(r, &previousSession, sessionID, true)
ctxSetSession(r, &previousSession, true)
returnOverrides = ReturnOverrides{
ResponseCode: 200,
}
Expand Down Expand Up @@ -206,11 +207,12 @@ func (e *RegexExtractor) ExtractAndCheck(r *http.Request) (SessionID string, ret
}

SessionID = e.GenerateSessionID(regexOutput[e.cfg.RegexMatchIndex], e.BaseMid)
previousSession, keyExists := e.BaseMid.CheckSessionAndIdentityForValidKey(&SessionID, r)
previousSession, keyExists := e.BaseMid.CheckSessionAndIdentityForValidKey(SessionID, r)
SessionID = previousSession.KeyID

if keyExists {
if previousSession.IdExtractorDeadline > time.Now().Unix() {
ctxSetSession(r, &previousSession, SessionID, true)
ctxSetSession(r, &previousSession, true)
returnOverrides = ReturnOverrides{
ResponseCode: 200,
}
Expand Down Expand Up @@ -282,10 +284,11 @@ func (e *XPathExtractor) ExtractAndCheck(r *http.Request) (SessionID string, ret

SessionID = e.GenerateSessionID(output, e.BaseMid)

previousSession, keyExists := e.BaseMid.CheckSessionAndIdentityForValidKey(&SessionID, r)
previousSession, keyExists := e.BaseMid.CheckSessionAndIdentityForValidKey(SessionID, r)
SessionID = previousSession.KeyID
if keyExists {
if previousSession.IdExtractorDeadline > time.Now().Unix() {
ctxSetSession(r, &previousSession, SessionID, true)
ctxSetSession(r, &previousSession, true)
returnOverrides = ReturnOverrides{
ResponseCode: 200,
}
Expand Down
17 changes: 11 additions & 6 deletions gateway/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -654,8 +654,8 @@ func (t BaseMiddleware) ApplyPolicies(session *user.SessionState) error {

// CheckSessionAndIdentityForValidKey will check first the Session store for a valid key, if not found, it will try
// the Auth Handler, if not found it will fail
func (t BaseMiddleware) CheckSessionAndIdentityForValidKey(originalKey *string, r *http.Request) (user.SessionState, bool) {
key := *originalKey
func (t BaseMiddleware) CheckSessionAndIdentityForValidKey(originalKey string, r *http.Request) (user.SessionState, bool) {
key := originalKey
minLength := t.Spec.GlobalConfig.MinTokenLength
if minLength == 0 {
// See https://github.com/TykTechnologies/tyk/issues/1681
Expand All @@ -671,7 +671,6 @@ func (t BaseMiddleware) CheckSessionAndIdentityForValidKey(originalKey *string,
keyHash := key
cacheKey := key
if t.Spec.GlobalConfig.HashKeys {
keyHash = storage.HashStr(key)
cacheKey = storage.HashStr(key, storage.HashMurmur64) // always hash cache keys with murmur64 to prevent collisions
}

Expand All @@ -692,7 +691,11 @@ func (t BaseMiddleware) CheckSessionAndIdentityForValidKey(originalKey *string,
// Check session store
t.Logger().Debug("Querying keystore")
session, found := GlobalSessionManager.SessionDetail(t.Spec.OrgID, key, false)

if found {
if t.Spec.GlobalConfig.HashKeys {
keyHash = storage.HashStr(session.KeyID)
}
session := session.Clone()
session.SetKeyHash(keyHash)
// If exists, assume it has been authorized and pass on
Expand All @@ -719,16 +722,15 @@ func (t BaseMiddleware) CheckSessionAndIdentityForValidKey(originalKey *string,
// 2. If not there, get it from the AuthorizationHandler
session, found = t.Spec.AuthManager.SessionDetail(t.Spec.OrgID, key, false)
if found {
// update value of originalKey, as for custom-keys it might get updated (the key is generated again using alias)
*originalKey = key
key = session.KeyID

session := session.Clone()
session.SetKeyHash(keyHash)
// If not in Session, and got it from AuthHandler, create a session with a new TTL

t.Logger().Info("Recreating session for key: ", obfuscateKey(key))

// cache it

if !t.Spec.GlobalConfig.LocalSessionCache.DisableCacheSessionState {
SessionCache.Set(cacheKey, session, cache.DefaultExpiration)
}
Expand All @@ -741,6 +743,9 @@ func (t BaseMiddleware) CheckSessionAndIdentityForValidKey(originalKey *string,

t.Logger().Debug("Lifetime is: ", session.Lifetime(t.Spec.SessionLifetime))
ctxScheduleSessionUpdate(r)
} else {
// defaulting
session.KeyID = key
}

return session, found
Expand Down
25 changes: 20 additions & 5 deletions gateway/mw_auth_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"strings"

"github.com/TykTechnologies/tyk/certs"
"github.com/TykTechnologies/tyk/storage"

"github.com/TykTechnologies/tyk/user"

Expand Down Expand Up @@ -102,19 +103,33 @@ func (k *AuthKey) ProcessRequest(_ http.ResponseWriter, r *http.Request, _ inter
return errorAndStatusCode(ErrAuthAuthorizationFieldMissing)
}

session, keyExists = k.CheckSessionAndIdentityForValidKey(&key, r)
session, keyExists = k.CheckSessionAndIdentityForValidKey(key, r)
key = session.KeyID
if !keyExists {
// fallback to search by cert
session, keyExists = k.CheckSessionAndIdentityForValidKey(&certHash, r)
session, keyExists = k.CheckSessionAndIdentityForValidKey(certHash, r)
certHash = session.KeyID
if !keyExists {
return k.reportInvalidKey(key, r, MsgNonExistentKey, ErrAuthKeyNotFound)
}
}

if authConfig.UseCertificate {
certID := session.OrgID + certHash
if _, err := CertificateManager.GetRaw(certID); err != nil {
return k.reportInvalidKey(key, r, MsgNonExistentCert, ErrAuthCertNotFound)
_, err := CertificateManager.GetRaw(certID)
if err != nil {
// Try alternative approach:
id, err := storage.TokenID(session.KeyID)
if err != nil {
log.Error(err)
return k.reportInvalidKey(key, r, MsgNonExistentCert, ErrAuthCertNotFound)
}

certID = session.OrgID + id
_, err = CertificateManager.GetRaw(certID)
if err != nil {
return k.reportInvalidKey(key, r, MsgNonExistentCert, ErrAuthCertNotFound)
}
}

if session.Certificate != certID {
Expand All @@ -124,7 +139,7 @@ func (k *AuthKey) ProcessRequest(_ http.ResponseWriter, r *http.Request, _ inter
// Set session state on context, we will need it later
switch k.Spec.BaseIdentityProvidedBy {
case apidef.AuthToken, apidef.UnsetAuth:
ctxSetSession(r, &session, key, false)
ctxSetSession(r, &session, false)
k.setContextVars(r, key)
}

Expand Down
Loading

0 comments on commit 529518d

Please sign in to comment.