KNOX-2712 - Managing custom Knox Token metadata #542

smolnar82 · 2022-03-04T09:14:23Z

What changes were proposed in this pull request?

The following enhancements were added by this change:

the simple GET API is extended to handle custom metadata information
the getUserTokens API endpoint is extended to filter tokens by metadata name/value
additional metadata information is displayed on the Knox Token Management page

How was this patch tested?

Adjusted and ran JUnit tests:

$ mvn clean -Dshellcheck=true verify -Prelease,package -am -pl gateway-service-knoxtoken
...
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  06:01 min
[INFO] Finished at: 2022-03-04T09:32:59+01:00
[INFO] ------------------------------------------------------------------------

Additionally, I tested the API changes with the following curl commands:

curl -iku admin:admin-password "https://localhost:8443/gateway/sandbox/knoxtoken/api/v1/token?md_notebookName=accountantKnoxToken&md_souldBeRemovedBy=31March2022&md_otherMeaningfuMetadata=KnoxIsCool"

curl -iku admin:admin-password "https://localhost:8443/gateway/sandbox/knoxtoken/api/v1/token?lifespan=P0DT1H0M&md_notebookName=DROP%20TABLE%20knox_token_metadata"

curl -iku admin:admin-password "https://localhost:8443/gateway/sandbox/knoxtoken/api/v1/token?lifespan=P0DT1H0M&md_notebookName=%3Cscript%3Ealert%28%27smolnar%27%29%3B%3C%2Fscript%3E"

curl -iku admin:admin-password "https://localhost:8443/gateway/sandbox/knoxtoken/api/v1/token?lifespan=P0DT1H0M&md_notebookName=<script>alert(\"smolnar\")</script>&md_otherMetadata=MyOtherMetadata"

curl -iku admin:admin-password "https://localhost:8443/gateway/sandbox/knoxtoken/api/v1/token?lifespan=P0DT1H0M&md_notebookName=%3Ca%20href%3D%22%23%22%20onClick%3D%22alert%28%27Hello%20World%21%27%29%22%3E%3Cimg%20title%3D%22The%20Link%22%20%2F%3E%3C%2Fa%3E"

curl -iku admin:admin-password "https://localhost:8443/gateway/sandbox/knoxtoken/api/v1/token?lifespan=P0DT1H0M&md_smolnarMd=dummyMd"

As you can see I tried to challenge the GET API with

SQL commands to make sure SQL injection is not an issue (in TokenStateDatabase we use PreparedStatement objects to communicate with the DB, so we are safe)
different HTML scripts to make sure XSS attacks are not an issue. The prevention of XSS comes OOTB with Angular as we only use interpolated values in curly brackets that are escaped in Angular.

I also tested the updated getUserTokens API:

$ curl -iku admin:admin-password "https://localhost:8443/gateway/sandbox/knoxtoken/api/v1/token/getUserTokens?userName=admin&md_smolnarMd"
HTTP/1.1 200 OK
...

{
	"tokens": [{
		"tokenId": "97de921a-aa54-4308-803c-6c20cebcc1f9",
		"issueTime": "2022-03-04T10:07:49.093+0100",
		"expiration": "2022-03-04T11:07:48.928+0100",
		"maxLifetime": "2022-03-11T10:07:49.093+0100",
		"metadata": {
			"customMetadataMap": {
				"smolnarMd": "dummyMd"
			},
			"comment": null,
			"enabled": true,
			"userName": "admin"
		},
		"issueTimeLong": 1646384869093,
		"expirationLong": 1646388468928,
		"maxLifetimeLong": 1646989669093
	}]
}

$ curl -iku admin:admin-password "https://localhost:8443/gateway/sandbox/knoxtoken/api/v1/token/getUserTokens?userName=admin&md_otherMetadata"
HTTP/1.1 200 OK
....

{
	"tokens": [{
		"tokenId": "e8e578ca-a782-4cd7-8fa1-3580b7a79541",
		"issueTime": "2022-03-04T10:00:31.655+0100",
		"expiration": "2022-03-04T11:00:31.645+0100",
		"maxLifetime": "2022-03-11T10:00:31.655+0100",
		"metadata": {
			"customMetadataMap": {
				"notebookName": "<script>alert(\"smolnar\")</script>",
				"otherMetadata": "MyOtherMetadata"
			},
			"comment": null,
			"enabled": true,
			"userName": "admin"
		},
		"issueTimeLong": 1646384431655,
		"expirationLong": 1646388031645,
		"maxLifetimeLong": 1646989231655
	}]
}

$ curl -iku admin:admin-password "https://localhost:8443/gateway/sandbox/knoxtoken/api/v1/token/getUserTokens?userName=admin&md_notebookName=accountantKnoxToken"
HTTP/1.1 200 OK
...
{
	"tokens": [{
		"tokenId": "1487647b-b986-4b73-a250-854843aade9b",
		"issueTime": "2022-03-04T09:58:03.155+0100",
		"expiration": "2022-07-02T10:58:02.979+0200",
		"maxLifetime": "2022-03-11T09:58:03.155+0100",
		"metadata": {
			"customMetadataMap": {
				"souldBeRemovedBy": "31March2022",
				"notebookName": "accountantKnoxToken",
				"otherMeaningfuMetadata": "KnoxIsCool"
			},
			"comment": null,
			"enabled": true,
			"userName": "admin"
		},
		"issueTimeLong": 1646384283155,
		"expirationLong": 1656752282979,
		"maxLifetimeLong": 1646989083155
	}]
}

$ curl -iku admin:admin-password "https://localhost:8443/gateway/sandbox/knoxtoken/api/v1/token/getUserTokens?userName=admin&md_notebookName=accountantKnoxToken&md_smolnarMd"
HTTP/1.1 200 OK
...
{
	"tokens": [{
		"tokenId": "8501a9a0-d14d-4780-b2b8-52becaedd6f0",
		"issueTime": "2022-03-09T10:42:34.176+0100",
		"expiration": "2022-07-07T11:42:34.153+0200",
		"maxLifetime": "2022-03-16T10:42:34.176+0100",
		"metadata": {
			"customMetadataMap": {
				"souldBeRemovedBy": "31March2022",
				"notebookName": "accountantKnoxToken",
				"otherMeaningfuMetadata": "KnoxIsCool"
			},
			"comment": null,
			"enabled": true,
			"userName": "admin"
		},
		"issueTimeLong": 1646818954176,
		"expirationLong": 1657186954153,
		"maxLifetimeLong": 1647423754176
	}, {
		"tokenId": "44a773d4-3d6a-410e-bb06-82973b6e5d56",
		"issueTime": "2022-03-09T11:52:56.885+0100",
		"expiration": "2022-03-09T12:52:56.867+0100",
		"maxLifetime": "2022-03-16T11:52:56.885+0100",
		"metadata": {
			"customMetadataMap": {
				"smolnarMd": "dummyMd"
			},
			"comment": null,
			"enabled": true,
			"userName": "admin"
		},
		"issueTimeLong": 1646823176885,
		"expirationLong": 1646826776867,
		"maxLifetimeLong": 1647427976885
	}]
}

...service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java

...xtoken/src/test/java/org/apache/knox/gateway/service/knoxtoken/TokenServiceResourceTest.java

moresandeep

May be i am misunderstanding, but using GET to add metadata seems odd.

pzampino · 2022-03-08T21:09:24Z

May be i am misunderstanding, but using GET to add metadata seems odd.

As I understand it, you're GETting a token with the specified metadata.

smolnar82 · 2022-03-09T12:33:36Z

@pzampino - all of your review comments are addressed/fixed. Could you please take another look? Thanks!

pzampino

LGTM...just a comment about a possible performance enhancement not directly related to these changes.

pzampino · 2022-03-09T16:21:13Z

...service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java

+          for (Map.Entry<String, String> entry : metadataMap.entrySet()) {
+            if (StringUtils.isBlank(entry.getValue()) || "*".equals(entry.getValue())) {
+              // we should only filter tokens by metadata name
+              if (knoxToken.getMetadata().getMetadataMap().containsKey(entry.getKey())) {


Not necessarily related to this change, but it seems to me that TokenMetadata could have a generic get(String key) method, rather than requiring making a copy of the metadataMap every time a value is needed to be looked up (knoxToken.getMetadata().getMetadataMap()).

public String get(final String key) { return metadataMap.get(key); }

Such that lookups could be more simply knoxToken.getMetadata().get(key).

Fixed. Thanks for your time to review my changes.

Change-Id: I68b16236000dafda0ae627a98426d93948dca9d4

smolnar82 requested review from zeroflag, moresandeep and pzampino March 4, 2022 09:14

smolnar82 self-assigned this Mar 4, 2022

smolnar82 added the knoxtoken label Mar 4, 2022

pzampino reviewed Mar 7, 2022

View reviewed changes

moresandeep requested changes Mar 8, 2022

View reviewed changes

smolnar82 force-pushed the KNOX-2712 branch from e352ba4 to f330490 Compare March 9, 2022 12:28

pzampino approved these changes Mar 9, 2022

View reviewed changes

KNOX-2712 - Managing custom Knox Token metadata

5903306

smolnar82 force-pushed the KNOX-2712 branch from f330490 to 5903306 Compare March 9, 2022 22:35

smolnar82 requested a review from moresandeep March 9, 2022 22:37

smolnar82 merged commit 5b4040f into apache:master Mar 10, 2022

smolnar82 deleted the KNOX-2712 branch March 10, 2022 20:27

stoty pushed a commit to stoty/knox that referenced this pull request May 14, 2024

CDPD-35799 KNOX-2712 - Managing custom Knox Token metadata (apache#542)

483bc15

Change-Id: I68b16236000dafda0ae627a98426d93948dca9d4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KNOX-2712 - Managing custom Knox Token metadata #542

KNOX-2712 - Managing custom Knox Token metadata #542

smolnar82 commented Mar 4, 2022 •

edited

Loading

moresandeep left a comment

pzampino commented Mar 8, 2022

smolnar82 commented Mar 9, 2022

pzampino left a comment

pzampino Mar 9, 2022 •

edited

Loading

smolnar82 Mar 9, 2022

KNOX-2712 - Managing custom Knox Token metadata #542

KNOX-2712 - Managing custom Knox Token metadata #542

Conversation

smolnar82 commented Mar 4, 2022 • edited Loading

What changes were proposed in this pull request?

How was this patch tested?

moresandeep left a comment

Choose a reason for hiding this comment

pzampino commented Mar 8, 2022

smolnar82 commented Mar 9, 2022

pzampino left a comment

Choose a reason for hiding this comment

pzampino Mar 9, 2022 • edited Loading

Choose a reason for hiding this comment

smolnar82 Mar 9, 2022

Choose a reason for hiding this comment

smolnar82 commented Mar 4, 2022 •

edited

Loading

pzampino Mar 9, 2022 •

edited

Loading