From 02efa93e3a8927738c7e668e805841f0131362d4 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sun, 23 Dec 2018 18:24:33 +0000 Subject: [PATCH] PlayerPreLoginEvent: New, more elegant way to control authentication requirement Previously the only way to deal with this was to cancel the PlayerKickEvent generated by lack of authentication. Now, plugins can decide whether auth should be required for a specific player. The default is whatever xbox-auth is set to in server.properties. cc @Johnmacrocraft --- src/pocketmine/Player.php | 11 +++++----- .../event/player/PlayerPreLoginEvent.php | 20 ++++++++++++++++++- .../network/mcpe/ProcessLoginTask.php | 7 +++++-- 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/src/pocketmine/Player.php b/src/pocketmine/Player.php index 56b8d4972..d00f4e2c6 100644 --- a/src/pocketmine/Player.php +++ b/src/pocketmine/Player.php @@ -1732,7 +1732,7 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{ $this->setSkin($packet->skin); - $ev = new PlayerPreLoginEvent($this, "Plugin reason"); + $ev = new PlayerPreLoginEvent($this, "Plugin reason", $this->server->requiresAuthentication()); $ev->call(); if($ev->isCancelled()){ $this->close("", $ev->getKickMessage()); @@ -1756,16 +1756,16 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{ } if(!$packet->skipVerification){ - $this->server->getAsyncPool()->submitTask(new ProcessLoginTask($this, $packet, NetworkCipher::$ENABLED)); + $this->server->getAsyncPool()->submitTask(new ProcessLoginTask($this, $packet, $ev->isAuthRequired(), NetworkCipher::$ENABLED)); }else{ - $this->setAuthenticationStatus(true, null); + $this->setAuthenticationStatus(false, false, null); $this->networkSession->onLoginSuccess(); } return true; } - public function setAuthenticationStatus(bool $authenticated, ?string $error) : bool{ + public function setAuthenticationStatus(bool $authenticated, bool $authRequired, ?string $error) : bool{ if($this->networkSession === null){ return false; } @@ -1783,7 +1783,8 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{ $this->authenticated = $authenticated; if(!$this->authenticated){ - if($this->server->requiresAuthentication() and $this->kick("disconnectionScreen.notAuthenticated", false)){ //use kick to allow plugins to cancel this + if($authRequired){ + $this->close("", "disconnectionScreen.notAuthenticated"); return false; } diff --git a/src/pocketmine/event/player/PlayerPreLoginEvent.php b/src/pocketmine/event/player/PlayerPreLoginEvent.php index 6ad2ce22f..3ea25fb4d 100644 --- a/src/pocketmine/event/player/PlayerPreLoginEvent.php +++ b/src/pocketmine/event/player/PlayerPreLoginEvent.php @@ -42,14 +42,18 @@ use pocketmine\Player; class PlayerPreLoginEvent extends PlayerEvent implements Cancellable{ /** @var string */ protected $kickMessage; + /** @var bool */ + protected $authRequired; /** * @param Player $player * @param string $kickMessage + * @param bool $authRequired */ - public function __construct(Player $player, string $kickMessage){ + public function __construct(Player $player, string $kickMessage, bool $authRequired){ $this->player = $player; $this->kickMessage = $kickMessage; + $this->authRequired = $authRequired; } /** @@ -65,4 +69,18 @@ class PlayerPreLoginEvent extends PlayerEvent implements Cancellable{ public function getKickMessage() : string{ return $this->kickMessage; } + + /** + * @return bool + */ + public function isAuthRequired() : bool{ + return $this->authRequired; + } + + /** + * @param bool $v + */ + public function setAuthRequired(bool $v) : void{ + $this->authRequired = $v; + } } diff --git a/src/pocketmine/network/mcpe/ProcessLoginTask.php b/src/pocketmine/network/mcpe/ProcessLoginTask.php index c130e93c0..89460145d 100644 --- a/src/pocketmine/network/mcpe/ProcessLoginTask.php +++ b/src/pocketmine/network/mcpe/ProcessLoginTask.php @@ -59,6 +59,8 @@ class ProcessLoginTask extends AsyncTask{ * root public key. */ private $authenticated = false; + /** @var bool */ + private $authRequired; /** * @var bool @@ -74,9 +76,10 @@ class ProcessLoginTask extends AsyncTask{ /** @var string|null */ private $handshakeJwt = null; - public function __construct(Player $player, LoginPacket $packet, bool $useEncryption = true){ + public function __construct(Player $player, LoginPacket $packet, bool $authRequired, bool $useEncryption = true){ $this->storeLocal($player); $this->packet = $packet; + $this->authRequired = $authRequired; $this->useEncryption = $useEncryption; if($useEncryption){ if(self::$SERVER_PRIVATE_KEY === null){ @@ -220,7 +223,7 @@ class ProcessLoginTask extends AsyncTask{ $player = $this->fetchLocal(); if(!$player->isConnected()){ $this->worker->getLogger()->error("Player " . $player->getName() . " was disconnected before their login could be verified"); - }elseif($player->setAuthenticationStatus($this->authenticated, $this->error)){ + }elseif($player->setAuthenticationStatus($this->authenticated, $this->authRequired, $this->error)){ if(!$this->useEncryption){ $player->getNetworkSession()->onLoginSuccess(); }else{