fix authentication cr

wso2 · Sep 7, 2023 · ef04ea3 · ef04ea3
1 parent 94daffa
commit ef04ea3
Show file tree

Hide file tree

Showing 8 changed files with 69 additions and 29 deletions.
diff --git a/adapter/internal/oasparser/model/http_route.go b/adapter/internal/oasparser/model/http_route.go
@@ -564,34 +564,35 @@ func concatAuthSchemes(schemeUp *dpv1alpha1.Authentication, schemeDown *dpv1alph
 // make sure authscheme only has external service override values. (i.e. empty default values)
 // tip: use concatScheme method
 func getSecurity(authScheme *dpv1alpha1.Authentication) *Authentication {
+	resolvedAuthScheme := concatAuthSchemes(nil, authScheme)
 	authHeader := constants.AuthorizationHeader
-	if authScheme != nil && authScheme.Spec.Override.AuthTypes != nil && len(authScheme.Spec.Override.AuthTypes.Oauth2.Header) > 0 {
-		authHeader = authScheme.Spec.Override.AuthTypes.Oauth2.Header
+	if resolvedAuthScheme != nil && resolvedAuthScheme.Spec.Override != nil && resolvedAuthScheme.Spec.Override.AuthTypes != nil && len(resolvedAuthScheme.Spec.Override.AuthTypes.Oauth2.Header) > 0 {
+		authHeader = resolvedAuthScheme.Spec.Override.AuthTypes.Oauth2.Header
 	}
 	sendTokenToUpstream := false
-	if authScheme != nil && authScheme.Spec.Override.AuthTypes != nil {
-		sendTokenToUpstream = authScheme.Spec.Override.AuthTypes.Oauth2.SendTokenToUpstream
+	if resolvedAuthScheme != nil && resolvedAuthScheme.Spec.Override != nil && resolvedAuthScheme.Spec.Override.AuthTypes != nil {
+		sendTokenToUpstream = resolvedAuthScheme.Spec.Override.AuthTypes.Oauth2.SendTokenToUpstream
 	}
 	auth := &Authentication{Disabled: false,
 		TestConsoleKey: &TestConsoleKey{Header: constants.TestConsoleKeyHeader},
 		JWT:            &JWT{Header: authHeader, SendTokenToUpstream: sendTokenToUpstream},
 	}
-	if authScheme != nil {
-		if authScheme.Spec.Override.Disabled != nil && *authScheme.Spec.Override.Disabled {
+	if resolvedAuthScheme != nil && resolvedAuthScheme.Spec.Override != nil {
+		if resolvedAuthScheme.Spec.Override.Disabled != nil && *resolvedAuthScheme.Spec.Override.Disabled {
 			return &Authentication{Disabled: true}
 		}
 		authFound := false
-		if authScheme.Spec.Override.AuthTypes != nil && authScheme.Spec.Override.AuthTypes.Oauth2.Disabled {
+		if resolvedAuthScheme.Spec.Override.AuthTypes != nil && resolvedAuthScheme.Spec.Override.AuthTypes.Oauth2.Disabled {
 			auth = &Authentication{Disabled: false,
 				TestConsoleKey: &TestConsoleKey{Header: constants.TestConsoleKeyHeader},
 			}
 		} else {
 			authFound = true
 		}
-		if authScheme.Spec.Override.AuthTypes.APIKey != nil {
-			authFound = authFound || len(authScheme.Spec.Override.AuthTypes.APIKey) > 0
+		if resolvedAuthScheme.Spec.Override.AuthTypes.APIKey != nil {
+			authFound = authFound || len(resolvedAuthScheme.Spec.Override.AuthTypes.APIKey) > 0
 			var apiKeys []APIKey
-			for _, apiKey := range authScheme.Spec.Override.AuthTypes.APIKey {
+			for _, apiKey := range resolvedAuthScheme.Spec.Override.AuthTypes.APIKey {
 				apiKeys = append(apiKeys, APIKey{
 					Name:                apiKey.Name,
 					In:                  apiKey.In,

diff --git a/runtime/config-deployer-service/ballerina/APIClient.bal b/runtime/config-deployer-service/ballerina/APIClient.bal
@@ -348,27 +348,27 @@ public class APIClient {
         string[] keys = createdEndpointMap.keys();
         log:printDebug("createdEndpointMap.keys:" + createdEndpointMap.keys().toString());
         foreach string endpointType in keys {
-            string disableAuthenticationRefName = self.retrieveDisableAuthenticationRefName(apkConf, endpointType, organization);
-            log:printDebug("disableAuthenticationRefName:" + disableAuthenticationRefName);
+            string authenticationRefName = self.retrieveAuthenticationRefName(apkConf, endpointType, organization);
+            log:printDebug("authenticationRefName:" + authenticationRefName);
             model:Authentication authentication = {
                 metadata: {
-                    name: disableAuthenticationRefName,
+                    name: authenticationRefName,
                     labels: self.getLabels(apkConf, organization)
                 },
                 spec: {
-                    override: {
+                    default: {
                         disabled: false,
                         authTypes: authTypes
                     },
                     targetRef: {
                         group: "gateway.networking.k8s.io",
-                        kind: "Resource",
+                        kind: "API",
                         name: apiArtifact.uniqueId
                     }
                 }
             };
             log:printDebug("Authentication CR:" + authentication.toString());
-            authenticationMap[disableAuthenticationRefName] = authentication;
+            authenticationMap[authenticationRefName] = authentication;
         }
         log:printDebug("Authentication Map:" + authenticationMap.toString());
         apiArtifact.authenticationMap = authenticationMap;
@@ -430,9 +430,12 @@ public class APIClient {
     }
 
     private isolated function retrieveDisableAuthenticationRefName(APKConf apkConf, string 'type, commons:Organization organization) returns string {
-        return self.getUniqueIdForAPI(apkConf.name, apkConf.'version, organization) + "-" + 'type + "-authentication";
+        return self.getUniqueIdForAPI(apkConf.name, apkConf.'version, organization) + "-" + 'type + "-no-authentication";
     }
 
+    private isolated function retrieveAuthenticationRefName(APKConf apkConf, string 'type, commons:Organization organization) returns string {
+        return self.getUniqueIdForAPI(apkConf.name, apkConf.'version, organization) + "-" + 'type + "-authentication";
+    }
     private isolated function setHttpRoute(model:APIArtifact apiArtifact, APKConf apkConf, model:Endpoint? endpoint, string uniqueId, string endpointType, commons:Organization organization) returns commons:APKError|error? {
         APKOperations[] apiOperations = apkConf.operations ?: [];
         APKOperations[][] operationsArray = [];
@@ -507,9 +510,6 @@ public class APIClient {
                         }
                         model:HTTPRouteFilter disableAuthenticationFilter = {'type: "ExtensionRef", extensionRef: {group: "dp.wso2.com", kind: "Authentication", name: disableAuthenticationRefName}};
                         (<model:HTTPRouteFilter[]>filters).push(disableAuthenticationFilter);
-                    } else if apiArtifact.authenticationMap.hasKey(disableAuthenticationRefName) {
-                        model:HTTPRouteFilter authenticationFilter = {'type: "ExtensionRef", extensionRef: {group: "dp.wso2.com", kind: "Authentication", name: disableAuthenticationRefName}};
-                        (<model:HTTPRouteFilter[]>filters).push(authenticationFilter);
                     }
                     string[]? scopes = operation.scopes;
                     if scopes is string[] {
@@ -604,9 +604,9 @@ public class APIClient {
                 targetRef: {
                     group: "",
                     kind: "Resource",
-                    name: self.getUniqueIdForAPI(apkConf.name, apkConf.'version, organization)
+                    name: apiArtifact.uniqueId
                 },
-                override: {
+                default: {
                     disabled: true
                 }
             }

diff --git a/runtime/config-deployer-service/ballerina/Dependencies.toml b/runtime/config-deployer-service/ballerina/Dependencies.toml
@@ -44,7 +44,7 @@ modules = [
 [[package]]
 org = "ballerina"
 name = "crypto"
-version = "2.4.0"
+version = "2.4.1"
 dependencies = [
 	{org = "ballerina", name = "jballerina.java"},
 	{org = "ballerina", name = "time"}

diff --git a/runtime/config-deployer-service/ballerina/Dependencies.toml.template b/runtime/config-deployer-service/ballerina/Dependencies.toml.template
@@ -44,7 +44,7 @@ modules = [
 [[package]]
 org = "ballerina"
 name = "crypto"
-version = "2.4.0"
+version = "2.4.1"
 dependencies = [
 	{org = "ballerina", name = "jballerina.java"},
 	{org = "ballerina", name = "time"}

diff --git a/runtime/config-deployer-service/ballerina/tests/APIClientTest.bal b/runtime/config-deployer-service/ballerina/tests/APIClientTest.bal
@@ -476,8 +476,16 @@ public function testJWTAuthenticationOnlyEnable() returns error? {
             }
         }
     };
+    model:AuthenticationData expectedNoAuthentication = {
+        disabled: true
+    };
+
     foreach model:Authentication item in apiArtifact.authenticationMap {
-        test:assertEquals(item.spec.override, expectedAuthenticationData);
+        if string:endsWith(item.metadata.name, "-no-authentication") {
+            test:assertEquals(item.spec.default, expectedNoAuthentication);
+        } else {
+            test:assertEquals(item.spec.default, expectedAuthenticationData);
+        }
     }
 }
 
@@ -508,8 +516,16 @@ public function testAPIKeyOnlyEnable() returns error? {
             ]
         }
     };
+
+    model:AuthenticationData expectedNoAuthentication = {
+        disabled: true
+    };
     foreach model:Authentication item in apiArtifact.authenticationMap {
-        test:assertEquals(item.spec.override, expectedAuthenticationData);
+        if string:endsWith(item.metadata.name, "-no-authentication") {
+            test:assertEquals(item.spec.default, expectedNoAuthentication);
+        } else {
+            test:assertEquals(item.spec.default, expectedAuthenticationData);
+        }
     }
 }
 
@@ -545,8 +561,15 @@ public function testAPIKeyAndJWTEnable() returns error? {
             }
         }
     };
+    model:AuthenticationData expectedNoAuthentication = {
+        disabled: true
+    };
     foreach model:Authentication item in apiArtifact.authenticationMap {
-        test:assertEquals(item.spec.override, expectedAuthenticationData);
+        if string:endsWith(item.metadata.name, "-no-authentication") {
+            test:assertEquals(item.spec.default, expectedNoAuthentication);
+        } else {
+            test:assertEquals(item.spec.default, expectedAuthenticationData);
+        }
     }
 }
 

diff --git a/test/cucumber-tests/src/test/java/org/wso2/apk/integration/api/BaseSteps.java b/test/cucumber-tests/src/test/java/org/wso2/apk/integration/api/BaseSteps.java
@@ -131,7 +131,7 @@ public void sendHttpRequest(String httpMethod, String url, String body) throws I
             sharedContext.setResponse(httpClient.doPut(url, sharedContext.getHeaders(), body, null));
             sharedContext.setResponseBody(SimpleHTTPClient.responseEntityBodyToString(sharedContext.getResponse()));
         } else if (CurlOption.HttpMethod.DELETE.toString().toLowerCase().equals(httpMethod.toLowerCase())) {
-            sharedContext.setResponse(httpClient.doPut(url, sharedContext.getHeaders(), body, null));
+            sharedContext.setResponse(httpClient.doDelete(url, sharedContext.getHeaders()));
             sharedContext.setResponseBody(SimpleHTTPClient.responseEntityBodyToString(sharedContext.getResponse()));
         } else if (CurlOption.HttpMethod.OPTIONS.toString().toLowerCase().equals(httpMethod.toLowerCase())) {
             sharedContext.setResponse(httpClient.doOptions(url, sharedContext.getHeaders(), null, null));

diff --git a/test/cucumber-tests/src/test/resources/artifacts/apk-confs/jwt_basic_conf.yaml b/test/cucumber-tests/src/test/resources/artifacts/apk-confs/jwt_basic_conf.yaml
@@ -23,7 +23,7 @@ operations:
     scopes: []
   - target: "/employee/{employeeId}"
     verb: "DELETE"
-    secured: true
+    secured: false
     scopes: []
 authentication:
  - authType: OAuth2

diff --git a/test/cucumber-tests/src/test/resources/tests/api/JWT.feature b/test/cucumber-tests/src/test/resources/tests/api/JWT.feature
@@ -25,7 +25,23 @@ Feature: Test JWT related functionalities
     And I eventually receive 200 response code, not accepting
       |429|
       |401|
+    Then I set headers
+      |Authorization|bearer "${idp-1-token}h"|
+    And I send "GET" request to "https://default.gw.wso2.com:9095/jwt-basic/3.14/employee/" with body ""
+    And I eventually receive 401 response code, not accepting
+      |429|
+      |200|
+    Then I set headers
+      |Authorization|bearer ${idp-1-token}|
+    And I send "GET" request to "https://default.gw.wso2.com:9095/jwt-basic/3.14/employee/" with body ""
+    And I eventually receive 401 response code, not accepting
+      |429|
+      |200|
     Then I generate JWT token from idp1 with kid "456-789"
+    And I send "DELETE" request to "https://default.gw.wso2.com:9095/jwt-basic/3.14/employee/1234" with body ""
+    And I eventually receive 200 response code, not accepting
+      |429|
+      |401|
     Then I set headers
       |Authorization|bearer ${idp-1-token}|
     And I send "GET" request to "https://default.gw.wso2.com:9095/jwt-basic/3.14/employee/" with body ""