Skip to content
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

fix(server/functions): ban checking edge case #519

Merged
merged 1 commit into from
Jul 24, 2024
Merged

Conversation

mafewtm
Copy link
Member

@mafewtm mafewtm commented Jul 19, 2024

Description

There seems to be a weird edge case in ban checking which I believe is the issue in #463.

Currently we pull whichever license the player has (license2 first then license) and then check it against the bans table to see if the individual is banned. If you have an admin menu which bans by license but you have a license2, the check will give a false positive and allow the player through because it did not check by license. This fixes that by first checking by license2 and then by license to make sure we catch both cases.

Feel free to comment or start a discussion below. Idk if there's a better way we'd like to do this.

Checklist

  • I have personally loaded this code into an updated Qbox project and checked all of its functionality.
  • My pull request fits the contribution guidelines & code conventions.

@mafewtm mafewtm requested review from solareon and Manason July 19, 2024 05:20
@mafewtm
Copy link
Member Author

mafewtm commented Jul 19, 2024

Now that I think about it, editing the fetchBan SQL is probably a better alternative than this. I'll do that tomorrow if y'all agree.

@solareon
Copy link
Contributor

Now that I think about it, editing the fetchBan SQL is probably a better alternative than this. I'll do that tomorrow if y'all agree.

Yeah to use the same lookup as the fetch characters.

@solareon solareon merged commit 6ff85eb into main Jul 24, 2024
4 checks passed
@solareon solareon deleted the mafew-patch-1 branch July 24, 2024 07:38
@mmoftah mmoftah mentioned this pull request Aug 12, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants