This fixes false positive cheat detection when throwing rotten eggs at town NPCs while wearing Frost armor set. Also made the debug and kick messages more clear for future reference.
TShock was originally designed to handle many things that Terraria did
not. Therefore, TShock always "took over" for the server password
prompt. We then added the ability to login via the password prompt if
you had an account, so that you could play on a server and login without
having to run /login in the chat window. Then, UUIDs were introduced,
and we added the ability to login via UUID.
This has created a cascading scenario where users are potentially
affected by many different things. We have always treated a user's
runtime intent as the most important: if a user sets something on the
console, it should be taken as the "most true" setting. In other words,
we believe that the most recent choice the user made is the valid one.
But for some of the config settings we have, we've made it opaque as to
how this decision making works. We also aren't clear what certain things
do by default.
Currently, if UUID login is enabled, a user will login "magically" and
bypass any password prompt. Even if this is disabled, though, users are,
by default, allowed to enter their passwords at the password prompt
instead of the server password. Both of these take priority over the
runtime setting.
The problem is that we haven't really made it clear if we should
override the runtime setting here. This is because the Terraria
interactive prompt asks for a server password, and one of the two
"bypass" settings is not a password setting at all. What do we respect?
I decided that the best approach is to just communicate really loudly
about these settings. If a runtime password is set, we'll warn users if
either of the bypass settings are "in play." If it's not set, we'll warn
users if the server password was set in config.json, just so they know
which password is being used.
If UUID logins are enabled we'll also warn users about that and the
security risks attached, no matter what. I don't know that we should
really have this feature, but we shouldn't get rid of it, imho.
The only thing I don't think we need to warn about is if login before
join is enabled. Login before join just acts as a way to speed up logins
for registered users. In an ideal world, users who shouldn't be able to
login should be banned. But I split the difference since we're warning
about UUID logins.
The only real downside to this change is that the PostInit hook gets
bigger. But dumping this stuff in another file/area/etc., seems dumb
since some of the logic exists here already. I think we can refactor
this later, but it's not my most pressing priority.
This whole change was inspired by the fact that @Onusai tried to lock
down their server but failed because of these settings enabled. We need
to be more transparent about logins, and this is a good first step.
This commit adds a fallback to address problems with FindByNameOrID
potentially returning ambiguous results. Now, in response to a multiple
match error, a player can specify tsi:[number] or tsn:[exact name] to
match a user ID or name exactly. This behaves analogous to the old
behavior of the search method.
Currently, the TSPlayer FindbyNameOrID method aborts if it finds an
"exact match" based on this criteria:
1. If the player ID is on the server, it must be the thing we're looking
for. Therefore, return that.
2. If the case sensitive "exact match" is on the server that isn't an
ID, that must be what we're looking for. Therefore, return that.
3. Just yolo and downcase everything and return any number of matching
players next.
This commit changes the behavior because some players have been joining
servers with ambiguous names, like `1`. In the current system, this
player is difficult to query because they're an "ID" and therefore an
exact match will be returned even if a player name exists that matches
the criteria.
This also alleviates the issue of a case exact match falling down the
same trap. It's ambiguous enough in all of these situations that an
admin should just be using a player ID instead.`
Okay, now we're at problem 74 with github actions. Basically, github
actions doesn't send secrets to forks because duh, that makes sense. So
even if you make a super restricted token you still can't send it to
forks because github still doesn't understand how to make a security
platform when they just copy paste azure pipelines into github and then
say "well looks good to me" and ship fucking arbitrary code execution to
the entire fucking world and then try to retroactively fix all of their
mistakes and fail miserably in the process
Look, let's just be real here: GitHub needs to redo the entire
permission model for GitHub. There is no way to create a secure
combination of the following elements: post comment, edit comment, and
post status check.
If you want to be able to post comments, you have to authorize a token
or app to have full authority to do literally anything that the user can
do on a public repo. Full stop.
If you want to post a status check, you have to give the user write
access to the entire repo, which makes the first issue a problem.
You can't just explicitly make a token that says "only allow this user
to post and edit its own comments" and "allow this user to post status
checks" because write access on the repo implies authority over all
other issues/PRs opened by other people.
Now Cardinal's token is restricted to just status checks, and we're
using a different action.
Thanks a ton for the huge mess Github.
This change uses Cardinal's PAT for GitHub Actions CI. The way this
works is very convoluted, but it makes sense in theory.
1. Cardinal is a member of the Pryaxis org, in a group called "untrusted
robots." She has write access to Pryaxis/TShock, so she can create
status messages. This is because GitHub only allows status messages to
be created if a user has write access.
2. Cardinal has a PAT, and that PAT only has access to creating
repository status messages.
3. Danger requires permission to post comments and update CI status.
4. Cardinal's PAT is only authorized to create repo status messages, and
cannot privilege escalate.
5. GitHub implicitly gives everyone the ability to post comments on
public repositories.
Thus, this really interesting and weird flow should mean that Cardinal
can post comments and update status messages, by having write access but
functionally being unable to use it.
At least, that's the theory.
UsingBiomeTorches: Whether or not the player has the torchgod biometorches ability enabled
HappyFunTorchTime: Whether or not the player has fought the torchgod before (for logic that checks for torchgod spawning)
unlockedBiomeTorches: Whether or not the player has the torchgod biome torches ability unlocked
This fixes a ridiculous typo in GetDataHandlers where we were setting
the UsingBiomeTorches flag based on having unlocked biome torches,
rather than actually being used. Thanks to @Arthri for the tip!
If a player has the tshock.ignore.ssc permission, odds are that they may
want to know that their data isn't being saved or not. This change
allows users to be notified if they have SSC data stored in the DB but
they aren't having it loaded due to the aforementioned permission.
This permission causes great confusion, but we can't really change it
because we would break existing setups. This is an easy change that
gives people a reason why they suddenly "have no items."
This new option can be turned off in the config file for SSC if it's not
desired.
This change also modifies some of the log messages so that it's clear
why the SSC save didn't occur for a given player.
This commit adds Danger via GitHub Actions. Dangerfiles are ruby files
that have a DSL for interacting with GitHub. They can do arbitrary
things. See: https://danger.systems/reference.html
The point of this commit is to automate the process of asking people to
update the changelog. This is a really really annoying thing that we
have to do too often. Editing a pull request will automatically re-run
the check.
Truly trivial commits can be marked as trivial easily by using the
hashtag trivial in the PR body. This is really just useful for actually
trivial things. Most commits actually do need to have associated
changelog entries.
For the purposes of making it easier to understand, the Terraria Server
API is now being called "TShock Scaffold API." This is actually just an
elaborate measure so that TSAPI can be called TSAPI but to alleviate
some confusion.
