Change /modlogin to only inherit rights, hide them better in !players

Fixes #673

Author: RussellLVP

data/server/callbacks.txt

@@ -9,10 +9,11 @@ forward OnPlayerActivityChange(playerid, activity);
 [Deferred] forward OnPlayerChecksumAvailable(playerid, address, checksum);
 forward OnPlayerLeaveActivity(playerid);
 forward OnPlayerLevelChange(playerid, newlevel, temporary);
-forward OnPlayerLogin(playerid, userid, vip, gangid, undercover);
+forward OnPlayerLogin(playerid, userid, vip, gangid);
 [Deferred] forward OnPlayerResolvedDeath(playerid, killerid, reason);
 forward OnSetiOwnershipChange(playerid);
 forward OnPlayerGuestLogin(playerId);
+forward OnPlayerModLogin(playerid, level, vip);
 
 # Streamer callbacks
 [Deferred] forward OnDynamicObjectMoved(objectid);

javascript/entities/player_manager.js

@@ -220,11 +220,25 @@ export class PlayerManager {
 
         player.userId = event.userid;
         player.setVip(!!event.vip);
-        player.setUndercover(!!event.undercover);
 
         this.notifyObservers('onPlayerLogin', player, event);
     }
 
+    // Called when a player logs in to their account whilst undercover. This will mark them as such,
+    // without overriding their account information to protect their identity.
+    onPlayerModLogin(event) {
+        const player = this.players_.get(event.playerid);
+        if (!player)
+            return;  // the event has been received for an invalid player
+
+        player.level = event.level;
+
+        player.setUndercover(true);
+        player.setVip(!!event.vip);
+
+        this.notifyObservers('onPlayerModLogin', player, event);
+    }
+
     // Called when a player has selected an object.
     onPlayerSelectObject(event) {
         const player = this.players_.get(event.playerid);

javascript/entities/test/mock_player.js

@@ -405,7 +405,7 @@ export class MockPlayer extends Player {
     // ---------------------------------------------------------------------------------------------
 
     // Identifies the player to a fake account. The options can be specified optionally.
-    async identify({ userId = 42, vip = 0, gangId = 0, undercover = 0 } = {}) {
+    async identify({ userId = 42, level = null, vip = 0, gangId = 0, undercover = 0 } = {}) {
         let resolver = null;
 
         const observerPromise = new Promise(resolve => resolver = resolve);
@@ -414,15 +414,28 @@ export class MockPlayer extends Player {
                 server.playerManager.removeObserver(observer);
                 resolver();
             }
+
+            onPlayerModLogin(player) {
+                server.playerManager.removeObserver(observer);
+                resolver();
+            }
         };
 
         server.playerManager.addObserver(observer);
-        dispatchEvent('playerlogin', {
-            playerid: this.id,
-            userid: userId,
-            gangid: gangId,
-            undercover, vip,
-        });
+        if (undercover) {
+            dispatchEvent('playermodlogin', {
+                playerid: this.id,
+                level: level ?? Player.LEVEL_PLAYER,
+                vip
+            });
+        } else {
+            dispatchEvent('playerlogin', {
+                playerid: this.id,
+                userid: userId,
+                gangid: gangId,
+                undercover, vip,
+            });
+        }
 
         await observerPromise;
     }

javascript/features/account/account_nuwani_commands.js

@@ -332,10 +332,10 @@ export class AccountNuwaniCommands {
 
             const name = player.name;
             const registered = player.account.isRegistered();
-            const vip = player.isVip();
+            const vip = player.account.isVip();
             const minimized = isPlayerMinimized(player.id);
             const temporary = player.isTemporaryAdministrator();
-            const level = player.isUndercover() ? Player.LEVEL_PLAYER
+            const level = player.isUndercover() ? player.account.level
                                                 : player.level;
 
             players.push({ name, registered, vip, temporary, level, minimized });

javascript/features/account_provider/account_manager.js

@@ -20,6 +20,8 @@ export class AccountManager {
             'playerlogin', AccountManager.prototype.onPlayerLoginEvent.bind(this));
         this.callbacks_.addEventListener(
             'playerguestlogin', AccountManager.prototype.onPlayerGuestLoginEvent.bind(this));
+        this.callbacks_.addEventListener(
+            'playermodlogin', AccountManager.prototype.onPlayerModLoginEvent.bind(this));
 
         provideNative('SetIsRegistered', 'ii', AccountManager.prototype.setIsRegistered.bind(this));
 
@@ -87,6 +89,19 @@ export class AccountManager {
         server.playerManager.onPlayerGuestSession(player);
     }
 
+    // Called when a player has logged in to an administrator's account, and their original level
+    // and rights should be applied. Statistics will continue to aggregate as the other account.
+    onPlayerModLoginEvent(event) {
+        const player = server.playerManager.getById(event.playerid);
+        if (!player)
+            return;  // the |player| does not exist (anymore)
+
+        // TODO: Mutate the |PlayerAccountSupplement| with the appropriate data when that's made
+        // responsible for account information, including for undercover players.
+
+        server.playerManager.onPlayerModLogin(event);
+    }
+
     // Called when the |player| has disconnected from the server.
     onPlayerDisconnect(player) {
         if (!player.account.isIdentified())

javascript/features/account_provider/account_manager.test.js

@@ -20,4 +20,36 @@ describe('AccountManager', (it, beforeEach) => {
 
         assert.notEqual(gunther.name, 'Gunther');
     });
+
+    it('is able to log in players undercover, override their rights', async (assert) => {
+        const gunther = server.playerManager.getById(/* Gunther= */ 0);
+
+        assert.equal(gunther.level, Player.LEVEL_PLAYER);
+        assert.isFalse(gunther.isVip());
+        assert.isFalse(gunther.isUndercover());
+        assert.isFalse(gunther.account.isRegistered());
+        assert.isFalse(gunther.account.isIdentified());
+
+        await gunther.identify({ userId: 42 });
+
+        assert.equal(gunther.level, Player.LEVEL_PLAYER);
+        assert.isFalse(gunther.isVip());
+        assert.isFalse(gunther.isUndercover());
+        assert.isTrue(gunther.account.isRegistered());
+        assert.isTrue(gunther.account.isIdentified());
+        assert.equal(gunther.account.userId, 42);
+
+        dispatchEvent('playermodlogin', {
+            playerid: gunther.id,
+            level: Player.LEVEL_ADMINISTRATOR,
+            vip: true,
+        });
+
+        assert.equal(gunther.level, Player.LEVEL_ADMINISTRATOR);
+        assert.isTrue(gunther.isVip());
+        assert.isTrue(gunther.isUndercover());
+        assert.isTrue(gunther.account.isRegistered());
+        assert.isTrue(gunther.account.isIdentified());
+        assert.equal(gunther.account.userId, 42);
+    });
 });

javascript/features/account_provider/account_provider_database.js

@@ -6,6 +6,8 @@
 const ACCOUNT_LOAD_QUERY = `
     SELECT
         users_mutable.user_id,
+        users.level,
+        users.is_vip,
         users_mutable.online_time,
         users_mutable.kill_count,
         users_mutable.death_count,
@@ -20,6 +22,8 @@ const ACCOUNT_LOAD_QUERY = `
         users_mutable.muted
     FROM
         users_mutable
+    LEFT JOIN
+        users ON users.user_id = users_mutable.user_id
     WHERE
         users_mutable.user_id = ?`;
 

javascript/features/account_provider/player_account_supplement.js

@@ -4,6 +4,18 @@
 
 import { Supplement } from 'base/supplementable.js';
 
+// Converts the database-bound level string to a Player.LEVEL_* constant.
+function toPlayerLevel(level) {
+    switch (level) {
+        case 'Management':
+            return Player.LEVEL_MANAGEMENT;
+        case 'Administrator':
+            return Player.LEVEL_ADMINISTRATOR;
+    }
+
+    return Player.LEVEL_PLAYER;
+}
+
 // Supplements the Player object with an `account` accessor, giving other features access to the
 // information associated with a player's account. This supplement will be created for players who
 // are registered on the server, as well as players who are visiting us as a guest.
@@ -14,6 +26,9 @@ export class PlayerAccountSupplement extends Supplement {
     hasRequestedUpdate_ = false;
 
     userId_ = null;
+    level_ = null;
+    isVip_ = false;
+
     bankAccountBalance_ = 0;
     cashBalance_ = 0;
     reactionTests_ = 0;
@@ -22,6 +37,12 @@ export class PlayerAccountSupplement extends Supplement {
     // Gets the permanent user Id that has been assigned to this user. Read-only.
     get userId() { return this.userId_; }
 
+    // Gets the level of this player as associated with their account. Read-only.
+    get level() { return this.level_; }
+
+    // Gets whether this player is a VIP, as set in their account. Read-only.
+    isVip() { return this.isVip_; }
+
     // Gets or sets the balance this user has on their bank account. Writes will be processed as
     // high priority, because 
     get bankAccountBalance() { return this.bankAccountBalance_; }
@@ -58,6 +79,9 @@ export class PlayerAccountSupplement extends Supplement {
     // data transformations to make the data types appropriate for JavaScript. (E.g. colours.)
     initializeFromDatabase(player, databaseRow) {
         this.userId_ = databaseRow.user_id;
+        this.level_ = toPlayerLevel(databaseRow.level);
+        this.isVip_ = !!databaseRow.is_vip;
+
         this.bankAccountBalance_ = databaseRow.money_bank;
         this.cashBalance_ = databaseRow.money_cash;
         this.reactionTests_ = databaseRow.stats_reaction;

javascript/features/account_provider/test/mock_account_provider_database.js

@@ -11,6 +11,8 @@ export class MockAccountProviderDatabase extends AccountProviderDatabase {
     async loadAccountData(userId) {
         return {
             user_id: userId,
+            level: 'Player',
+            is_vip: 0,
             online_time: 0,
             kill_count: 0,
             death_count: 0,

pawn/Features/Account/Account.pwn

@@ -183,7 +183,7 @@ class Account <playerId (MAX_PLAYERS)> {
         Annotation::ExpandList<OnPlayerLogin>(playerId);
 
         // Broadcast an OnPlayerLogin callback that can be intercepted by other scripts.
-        CallRemoteFunction("OnPlayerLogin", "iiiii", playerId, m_userId, Player(playerId)->isVip(), AccountData(playerId)->gangId(), 0 /* undercover */);
+        CallRemoteFunction("OnPlayerLogin", "iiii", playerId, m_userId, Player(playerId)->isVip(), AccountData(playerId)->gangId());
     }
 
     /**
@@ -244,6 +244,7 @@ class Account <playerId (MAX_PLAYERS)> {
      */
     public onSuccessfulModLoginAttempt(PlayerAccessLevel: level, originalUsername[], originalUserId) {
         AccountData(playerId)->applyPlayerLevel(level);
+
         UndercoverAdministrator(playerId)->setIsUndercoverAdministrator(true);
         UndercoverAdministrator(playerId)->resetUndercoverLoginAttemptCount();
         UndercoverAdministrator(playerId)->setOriginalUsername(originalUsername);
@@ -260,12 +261,10 @@ class Account <playerId (MAX_PLAYERS)> {
 
         EchoMessage("notice-crew", "z", notice);
 
-        // Broadcast an OnPlayerLogin callback that can be intercepted by other scripts.
-        CallRemoteFunction("OnPlayerLogin", "iiiii", playerId, originalUserId, Player(playerId)->isVip(), AccountData(playerId)->gangId(), 1 /* undercover */);
+        // Broadcast an OnPlayerModLogin callback that can be intercepted by other scripts.
+        CallRemoteFunction("OnPlayerModLogin", "iii", playerId, _: level, Player(playerId)->isVip());
 
         Annotation::ExpandList<OnPlayerModLogin>(playerId);
-
-        // TODO(Russell): Should this broadcast an event similar to OnPlayerLogin as well?
     }
 
     /**
@@ -289,8 +288,11 @@ class Account <playerId (MAX_PLAYERS)> {
     }
 };
 
-forward OnPlayerLogin(playerid, userid, vip, gangId, undercover);
-public OnPlayerLogin(playerid, userid, vip, gangId, undercover) {}
+forward OnPlayerLogin(playerid, userid, vip, gangId);
+public OnPlayerLogin(playerid, userid, vip, gangId) {}
 
 forward OnPlayerGuestLogin(playerId);
 public OnPlayerGuestLogin(playerId) {}
+
+forward OnPlayerModLogin(playerid, level, vip);
+public OnPlayerModLogin(playerid, level, vip) {}