Skip to content

Commit 36b0e5b

Browse files
imogenkraakimogenkraak
authored
Merge branch 'master' into TT-15354-improve-jwt-logging
2 parents 8d0f155 + adc2fa5 commit 36b0e5b

6 files changed

Lines changed: 877 additions & 39 deletions

File tree

‎apidef/oas/servers_regeneration.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,29 @@ func (s *OAS) RegenerateServers(
9494
return nil
9595
}
9696

97+
// GenerateTykServers generates and returns only Tyk-managed server URLs for an API.
98+
// This is a convenience method that generates servers without modifying the OAS spec.
99+
// Unlike RegenerateServers, this does not include user-defined servers and does not
100+
// modify the OAS servers array.
101+
func (s *OAS) GenerateTykServers(
102+
apiData *apidef.APIDefinition,
103+
baseAPI *apidef.APIDefinition,
104+
config ServerRegenerationConfig,
105+
versionName string,
106+
) []*openapi3.Server {
107+
serverInfos := generateTykServers(apiData, baseAPI, config, versionName)
108+
109+
servers := make([]*openapi3.Server, len(serverInfos))
110+
for i, info := range serverInfos {
111+
servers[i] = &openapi3.Server{
112+
URL: info.url,
113+
Description: info.description,
114+
}
115+
}
116+
117+
return servers
118+
}
119+
97120
// generateTykServers generates all Tyk-managed server URLs for an API.
98121
func generateTykServers(
99122
apiData *apidef.APIDefinition,

‎apidef/oas/servers_regeneration_test.go‎

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -757,6 +757,167 @@ func TestGenerateTykServersBaseAPIWithVersioning(t *testing.T) {
757757
})
758758
}
759759

760+
func TestOAS_GenerateTykServers(t *testing.T) {
761+
t.Parallel()
762+
763+
t.Run("standard non-versioned API", func(t *testing.T) {
764+
t.Parallel()
765+
766+
oas := &OAS{}
767+
apiDef := &apidef.APIDefinition{
768+
APIID: "test-api",
769+
Proxy: apidef.ProxyConfig{
770+
ListenPath: "/test",
771+
},
772+
}
773+
774+
config := ServerRegenerationConfig{
775+
Protocol: "http://",
776+
DefaultHost: "localhost:8080",
777+
}
778+
779+
servers := oas.GenerateTykServers(apiDef, nil, config, "")
780+
781+
require.Len(t, servers, 1)
782+
assert.Equal(t, "http://localhost:8080/test", servers[0].URL)
783+
})
784+
785+
t.Run("versioned child API with URL path versioning", func(t *testing.T) {
786+
t.Parallel()
787+
788+
oas := &OAS{}
789+
baseAPI := &apidef.APIDefinition{
790+
APIID: "base-id",
791+
Proxy: apidef.ProxyConfig{
792+
ListenPath: "/products",
793+
},
794+
VersionDefinition: apidef.VersionDefinition{
795+
Enabled: true,
796+
Location: "url",
797+
Key: "version",
798+
Versions: map[string]string{
799+
"v1": "base-id",
800+
"v2": "child-id",
801+
},
802+
},
803+
}
804+
805+
childAPI := &apidef.APIDefinition{
806+
APIID: "child-id",
807+
Proxy: apidef.ProxyConfig{
808+
ListenPath: "/products-v2",
809+
},
810+
VersionData: apidef.VersionData{
811+
NotVersioned: false,
812+
Versions: map[string]apidef.VersionInfo{
813+
"v2": {},
814+
},
815+
},
816+
}
817+
818+
config := ServerRegenerationConfig{
819+
Protocol: "https://",
820+
DefaultHost: "api.example.com",
821+
}
822+
823+
servers := oas.GenerateTykServers(childAPI, baseAPI, config, "v2")
824+
825+
// Should have versioned URL
826+
require.NotEmpty(t, servers)
827+
found := false
828+
for _, server := range servers {
829+
if server.URL == "https://api.example.com/products/v2" {
830+
found = true
831+
break
832+
}
833+
}
834+
assert.True(t, found, "Expected to find versioned URL https://api.example.com/products/v2")
835+
})
836+
837+
t.Run("API with custom domain", func(t *testing.T) {
838+
t.Parallel()
839+
840+
oas := &OAS{}
841+
apiDef := &apidef.APIDefinition{
842+
APIID: "test-api",
843+
Domain: "custom.example.com",
844+
Proxy: apidef.ProxyConfig{
845+
ListenPath: "/api",
846+
},
847+
}
848+
849+
config := ServerRegenerationConfig{
850+
Protocol: "https://",
851+
DefaultHost: "localhost:8080",
852+
}
853+
854+
servers := oas.GenerateTykServers(apiDef, nil, config, "")
855+
856+
require.Len(t, servers, 1)
857+
assert.Equal(t, "https://custom.example.com/api", servers[0].URL)
858+
})
859+
860+
t.Run("base API with versioning and fallback", func(t *testing.T) {
861+
t.Parallel()
862+
863+
oas := &OAS{}
864+
baseAPI := &apidef.APIDefinition{
865+
APIID: "base-id",
866+
Proxy: apidef.ProxyConfig{
867+
ListenPath: "/products",
868+
},
869+
VersionDefinition: apidef.VersionDefinition{
870+
Enabled: true,
871+
Name: "v1",
872+
Location: "url",
873+
Default: "v1",
874+
FallbackToDefault: true,
875+
Versions: map[string]string{
876+
"v1": "base-id",
877+
},
878+
},
879+
}
880+
881+
config := ServerRegenerationConfig{
882+
Protocol: "http://",
883+
DefaultHost: "localhost:8080",
884+
}
885+
886+
servers := oas.GenerateTykServers(baseAPI, nil, config, "")
887+
888+
// Should have both versioned URL and fallback URL
889+
require.Len(t, servers, 2)
890+
urls := []string{servers[0].URL, servers[1].URL}
891+
assert.Contains(t, urls, "http://localhost:8080/products/v1")
892+
assert.Contains(t, urls, "http://localhost:8080/products")
893+
})
894+
895+
t.Run("returns openapi3.Server type not serverInfo", func(t *testing.T) {
896+
t.Parallel()
897+
898+
oas := &OAS{}
899+
apiDef := &apidef.APIDefinition{
900+
APIID: "test-api",
901+
Proxy: apidef.ProxyConfig{
902+
ListenPath: "/test",
903+
},
904+
}
905+
906+
config := ServerRegenerationConfig{
907+
Protocol: "http://",
908+
DefaultHost: "localhost:8080",
909+
}
910+
911+
servers := oas.GenerateTykServers(apiDef, nil, config, "")
912+
913+
// Verify it returns the correct type
914+
require.NotNil(t, servers)
915+
require.IsType(t, []*openapi3.Server{}, servers)
916+
require.Len(t, servers, 1)
917+
require.IsType(t, &openapi3.Server{}, servers[0])
918+
})
919+
}
920+
760921
func TestShouldUpdateChildAPIs(t *testing.T) {
761922
t.Parallel()
762923

‎gateway/mw_jwt.go‎

Lines changed: 59 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -735,36 +735,38 @@ func (k *JWTMiddleware) processCentralisedJWT(r *http.Request, token *jwt.Token)
735735
// We need a base policy as a template, either get it from the token itself OR a proxy client ID within Tyk
736736
basePolicyID, foundPolicy = k.getBasePolicyID(r, claims)
737737
if !foundPolicy {
738-
if len(k.Spec.JWTDefaultPolicies) == 0 {
739-
k.reportLoginFailure(baseFieldData, r)
740-
return errors.New("key not authorized: no matching policy found"), http.StatusForbidden
741-
} else {
738+
// Only use default policies if configured - scope mapping may provide policies later
739+
if len(k.Spec.JWTDefaultPolicies) > 0 {
742740
isDefaultPol = true
743741
basePolicyID = k.Spec.JWTDefaultPolicies[0]
744742
}
745743
}
746744

747-
session, err = k.Gw.generateSessionFromPolicy(basePolicyID,
748-
k.Spec.OrgID,
749-
true)
745+
// Only generate from policy if we have a base policy ID
746+
if basePolicyID != "" {
747+
session, err = k.Gw.generateSessionFromPolicy(basePolicyID,
748+
k.Spec.OrgID,
749+
true)
750750

751-
// If base policy is one of the defaults, apply other ones as well
752-
if isDefaultPol {
753-
for _, pol := range k.Spec.JWTDefaultPolicies {
754-
if !contains(session.ApplyPolicies, pol) {
755-
session.ApplyPolicies = append(session.ApplyPolicies, pol)
751+
if isDefaultPol {
752+
for _, pol := range k.Spec.JWTDefaultPolicies {
753+
if !contains(session.ApplyPolicies, pol) {
754+
session.ApplyPolicies = append(session.ApplyPolicies, pol)
755+
}
756756
}
757757
}
758-
}
759758

760-
if err := k.ApplyPolicies(&session); err != nil {
761-
return errors.New("failed to create key: " + err.Error()), http.StatusInternalServerError
762-
}
759+
if err := k.ApplyPolicies(&session); err != nil {
760+
return errors.New("failed to create key: " + err.Error()), http.StatusInternalServerError
761+
}
763762

764-
if err != nil {
765-
k.reportLoginFailure(baseFieldData, r)
766-
k.Logger().Error("Could not find a valid policy to apply to this token!")
767-
return errors.New("key not authorized: no matching policy"), http.StatusForbidden
763+
if err != nil {
764+
k.reportLoginFailure(baseFieldData, r)
765+
k.Logger().Error("Could not find a valid policy to apply to this token!")
766+
return errors.New("key not authorized: no matching policy"), http.StatusForbidden
767+
}
768+
} else {
769+
session = user.SessionState{OrgID: k.Spec.OrgID}
768770
}
769771

770772
//override session expiry with JWT if longer lived
@@ -784,22 +786,23 @@ func (k *JWTMiddleware) processCentralisedJWT(r *http.Request, token *jwt.Token)
784786
// extract policy ID from JWT token
785787
basePolicyID, foundPolicy = k.getBasePolicyID(r, claims)
786788
if !foundPolicy {
787-
if len(k.Spec.JWTDefaultPolicies) == 0 {
788-
k.reportLoginFailure(baseFieldData, r)
789-
return errors.New("key not authorized: no matching policy found"), http.StatusForbidden
790-
} else {
789+
if len(k.Spec.JWTDefaultPolicies) > 0 {
791790
isDefaultPol = true
792791
basePolicyID = k.Spec.JWTDefaultPolicies[0]
793792
}
794793
}
795-
// check if we received a valid policy ID in claim
796-
k.Gw.policiesMu.RLock()
797-
policy, ok := k.Gw.policiesByID[basePolicyID]
798-
k.Gw.policiesMu.RUnlock()
799-
if !ok {
800-
k.reportLoginFailure(baseFieldData, r)
801-
k.Logger().Error("Policy ID found is invalid!")
802-
return errors.New("key not authorized: no matching policy"), http.StatusForbidden
794+
// check if we received a valid policy ID in claim (skip if no base policy for scope-only auth)
795+
var policy user.Policy
796+
var ok bool
797+
if basePolicyID != "" {
798+
k.Gw.policiesMu.RLock()
799+
policy, ok = k.Gw.policiesByID[basePolicyID]
800+
k.Gw.policiesMu.RUnlock()
801+
if !ok {
802+
k.reportLoginFailure(baseFieldData, r)
803+
k.Logger().Error("Policy ID found is invalid!")
804+
return errors.New("key not authorized: no matching policy"), http.StatusForbidden
805+
}
803806
}
804807
// check if token for this session was switched to another valid policy
805808
pols := session.PolicyIDs()
@@ -827,7 +830,7 @@ func (k *JWTMiddleware) processCentralisedJWT(r *http.Request, token *jwt.Token)
827830
}
828831
}
829832

830-
if !contains(pols, basePolicyID) || defaultPolicyListChanged {
833+
if basePolicyID != "" && (!contains(pols, basePolicyID) || defaultPolicyListChanged) {
831834
if policy.OrgID != k.Spec.OrgID {
832835
k.reportLoginFailure(baseFieldData, r)
833836
k.Logger().Error("Policy ID found is invalid (wrong ownership)!")
@@ -872,11 +875,13 @@ func (k *JWTMiddleware) processCentralisedJWT(r *http.Request, token *jwt.Token)
872875
}
873876

874877
if scope := getScopeFromClaim(claims, scopeClaimName); len(scope) > 0 {
875-
polIDs := []string{
876-
basePolicyID, // add base policy as a first one
878+
// Start with base policy if it exists
879+
polIDs := []string{}
880+
if basePolicyID != "" {
881+
polIDs = []string{basePolicyID}
877882
}
878883

879-
// // If specified, scopes should not use default policy
884+
// If specified, scopes should not use default policy
880885
if isDefaultPol {
881886
polIDs = []string{}
882887
}
@@ -910,6 +915,24 @@ func (k *JWTMiddleware) processCentralisedJWT(r *http.Request, token *jwt.Token)
910915
return errors.New("key not authorized: could not apply several policies"), http.StatusForbidden
911916
}
912917

918+
} else if basePolicyID == "" && exists {
919+
// Security: existing session with no scope in token and no base policy
920+
// Reject to prevent privilege escalation (token should reset policies)
921+
k.reportLoginFailure(baseFieldData, r)
922+
k.Logger().Error("Existing session requires scope or base policy when scope mapping is configured")
923+
return errors.New("key not authorized: no scope or policy in token"), http.StatusForbidden
924+
}
925+
}
926+
927+
if basePolicyID == "" && len(k.Spec.JWTDefaultPolicies) == 0 {
928+
if len(session.PolicyIDs()) == 0 {
929+
k.reportLoginFailure(baseFieldData, r)
930+
k.Logger().Error("No policies could be determined from token (no base policy, no valid scopes)")
931+
return errors.New("key not authorized: no matching policy found"), http.StatusForbidden
932+
} else if exists && len(k.Spec.GetScopeToPolicyMapping()) == 0 {
933+
k.reportLoginFailure(baseFieldData, r)
934+
k.Logger().Error("Existing session requires policy in token when no defaults configured")
935+
return errors.New("key not authorized: no matching policy found"), http.StatusForbidden
913936
}
914937
}
915938

0 commit comments

Comments
 (0)