Skip to content

Commit

Permalink
[TT-5988] Use defaults APISpec if no API ID found (inactive or delete…
Browse files Browse the repository at this point in the history 
…d) (#5423)

https://tyktech.atlassian.net/browse/TT-5988

Before:


![image](https://github.com/TykTechnologies/tyk/assets/233360/8e2b6b60-5ef1-49a3-a1b7-6014b3de5218)

After:


![image](https://github.com/TykTechnologies/tyk/assets/233360/c42fd8f0-de85-4905-b3c0-fdf3f5685993)

Final recorded key data:

```
{
  "last_check": 0,
  "allowance": 1000,
  "rate": 1000,
  "per": 60,
  "throttle_interval": -1,
  "throttle_retry_limit": -1,
  "max_query_depth": -1,
  "date_created": "2023-08-16T12:21:53.662653883Z",
  "expires": 1693398822,
  "quota_max": -1,
  "quota_renews": 1692190432,
  "quota_remaining": 0,
  "quota_renewal_rate": -1,
  "access_rights": {
    "b07c5d3d288f42674a2770aa31981f48": {
      "api_name": "api 2",
      "api_id": "b07c5d3d288f42674a2770aa31981f48",
      "versions": [
        "Default"
      ],
      "allowed_urls": null,
      "restricted_types": [],
      "allowed_types": null,
      "limit": {
        "rate": 0,
        "per": 0,
        "throttle_interval": 0,
        "throttle_retry_limit": 0,
        "max_query_depth": 0,
        "quota_max": 0,
        "quota_renews": 0,
        "quota_remaining": 0,
        "quota_renewal_rate": 0
      },
      "field_access_rights": [],
      "disable_introspection": false,
      "allowance_scope": ""
    },
    "d77069245b9444766c399015c903c1fb": {
      "api_name": "api 1",
      "api_id": "d77069245b9444766c399015c903c1fb",
      "versions": [
        "Default"
      ],
      "allowed_urls": null,
      "restricted_types": [],
      "allowed_types": null,
      "limit": {
        "rate": 0,
        "per": 0,
        "throttle_interval": 0,
        "throttle_retry_limit": 0,
        "max_query_depth": 0,
        "quota_max": 0,
        "quota_renews": 0,
        "quota_remaining": 0,
        "quota_renewal_rate": 0
      },
      "field_access_rights": [],
      "disable_introspection": false,
      "allowance_scope": ""
    }
  },
  "org_id": "64dcbf11a164cd000109d282",
  "oauth_client_id": "",
  "oauth_keys": null,
  "certificate": "",
  "basic_auth_data": {
    "password": "",
    "hash_type": ""
  },
  "jwt_data": {
    "secret": ""
  },
  "hmac_enabled": false,
  "enable_http_signature_validation": false,
  "hmac_string": "",
  "rsa_certificate_id": "",
  "is_inactive": false,
  "apply_policy_id": "",
  "apply_policies": [],
  "data_expires": 0,
  "monitor": {
    "trigger_limits": null
  },
  "enable_detail_recording": false,
  "enable_detailed_recording": false,
  "meta_data": {},
  "tags": [],
  "alias": "",
  "last_updated": "1692190433",
  "id_extractor_deadline": 0,
  "session_lifetime": 0
}
```

API 2 is inactive and honored in access_rights above:

![image](https://github.com/TykTechnologies/tyk/assets/233360/bf75d695-6543-4038-82a9-e80905654df7)

PR also addresses some found defects:

1. logging was inconsistent/spaghetti code (now we log with same log
fields in function),
2. pre-existing errors - added a `test.JSONMarshal(t)` utility to assert
no errors occured (sonarcloud)
3. updated tests to ensure coverage for the change

---------

Co-authored-by: Tit Petric <tit@tyk.io>

(cherry picked from commit b7f2e2f)
  • Loading branch information
@titpetric
titpetric authored and Tyk Bot committed Aug 22, 2023
1 parent aa7ce33 commit 18caaa8
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 58 deletions.
103 changes: 45 additions & 58 deletions gateway/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,11 +262,8 @@ func (gw *Gateway) applyPoliciesAndSave(keyName string, session *user.SessionSta

// calculate lifetime considering access rights
lifetime := gw.ApplyLifetime(session, spec)
if err := gw.GlobalSessionManager.UpdateSession(keyName, session, lifetime, isHashed); err != nil {
return err
}

return nil
return gw.GlobalSessionManager.UpdateSession(keyName, session, lifetime, isHashed)
}

// GetApiSpecsFromAccessRights from the session.AccessRights returns the collection of api specs
Expand Down Expand Up @@ -322,48 +319,54 @@ func (gw *Gateway) doAddOrUpdate(keyName string, newSession *user.SessionState,
newSession.LastUpdated = strconv.Itoa(int(time.Now().Unix()))
}

logger := log.WithFields(logrus.Fields{
"prefix": "api",
"key": gw.obfuscateKey(keyName),
"org_id": newSession.OrgID,
"expires": newSession.Expires,
"api_id": "--",
"user_id": "system",
"user_ip": "--",
"path": "--",
"server_name": "system",
})

if len(newSession.AccessRights) > 0 {
// reset API-level limit to empty APILimit if any has a zero-value
resetAPILimits(newSession.AccessRights)

// We have a specific list of access rules, only add / update those
for apiId := range newSession.AccessRights {
apiSpec := gw.getApiSpec(apiId)
if apiSpec == nil {
log.WithFields(logrus.Fields{
"prefix": "api",
"key": keyName,
"org_id": newSession.OrgID,
"api_id": apiId,
"user_id": "system",
"user_ip": "--",
"path": "--",
"server_name": "system",
}).Error("Could not add key for this API ID, API doesn't exist.")
return errors.New("API must be active to add keys")
logger.WithField("api_id", apiId).Warn("Can't find active API, storing anyway")
}

if apiSpec != nil {
gw.checkAndApplyTrialPeriod(keyName, newSession, isHashed)
}
gw.checkAndApplyTrialPeriod(keyName, newSession, isHashed)

// Lets reset keys if they are edited by admin
if !apiSpec.DontSetQuotasOnCreate {
if apiSpec == nil || !apiSpec.DontSetQuotasOnCreate {
// Reset quote by default
if !dontReset {
gw.GlobalSessionManager.ResetQuota(keyName, newSession, isHashed)
newSession.QuotaRenews = time.Now().Unix() + newSession.QuotaRenewalRate
}
}

// apply polices (if any) and save key
if err := gw.applyPoliciesAndSave(keyName, newSession, apiSpec, isHashed); err != nil {
return err
}
// apply polices (if any) and save key
if err := gw.applyPoliciesAndSave(keyName, newSession, apiSpec, isHashed); err != nil {
return err
}
}
} else {
// nothing defined, add key to ALL
if !gw.GetConfig().AllowMasterKeys {
log.Error("Master keys disallowed in configuration, key not added.")
logger.Error("Master keys disallowed in configuration, key not added.")
return errors.New("Master keys not allowed")
}
log.Warning("No API Access Rights set, adding key to ALL.")
logger.Warning("No API Access Rights set, adding key to ALL.")
gw.apisMu.RLock()
defer gw.apisMu.RUnlock()

Expand All @@ -380,17 +383,7 @@ func (gw *Gateway) doAddOrUpdate(keyName string, newSession *user.SessionState,
}
}

log.WithFields(logrus.Fields{
"prefix": "api",
"key": gw.obfuscateKey(keyName),
"expires": newSession.Expires,
"org_id": newSession.OrgID,
"api_id": "--",
"user_id": "system",
"user_ip": "--",
"path": "--",
"server_name": "system",
}).Info("Key added or updated.")
logger.Info("Key added or updated.")
return nil
}

Expand Down Expand Up @@ -1979,9 +1972,13 @@ func (gw *Gateway) createKeyHandler(w http.ResponseWriter, r *http.Request) {
newSession.LastUpdated = strconv.Itoa(int(time.Now().Unix()))
newSession.DateCreated = time.Now()

sessionManager := gw.GlobalSessionManager

mw := BaseMiddleware{Gw: gw}
// TODO: handle apply policies error
mw.ApplyPolicies(newSession)
if err := mw.ApplyPolicies(newSession); err != nil {
doJSONWrite(w, http.StatusInternalServerError, apiError("Failed to create key - "+err.Error()))
return
}

if len(newSession.AccessRights) > 0 {
// reset API-level limit to nil if any has a zero-value
Expand All @@ -1990,28 +1987,18 @@ func (gw *Gateway) createKeyHandler(w http.ResponseWriter, r *http.Request) {
apiSpec := gw.getApiSpec(apiID)
if apiSpec != nil {
gw.checkAndApplyTrialPeriod(newKey, newSession, false)
// If we have enabled HMAC checking for keys, we need to generate a secret for the client to use
if !apiSpec.DontSetQuotasOnCreate {
// Reset quota by default
gw.GlobalSessionManager.ResetQuota(newKey, newSession, false)
newSession.QuotaRenews = time.Now().Unix() + newSession.QuotaRenewalRate
}
// apply polices (if any) and save key
if err := gw.applyPoliciesAndSave(newKey, newSession, apiSpec, false); err != nil {
doJSONWrite(w, http.StatusInternalServerError, apiError("Failed to create key - "+err.Error()))
return
}
} else {
// Use fallback
sessionManager := gw.GlobalSessionManager
}

if apiSpec == nil || !apiSpec.DontSetQuotasOnCreate {
// Reset quota by default
newSession.QuotaRenews = time.Now().Unix() + newSession.QuotaRenewalRate
sessionManager.ResetQuota(newKey, newSession, false)
// apply polices (if any) and save key
err := sessionManager.UpdateSession(newKey, newSession, -1, false)
if err != nil {
doJSONWrite(w, http.StatusInternalServerError, apiError("Failed to create key - "+err.Error()))
return
}
}

// apply polices (if any) and save key
if err := gw.applyPoliciesAndSave(newKey, newSession, apiSpec, false); err != nil {
doJSONWrite(w, http.StatusInternalServerError, apiError("Failed to create key - "+err.Error()))
return
}
}
} else {
Expand All @@ -2033,8 +2020,8 @@ func (gw *Gateway) createKeyHandler(w http.ResponseWriter, r *http.Request) {
for _, spec := range gw.apisByID {
gw.checkAndApplyTrialPeriod(newKey, newSession, false)
if !spec.DontSetQuotasOnCreate {
// Reset quote by default
gw.GlobalSessionManager.ResetQuota(newKey, newSession, false)
// Reset quota by default
sessionManager.ResetQuota(newKey, newSession, false)
newSession.QuotaRenews = time.Now().Unix() + newSession.QuotaRenewalRate
}
if err := gw.applyPoliciesAndSave(newKey, newSession, spec, false); err != nil {
Expand Down
7 changes: 7 additions & 0 deletions gateway/api_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,12 @@ func TestKeyHandler(t *testing.T) {
}
withBadPolicyJSON := test.MarshalJSON(t)(withBadPolicy)

withUnknownAPI := CreateStandardSession()
withUnknownAPI.AccessRights = map[string]user.AccessDefinition{"unknown": {
APIID: "unknown", Versions: []string{"v1"},
}}
withUnknownAPIJSON := test.MarshalJSON(t)(withUnknownAPI)

t.Run("Create key", func(t *testing.T) {
_, _ = ts.Run(t, []test.TestCase{
// Master keys should be disabled by default
Expand Down Expand Up @@ -405,6 +411,7 @@ func TestKeyHandler(t *testing.T) {
// Without data
{Method: "PUT", Path: "/tyk/keys/" + knownKey, AdminAuth: true, Code: 400},
{Method: "PUT", Path: "/tyk/keys/" + knownKey, Data: string(withAccessJSON), AdminAuth: true, Code: 200},
{Method: "PUT", Path: "/tyk/keys/" + knownKey, Data: string(withUnknownAPIJSON), AdminAuth: true, Code: 200},
{Method: "PUT", Path: "/tyk/keys/" + knownKey + "?api_id=test", Data: string(withAccessJSON), AdminAuth: true, Code: 200},
{Method: "PUT", Path: "/tyk/keys/" + knownKey + "?api_id=none", Data: string(withAccessJSON), AdminAuth: true, Code: 200},
}...)
Expand Down

0 comments on commit 18caaa8

Please sign in to comment.