jwt blacklist stop play/publish using jwt #4847

lastpeony · 2023-02-28T12:45:20Z

This pull request introduces a JWT blacklist implementation, which allows users to ban viewers who are already watching or publishing streams. Since JWTs are not meant to be stored in a database, this feature is not enabled by default. However, a new setting called "jwtBlacklistEnabled" has been added to app settings. Otherwise, each request with a JWT would trigger a database lookup on disk, leading to degraded performance. This feature has several use cases, such as stopping HLS/DASH and WebRTC viewers who are already watching, and stoping WebRTC publishers from publishing who are already publishing. Mostly useful for AMS users who are mapping their viewers using JWT tokens by including some userId in token.

There is no new data storage created to avoid token duplication since tokens are already stored in database on disk. Adding a token to blacklist means setting its blackListed flag to true. If token doesnt exist it inserts. Whitelisting a token means setting its blackListed flag to false.

For example, AMS users who wants to prevent a viewer who is already watching a stream with HLS/DASH can enable the JWT blacklist feature and add the user's JWT token to the blacklist(set its blacklisted flag to true, if not exists insert with blacklisted flag true) using a new API call. When the viewer requests the next video chunk, the server won't return it because the token is blacklisted. To allow the viewer to watch again, the user can whitelist the token by using same API call with whiteFlag parameter true.
Example blacklist http call:
curl --location --request POST 'http://localhost:5080/WebRTCAppEE/rest/v2/broadcasts/jwt-black-list?jwt=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJzdHJlYW1JZCI6InRlc3RzdHJlYW0iLCJ0eXBlIjoicGxheSIsImV4cCI6Mjg4NzUwOTMwMX0.pZ2X8HxqXiUn_pSheky_J61z2HEs-cgjG8ikB4MZ1AE' \

Example whitelist http call:
curl --location --request POST 'http://localhost:5080/WebRTCAppEE/rest/v2/broadcasts/jwt-black-list?jwt=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJzdHJlYW1JZCI6InRlc3RzdHJlYW0iLCJ0eXBlIjoicGxheSIsImV4cCI6Mjg4NzUwOTMwMX0.pZ2X8HxqXiUn_pSheky_J61z2HEs-cgjG8ikB4MZ1AE&whiteList=true' \

Similarly, if an AMS user wants to prevent a WebRTC viewer from watching a stream, they can stop the viewer using the "/webrtc-viewers/stop" API route with users JWT. This call will disconnect webrtc viewer. Then AMS user can add JWT to blacklist so when page is refreshed stream wont play.
https://github.com/ant-media/Ant-Media-Server/pull/4738/files#diff-c0d849875e95960ae6cb3780196fe1dcb6d87a19f9e5f2784627c074752edac0

JWT's in AMS needs to contain streamId. AMS user can use stopStreamingV2 api call to stop given stream by extracting stream Id from users JWT. After stoping publish, user's JWT can be added to the blacklist, preventing them from publishing again until the token is whitelisted with api call.

There are also two other utility api calls /jwt-black-list-delete-expired and /jwt-black-list-clear
/jwt-black-list-delete-expired deletes all expired(invalid) and blacklisted tokens from database.

/jwt-black-list-clear white lists all jwts.

jwt-black-list-delete-expired can be valuable if used periodically to prevent the tokenlist from growing infinitely when too many items are added.

Overall, this pull request enhances the functionality of AMS by adding a JWT blacklist feature that gives users greater control over who can watch and publish streams. Mostly useful to stop playing and publishing users who are already doing it.

API calls can be refactored to different names and style if found complicated.

src/main/java/io/antmedia/rest/BroadcastRestService.java

sonatype-lift · 2023-02-28T12:58:05Z

src/main/java/io/antmedia/rest/BroadcastRestService.java

+			final JSONParser parser = new JSONParser();
+			try {
+				final JSONObject jwtPayload = (JSONObject) parser.parse(payload);
+				final String tokenType = jwtPayload.get("type").toString();


NULLPTR_DEREFERENCE: null (last assigned on line 1283) is dereferenced.

ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage

@sonatype-lift ignore Leave out the above finding from this PR

@sonatype-lift ignoreall Leave out all the existing findings from this PR

@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
_{Click here to add LiftBot to another repo.}

Help us improve LIFT! (Sonatype LiftBot external survey)

Was this a good recommendation for you? _{Answering this survey will not impact your Lift settings.}

[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

sonatype-lift · 2023-02-28T12:58:08Z

src/main/java/io/antmedia/datastore/db/MapBasedDataStore.java

+		logger.info("Deleting all expired JWTs from black list.");
+
+		AtomicInteger deletedTokenCount = new AtomicInteger();
+			tokenBlacklistMap.forEach((key, value) -> {


THREAD_SAFETY_VIOLATION: Read/Write race. Non-private method MapBasedDataStore.deleteAllExpiredJwtFromBlacklist(...) reads without synchronization from container this.tokenBlacklistMap via call to Map.forEach(...). Potentially races with write in method MapBasedDataStore.deleteTokenFromBlacklist(...).
Reporting because another access to the same memory occurs on a background thread, although this access may not.

ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage

@sonatype-lift ignore Leave out the above finding from this PR

@sonatype-lift ignoreall Leave out all the existing findings from this PR

@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
_{Click here to add LiftBot to another repo.}

Help us improve LIFT! (Sonatype LiftBot external survey)

Was this a good recommendation for you? _{Answering this survey will not impact your Lift settings.}

[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

src/main/java/io/antmedia/rest/BroadcastRestService.java

sonatype-lift · 2023-02-28T12:58:12Z

🛠 Lift Auto-fix

Some of the Lift findings in this PR can be automatically fixed. You can download and apply these changes in your local project directory of your branch to review the suggestions before committing.¹

# Download the patch
curl https://lift.sonatype.com/api/patch/github.com/ant-media/Ant-Media-Server/4847.diff -o lift-autofixes.diff

# Apply the patch with git
git apply lift-autofixes.diff

# Review the changes
git diff

Want it all in a single command? Open a terminal in your project's directory and copy and paste the following command:

curl https://lift.sonatype.com/api/patch/github.com/ant-media/Ant-Media-Server/4847.diff | git apply

Once you're satisfied, commit and push your changes in your project.

You can preview the patch by opening the patch URL in the browser. ↩

… jwt api call

sonatype-lift · 2023-03-01T11:18:18Z

src/main/java/io/antmedia/rest/BroadcastRestService.java

+	@Produces(MediaType.APPLICATION_JSON)
+	public List<String> getJwtBlacklist()
+	{
+		if(getAppSettings().isJwtBlacklistEnabled()) {


NULL_DEREFERENCE: object returned by getAppSettings() could be null and is dereferenced at line 687.

ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage

@sonatype-lift ignore Leave out the above finding from this PR

@sonatype-lift ignoreall Leave out all the existing findings from this PR

@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
_{Click here to add LiftBot to another repo.}

Help us improve LIFT! (Sonatype LiftBot external survey)

Was this a good recommendation for you? _{Answering this survey will not impact your Lift settings.}

[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

jwt. blacklist using already existed token db with flag

sonatype-lift · 2023-03-02T12:56:20Z

src/main/java/io/antmedia/rest/BroadcastRestService.java

+	public Result blackListOrWhitelistJwt(@ApiParam(value = "jwt to be added to blacklist.", required = true) @QueryParam("jwt") String jwt,
+										  @ApiParam(value = "if true whitelist jwt (unblacklist)", required = false) @QueryParam("whiteList") boolean whiteList)
+	{
+		if(getAppSettings().isJwtBlacklistEnabled()){


NULL_DEREFERENCE: object returned by getAppSettings() could be null and is dereferenced at line 618.

ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage

@sonatype-lift ignore Leave out the above finding from this PR

@sonatype-lift ignoreall Leave out all the existing findings from this PR

@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
_{Click here to add LiftBot to another repo.}

Help us improve LIFT! (Sonatype LiftBot external survey)

Was this a good recommendation for you? _{Answering this survey will not impact your Lift settings.}

[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

unrelated but i didnt like the naming of this blackListOrWhitelistJwt function but couldnt find something better. Previously there were 2 different api calls one for blacklisting other for whitelisting but i changed to this. im sure someone can come up with a better naming.

sonarqubecloud · 2023-03-02T14:03:54Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
6 Code Smells

81.4% Coverage
5.3% Duplication

jantekb · 2023-03-15T22:46:59Z

src/main/java/io/antmedia/rest/BroadcastRestService.java

+	@Path("/jwt-black-list")
+	@Produces(MediaType.APPLICATION_JSON)
+	public Result blackListOrWhitelistJwt(@ApiParam(value = "jwt to be added to blacklist.", required = true) @QueryParam("jwt") String jwt,
+										  @ApiParam(value = "if true whitelist jwt (unblacklist)", required = false) @QueryParam("whiteList") boolean whiteList)


Sending a POST to /jwt-black-list?whiteList=true seems a bit weird to me, just read back the docs: "unblacklist".. Would it not be clearer if one had to send a DELETE request to the collection /jwt-black-list to get something removed?

this is not removing anything from disk thats why i made it a post call.
But we can also make it a delete

It is not the disk that matters, but it does remove someone from the blacklist, so I think a DELETE would be a better choice.

jantekb · 2023-03-15T22:49:17Z

src/main/java/io/antmedia/datastore/db/DataStore.java

+	 *
+	 * @return
+	 */
+	public abstract boolean whiteListAllTokens();


The concept of allowing only people with valid tokens is implicitly a whitelist, so I think you should not give the impression that there is a separate whitelist and a blacklist. Is it not only clearing the blacklist..?

jantekb · 2023-03-15T22:53:02Z

src/main/java/io/antmedia/datastore/db/MapBasedDataStore.java

+	@Override
+	public boolean whiteListAllTokens(){
+
+		synchronized (this) {


Using this as a synchronization point might not be a good choice as other classes can still hold a lock on this class. I would suggest using a non-public internal locking mechanism for synchronization purposes, which is more fine-grained and only applies to operations on the tokenMap, e.g. a ReadWriteLock or similar.

jantekb · 2023-03-15T22:57:45Z

src/main/java/io/antmedia/datastore/db/MapBasedDataStore.java

+	public boolean blackListToken(Token token) {
+		boolean result = false;
+
+		synchronized (this) {


I'd revisit the usage of synchronized in this class, as there seems to be some overuse and overscoping. Using this as a monitor is not very fortunate, as it can be held externally. Here the monitor is obtained too early, only saveToken seems to be critical, but as far as I can see saveToken itself also takes care of synchronization using this, so this might be completely unnecessary.

jantekb · 2023-03-15T23:01:20Z

src/main/java/io/antmedia/datastore/db/MongoStore.java

+	public boolean blackListToken(Token token) {
+		boolean result = false;
+		//update if exists, else insert
+		synchronized (this) {


The scope of the synchronized block looks unnecessarily broad and blocks other concurrent operations in the store. Sorry if I misunderstand, but if you are trying to ensure some kind of atomic transaction here so that the find-update-save cannot be interrupted, then what about other JVM participants in the cluster, that are not held up by the lock, hence can still concurrently fiddle with Mongo?

jantekb · 2023-03-15T23:02:21Z

src/main/java/io/antmedia/datastore/db/MongoStore.java

+		synchronized (this){
+			Query<Token> query = tokenDatastore.find(Token.class).filter(Filters.eq(BLACKLISTED, true));
+			for (Token token : query) {
+				tokenBlacklist.add(token.getTokenId());


Is it necessary to hold the monitor as long as the iteration processes all the tokens?

jantekb · 2023-03-20T09:31:08Z

I would suggest not using query parameters for mandatory data required for a POST to happen. Concretely I would not recommend sending the blacklisted JWT via query parameter, but POST body. Query parameters could be used for optional things instead (remember the ? mark), that narrow down or refine the results of a GET request.

sonatype-lift · 2023-06-13T18:54:43Z

src/main/java/io/antmedia/rest/BroadcastRestService.java

+	@Produces(MediaType.APPLICATION_JSON)
+	public Result blackListJwt(@ApiParam(value = "jwt to be added to blacklist.", required = true) @RequestBody Jwt jwt)
+	{
+		if(getAppSettings().isJwtBlacklistEnabled()){


NULL_DEREFERENCE: object returned by getAppSettings() could be null and is dereferenced at line 618.

ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage

@sonatype-lift ignore Leave out the above finding from this PR

@sonatype-lift ignoreall Leave out all the existing findings from this PR

@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
_{Click here to add LiftBot to another repo.}

sonatype-lift · 2023-06-13T18:54:45Z

src/main/java/io/antmedia/rest/BroadcastRestService.java

+	@Produces(MediaType.APPLICATION_JSON)
+	public Result whiteListJwt(@ApiParam(value = "Jwt to be removed from blacklist.", required = true) @QueryParam("jwt") String jwt)
+	{
+		if(getAppSettings().isJwtBlacklistEnabled()){


NULL_DEREFERENCE: object returned by getAppSettings() could be null and is dereferenced at line 667.

ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage

@sonatype-lift ignore Leave out the above finding from this PR

@sonatype-lift ignoreall Leave out all the existing findings from this PR

@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
_{Click here to add LiftBot to another repo.}

sonatype-lift · 2023-06-20T06:45:52Z

src/main/java/io/antmedia/rest/BroadcastRestService.java

+	@Produces(MediaType.APPLICATION_JSON)
+	public Result whiteListJwt(@ApiParam(value = "Jwt to be removed from blacklist.", required = true) Jwt jwt)
+	{
+		if(getAppSettings().isJwtBlacklistEnabled()){


NULL_DEREFERENCE: object returned by getAppSettings() could be null and is dereferenced at line 667.

ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage

@sonatype-lift ignore Leave out the above finding from this PR

@sonatype-lift ignoreall Leave out all the existing findings from this PR

@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
_{Click here to add LiftBot to another repo.}

sonarqubecloud · 2023-06-20T07:06:13Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
7 Code Smells

66.0% Coverage
10.0% Duplication

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

sonatype-lift bot reviewed Feb 28, 2023

View reviewed changes

src/main/java/io/antmedia/rest/BroadcastRestService.java Outdated Show resolved Hide resolved

sonatype-lift bot reviewed Feb 28, 2023

View reviewed changes

src/main/java/io/antmedia/rest/BroadcastRestService.java Outdated Show resolved Hide resolved

sonatype-lift bot reviewed Feb 28, 2023

View reviewed changes

src/main/java/io/antmedia/rest/BroadcastRestService.java Show resolved Hide resolved

lastpeony force-pushed the tokenBlacklist branch 2 times, most recently from d36062e to 9542943 Compare February 28, 2023 13:27

lastpeony mentioned this pull request Feb 28, 2023

new app setting jwtBlacklistEnabled added ant-media/StreamApp#341

Closed

lastpeony force-pushed the tokenBlacklist branch 4 times, most recently from f7d1669 to 94b6fd0 Compare March 1, 2023 09:21

issue #4809 jwt blacklist implementation and stop play/ publish using…

fc28eed

… jwt api call

lastpeony force-pushed the tokenBlacklist branch from 94b6fd0 to fc28eed Compare March 1, 2023 11:02

sonatype-lift bot reviewed Mar 1, 2023

View reviewed changes

lastpeony force-pushed the tokenBlacklist branch from 19945a7 to 0bd2319 Compare March 2, 2023 12:31

issue #4809 jwt blacklist implementation and stop play/ publish using

7543ea1

jwt. blacklist using already existed token db with flag

lastpeony force-pushed the tokenBlacklist branch from 0bd2319 to 7543ea1 Compare March 2, 2023 12:40

sonatype-lift bot reviewed Mar 2, 2023

View reviewed changes

jantekb reviewed Mar 15, 2023

View reviewed changes

jantekb mentioned this pull request Mar 20, 2023

Rest API suggestions #4918

Open

Merge branch 'master' into tokenBlacklist

02841c6

lastpeony added 2 commits June 13, 2023 19:53

refactor jwt blacklist rest

2f5971d

refactor integration test

9002d6c

sonatype-lift bot reviewed Jun 13, 2023

View reviewed changes

lastpeony force-pushed the tokenBlacklist branch 2 times, most recently from a16a4c1 to 06b4627 Compare June 20, 2023 06:34

sonatype-lift bot reviewed Jun 20, 2023

View reviewed changes

jwt blacklist for inmemory db

2fc0ea8

lastpeony force-pushed the tokenBlacklist branch from 7eb7818 to 2fc0ea8 Compare June 20, 2023 10:26

Merge branch 'master' into tokenBlacklist

c128bc6

lastpeony mentioned this pull request Jun 21, 2023

jwt blacklist guide added to stream-security.md ant-media/ant-media-documentation#103

Merged

asharamseervi mentioned this pull request Jun 30, 2023

Revert "jwt blacklist guide added to stream-security.md" ant-media/ant-media-documentation#107

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

jwt blacklist stop play/publish using jwt #4847

jwt blacklist stop play/publish using jwt #4847

lastpeony commented Feb 28, 2023 •

edited

Loading

sonatype-lift bot Feb 28, 2023

sonatype-lift bot Feb 28, 2023

sonatype-lift bot commented Feb 28, 2023

sonatype-lift bot Mar 1, 2023

sonatype-lift bot Mar 2, 2023

lastpeony Mar 2, 2023

sonarqubecloud bot commented Mar 2, 2023

jantekb Mar 15, 2023

lastpeony Mar 18, 2023

jantekb Mar 20, 2023

jantekb Mar 15, 2023

jantekb Mar 15, 2023

jantekb Mar 15, 2023

lastpeony Mar 18, 2023

jantekb Mar 15, 2023

jantekb Mar 15, 2023

jantekb commented Mar 20, 2023

sonatype-lift bot Jun 13, 2023

sonatype-lift bot Jun 13, 2023

sonatype-lift bot Jun 20, 2023

sonarqubecloud bot commented Jun 20, 2023

Command	Usage
`@sonatype-lift ignore`	Leave out the above finding from this PR
`@sonatype-lift ignoreall`	Leave out all the existing findings from this PR
`@sonatype-lift exclude <file\|issue\|path\|tool>`	Exclude specified `file\|issue\|path\|tool` from Lift findings by updating your config.toml file

jwt blacklist stop play/publish using jwt #4847

Are you sure you want to change the base?

jwt blacklist stop play/publish using jwt #4847

Conversation

lastpeony commented Feb 28, 2023 • edited Loading

sonatype-lift bot Feb 28, 2023

Choose a reason for hiding this comment

sonatype-lift bot Feb 28, 2023

Choose a reason for hiding this comment

sonatype-lift bot commented Feb 28, 2023

🛠 Lift Auto-fix

Footnotes

sonatype-lift bot Mar 1, 2023

Choose a reason for hiding this comment

sonatype-lift bot Mar 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Mar 2, 2023

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

jantekb commented Mar 20, 2023

sonatype-lift bot Jun 13, 2023

Choose a reason for hiding this comment

sonatype-lift bot Jun 13, 2023

Choose a reason for hiding this comment

sonatype-lift bot Jun 20, 2023

Choose a reason for hiding this comment

sonarqubecloud bot commented Jun 20, 2023

lastpeony commented Feb 28, 2023 •

edited

Loading