Skip to content

Commit

Permalink
Merging to release-5.7.0: [TT-13535/TT-13566] Make upstream oauth flo…
Browse files Browse the repository at this point in the history
…w client secret omitempty (#6708) (#6710)

### **User description**
[TT-13535/TT-13566] Make upstream oauth flow client secret omitempty
(#6708)

### **User description**
<details open>
<summary><a href="https://tyktech.atlassian.net/browse/TT-13566"
title="TT-13566" target="_blank">TT-13566</a></summary>
  <br />
  <table>
    <tr>
      <th>Summary</th>
<td>Make upstream auth oauth password client secret not required in oas
schema</td>
    </tr>
    <tr>
      <th>Type</th>
      <td>
<img alt="Sub-task"

src="https://tyktech.atlassian.net/rest/api/2/universal_avatar/view/type/issuetype/avatar/10316?size=medium"
/>
        Sub-task
      </td>
    </tr>
    <tr>
      <th>Status</th>
      <td>Ready for Testing</td>
    </tr>
    <tr>
      <th>Points</th>
      <td>N/A</td>
    </tr>
    <tr>
      <th>Labels</th>
      <td>-</td>
    </tr>
  </table>
</details>
<!--
  do not remove this marker as it will break jira-lint's functionality.
  added_by_jira_lint
-->

---

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

## Description

Make upstream oauth flow client secret omitempty to not break when an
API is created without `clientSecret` and saved later.

## Related Issue
Parent: https://tyktech.atlassian.net/browse/TT-13535
Subtask: https://tyktech.atlassian.net/browse/TT-13566

## Motivation and Context

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

## How This Has Been Tested

<!-- Please describe in detail how you tested your changes -->
<!-- Include details of your testing environment, and the tests -->
<!-- you ran to see how your change affects other areas of the code,
etc. -->
<!-- This information is helpful for reviewers and QA. -->

## 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**
enhancement


___

### **Description**
- Updated the `ClientAuthData` struct in `apidef/api_definitions.go` to
make the `ClientSecret` field optional by adding the `omitempty` tag.
- This change prevents errors when an API is created without a
`clientSecret` and saved later.



___



### **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>api_definitions.go</strong><dd><code>Make
`ClientSecret` optional in OAuth client auth data</code>&nbsp; &nbsp;
&nbsp; </dd></summary>
<hr>

apidef/api_definitions.go

<li>Made <code>ClientSecret</code> field optional by adding
<code>omitempty</code> tag.<br> <li> Updated JSON and BSON tags for
<code>ClientSecret</code> to reflect optional <br>status.<br>


</details>


  </td>
<td><a

href="https://github.com/TykTechnologies/tyk/pull/6708/files#diff-9961ccc89a48d32db5b47ba3006315ef52f6e5007fb4b09f8c5d6d299c669d67">+1/-1</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


___

### **PR Type**
Enhancement, Tests


___

### **Description**
- Made the `ClientSecret` field optional in the `ClientAuthData` struct
by adding the `omitempty` tag in `apidef/api_definitions.go` and
`apidef/oas/upstream.go`.
- Updated the schema in `apidef/schema.go` to remove `client_secret`
from the list of required fields.
- Enhanced test coverage in `apidef/api_definitions_test.go` by
configuring `UpstreamAuth` and including `ClientSecret` in test setup to
prevent schema errors.



___



### **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>api_definitions.go</strong><dd><code>Make
`ClientSecret` optional in OAuth client auth data</code>&nbsp; &nbsp;
&nbsp; </dd></summary>
<hr>

apidef/api_definitions.go

<li>Made <code>ClientSecret</code> field optional by adding
<code>omitempty</code> tag.<br> <li> Updated JSON and BSON tags for
<code>ClientSecret</code> to reflect optional <br>status.<br>


</details>


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

</tr>

<tr>
  <td>
    <details>
<summary><strong>upstream.go</strong><dd><code>Make `ClientSecret`
optional in OAuth client auth data</code>&nbsp; &nbsp; &nbsp;
</dd></summary>
<hr>

apidef/oas/upstream.go

<li>Made <code>ClientSecret</code> field optional by adding
<code>omitempty</code> tag.<br> <li> Updated JSON and BSON tags for
<code>ClientSecret</code> to reflect optional <br>status.<br>


</details>


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

</tr>

<tr>
  <td>
    <details>
<summary><strong>schema.go</strong><dd><code>Remove `client_secret` from
required schema fields</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; </dd></summary>
<hr>

apidef/schema.go

- Removed `client_secret` from required fields in schema.



</details>


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

</tr>
</table></td></tr><tr><td><strong>Tests</strong></td><td><table>
<tr>
  <td>
    <details>
<summary><strong>api_definitions_test.go</strong><dd><code>Add test
setup for optional `ClientSecret` in schema</code>&nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; </dd></summary>
<hr>

apidef/api_definitions_test.go

<li>Added <code>UpstreamAuth</code> configuration to test schema.<br>
<li> Included <code>ClientSecret</code> in test setup to avoid schema
errors.<br>


</details>


  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6710/files#diff-6af57c2148f42dce2ee2b93b77d65412024a802ddbd26b63f1d8bd339f4ef760">+22/-0</a>&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: Jeffy Mathew <[email protected]>
  • Loading branch information
buger and jeffy-mathew authored Nov 18, 2024
1 parent 4945049 commit c37dc2e
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 3 deletions.
2 changes: 1 addition & 1 deletion apidef/api_definitions.go
Original file line number Diff line number Diff line change
Expand Up @@ -849,7 +849,7 @@ type ClientAuthData struct {
// ClientID is the application's ID.
ClientID string `bson:"client_id" json:"client_id"`
// ClientSecret is the application's secret.
ClientSecret string `bson:"client_secret" json:"client_secret"`
ClientSecret string `bson:"client_secret,omitempty" json:"client_secret,omitempty"` // client secret is optional for password flow
}

// ClientCredentials holds the client credentials for upstream OAuth2 authentication.
Expand Down
22 changes: 22 additions & 0 deletions apidef/api_definitions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,17 @@ func TestSchema(t *testing.T) {
schemaLoader := schema.NewBytesLoader([]byte(Schema))

spec := DummyAPI()
spec.UpstreamAuth = UpstreamAuth{
Enabled: true,
OAuth: UpstreamOAuth{
Enabled: true,
ClientCredentials: ClientCredentials{
ClientAuthData: ClientAuthData{
ClientSecret: "dummy", // workaround to fix schema error
},
},
},
}
goLoader := schema.NewGoLoader(spec)
result, err := schema.Validate(schemaLoader, goLoader)
if err != nil {
Expand Down Expand Up @@ -100,6 +111,17 @@ func TestSchemaGraphqlConfig(t *testing.T) {
schemaLoader := schema.NewBytesLoader([]byte(Schema))

spec := DummyAPI()
spec.UpstreamAuth = UpstreamAuth{
Enabled: true,
OAuth: UpstreamOAuth{
Enabled: true,
ClientCredentials: ClientCredentials{
ClientAuthData: ClientAuthData{
ClientSecret: "dummy", // workaround to fix schema error
},
},
},
}
spec.GraphQL.ExecutionMode = ""

goLoader := schema.NewGoLoader(spec)
Expand Down
2 changes: 1 addition & 1 deletion apidef/oas/upstream.go
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,7 @@ type ClientAuthData struct {
// ClientID is the application's ID.
ClientID string `bson:"clientId" json:"clientId"`
// ClientSecret is the application's secret.
ClientSecret string `bson:"clientSecret" json:"clientSecret"`
ClientSecret string `bson:"clientSecret,omitempty" json:"clientSecret,omitempty"` // client secret is optional for password flow
}

// ClientCredentials holds the configuration for OAuth2 Client Credentials flow.
Expand Down
1 change: 0 additions & 1 deletion apidef/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -1107,7 +1107,6 @@ const Schema = `{
},
"required": [
"client_id",
"client_secret",
"token_url",
"username",
"password"
Expand Down

0 comments on commit c37dc2e

Please sign in to comment.