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

Premium Players Support Reworked #197

Closed
wants to merge 5 commits into from

Conversation

scaik
Copy link
Contributor

@scaik scaik commented Jul 20, 2024

Pull Request Etiquette

  • I have checked the PRs for upcoming features/bug fixes.

Changes

  • Internal code
  • API (affecting end-user code, backwards compatible)
  • Other: _____

Closes Issue: #39

Description

This pull request implements the following features:

  • Nickname length max & min limit
  • Premium account auto-detection on login
  • Separated authentication scenarios for premium and cracked players
  • Premium nickname change handling
  • Cracked and premium conflict handling

Now this works a bit different compared to previous PR.
The PreLogin scenario look like this:

  1. Validate nickname (Moved from PostLogin)
  2. Check if player's IP + Nickname is in the PendingLoginBucket
    • If found, it means that this is a cracked player
    • Continue to regular authorization
  3. Retrieve player UUID from mojang
    • If not found, continue to regular authorization
  4. If there is no account with player's nickname or there is a non-premium account,
    we should check player's mode, because that can be just a cracked player with premium name.
    (By the way, there is an option in config to prevent such players from connecting)
    • Add player to PendingLoginBucket
    • Force online mode
  5. Otherwise we force offline mode

There is a slight downsight in this approach, which is that whenever a cracked player
with a premium name logs in, he will get kicked and he will have to reconnect,
but i couldn't manage to find any workarounds.

Inspired by LibreLogin and FastLogin

I really hope that this is what the plugin needs and my code will work as expected :)

@bivashy bivashy self-assigned this Jul 21, 2024
@bivashy bivashy added the status: needs testing Issue or PR should be tested label Jul 21, 2024
@bivashy
Copy link
Owner

bivashy commented Jul 21, 2024

First of all, great job! I reviewed your code and you nailed it. I'm going to test your changes and then try to fix some issues you mentioned. I will modify the code if I find any bugs or minor fixes.

This will definitely be merged, so you can be sure that the next release will include this feature.

Testing TODO:

  • Test with id-type: NAME and premium enabled.
    Expected behavior: Plugin should behave as usual, commands with player name should work as expected.

  • Enable premium, change player name in database to wrong one (without profile-conflict)
    Expected behavior: Plugin should change player name in database to valid one

  • Test every profile-conflict.
    Expected behavior: Plugin should work depending on profile-conflict.

  • Try to enter with enabled premium mode without online account
    Expected behavior: Plugin should kick player with error

  • Use wrong name when entering premium mode account
    Expected behavior: Plugin should kick player with error

  • Test Mojang rate limit and premium account auto-detection
    Expected behavior: Plugin should work without premium functionality.

If you have other test cases, reply to this comment.

@scaik
Copy link
Contributor Author

scaik commented Jul 21, 2024

Thanks for reviewing my code, i’m glad to hear that! tbh i tested it and it worked as expected, but my tests were very superficial. i think there is another test case with premium auth steps.

core/pom.xml Show resolved Hide resolved
@bivashy bivashy removed their assignment Jul 21, 2024
@scaik
Copy link
Contributor Author

scaik commented Jul 24, 2024

Relocation didn't help, idk why. So i just let bungee download the library.

bivashy and others added 2 commits July 26, 2024 18:06
@scaik scaik closed this by deleting the head repository Dec 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs testing Issue or PR should be tested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants