Merging to release-5.7: TT-13513 TT-12767 TT-12768 ensure to save oau…

…th clients locally when pulled from rpc (#6740) (#6799)

### **User description**
TT-13513 TT-12767 TT-12768 ensure to save oauth clients locally when
pulled from rpc (#6740)

### **User description**
<!-- Provide a general summary of your changes in the Title above -->

## Description

The Oauth client was not being cached in the local redis when the
gateway was running as an edge in an MDCB setup. This PR then:
- Ensures that the first time that the oauthclient is pulled from RPC
then we cache it in redis
- Refactor code of the MDCB storage into multiple smaller functions so
is eaasy to read the code and test
- created mock for the storage handler interface...later we should
remove all mentions to DummyStorage and use the mock instead
- Created tests for the mdcb storage
- Certificates caching doesnt works in the same way, as they depend on
the certificate manager and secret set to encode the content

## Related Issue

<!-- This project only accepts pull requests related to open issues. -->
<!-- If suggesting a new feature or change, please discuss it in an
issue first. -->
<!-- If fixing a bug, there should be an issue describing it with steps
to reproduce. -->
<!-- OSS: Please link to the issue here. Tyk: please create/link the
JIRA ticket. -->

## Motivation and Context

<!-- Why is this change required? What problem does it solve? -->

## How This Has Been Tested

- Run MDCB setup with synchroniser disabled
- Created api and policy via dashboard. 
- Protect the api using oauth 2.0
- Created an oauth client via dashboard api
- Create a token in the edge node using the created oauth client
- use the token to consume the api in that edge node
- shut down mdcb
- attempt to generate another token using the edge node
- At this point you should be allowed to create that new token and use
it against the api

## Screenshots (if appropriate)

## Types of changes

<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [ ] 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 there are no documentation updates required, mark the item as
checked. -->
<!-- Raise up any additional concerns not covered by the checklist. -->

- [ ] I ensured that the documentation is up to date
- [ ] I explained why this PR updates go.mod in detail with reasoning
why it's required
- [ ] I would like a code coverage CI quality gate exception and have
explained why


___

### **PR Type**
Bug fix, Tests, Enhancement


___

### **Description**
- Refactored the `GetKey` method to separate local and RPC retrieval
logic, improving maintainability.
- Introduced caching mechanisms for OAuth clients and certificates,
ensuring resources pulled from RPC are stored locally.
- Added constants for resource types to improve code readability and
maintainability.
- Renamed callback function for certificate pull consistency.
- Added extensive unit tests for new caching and retrieval logic,
improving test coverage.
- Generated a mock for the `Handler` interface using GoMock to
facilitate isolated testing of storage interactions.



___



### **Changes walkthrough** 📝
<table><thead><tr><th></th><th align="left">Relevant

files</th></tr></thead><tbody><tr><td><strong>Enhancement</strong></td><td><table>
<tr>
  <td>
    <details>
<summary><strong>manager.go</strong><dd><code>Rename callback function
for certificate pull consistency</code></dd></summary>
<hr>

certs/manager.go

<li>Renamed <code>CallbackonPullfromRPC</code> to
<code>CallbackOnPullCertificateFromRPC</code> for <br>consistency.<br>
<li> Updated the initialization of <code>mdcbStorage</code> with the
renamed callback.<br>


</details>


  </td>
<td><a

href="https://github.com/TykTechnologies/tyk/pull/6740/files#diff-78e768b2719ac9f70038499f847de2843db20d8ca21a963ea63b82010d711039">+1/-1</a>&nbsp;
&nbsp; &nbsp; </td>

</tr>

<tr>
  <td>
    <details>
<summary><strong>mdcb_storage.go</strong><dd><code>Refactor key
retrieval and add caching mechanisms</code>&nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

storage/mdcb_storage.go

<li>Added constants for resource types
(<code>resourceOauthClient</code>, <br><code>resourceCertificate</code>,
etc.).<br> <li> Refactored <code>GetKey</code> to separate local and RPC
retrieval logic.<br> <li> Introduced caching mechanisms for OAuth
clients and certificates.<br> <li> Added helper methods like
<code>getFromRPCAndCache</code>, <code>cacheCertificate</code>, and
<br><code>cacheOAuthClient</code>.<br>


</details>


  </td>
<td><a

href="https://github.com/TykTechnologies/tyk/pull/6740/files#diff-c5739d542a422343ec22585ffa5e4ad7e2e91358db018a157dc23cb5096c04d2">+74/-32</a>&nbsp;
</td>

</tr>

<tr>
  <td>
    <details>
<summary><strong>storage.go</strong><dd><code>Add GoMock directive for
Handler interface</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

storage/storage.go

<li>Added GoMock generation directive for the <code>Handler</code>
interface.<br> <li> Prepared the file for mock generation to support
testing.<br>


</details>


  </td>
<td><a

href="https://github.com/TykTechnologies/tyk/pull/6740/files#diff-2a93e444b612bd9853c32889fb82c4041760536f84356bb0db04738c19b62dde">+2/-0</a>&nbsp;
&nbsp; &nbsp; </td>

</tr>
</table></td></tr><tr><td><strong>Tests</strong></td><td><table>
<tr>
  <td>
    <details>
<summary><strong>mdcb_storage_test.go</strong><dd><code>Add unit tests
for caching and retrieval logic</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

storage/mdcb_storage_test.go

<li>Added test setup utility for mocking dependencies.<br> <li>
Implemented unit tests for new caching and retrieval methods.<br> <li>
Enhanced test coverage for resource type processing and error
<br>handling.<br>


</details>


  </td>
<td><a

href="https://github.com/TykTechnologies/tyk/pull/6740/files#diff-6a40b704ea7dc3b61069eebd5d56464a66bb1c61095909aa9cc5e423c5c88422">+323/-4</a>&nbsp;
</td>

</tr>

<tr>
  <td>
    <details>
<summary><strong>storage.go</strong><dd><code>Add GoMock-generated mock
for Handler interface</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

storage/mock/storage.go

<li>Added a generated mock for the <code>Handler</code> interface using
GoMock.<br> <li> Enables testing of storage interactions in
isolation.<br>


</details>


  </td>
<td><a

href="https://github.com/TykTechnologies/tyk/pull/6740/files#diff-0e75f439d0385d9272ea3afa9fc465dcae08554f19ff821e0743ad096325df40">+501/-0</a>&nbsp;
</td>

</tr>
</table></td></tr></tr></tbody></table>

___

> 💡 **PR-Agent usage**: Comment `/help "your question"` on any pull
request to receive relevant information

---------

Co-authored-by: sredny buitrago <[email protected]>
Co-authored-by: Copilot Autofix powered by AI
<62310815+github-advanced-security[bot]@users.noreply.github.com>
Co-authored-by: Matias <[email protected]>
Co-authored-by: Mladen Kolavcic
<[email protected]>


___

### **PR Type**
Bug fix, Enhancement, Tests


___

### **Description**
- Refactored the `GetKey` method to separate local and RPC retrieval
logic, improving maintainability.
- Introduced caching mechanisms for OAuth clients and certificates,
ensuring resources pulled from RPC are stored locally.
- Added constants for resource types to improve code readability and
maintainability.
- Renamed callback function for certificate pull consistency.
- Added extensive unit tests for new caching and retrieval logic,
improving test coverage.
- Generated a mock for the `Handler` interface using GoMock to
facilitate isolated testing of storage interactions.



___



### **Changes walkthrough** 📝
<table><thead><tr><th></th><th align="left">Relevant
files</th></tr></thead><tbody><tr><td><strong>Enhancement</strong></td><td><table>
<tr>
  <td>
    <details>
<summary><strong>manager.go</strong><dd><code>Rename callback function
for certificate pull consistency</code></dd></summary>
<hr>

certs/manager.go

<li>Renamed <code>CallbackonPullfromRPC</code> to
<code>CallbackOnPullCertificateFromRPC</code> for <br>consistency.<br>
<li> Updated initialization of <code>mdcbStorage</code> with the renamed
callback.<br>


</details>


  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6799/files#diff-78e768b2719ac9f70038499f847de2843db20d8ca21a963ea63b82010d711039">+1/-3</a>&nbsp;
&nbsp; &nbsp; </td>

</tr>

<tr>
  <td>
    <details>
<summary><strong>server.go</strong><dd><code>Update MDCB storage handler
initialization</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

gateway/server.go

<li>Added a <code>nil</code> callback parameter when initializing MDCB
storage handler.<br> <br>


</details>


  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6799/files#diff-4652d1bf175a0be8f5e61ef7177c9666f23e077d8626b73ac9d13358fa8b525b">+1/-0</a>&nbsp;
&nbsp; &nbsp; </td>

</tr>

<tr>
  <td>
    <details>
<summary><strong>mdcb_storage.go</strong><dd><code>Refactor key
retrieval and add caching mechanisms</code>&nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

storage/mdcb_storage.go

<li>Refactored <code>GetKey</code> to separate local and RPC retrieval
logic.<br> <li> Added constants for resource types to improve
readability.<br> <li> Introduced caching mechanisms for OAuth clients
and certificates.<br> <li> Added helper methods for caching and
retrieval logic.<br>


</details>


  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6799/files#diff-c5739d542a422343ec22585ffa5e4ad7e2e91358db018a157dc23cb5096c04d2">+69/-36</a>&nbsp;
</td>

</tr>
</table></td></tr><tr><td><strong>Tests</strong></td><td><table>
<tr>
  <td>
    <details>
<summary><strong>mdcb_storage_test.go</strong><dd><code>Add unit tests
for caching and retrieval logic</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

storage/mdcb_storage_test.go

<li>Added unit tests for new caching and retrieval logic.<br> <li>
Implemented test setup utility for mocking dependencies.<br> <li>
Enhanced test coverage for resource type processing and error
<br>handling.<br>


</details>


  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6799/files#diff-6a40b704ea7dc3b61069eebd5d56464a66bb1c61095909aa9cc5e423c5c88422">+323/-5</a>&nbsp;
</td>

</tr>

<tr>
  <td>
    <details>
<summary><strong>storage.go</strong><dd><code>Add GoMock-generated mock
for Handler interface</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

storage/mock/storage.go

<li>Added a generated mock for the <code>Handler</code> interface using
GoMock.<br> <li> Enables isolated testing of storage interactions.<br>


</details>


  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6799/files#diff-0e75f439d0385d9272ea3afa9fc465dcae08554f19ff821e0743ad096325df40">+502/-0</a>&nbsp;
</td>

</tr>

<tr>
  <td>
    <details>
<summary><strong>storage.go</strong><dd><code>Add GoMock directive for
Handler interface</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

storage/storage.go

<li>Added GoMock generation directive for the <code>Handler</code>
interface.<br> <li> Prepared the file for mock generation to support
testing.<br>


</details>


  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6799/files#diff-2a93e444b612bd9853c32889fb82c4041760536f84356bb0db04738c19b62dde">+2/-0</a>&nbsp;
&nbsp; &nbsp; </td>

</tr>
</table></td></tr></tr></tbody></table>

___

> 💡 **PR-Agent usage**: Comment `/help "your question"` on any pull
request to receive relevant information

Co-authored-by: Sredny M. <[email protected]>

certs/manager.go

@@ -101,9 +101,7 @@ func NewSlaveCertManager(localStorage, rpcStorage storage.Handler, secret string
 		return err
 	}
 
-	mdcbStorage := storage.NewMdcbStorage(localStorage, rpcStorage, log)
-	mdcbStorage.CallbackonPullfromRPC = &callbackOnPullCertFromRPC
-
+	mdcbStorage := storage.NewMdcbStorage(localStorage, rpcStorage, log, callbackOnPullCertFromRPC)
 	cm.storage = mdcbStorage
 	return cm
 }

gateway/server.go

@@ -1587,6 +1587,7 @@ func (gw *Gateway) getGlobalMDCBStorageHandler(keyPrefix string, hashKeys bool)
 				Gw:        gw,
 			},
 			logger,
+			nil,
 		)
 	}
 	return localStorage

storage/mdcb_storage.go

@@ -8,60 +8,50 @@ import (
 )
 
 type MdcbStorage struct {
-	local                 Handler
-	rpc                   Handler
-	logger                *logrus.Entry
-	CallbackonPullfromRPC *func(key string, val string) error
+	local         Handler
+	rpc           Handler
+	logger        *logrus.Entry
+	OnRPCCertPull func(key string, val string) error
 }
 
-func NewMdcbStorage(local, rpc Handler, log *logrus.Entry) *MdcbStorage {
+const (
+	resourceOauthClient = "OauthClient"
+	resourceCertificate = "Certificate"
+	resourceApiKey      = "ApiKey"
+	resourceKey         = "Key"
+)
+
+func NewMdcbStorage(local, rpc Handler, log *logrus.Entry, OnRPCCertPull func(key string, val string) error) *MdcbStorage {
 	return &MdcbStorage{
-		local:  local,
-		rpc:    rpc,
-		logger: log,
+		local:         local,
+		rpc:           rpc,
+		logger:        log,
+		OnRPCCertPull: OnRPCCertPull,
 	}
 }
 
 func (m MdcbStorage) GetKey(key string) (string, error) {
-	var val string
-	var err error
-
-	if m.local == nil {
-		return m.rpc.GetKey(key)
-	}
-
-	val, err = m.local.GetKey(key)
-	if err != nil {
-		m.logger.Infof("Retrieving key from rpc.")
-		val, err = m.rpc.GetKey(key)
-
-		if err != nil {
-			resourceType := getResourceType(key)
-			m.logger.Errorf("cannot retrieve %v from rpc: %v", resourceType, err.Error())
-			return val, err
-		}
-
-		if m.CallbackonPullfromRPC != nil {
-			err := (*m.CallbackonPullfromRPC)(key, val)
-			if err != nil {
-				m.logger.Error(err)
-			}
+	if m.local != nil {
+		val, err := m.getFromLocal(key)
+		if err == nil {
+			return val, nil
 		}
+		m.logger.Debugf("Key not present locally, pulling from rpc layer: %v", err)
 	}
 
-	return val, err
+	return m.getFromRPCAndCache(key)
 }
 
 func getResourceType(key string) string {
 	switch {
 	case strings.Contains(key, "oauth-clientid."):
-		return "Oauth Client"
+		return resourceOauthClient
 	case strings.HasPrefix(key, "cert"):
-		return "certificate"
+		return resourceCertificate
 	case strings.HasPrefix(key, "apikey"):
-		return "api key"
+		return resourceApiKey
 	default:
-		return "key"
+		return resourceKey
 	}
 }
 
@@ -256,3 +246,46 @@ func (m MdcbStorage) Exists(key string) (bool, error) {
 
 	return foundLocal && foundRpc, nil
 }
+
+// cacheCertificate saves locally resourceCertificate after pull from rpc
+func (m MdcbStorage) cacheCertificate(key, val string) error {
+	if m.OnRPCCertPull == nil {
+		return nil
+	}
+	return m.OnRPCCertPull(key, val)
+}
+
+// cacheOAuthClient saved oauth data in local storage after pull from rpc
+func (m MdcbStorage) cacheOAuthClient(key, val string) error {
+	return m.local.SetKey(key, val, 0)
+}
+
+// processResourceByType based on the type of key it will trigger the proper
+// caching mechanism
+func (m MdcbStorage) processResourceByType(key, val string) error {
+
+	resourceType := getResourceType(key)
+	switch resourceType {
+	case resourceOauthClient:
+		return m.cacheOAuthClient(key, val)
+	case resourceCertificate:
+		return m.cacheCertificate(key, val)
+	}
+	return nil
+}
+
+// getFromRPCAndCache pulls a resource from rpc and stores it in local redis for caching
+func (m MdcbStorage) getFromRPCAndCache(key string) (string, error) {
+	val, err := m.rpc.GetKey(key)
+	if err != nil {
+		return "", err
+	}
+
+	err = m.processResourceByType(key, val)
+	return val, err
+}
+
+// getFromLocal get a key from local storage
+func (m MdcbStorage) getFromLocal(key string) (string, error) {
+	return m.local.GetKey(key)
+}