Encrypted Online Rock Paper Scissors Game

Question

I made an RPS Game with python with the Crypto and socket modules.

I would like you to look at the code and tell me if anything can be improved, and if there are mistakes, etc.

The game is working like this, the server is setting up the socket connection, and then listening for 2 clients at least, when there are 2 clients in the session, they need to choose a nickname in order to play. Once they choose, the need to choice R/P/S. The server sends to the clients his public key, they are encrypting their choice and sending it back to the server. The server decrypt their choice and calculating the result. The result will sent to both clients.

Note* - Only the clients's choice is Encrypted, there is no need to encrypt all the other data over the network.

Server:

import socket
from Crypto.PublicKey import RSA
from Crypto.Cipher import PKCS1_OAEP
from time import sleep

def rules(first_choice, second_choice, mem1, mem2) -> str:
    first_choice = first_choice.decode(); second_choice = second_choice.decode()
    if (first_choice == 'R' and second_choice == 'P' 
        or first_choice == 'P' and second_choice == 'S'
            or first_choice == 'S' and second_choice == 'R'):
                return f'\nResult: {mem2} Won\n{mem1} Choice - {first_choice}\n{mem2} Choice - {second_choice}'
    elif (first_choice == 'S' and second_choice == 'S'
           or first_choice == 'P' and second_choice == 'P'
             or first_choice == 'R' and second_choice == 'R'):
                return f'\nResult: Tie!\n{mem1} Choice - {first_choice}\n{mem2} Choice - {second_choice}'
    else:
        return f'\nResult: {mem1} Won!\n{mem1} Choice - {first_choice}\n{mem2} Choice - {second_choice}'

class Connect:
    def __init__(self):
        players = 0
        self.prikey = RSA.generate(1024)
        self.pubkey = self.prikey.publickey()
        self.token = PKCS1_OAEP.new(self.prikey)
        with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock:
            sock.bind(('0.0.0.0', 21523))
            sock.listen(2)
            print('Waiting for at least 2 players, please wait.')
            while True:
                self.conn, self.addr = sock.accept()
                players += 1
                if players == 1:
                    self.nick1 = self.conn.recv(1024).decode()
                    print(f'Player 1 is {self.nick1}')
                    self.player1_conn = self.conn
                elif players == 2:
                    self.nick2 = self.conn.recv(1024).decode()
                    print(f'Player 2 is {self.nick2}')
                    self.player2_conn = self.conn
                    break
            self.connection()

    def connection(self) -> None:
        print('2 Players have joined, starting game in 5 seconds.\n')
        sleep(5)
        self.player1_conn.send('Y'.encode())
        self.player2_conn.send('Y'.encode())
        self.game_play()

    def game_play(self) -> None:
        self.player1_conn.send(self.pubkey.exportKey())
        self.player2_conn.send(self.pubkey.exportKey())
        choice_1_cipher = self.player1_conn.recv(1024)
        choice_1_plain = self.token.decrypt(choice_1_cipher)
        print('Got first choice, waiting for another choice..')
        choice_2_cipher = self.player2_conn.recv(1024)
        choice_2_plain = self.token.decrypt(choice_2_cipher)
        print('Got second answer, calculating winner!')
        print(rules(choice_1_plain, choice_2_plain, self.nick1, self.nick2))
        self.player1_conn.send(f'{rules(choice_1_plain, choice_2_plain, self.nick1, self.nick2)}'.encode())
        self.player2_conn.send(f'{rules(choice_1_plain, choice_2_plain, self.nick1, self.nick2)}'.encode())
        exit(0)

if __name__ == '__main__':
    Connect()

Client:

import socket
from time import sleep
from Crypto.Cipher import PKCS1_OAEP
from Crypto.PublicKey import RSA

nickname = input('Enter Nickname: ').encode()

class Client:
    def __init__(self):
        self.prikey = RSA.generate(2048)
        self.pubkey = self.prikey.publickey()
        with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as self.sock:
            self.sock.connect(('10.0.0.42', 21523))
            self.sock.send(nickname)
            data = self.sock.recv(1024).decode()
            if data == 'Y':
                self.start_game()

    def start_game(self) -> None:
        print('\n [R]ock | [P]aper | [S]cissors - ')
        while True:
            my_choice = input().upper()
            if my_choice not in ['R', 'P', 'S']:
                print('Invalid Input, input must be one of those R\\P\\S')
            else:
                user_pubkey = RSA.importKey(self.sock.recv(2048))
                token = PKCS1_OAEP.new(user_pubkey)
                cipher_choice = token.encrypt(my_choice.encode())
                self.sock.send(cipher_choice)
                break
        self.recv_and_exit()

    def recv_and_exit(self):
        print(self.sock.recv(1024).decode()) # Result
        sleep(5)
        exit(0)

if __name__ == '__main__':
    try:
        Client()
    except Exception as e:
        print(f'err: {e}')
    except KeyboardInterrupt:
        print('A player has pressed [Ctrl + C] to quit the game, game ended!')

Answer 1

I am not familiar with the modules you use and will not comment on functional correctness. However, I will try to give some suggestions on improving the design of your code.

Your main methods makes some non-intuitive use of Object constructors (which should be deterministic in general). I would expect something more like this:

if __name__ == '__main__':
  rpsHost = RPSHost()
  rpsResult = rpsHost.host()
  rpsResult.print()

(I also introduced a result object, which could take over the text output that has crept into your rules() method. (A method should do one thing, do it well, do it only))

Your Client main method knows some very specific stuff about Exception handling (again, do one thing...). That should be moved down into the Client class, and any feedback could be packed into a result object. Some more non-intuitive behavior is the setting of the nickname, which is done once your module is loaded. I would move that into the game client or pass it as a constructor argument, if it really needs to stay at top-level.

if __name__ == '__main__':
  rpsClient = RPSClient()
  rpsResult = RPSClient.play()  # <- select nickname in here
  rpsResult.print()

That's about as much time as I have. Other things that I would change are:

• The phases of the game are chained method calls. A master method should call them, one after the other.
• Game hosting issues (i.e. exchanged symbols, waiting for players) are mixed with Socket/RSA issues. I would move connections issues into an RSPConnection class, and operate the game like this:

rspConnection.wait_for_2_players()
rspConnection.send_to_player_1('Y')
player_1_message = rspConnection.get_message_from_player_1()

If you have time for an exercise in TDD, I would recommend writing some acceptance tests for your current game (in pytest for instance) and then refactor it. It's a great method for improving your code, since you know immediately when you're breaking the core behavior.