Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions code/framework/src/networking/rpc/client_identity.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ namespace Framework::Networking::RPC {
std::string steamId;
std::string discordId;
std::string hardwareId;
// Authenticated Epic account id; empty when the game was not launched through Epic. Ordered
// to match Decode(), which is strict: a payload missing any field is refused outright.
// Client-reported Epic account id, not authenticated by the server; empty when absent.
// Ordered to match Decode(), which refuses a payload missing any field.
std::string epicId;

// Opaque string the client was launched with (a launcher-issued join ticket, typically).
Expand Down
6 changes: 4 additions & 2 deletions code/tests/modules/connection_admission_ut.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,11 @@ class ConnectionAdmissionRig {
return static_cast<bool>(client->Connect("127.0.0.1", port, "", sessionPayload));
}

static std::string IdentityPayload(const char *name, const char *ticket) {
static std::string IdentityPayload(const char *name, const char *ticket, const char *epicId = "") {
Framework::Networking::RPC::ClientIdentity identity;
identity.name = name;
identity.ticket = ticket;
identity.epicId = epicId;
return identity.Encode();
}

Expand Down Expand Up @@ -138,12 +139,13 @@ MODULE(connection_admission, {
IT("hands the server the identity and ticket the client connected with, before any connection", {
Rig rig;
EQUALS(rig.StartServer(true), true);
EQUALS(rig.Connect(Rig::IdentityPayload("Jan", "tkt-abc+123")), true);
EQUALS(rig.Connect(Rig::IdentityPayload("Jan", "tkt-abc+123", "0123456789abcdef0123456789abcdef")), true);

EQUALS(rig.PumpUntil([&] { return !rig.seen.requests.empty(); }), true);
EQUALS(rig.seen.requests[0].has_value(), true);
STREQUALS(rig.seen.requests[0]->name.c_str(), "Jan");
STREQUALS(rig.seen.requests[0]->ticket.c_str(), "tkt-abc+123");
STREQUALS(rig.seen.requests[0]->epicId.c_str(), "0123456789abcdef0123456789abcdef");

// Waiting: nobody is connected on either side, and the player count says so.
rig.PumpFor(300);
Expand Down
106 changes: 104 additions & 2 deletions code/tests/modules/connection_gate_ut.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,14 @@

#pragma once

#include "connection_admission_ut.h"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This couples the gate tests to the admission test module only for the loopback Player.epicId test. If that test goes, so should this include and the replication/player includes below.


#include "core_modules.h"
#include "integrations/server/connection_gate.h"
#include "integrations/server/scripting/module.h"
#include "networking/replication/network_entity.h"
#include "networking/replication/replication_manager.h"
#include "scripting/builtins/player.h"
#include "scripting/node_engine.h"
#include "scripting/resource/resource_manager.h"

Expand All @@ -26,10 +31,32 @@

// The admission gate against a real Node engine and a real resource: what a playerConnecting
// handler does -- return, reject, throw, await, hang -- and what the gate decides for it. The
// network side is left out (no NetworkServer, so update() sends nothing); the decisions are what
// the instance acts on, and those are what is asserted.
// decisions are what the instance acts on. A loopback admission also checks the joined player's
// script-visible identity.
class ConnectionGateTestRig {
public:
// Register an admitted peer's avatar for the same owner-guid lookup used by server scripts.
struct JoinedPlayer {
Framework::Networking::NetworkServer &server;
Framework::Networking::Replication::NetworkEntity avatar;

JoinedPlayer(Framework::Networking::NetworkServer &peer, MafiaNet::RakNetGUID guid): server(peer) {
avatar.SetNetworkID(1);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JoinedPlayer hard-codes NetworkID(1) and swaps process-wide CoreModules state (SetNetworkPeer/SetReplication) to fake a joined player. That's fragile across test ordering, and it's a lot of setup to exercise a one-line getter (Player::GetEpicId).

avatar.ownerGUID = MafiaNet::ToPeerGuid(guid);
server.GetReplicationManager()->Reference(&avatar);
server.GetReplicationManager()->SetViewer(avatar.ownerGUID, &avatar);
Framework::CoreModules::SetNetworkPeer(&server);
Framework::CoreModules::SetReplication(server.GetReplicationManager());
}

~JoinedPlayer() {
Framework::CoreModules::SetReplication(nullptr);
Framework::CoreModules::SetNetworkPeer(nullptr);
server.GetReplicationManager()->ClearViewer(avatar.ownerGUID);
server.GetReplicationManager()->Dereference(&avatar);
}
};

static std::string ResourcesPath() {
#ifdef _WIN32
const char *temp = std::getenv("TEMP");
Expand Down Expand Up @@ -64,6 +91,12 @@ class ConnectionGateTestRig {
globalThis.__probe = connection;
globalThis.__pendingDuring = connection.isPending() ? 1 : 0;
return;
case "epic-whitelist":

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"0123456789abcdef0123456789abcdef" is repeated 5 times across this file and connection_admission_ut.h, and the reject reason 4 times. Use one shared constant for each (kTestEpicId, kEpicWhitelistReason) and build this handler string from them.

globalThis.__epicId = connection.epicId;
if (connection.epicId !== "0123456789abcdef0123456789abcdef") {
connection.reject("Your Epic account is not on the whitelist.");
}
return;
}
});
)";
Expand Down Expand Up @@ -186,6 +219,75 @@ MODULE(connection_gate, {
STREQUALS(Rig::EvalString(module.GetEngine(), "globalThis.__seen").c_str(), "admit|one-time-ticket|10.0.0.2|76561198000000000");
});

IT("admits an Epic whitelist entry using the client-reported ID without a ticket", {
auto identity = Rig::Identity("epic-whitelist");
identity.steamId.clear();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block (clear steamId, clear ticket, set epicId) is duplicated at L280. Suggest a Rig::EpicIdentity(const char *epicId) helper. The size/admitted/reason assertions in the two refusal tests could share an ExpectRefused(decisions, reason) helper too.

identity.ticket.clear();
identity.epicId = "0123456789abcdef0123456789abcdef";
EQUALS(gate.Begin(Rig::Guid(12), identity, "10.0.0.12"), true);
const auto decisions = Rig::Settle(module, gate);
EQUALS(decisions.size(), 1u);
EQUALS(decisions[0].admitted, true);
STREQUALS(Rig::EvalString(module.GetEngine(), "globalThis.__epicId").c_str(), identity.epicId.c_str());
});

IT("exposes the admitted peer's Epic ID through script-visible Player.epicId after joining", {
ConnectionAdmissionRig network;
EQUALS(network.StartServer(true), true);
const char *epicId = "0123456789abcdef0123456789abcdef";
EQUALS(network.Connect(ConnectionAdmissionRig::IdentityPayload("epic-whitelist", "", epicId)), true);
EQUALS(network.PumpUntil([&] { return !network.seen.requests.empty(); }), true);
EQUALS(network.seen.requests[0].has_value(), true);

const auto guid = network.seen.requestGuids[0];
const auto &identity = *network.seen.requests[0];
// The instance retains the received identity before running playerConnecting.
network.server->SetPeerIdentity(guid, identity);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This copies instance.cpp:737 instead of running it, so the hex/length sanitisation at instance.cpp:730 is skipped, and that's the only real server behaviour for epicId. The test then reads back what it injected. Please drive the instance's actual handshake path, or test the sanitiser directly with oversized, non-hex and uppercase IDs.

EQUALS(gate.Begin(guid, identity, network.server->GetAddress(guid)), true);
const auto decisions = Rig::Settle(module, gate);
EQUALS(decisions.size(), 1u);
EQUALS(decisions[0].guid.g, guid.g);
EQUALS(decisions[0].admitted, true);
network.server->AcceptSession(decisions[0].guid);
EQUALS(network.PumpUntil([&] { return network.seen.clientConnected && network.server->GetPlayerCount() == 1u; }), true);
EQUALS(network.seen.clientDisconnected, false);

Rig::JoinedPlayer player(*network.server, guid);
auto *engine = module.GetEngine();
v8::Isolate *isolate = engine->GetResourceRuntime("gate")->GetIsolate();
{
v8::Locker locker(isolate);
v8::Isolate::Scope isolateScope(isolate);
v8::HandleScope handleScope(isolate);
v8::Local<v8::Context> context = engine->GetContext();
v8::Context::Scope contextScope(context);
Framework::Scripting::Builtins::Player::Register(isolate, context->Global());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-registering the Player builtin with a manual v8::Locker inside a test is a sign this test reaches past the gate's responsibility. I'd drop it: Player::GetEpicId is a trivial passthrough over the same PeerIdentity the other identity getters use.

}
STREQUALS(Rig::EvalString(engine, "new Player(1).epicId").c_str(), epicId);
});

IT("lets an Epic whitelist refuse a connection without an Epic ID", {
EQUALS(gate.Begin(Rig::Guid(13), Rig::Identity("epic-whitelist"), "10.0.0.13"), true);
const auto decisions = Rig::Settle(module, gate);
EQUALS(decisions.size(), 1u);
EQUALS(decisions[0].admitted, false);
STREQUALS(decisions[0].reason.c_str(), "Your Epic account is not on the whitelist.");
STREQUALS(Rig::EvalString(module.GetEngine(), "globalThis.__epicId").c_str(), "");
});

IT("lets an Epic whitelist refuse a nonempty ID that is not listed", {
auto identity = Rig::Identity("epic-whitelist");
identity.steamId.clear();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate of the setup at L224, see above.

identity.ticket.clear();
identity.epicId = "fedcba9876543210fedcba9876543210";
EQUALS(gate.Begin(Rig::Guid(14), identity, "10.0.0.14"), true);
const auto decisions = Rig::Settle(module, gate);
EQUALS(decisions.size(), 1u);
EQUALS(decisions[0].admitted, false);
STREQUALS(decisions[0].reason.c_str(), "Your Epic account is not on the whitelist.");
STREQUALS(Rig::EvalString(module.GetEngine(), "globalThis.__epicId").c_str(), identity.epicId.c_str());
});

IT("refuses with the script's reason", {
EQUALS(gate.Begin(Rig::Guid(3), Rig::Identity("refuse"), "10.0.0.3"), true);
const auto decisions = Rig::Settle(module, gate);
Expand Down
22 changes: 22 additions & 0 deletions docs/connection_admission.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ waits up to two minutes for an answer; it is always the server that decides.
|---|---|
| `nickname` | the name the player asked to join under |
| `steamId`, `discordId`, `hardwareId` | as reported by the client; empty when absent |
| `epicId` | Epic account ID reported by the client; empty when absent |
| `ticket` | the `ticket` of the client's launch link, untouched |
| `ip` | remote address, no port |
| `reject(reason?)` | refuse; reason up to 512 bytes |
Expand All @@ -116,6 +117,27 @@ itself; `steamId` is read from an environment variable the launcher sets and is
not checked against Steam. A whitelist that must hold against a modified client
should check the `ticket` with whatever issued it.

The Epic launcher sets `MafiaHubEpicId` from its signed-in account, and the client
includes that ID in the existing connection handshake. The server applies a hex

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"applies a hex format check": the check accepts any length from 1 to 32 and either case without normalising. Either enforce exactly 32 and lowercase on the server, or say here that the ID may be uppercase or mixed case and that comparisons must be case-insensitive.

format check and exposes it as `PendingConnection.epicId` before admission and
`Player.epicId` after joining. It is an Epic account ID, not an EOS Product User
ID. No Epic proof or server-side account verification is performed.

For temporary, non-security filtering only, a handler can compare the
client-reported Epic ID with a list. This does not authenticate the player: a
modified client can submit a listed ID. For an authorization whitelist, validate
`connection.ticket` with its issuer before admitting the player.

The example below performs only the client-reported ID filter:

```js
Events.on("playerConnecting", async (connection) => {
if (!connection.epicId || !await whitelist.containsEpicAccount(connection.epicId)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This snippet will be copied into real auth whitelists regardless of the caveat above. Consider making the primary example the ticket check, and keeping the Epic ID filter as a short secondary note.

connection.reject("Your Epic account is not on the whitelist.");
}
});
```

Comment thread
Kheartz marked this conversation as resolved.
---

## 5. Passing a ticket
Expand Down
Loading