-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: replaced all whitelist and blacklist occurences by allowlis… #5277
base: develop
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -1,13 +1,13 @@ | ||||||||||
Modding API | ||||||||||
================= | ||||||||||
|
||||||||||
Terasology's engine uses a whitelisting approach to expose an API for modules using two primary methods and a rarely needed third one: | ||||||||||
Terasology's engine uses allowlisting approach to expose an API for modules using two primary methods and a rarely needed third one: | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or alternatively:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. applied |
||||||||||
|
||||||||||
* Classes or packages marked with the `@API` annotation | ||||||||||
* Classes or packages in the basic whitelist defined in `ExternalApiWhitelist.java` | ||||||||||
* Classes or packages in the basic allowlist defined in `ExternalApiAllowlist.java` | ||||||||||
* Rarely blocks of code in the engine may be hit in a way requiring use of `AccessController.doPrivileged(...)` - usually module authors do not need to worry about this but once in a while it could explain something quirky. | ||||||||||
|
||||||||||
This is to both protect the user's system from malicious code (for instance you cannot use `java.io.File` directly) and to better document what is available. If you attempt to use a class not on the whitelist you'll get log message like: | ||||||||||
This is to both protect the user's system from malicious code (for instance you cannot use `java.io.File` directly) and to better document what is available. If you attempt to use a class not on the allowlist you'll get log message like: | ||||||||||
|
||||||||||
`Denied access to class (not allowed with this module's permissions): some.package.and.class` | ||||||||||
|
||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,7 +19,7 @@ | |
|
||
/** | ||
* This class provides the methods needed to determine if a client is allowed to connect or not, | ||
* based on the blacklist and whitelist files. | ||
* based on the denylist and allowlist files. | ||
*/ | ||
|
||
public class ServerConnectListManager { | ||
|
@@ -28,14 +28,14 @@ public class ServerConnectListManager { | |
private static final Gson GSON = new Gson(); | ||
|
||
private Context context; | ||
private Set<String> blacklistedIDs; | ||
private Set<String> whitelistedIDs; | ||
private final Path blacklistPath; | ||
private final Path whitelistPath; | ||
private Set<String> denylistedIDs; | ||
private Set<String> allowlistedIDs; | ||
private final Path denylistPath; | ||
private final Path allowlistPath; | ||
|
||
public ServerConnectListManager(Context context) { | ||
blacklistPath = PathManager.getInstance().getHomePath().resolve("blacklist.json"); | ||
whitelistPath = PathManager.getInstance().getHomePath().resolve("whitelist.json"); | ||
denylistPath = PathManager.getInstance().getHomePath().resolve("denylist.json"); | ||
allowlistPath = PathManager.getInstance().getHomePath().resolve("allowlist.json"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this is likely to a breaking change otherwise, can we continue to check for the old file names if the newer ones do not exist? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. applied |
||
this.context = context; | ||
loadLists(); | ||
} | ||
|
@@ -44,29 +44,29 @@ public ServerConnectListManager(Context context) { | |
private void loadLists() { | ||
try { | ||
if (createFiles()) { | ||
blacklistedIDs = GSON.fromJson(Files.newBufferedReader(blacklistPath), Set.class); | ||
whitelistedIDs = GSON.fromJson(Files.newBufferedReader(whitelistPath), Set.class); | ||
if (blacklistedIDs == null) { | ||
blacklistedIDs = new HashSet<>(); | ||
denylistedIDs = GSON.fromJson(Files.newBufferedReader(denylistPath), Set.class); | ||
allowlistedIDs = GSON.fromJson(Files.newBufferedReader(allowlistPath), Set.class); | ||
if (denylistedIDs == null) { | ||
denylistedIDs = new HashSet<>(); | ||
} | ||
if (whitelistedIDs == null) { | ||
whitelistedIDs = new HashSet<>(); | ||
if (allowlistedIDs == null) { | ||
allowlistedIDs = new HashSet<>(); | ||
} | ||
} | ||
} catch (IOException e) { | ||
logger.error("Whitelist or blacklist files not found:", e); | ||
logger.error("Allowlist or denylist files not found:", e); | ||
} | ||
} | ||
|
||
private void saveLists() { | ||
try { | ||
if (createFiles()) { | ||
Writer blacklistWriter = Files.newBufferedWriter(blacklistPath); | ||
Writer whitelistWriter = Files.newBufferedWriter(whitelistPath); | ||
blacklistWriter.write(GSON.toJson(blacklistedIDs)); | ||
whitelistWriter.write(GSON.toJson(whitelistedIDs)); | ||
blacklistWriter.close(); | ||
whitelistWriter.close(); | ||
Writer denylistWriter = Files.newBufferedWriter(denylistPath); | ||
Writer allowlistWriter = Files.newBufferedWriter(allowlistPath); | ||
denylistWriter.write(GSON.toJson(denylistedIDs)); | ||
allowlistWriter.write(GSON.toJson(allowlistedIDs)); | ||
denylistWriter.close(); | ||
allowlistWriter.close(); | ||
} | ||
} catch (IOException e) { | ||
e.printStackTrace(); | ||
|
@@ -78,63 +78,63 @@ private boolean createFiles() throws IOException { | |
if (display == null || !display.isHeadless()) { | ||
return false; | ||
} | ||
if (!Files.exists(blacklistPath)) { | ||
Files.createFile(blacklistPath); | ||
if (!Files.exists(denylistPath)) { | ||
Files.createFile(denylistPath); | ||
} | ||
if (!Files.exists(whitelistPath)) { | ||
Files.createFile(whitelistPath); | ||
if (!Files.exists(allowlistPath)) { | ||
Files.createFile(allowlistPath); | ||
} | ||
return true; | ||
} | ||
|
||
public String getErrorMessage(String clientID) { | ||
if (isClientBlacklisted(clientID)) { | ||
return "client on blacklist"; | ||
if (isClientDenylisted(clientID)) { | ||
return "client on denylist"; | ||
} | ||
if (!isClientWhitelisted(clientID)) { | ||
return "client not on whitelist"; | ||
if (!isClientAllowlisted(clientID)) { | ||
return "client not on allowlist"; | ||
} | ||
return null; | ||
} | ||
|
||
@SuppressWarnings("BooleanMethodIsAlwaysInverted") | ||
public boolean isClientAllowedToConnect(String clientID) { | ||
return !isClientBlacklisted(clientID) && isClientWhitelisted(clientID); | ||
return !isClientDenylisted(clientID) && isClientAllowlisted(clientID); | ||
} | ||
|
||
public void addToWhitelist(String clientID) { | ||
whitelistedIDs.add(clientID); | ||
public void addToAllowlist(String clientID) { | ||
allowlistedIDs.add(clientID); | ||
saveLists(); | ||
} | ||
|
||
public void removeFromWhitelist(String clientID) { | ||
whitelistedIDs.remove(clientID); | ||
public void removeFromAllowlist(String clientID) { | ||
allowlistedIDs.remove(clientID); | ||
saveLists(); | ||
} | ||
|
||
public Set getWhitelist() { | ||
return Collections.unmodifiableSet(whitelistedIDs); | ||
public Set getAllowlist() { | ||
return Collections.unmodifiableSet(allowlistedIDs); | ||
} | ||
|
||
public void addToBlacklist(String clientID) { | ||
blacklistedIDs.add(clientID); | ||
public void addToDenylist(String clientID) { | ||
denylistedIDs.add(clientID); | ||
saveLists(); | ||
} | ||
|
||
public void removeFromBlacklist(String clientID) { | ||
blacklistedIDs.remove(clientID); | ||
public void removeFromDenylist(String clientID) { | ||
denylistedIDs.remove(clientID); | ||
saveLists(); | ||
} | ||
|
||
public Set getBlacklist() { | ||
return Collections.unmodifiableSet(blacklistedIDs); | ||
public Set getDenylist() { | ||
return Collections.unmodifiableSet(denylistedIDs); | ||
} | ||
|
||
private boolean isClientBlacklisted(String clientID) { | ||
return blacklistedIDs != null && blacklistedIDs.contains(clientID); | ||
private boolean isClientDenylisted(String clientID) { | ||
return denylistedIDs != null && denylistedIDs.contains(clientID); | ||
} | ||
|
||
private boolean isClientWhitelisted(String clientID) { | ||
return whitelistedIDs == null || whitelistedIDs.isEmpty() || whitelistedIDs.contains(clientID); | ||
private boolean isClientAllowlisted(String clientID) { | ||
return allowlistedIDs == null || allowlistedIDs.isEmpty() || allowlistedIDs.contains(clientID); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we please retain the old flle names in the gitignore so that they are not accidentally committed from older workspaces in future?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
applied