-
Notifications
You must be signed in to change notification settings - Fork 16
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
Implement joinserver.jsp and checkserver.jsp #98
Conversation
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.
Good so far, but the HTTP queries should always be HTTP 200 or the client cannot handle it and fails without returning the proper error.
session.go
Outdated
return func(c echo.Context) error { | ||
username := c.QueryParam("user") | ||
sessionID := c.QueryParam("sessionId") | ||
serverID := c.QueryParam("serverId") | ||
|
||
// If any parameters are missing, return NO | ||
if username == "" || sessionID == "" || serverID == "" { | ||
return c.String(http.StatusMethodNotAllowed, "NO") | ||
} | ||
|
||
// Parse sessionId. It has the form: | ||
// token:<accessToken>:<player UUID> | ||
split := strings.Split(sessionID, ":") | ||
if len(split) != 3 || split[0] != "token" { | ||
return c.String(http.StatusMethodNotAllowed, "NO") | ||
} | ||
accessToken := split[1] | ||
id := split[2] | ||
|
||
// Is the accessToken valid? | ||
client := app.GetClient(accessToken, StalePolicyDeny) | ||
if client == nil { | ||
return c.String(http.StatusMethodNotAllowed, "NO") | ||
} | ||
|
||
// If the player name corresponding to the access token doesn't match | ||
// the `user` param from the request, return NO | ||
user := client.User | ||
if user.PlayerName != username { | ||
return c.String(http.StatusMethodNotAllowed, "NO") | ||
} | ||
// If the player's UUID doesn't match the UUID in the sessionId, return | ||
// NO | ||
userID, err := UUIDToID(user.UUID) | ||
if err != nil { | ||
return err | ||
} | ||
if userID != id { | ||
return c.String(http.StatusMethodNotAllowed, "NO") | ||
} | ||
|
||
user.ServerID = MakeNullString(&serverID) | ||
result := app.DB.Save(&user) | ||
if result.Error != nil { | ||
return result.Error | ||
} | ||
|
||
return c.String(http.StatusOK, "YES") | ||
} |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
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.
Ah, I think I understand what you mean about the fallbackAPIServer functionality. The /session/minecraft/join
and /game/joinserver.jsp
routes should never/don't ever need to query fallback API servers. These routes are called by the Minecraft client when connecting to a Minecraft server. The client knows an accessToken
provided by the launcher as well as a URL for an authentication/session server. This URL can be either provided by the launcher, hardcoded/patched in, patched by a Java agent, redirected by a proxy, or some combination of those. The client's accessToken
MUST be issued by the session server pointed to by that URL. Otherwise, the client would send an accessToken
issued by server A to server B, and server B would be able to impersonate the client to server A.
So we never need to query fallback API servers for the "session join" routes. If everything is set up sanely, the accessToken
WILL be issued from the same authentication server that gets the join
request. If your client has a mismatched auth server URL and accessToken, then something has gone horribly wrong.
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.
I think I finally understand what you mean, I just realised how critical it was, a vanilla minecraft server directly authenticates through mojang's sessionserver domains, we only have to check from the serverside if the user actually joined or not, good point, my testing confused me as i kept thinking i was supposed to proxy the login part too to access from a mojang account through a proxied minecraft instance that redirects everything through drasl's servers. In the end yes, this does not need to be sent to drasl and can be disregarded.
Thanks for the insight! This can be disregarded.
// not known, don't query the fallback server. | ||
continue | ||
} | ||
base, err := url.Parse(fallbackAPIServer.SessionURL) |
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.
If we're goingwith the simple step (which is to use mojang's still working legacy login URL:
if legacy {
// Replace string sessionserver.mojang.com with session.minecraft.net in the URL
if strings.Contains(base.Host, "sessionserver.mojang.com") {
base.Host = strings.ReplaceAll(base.Host, "sessionserver.mojang.com", "session.minecraft.net")
}
}
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.
It's simpler to use the newer route in all cases.
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.
I tried to use the newer route but seems like it refuses to even accept it, i just relied on legacy route again as it worked, unlike the modern route. Did it work for you?
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.
Those changes made the game sccessfully load on the server side.
params.Add("username", playerName) | ||
params.Add("serverId", serverID) | ||
base.RawQuery = params.Encode() | ||
base.Path += "/session/minecraft/hasJoined" |
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.
- base.Path += "/session/minecraft/hasJoined"
+ if legacy {
+ base.Path += "/game/checkserver.jsp"
+ } else {
+ base.Path += "/session/minecraft/hasJoined"
+ }
716737c
to
7ae4a1a
Compare
Thanks, seems to be working now for both fallback and regular login. |
force pushing kinda confused me out as i couldn't see the new changes, but overall majority of the changes work, now to deal with the fallback APIs, I hid the fixed remarks (as i can't mark it as resolved) but left the relevant ones on |
The authlib-injector spec only requires /profiles/minecraft to be implemented, not necessarily /users/profiles/minecraft/:playerName, so the /profiles/minecraft at least should query fallback API servers at /profiles/minecraft. Also fixes potential DoS by introducing a limit of 10 players per request (also which prevents fallback API servers from being spammed)
Merging this. Even though Drasl should recommend ways to patch legacy versions to use the newer routes (e.g. https://github.com/craftycodie/OnlineModeFix), we should still add support for these legacy routes for those wanting to get legacy versions working using a local proxy server. |
For #52, if we end up adding support for old versions via an HTTP proxy or similar.
We may not need this since patching the game to use the new routes might be a better/simpler solution. For example, if we add support for the
-Dminecraft.api.*.host
system properties to https://github.com/betacraftuk/legacyfix and integrate it nicely into Fjord Launcher, Ely.by and Blessing Skin accounts would "just work" on old versions without those auth servers needing to implement the legacy API routes.An HTTP proxy like betacraft.uk uses also has the drawback that all traffic will be proxied. There doesn't seem to be a way to tell (ancient) Java to only proxy certain domains (I stand to be corrected), so some mods might break if they make requests to other URLs that Drasl doesn't respond to. We could have a configurable option to proxy all requests, but that'd be dangerous.