Skip to content

Commit 53d3a57

Browse files
MaciekMisMaciekMis
authored
[TT-11975] Lack of clear error message during failed import when API definition is not valid (#7077)
### **User description** <details open> <summary><a href="https://tyktech.atlassian.net/browse/TT-11975" title="TT-11975" target="_blank">TT-11975</a></summary> <br /> <table> <tr> <th>Summary</th> <td>Lack of clear error message during failed import when API definition is not valid</td> </tr> <tr> <th>Type</th> <td> <img alt="Bug" src="https://tyktech.atlassian.net/rest/api/2/universal_avatar/view/type/issuetype/avatar/10303?size=medium" /> Bug </td> </tr> <tr> <th>Status</th> <td>In Dev</td> </tr> <tr> <th>Points</th> <td>N/A</td> </tr> <tr> <th>Labels</th> <td><a href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20codilime_refined%20ORDER%20BY%20created%20DESC" title="codilime_refined">codilime_refined</a>, <a href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20customer_bug%20ORDER%20BY%20created%20DESC" title="customer_bug">customer_bug</a>, <a href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20jira_escalated%20ORDER%20BY%20created%20DESC" title="jira_escalated">jira_escalated</a></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 The system fails to properly validate and provide clear feedback when there's a mismatch between security schemes and security requirements in an OpenAPI description. Instead of returning a helpful validation error, the application crashes with a nil pointer dereference in the security extraction process, resulting in a generic 500 error. ## 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 <!-- 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: --> - [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 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 ___ ### **Description** - Added explicit validation for OAS security requirements. - Improved error messages for missing security schemes. - Prevented nil pointer dereference during OAS import. - Introduced comprehensive unit tests for security validation. ___ ### **Changes walkthrough** 📝 <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Bug fix</strong></td><td><table> <tr> <td> <details> <summary><strong>oas.go</strong><dd><code>Add OAS security requirements validation and error handling</code></dd></summary> <hr> apidef/oas/oas.go <li>Added <code>Validate</code> method to OAS for enhanced validation.<br> <li> Implemented <code>validateSecurity</code> to check security requirements.<br> <li> Improved error messages for missing security schemes.<br> <li> Prevented application crash on invalid security configuration. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7077/files#diff-80279b1d59499a41a77ff7a16a6e2c9b9b785a4fd1326c351da6884c867658d7">+35/-0</a>&nbsp; &nbsp; </td> </tr> </table></td></tr><tr><td><strong>Tests</strong></td><td><table> <tr> <td> <details> <summary><strong>oas_test.go</strong><dd><code>Add tests for OAS security validation and error reporting</code></dd></summary> <hr> apidef/oas/oas_test.go <li>Added unit tests for OAS security validation logic.<br> <li> Covered scenarios for missing, incomplete, and valid security schemes.<br> <li> Ensured error messages are tested for clarity and accuracy. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7077/files#diff-74029ee88132d30d6478c96a35f8bb2200e0c8e6f42f2c9b147dc6bb7ce74644">+164/-0</a>&nbsp; </td> </tr> </table></td></tr></tr></tbody></table> ___ > <details> <summary> Need help?</summary><li>Type <code>/help how to ...</code> in the comments thread for any questions about PR-Agent usage.</li><li>Check out the <a href="https://qodo-merge-docs.qodo.ai/usage-guide/">documentation</a> for more information.</li></details>
1 parent 6495337 commit 53d3a57

File tree

2 files changed

+199
-0
lines changed

2 files changed

+199
-0
lines changed

‎apidef/oas/oas.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package oas
33
import (
44
"context"
55
"encoding/json"
6+
"errors"
67
"fmt"
78
"strings"
89

@@ -439,6 +440,40 @@ func (s *OAS) ReplaceServers(apiURLs, oldAPIURLs []string) {
439440
s.Servers = append(newServers, userAddedServers...)
440441
}
441442

443+
// Validate validates OAS document by calling openapi3.T.Validate() function. In addition, it validates Security
444+
// Requirement section and it's requirements by calling OAS.validateSecurity() function.
445+
func (s *OAS) Validate(ctx context.Context, opts ...openapi3.ValidationOption) error {
446+
validationErr := s.T.Validate(ctx, opts...)
447+
securityErr := s.validateSecurity()
448+
449+
return errors.Join(validationErr, securityErr)
450+
}
451+
452+
// validateSecurity verifies that existing Security Requirement Objects has Security Schemes declared in the Security
453+
// Schemes under the Components Object. This function closes gap in validation provided by OAS.Validate func.
454+
func (s *OAS) validateSecurity() error {
455+
if len(s.Security) == 0 {
456+
return nil
457+
}
458+
459+
if s.Components == nil || s.Components.SecuritySchemes == nil || len(s.Components.SecuritySchemes) == 0 {
460+
return errors.New("No components or security schemes present in OAS")
461+
}
462+
463+
for _, requirement := range s.Security {
464+
for key := range requirement {
465+
if _, ok := s.Components.SecuritySchemes[key]; !ok {
466+
errorMsg := fmt.Sprintf("Missing required Security Scheme '%s' in Components.SecuritySchemes. "+
467+
"For more information please visit https://swagger.io/specification/#security-requirement-object",
468+
key)
469+
return errors.New(errorMsg)
470+
}
471+
}
472+
}
473+
474+
return nil
475+
}
476+
442477
// APIDef holds both OAS and Classic forms of an API definition.
443478
type APIDef struct {
444479
// OAS contains the OAS API definition.

‎apidef/oas/oas_test.go

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package oas
33
import (
44
"context"
55
"encoding/json"
6+
"fmt"
67
"net/http"
78
"testing"
89

@@ -1294,6 +1295,169 @@ func TestAPIContext_getValidationOptionsFromConfig(t *testing.T) {
12941295
})
12951296
}
12961297

1298+
func TestOAS_ValidateSecurity(t *testing.T) {
1299+
apiKey := "api_key"
1300+
oauth2 := "oauth2"
1301+
1302+
createSecurityReq := func(oas *OAS, key string, value []string) {
1303+
securityRequirement := openapi3.NewSecurityRequirement()
1304+
securityRequirement[key] = value
1305+
oas.Security = append(oas.Security, securityRequirement)
1306+
}
1307+
1308+
addSingleSecurityReq := func(oas *OAS) {
1309+
createSecurityReq(oas, apiKey, []string{})
1310+
}
1311+
1312+
addMultipleSecurityReq := func(oas *OAS) {
1313+
addSingleSecurityReq(oas)
1314+
createSecurityReq(oas, oauth2, []string{"read", "write"})
1315+
}
1316+
1317+
expectedErrorForNoComponentOrSchemes := "No components or security schemes present in OAS"
1318+
expectedErrorForMissingSchema := func(s string) string {
1319+
return fmt.Sprintf("Missing required Security Scheme '%s' in Components.SecuritySchemes", s)
1320+
}
1321+
1322+
tests := []struct {
1323+
name string
1324+
setupOAS func(oas *OAS)
1325+
expectedError string
1326+
}{
1327+
{
1328+
name: "no security requirements",
1329+
setupOAS: func(oas *OAS) {
1330+
oas.Security = openapi3.SecurityRequirements{}
1331+
},
1332+
expectedError: "",
1333+
},
1334+
{
1335+
name: "security requirements with matching security schemes",
1336+
setupOAS: func(oas *OAS) {
1337+
addSingleSecurityReq(oas)
1338+
1339+
if oas.Components == nil {
1340+
oas.Components = &openapi3.Components{}
1341+
}
1342+
if oas.Components.SecuritySchemes == nil {
1343+
oas.Components.SecuritySchemes = make(openapi3.SecuritySchemes)
1344+
}
1345+
oas.Components.SecuritySchemes[apiKey] = &openapi3.SecuritySchemeRef{
1346+
Value: openapi3.NewJWTSecurityScheme(),
1347+
}
1348+
},
1349+
expectedError: "",
1350+
},
1351+
{
1352+
name: "security requirements but no Components",
1353+
setupOAS: func(oas *OAS) {
1354+
addSingleSecurityReq(oas)
1355+
1356+
oas.Components = nil
1357+
},
1358+
expectedError: expectedErrorForNoComponentOrSchemes,
1359+
},
1360+
{
1361+
name: "security requirements but no SecuritySchemes",
1362+
setupOAS: func(oas *OAS) {
1363+
addSingleSecurityReq(oas)
1364+
1365+
// Add Components but not SecuritySchemes
1366+
oas.Components = &openapi3.Components{}
1367+
oas.Components.SecuritySchemes = nil
1368+
},
1369+
expectedError: expectedErrorForNoComponentOrSchemes,
1370+
},
1371+
{
1372+
name: "security requirements but empty SecuritySchemes",
1373+
setupOAS: func(oas *OAS) {
1374+
addSingleSecurityReq(oas)
1375+
1376+
oas.Components = &openapi3.Components{}
1377+
oas.Components.SecuritySchemes = make(openapi3.SecuritySchemes)
1378+
},
1379+
expectedError: expectedErrorForNoComponentOrSchemes,
1380+
},
1381+
{
1382+
name: "security requirements with missing security scheme for this requirement",
1383+
setupOAS: func(oas *OAS) {
1384+
addSingleSecurityReq(oas)
1385+
1386+
if oas.Components == nil {
1387+
oas.Components = &openapi3.Components{}
1388+
}
1389+
if oas.Components.SecuritySchemes == nil {
1390+
oas.Components.SecuritySchemes = make(openapi3.SecuritySchemes)
1391+
}
1392+
oas.Components.SecuritySchemes["other_key"] = &openapi3.SecuritySchemeRef{
1393+
Value: openapi3.NewJWTSecurityScheme(),
1394+
}
1395+
},
1396+
expectedError: expectedErrorForMissingSchema(apiKey),
1397+
},
1398+
{
1399+
name: "multiple security requirements with all matching security schemes",
1400+
setupOAS: func(oas *OAS) {
1401+
addMultipleSecurityReq(oas)
1402+
1403+
if oas.Components == nil {
1404+
oas.Components = &openapi3.Components{}
1405+
}
1406+
if oas.Components.SecuritySchemes == nil {
1407+
oas.Components.SecuritySchemes = make(openapi3.SecuritySchemes)
1408+
}
1409+
oas.Components.SecuritySchemes[apiKey] = &openapi3.SecuritySchemeRef{
1410+
Value: openapi3.NewJWTSecurityScheme(),
1411+
}
1412+
oas.Components.SecuritySchemes[oauth2] = &openapi3.SecuritySchemeRef{
1413+
Value: openapi3.NewJWTSecurityScheme(),
1414+
}
1415+
},
1416+
expectedError: "",
1417+
},
1418+
{
1419+
name: "multiple security requirements with one missing security scheme",
1420+
setupOAS: func(oas *OAS) {
1421+
addMultipleSecurityReq(oas)
1422+
1423+
if oas.Components == nil {
1424+
oas.Components = &openapi3.Components{}
1425+
}
1426+
if oas.Components.SecuritySchemes == nil {
1427+
oas.Components.SecuritySchemes = make(openapi3.SecuritySchemes)
1428+
}
1429+
oas.Components.SecuritySchemes[apiKey] = &openapi3.SecuritySchemeRef{}
1430+
},
1431+
expectedError: expectedErrorForMissingSchema(oauth2),
1432+
},
1433+
}
1434+
1435+
for _, tt := range tests {
1436+
t.Run(tt.name, func(t *testing.T) {
1437+
oas := &OAS{
1438+
T: openapi3.T{
1439+
OpenAPI: "3.0.3",
1440+
Info: &openapi3.Info{
1441+
Title: "Test API",
1442+
Version: "1.0.0",
1443+
},
1444+
Paths: openapi3.Paths{},
1445+
},
1446+
}
1447+
1448+
tt.setupOAS(oas)
1449+
1450+
err := oas.Validate(context.Background())
1451+
if tt.expectedError == "" {
1452+
assert.NoError(t, err)
1453+
} else {
1454+
assert.Error(t, err)
1455+
assert.Contains(t, err.Error(), tt.expectedError)
1456+
}
1457+
})
1458+
}
1459+
}
1460+
12971461
func TestYaml(t *testing.T) {
12981462
oasDoc := OAS{}
12991463
Fill(t, &oasDoc, 0)

0 commit comments

Comments
 (0)