Do not store proper objects in session data

Bug: T233146
Change-Id: I2f75261b276993d27f6c96e066ea7769cf7fc082

i18n/en.json

@@ -73,5 +73,6 @@
 	"oathauth-disable-method-warning": "If you disable $1 two-factor authentication method, you will no longer be able to use this method when logging in, and all the data associated with this authentication method will be deleted",
 	"oathauth-switch-method-warning-header": "Confirm switching to a different authentication method",
 	"oathauth-switch-method-warning": "By switching to $2 two-factor authentication method, current method ($1) will be disabled, and all the data associated with the current authentication method will be deleted",
-	"oathauth-totp-disable-warning": "You will no longer be able to use the authentication device registered with this account. All scratch-tokens associated with this account will be invalidated."
+	"oathauth-totp-disable-warning": "You will no longer be able to use the authentication device registered with this account. All scratch-tokens associated with this account will be invalidated.",
+	"oathauth-invalidrequest": "Invalid request"
 }

i18n/qqq.json

@@ -84,5 +84,6 @@
 	"oathauth-disable-method-warning": "Generic message warning the user of token/data loss when authentication method is disabled.\n$1 - Current method name",
 	"oathauth-switch-method-warning-header": "Page title for warning page when switching to an alternative 2FA method",
 	"oathauth-switch-method-warning": "Generic message warning the user of token/data loss when switching to an alternative method.\n$1 - Current method name, $2 - Name of the method that is being switched to",
-	"oathauth-totp-disable-warning": "TOTP specific warning message when disabling/switching to alternative 2FA method"
+	"oathauth-totp-disable-warning": "TOTP specific warning message when disabling/switching to alternative 2FA method",
+	"oathauth-invalidrequest": "Generic error message that is displayed when request cannot be processed due to an unpredicted reason"
 }

src/HTMLForm/TOTPEnableForm.php

@@ -27,11 +27,14 @@ class TOTPEnableForm extends OATHAuthOOUIHTMLForm implements IManageForm {
 	}
 
 	protected function getDescriptors() {
-		$key = $this->getRequest()->getSessionData( 'oathauth_totp_key' );
-
+		$keyData = $this->getRequest()->getSessionData( 'oathauth_totp_key' ) ?? [];
+		$key = TOTPKey::newFromArray( $keyData );
 		if ( !$key instanceof TOTPKey ) {
 			$key = TOTPKey::newFromRandom();
-			$this->getRequest()->setSessionData( 'oathauth_totp_key', $key );
+			$this->getRequest()->setSessionData(
+				'oathauth_totp_key',
+				$key->jsonSerialize()
+			);
 		}
 
 		$secret = $key->getSecret();
@@ -149,7 +152,11 @@ class TOTPEnableForm extends OATHAuthOOUIHTMLForm implements IManageForm {
 	 * @throws \MWException
 	 */
 	public function onSubmit( array $formData ) {
-		$key = $this->getRequest()->getSessionData( 'oathauth_totp_key' );
+		$keyData = $this->getRequest()->getSessionData( 'oathauth_totp_key' ) ?? [];
+		$key = TOTPKey::newFromArray( $keyData );
+		if ( !$key instanceof TOTPKey ) {
+			return [ 'oathauth-invalidrequest' ];
+		}
 
 		if ( $key->isScratchToken( $formData['token'] ) ) {
 			// A scratch token is not allowed for enrollment

src/IAuthKey.php

@@ -3,8 +3,9 @@
 namespace MediaWiki\Extension\OATHAuth;
 
 use stdClass;
+use JsonSerializable;
 
-interface IAuthKey {
+interface IAuthKey extends JsonSerializable {
 
 	/**
 	 * @param array|stdClass $data
@@ -12,4 +13,5 @@ interface IAuthKey {
 	 * @return mixed
 	 */
 	public function verify( $data, OATHUser $user );
+
 }

src/Key/TOTPKey.php

@@ -55,7 +55,7 @@ class TOTPKey implements IAuthKey {
 	private $secret;
 
 	/** @var string[] List of scratch tokens */
-	private $scratchTokens;
+	private $scratchTokens = [];
 
 	/**
 	 * @return TOTPKey
@@ -72,6 +72,31 @@ class TOTPKey implements IAuthKey {
 		return $object;
 	}
 
+	/**
+	 * Create key from json encoded string
+	 *
+	 * @param string $data
+	 * @return TOTPKey|null on invalid data
+	 */
+	public static function newFromString( $data ) {
+		$data = json_decode( $data, true );
+		if ( json_last_error() !== JSON_ERROR_NONE ) {
+			return null;
+		}
+		return static::newFromArray( $data );
+	}
+
+	/**
+	 * @param array $data
+	 * @return TOTPKey|null on invalid data
+	 */
+	public static function newFromArray( array $data ) {
+		if ( !isset( $data['secret'] ) || !isset( $data['scratch_tokens'] ) ) {
+			return null;
+		}
+		return new static( $data['secret'], $data['scratch_tokens'] );
+	}
+
 	/**
 	 * @param string $secret
 	 * @param array $scratchTokens
@@ -227,4 +252,11 @@ class TOTPKey implements IAuthKey {
 	private function getLogger() {
 		return LoggerFactory::getInstance( 'authentication' );
 	}
+
+	public function jsonSerialize() {
+		return [
+			'secret' => $this->getSecret(),
+			'scratch_tokens' => $this->getScratchTokens()
+		];
+	}
 }

src/Module/TOTP.php

@@ -42,10 +42,11 @@ class TOTP implements IModule {
 		if ( !isset( $data['secret'] ) || !isset( $data['scratch_tokens'] ) ) {
 			throw new MWException( 'oathauth-invalid-data-format' );
 		}
-		return new TOTPKey(
-			$data['secret'],
-			explode( ',', $data['scratch_tokens'] )
-		);
+		if ( is_string( $data['scratch_tokens' ] ) ) {
+			$data['scratch_tokens'] = explode( ',', $data['scratch_tokens'] );
+		}
+
+		return TOTPKey::newFromArray( $data );
 	}
 
 	/**
@@ -59,10 +60,7 @@ class TOTP implements IModule {
 			throw new MWException( 'oathauth-invalid-key-type' );
 		}
 		return [
-			'keys' => [ [
-				'secret' => $key->getSecret(),
-				'scratch_tokens' => implode( ',', $key->getScratchTokens() ),
-			] ]
+			'keys' => [ $key->jsonSerialize() ]
 		];
 	}