Api specific backend jwt generation #1153

BLasan · 2023-05-12T08:48:09Z

Purpose

Define API Specific Backend JWT Token generation configs.

Issue

Add backend JWT to CRD #782

Goals

This removes the usage of enforcer.jwtGenerator property defined in the main config file and allows users to provide configurations individually for APIs rather than as a shared one.

Approach

Added a new property for the APIPolicy to define the configurations. A sample will be as follows.

apiVersion: dp.wso2.com/v1alpha1
kind: APIPolicy
metadata:
  name: jwt-token-generator-policy
  namespace: gateway-integration-test-infra
spec:
  override:
    backendJwtToken:
      enabled: true
      encoding: "base64"
      signingAlgorithm: "SHA256withRSA"
      header: "X-JWT-Assertion"
  targetRef:
    group: gateway.networking.k8s.io
    kind: API
    name:  api-policy-with-jwt-generator

Automation tests

Added an integration test to check whether the backend request is holding the JWT header defined in the configuration if token generation is allowed. Otherwise, that header should not be in the request.

codecov-commenter · 2023-05-12T08:56:26Z

Codecov Report

Patch coverage has no change and project coverage change: -1.57 ⚠️

Comparison is base (2dfda53) 34.20% compared to head (be7f3e0) 32.64%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1153      +/-   ##
==========================================
- Coverage   34.20%   32.64%   -1.57%     
==========================================
  Files         350      183     -167     
  Lines       40398    20829   -19569     
  Branches    12754     6736    -6018     
==========================================
- Hits        13818     6799    -7019     
+ Misses      26336    13905   -12431     
+ Partials      244      125     -119

Flag	Coverage Δ
adapter	`25.09% <ø> (-1.10%)`	⬇️
admin-domain-service	`57.16% <ø> (+0.78%)`	⬆️
backoffice-domain-service	`50.29% <ø> (ø)`
devportal-domain-service	`31.52% <ø> (-20.14%)`	⬇️
idp-domain-service	`80.10% <ø> (ø)`
runtime-domain-service	`32.22% <ø> (+0.15%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 212 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

AmaliMatharaarachchi · 2023-05-12T10:43:55Z

adapter/api/proto/wso2/discovery/api/BackendJWTTokenInfo.proto

@@ -0,0 +1,34 @@
+//  Copyright (c) 2021, WSO2 LLC. (http://www.wso2.org) All Rights Reserved.


Suggested change

// Copyright (c) 2021, WSO2 LLC. (http://www.wso2.org) All Rights Reserved.

// Copyright (c) 2023, WSO2 LLC. (http://www.wso2.org) All Rights Reserved.

AmaliMatharaarachchi · 2023-05-15T06:35:19Z

adapter/api/proto/wso2/discovery/config/enforcer/jwt_generator.proto

-    string private_key_path = 11;
-
-    int32 token_ttl = 12;
+    int32 token_ttl = 3;


why did we keep it in the config, can't we move it to CR?

There's no such field for ttl under the jwt generator config. https://apim.docs.wso2.com/en/latest/deploy-and-publish/deploy-on-gateway/choreo-connect/passing-enduser-attributes-to-the-backend-via-choreo-connect/#enabling-the-default-backend-jwt-generator This value is populated in the code from some other config and kept under this jwt generator config. If it's needed, we can and support for api level ttl.

shall we add it to cr?

AmaliMatharaarachchi · 2023-05-15T06:42:02Z

.../main/java/org/wso2/apk/enforcer/commons/jwtgenerator/AbstractAPIMgtGatewayJWTGenerator.java

+            //useKid becomes always false
+            return JWTUtil.generateHeader(publicCert, signatureAlgorithm, false);


Keep it as it is till we fix the usekid properly

Suggested change

//useKid becomes always false

return JWTUtil.generateHeader(publicCert, signatureAlgorithm, false);

// TODO(benura) populate useKid accordingly, currently it's always false

return JWTUtil.generateHeader(publicCert, signatureAlgorithm, jwtConfigurationDto.useKid());

AmaliMatharaarachchi · 2023-05-15T06:55:02Z

helm-charts/templates/data-plane/gateway-components/log-conf.yaml

+    enabled = false
    encoding = "base64" # base64,base64url
    claimDialect = "http://wso2.org/claims"
    header = "X-JWT-Assertion"


There's no usage of this right? shall we remove it then?

AmaliMatharaarachchi · 2023-05-15T07:24:18Z

test/integration/integration/utils/testtypes/apipolicy_types.go

@@ -0,0 +1,145 @@
+/*
+ *  Copyright (c) 2022, WSO2 LLC. (http://www.wso2.org) All Rights Reserved.


Suggested change

* Copyright (c) 2022, WSO2 LLC. (http://www.wso2.org) All Rights Reserved.

* Copyright (c) 2023, WSO2 LLC. (http://www.wso2.org) All Rights Reserved.

AmaliMatharaarachchi · 2023-05-15T07:26:42Z

test/integration/integration/utils/testtypes/apipolicy_types.go

+// NOTE: json tags are required.  Any new fields you add must have json tags for the fields to be serialized.
+
+// APIPolicySpec defines the desired state of APIPolicy
+type APIPolicySpec struct {


why do we require this file, apipolicy type in test package? can't we remove this file?

I have used this to unmarshal the policy yaml file in the test case

AmaliMatharaarachchi · 2023-05-15T07:27:23Z

test/integration/integration/utils/http/http.go

@@ -247,6 +247,9 @@ func CompareRequest(req *roundtripper.Request, cReq *roundtripper.CapturedReques
 					if !ok {
 						return fmt.Errorf("expected %s header to be set by the enforcer", name)
 					}
+					if actualVal == nil && actualVal[0] == "" {


isn't it a or condition?

Suggested change

if actualVal == nil && actualVal[0] == "" {

if actualVal == nil || actualVal[0] == "" {

AmaliMatharaarachchi · 2023-05-15T07:28:48Z

adapter/api/proto/wso2/discovery/api/BackendJWTTokenInfo.proto

+	bool enabled = 1;
+	string encoding = 2;
+	string header = 3;
+	string signingAlgorithm = 4;


Haven't we added custom claims?

Aren't we going to add these in JWT issuer level?

no, it should come here. We were talking about adding claim mappings at the issuer level, not custom claims. They are 2 different features.

BLasan force-pushed the api-specific-backend-jwt-generation branch 3 times, most recently from fb280f6 to 4474ca9 Compare May 12, 2023 11:42

AmaliMatharaarachchi reviewed May 15, 2023

View reviewed changes

BLasan force-pushed the api-specific-backend-jwt-generation branch 2 times, most recently from b12ed8b to bd0ca7b Compare May 16, 2023 19:09

Revamp: Backend JWT Token generation for APIs

2397f2d

BLasan force-pushed the api-specific-backend-jwt-generation branch from bd0ca7b to 2397f2d Compare May 17, 2023 05:19

AmaliMatharaarachchi previously approved these changes May 18, 2023

View reviewed changes

add: api policy to enable backend jwt configs

95ae48c

BLasan dismissed AmaliMatharaarachchi’s stale review via 95ae48c May 18, 2023 10:36

BLasan added 2 commits May 18, 2023 17:16

enable jwt token generator in devportal

cdced8c

enable jwt in admin & idp

be7f3e0

tharindu1st approved these changes May 22, 2023

View reviewed changes

AmaliMatharaarachchi approved these changes May 22, 2023

View reviewed changes

tharindu1st merged commit 96e1e68 into main May 22, 2023

This was referenced May 23, 2023

Doc: Backend JWT Token manipulation wso2/docs-apk#190

Merged

Add backend JWT to CRD #782

Closed

BLasan deleted the api-specific-backend-jwt-generation branch June 27, 2023 04:43

tharindu1st linked an issue Jul 2, 2023 that may be closed by this pull request

[adapter] enable backendJWT per API. #632

Closed

BLasan mentioned this pull request Jul 6, 2023

Add: Custom Claim support for CRs #1362

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Api specific backend jwt generation #1153

Api specific backend jwt generation #1153

BLasan commented May 12, 2023 •

edited

Loading

codecov-commenter commented May 12, 2023 •

edited

Loading

AmaliMatharaarachchi May 12, 2023

AmaliMatharaarachchi May 15, 2023

BLasan May 16, 2023

AmaliMatharaarachchi May 17, 2023

AmaliMatharaarachchi May 15, 2023

AmaliMatharaarachchi May 15, 2023

AmaliMatharaarachchi May 15, 2023

AmaliMatharaarachchi May 15, 2023

BLasan May 16, 2023

AmaliMatharaarachchi May 15, 2023

AmaliMatharaarachchi May 15, 2023

BLasan May 16, 2023

AmaliMatharaarachchi May 16, 2023

		@@ -0,0 +1,34 @@
		// Copyright (c) 2021, WSO2 LLC. (http://www.wso2.org) All Rights Reserved.

	// Copyright (c) 2021, WSO2 LLC. (http://www.wso2.org) All Rights Reserved.
	// Copyright (c) 2023, WSO2 LLC. (http://www.wso2.org) All Rights Reserved.

		//useKid becomes always false
		return JWTUtil.generateHeader(publicCert, signatureAlgorithm, false);

		@@ -0,0 +1,145 @@
		/*
		* Copyright (c) 2022, WSO2 LLC. (http://www.wso2.org) All Rights Reserved.

	* Copyright (c) 2022, WSO2 LLC. (http://www.wso2.org) All Rights Reserved.
	* Copyright (c) 2023, WSO2 LLC. (http://www.wso2.org) All Rights Reserved.

	if actualVal == nil && actualVal[0] == "" {
	if actualVal == nil \|\| actualVal[0] == "" {

Api specific backend jwt generation #1153

Api specific backend jwt generation #1153

Conversation

BLasan commented May 12, 2023 • edited Loading

Purpose

Issue

Goals

Approach

Automation tests

codecov-commenter commented May 12, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BLasan commented May 12, 2023 •

edited

Loading

codecov-commenter commented May 12, 2023 •

edited

Loading