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

Support multiple players per user #107

Merged
merged 25 commits into from
Dec 29, 2024

Conversation

evan-goode
Copy link
Member

@evan-goode evan-goode commented Oct 14, 2024

For #75

  • Back-end functionality and refactoring
  • Disallow a user from claiming a Player Name the same as another user's Username
  • Disallow a user from claiming a Username the same as another user's Player Name
  • Send all connected players in availableProfiles
  • Support sending selectedProfile in /auth/refresh to move Client to another Player of the same user set selectedProfile for an unassigned client
  • Front-end changes
    • Allow admins to change an individual user's MaxPlayerCount
    • Basic functionality
  • Write new tests
    • availableProfiles, selectedProfile, refresh with selected profile
    • Set max player count in admin view
    • web create new player
    • web register existing player, w/skin verification
    • player name in use as another user's username
    • username in use as another user's player name
    • db migration
    • db backup
  • Fix existing tests
  • Add create/update player API routes
  • Gracefully deprecate v1 API
  • Documentation
  • DB Migration
  • Automatic DB backup

@evan-goode evan-goode force-pushed the evan-goode/multiple-profiles branch 2 times, most recently from cb38e69 to 94bc10c Compare November 27, 2024 16:24
@evan-goode evan-goode force-pushed the evan-goode/multiple-profiles branch from cc167ab to 349bd13 Compare December 6, 2024 02:13
@evan-goode evan-goode mentioned this pull request Dec 12, 2024
@evan-goode evan-goode changed the base branch from master to next December 14, 2024 21:54
@evan-goode evan-goode force-pushed the evan-goode/multiple-profiles branch from 4aa4cd3 to 9b79be9 Compare December 21, 2024 18:49
@IkyMax
Copy link

IkyMax commented Dec 24, 2024

i tried this feature on my testing vps and i found two bugs, in the hypotetical case where the database is empty it'll say this

2024/12/24 00:29:32 Loading config from /etc/drasl/config.toml
2024/12/24 00:29:32 Started migration of database version 3 to 4.
2024/12/24 00:29:32 Backing up old database to /var/lib/drasl/drasl.3.2024-12-24T00-29-32Z.db
2024/12/24 00:29:32 Database backed up, proceeding.
2024/12/24 00:29:32 Error migrating database: empty slice found

and whenever i try to login with the main account (via yggdrasil REST API) it will only throw the available profiles with the client and access tokens, it won't throw the selected profile or the user section (if requested)

Request:

{
    "agent": {
        "name": "Minecraft",
        "version": 1
    },
    "username": "Test1",
    "password": "Test",
    "clientToken": "abcdef123456",
    "requestUser": true
}

Expected response:

{
    "accessToken": "accessToken",
    "clientToken": "abcdef123456",
    "selectedProfile": {
        "id": "0c947ccefad24cb4b62d62e3480d9860",
        "name": "Test1"
    },
    "availableProfiles": [{
        "id": "dd81cb374dae4f8889c9b5cb62c4be2f",
        "name": "Test1"
    }, {
        "id": "0c947ccefad24cb4b62d62e3480d9860",
        "name": "Test2"
    }],
    "user": {
        "id": "dd81cb374dae4f8889c9b5cb62c4be2f",
        "properties": [{
            "name": "preferredLanguage",
            "value": "en"
        }]
    }
}

Actual response:

{
    "accessToken": "accessToken",
    "clientToken": "abcdef123456",
    "availableProfiles": [{
        "id": "dd81cb374dae4f8889c9b5cb62c4be2f",
        "name": "Test1"
    }, {
        "id": "0c947ccefad24cb4b62d62e3480d9860",
        "name": "Test2"
    }]
}

@evan-goode
Copy link
Member Author

Fixed the empty slice error, it was a case of go-gorm/gorm#4076, and added a regression test.

I also made /authenticate always honor requestUser and return the user ID instead of the selectedProfile ID. I think this is the correct behavior according to authlib-injector: https://github-com.translate.goog/yushijinhun/authlib-injector/wiki/Yggdrasil-%E6%9C%8D%E5%8A%A1%E7%AB%AF%E6%8A%80%E6%9C%AF%E8%A7%84%E8%8C%83?_x_tr_sl=auto&_x_tr_tl=en&_x_tr_hl=en-US#%E7%94%A8%E6%88%B7%E4%BF%A1%E6%81%AF%E7%9A%84%E5%BA%8F%E5%88%97%E5%8C%96. I also updated the migration routine to generate new UUIDs for existing users so they are always distinct from any player UUID.

As far as the missing selectedProfile, IMO this is the more useful behavior. The authlib-injector spec isn't clear on disambiguating usernames/player names since it assumes you use email addresses as usernames. In my implementation, the username is matched first, then the player name. So if you have a user named User1 with players User1 and User2, selectedProfile will be empty and the launcher should prompt you which profile you want to use. But if user Foo owns players Bar and Baz, and you log in as Bar (no ambiguity), selectedProfile will be the Bar player.

@evan-goode evan-goode force-pushed the evan-goode/multiple-profiles branch from 63cc5c2 to 8cd97d0 Compare December 24, 2024 03:47
@evan-goode evan-goode marked this pull request as ready for review December 24, 2024 17:07
@evan-goode evan-goode merged commit f562e8b into unmojang:next Dec 29, 2024
1 check passed
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.

2 participants