TCP server with user accounts, connection threading, and groups/permissions

Question

This is a TCP server I am currently working on. It is accessed through telnet (I know it isn't secure, its just a project for uni). The server is programmed in Python 3.

I am currently trying to figure out how to implement thread locking in my program, how to create user sessions, and the best way to implement a set of commands with authorization and group checking. Each user is assigned a group (either member or admin). I want to be able to put the commands in a dictionary or class (not sure which would be best). I also don't know how I will remove the connection thread from the dictionary when the person leaves. I am open to all constructive criticism. Tell me what I can improve on. I am very new to coding (couple months in Python and C).

To test the server yourself, run the Python script and telnet into your internal IP address.

# TCP Server for user database

# Import modules
from threading import Thread
from datetime import datetime
import socket

# Global variables
host = socket.gethostbyname(socket.gethostname()) # Host
port = 23                                         # Port
addr = (host, port)                               # Tuple address
form_addr = host + ":" + str(port)                # Formatted Address
buff = 2048                                       # Buffer rate

connections = {}
users = {}

# Function to validate forms - Have not used it yet
def validate(form_names, forms, min_len=0, max_len=250):
    for form in forms and form_name in form_names:
        if form >= min_len and form <= max_len:
            if form_name == "username":
                return form in users
            return True
        return False

# User class to store individual data for users
class User():
    def __init__(self, password, group, join_date):
        self.password = password
        self.group = group
        self.join_date = join_date


# Threaded connections - Gives each connection its own thread
class Connection(Thread):
    def __init__(self, conn, conn_addr):
        Thread.__init__(self)
        self.conn = conn                     # Connection object
        self.conn_addr = conn_addr           # Connection address (string)

    def send(self, message):                 # Sends data to the client
        try:
            self.conn.send(message.encode())
        except UnicodeEncodeError as e:
            print(str(e))

    def recieve(self, form_name=""):         # Recieves data from the client
        self.send(form_name + " -> ")
        try:
            return self.conn.recv(buff).decode()
        except UnicodeDecodeError:
            self.send("Type exit exit")

    def register(self):
        username = self.recieve(form_name="New Username")
        password = self.recieve(form_name="New Password")

        # Assign new user to object
        users[username] = \
        User(password, "Member", 
             datetime.now().strftime("%m/%d/%Y"))

    def login(self):
        username = self.recieve(form_name="Username")
        password = self.recieve(form_name="Password")

        # Perform check
        if username in users:
            if users[username].password == password:
                self.send("Successful login")  # Haven't figured out what to do here yet

    def run(self):
        self.send("Welcome to the server\n")   # Runs when the thread initializes
        while True:
            command = self.recieve().split()
            if not command:  # Checks for user connection loss
                break

# Create socket instance
sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)

# Bind to local ip/port
print("\nStarting server on " + form_addr)
try:
    sock.bind(addr)
except socket.error as e:  # Check for port binding error
    print(str(e))

# Listen and accept incoming connections
sock.listen(3)

def listener():
    conn, conn_addr = sock.accept()     # Waits for incoming connection
    conn_addr = conn_addr[0] + ":" + \
                str(conn_addr[1])

    print("Connection from client: " + conn_addr + "\n")

    connections[conn_addr] = Connection(conn, conn_addr) # Initiates connection thread
    connections[conn_addr].start() # Starts run sequence
    # After here the listener goes back to wait for multiple connections

Thread(target=listener).start()

while True:
    command = input("Server command: ")  # Nothing here yet, work in progress

Answer 1

There are a few problems with your validate function. First, you have a comment that would be a docstring if it contained useful infromation. It's already clear its' a function that validates, and since it takes the parameters form_names and forms we can guess what it validates. Why not explain what it validates. Then put it as a docstring

def validate(form_names, forms, min_len=0, max_len=250):
    """Validates if forms are correctly fooed and bared"""

    for form in forms and form_name in form_names:
        if form >= min_len and form <= max_len:
            if form_name == "username":
                return form in users
            return True
        return False

I didn't even know for form in forms and form_name in form_names was valid syntax. It doesn't work in Python 2.7, but perhaps it's in 3.x. Either way, it's less readable than using the zip function.

    for form, form_name in zip(forms, form_names):

zip just creates a list of tuples from each element of both lists. It will give values to both form and form_name simultaneously.

Also you can compress the form length check into one comparison because Python allows multiple operators in one expression:

        if min_len <= form <= max_len:

However, even still one thing confuses me here. In Every outcome you're returning a value on the first iteration. I'm unsure if you didn't realise that or you misunderstand return values. return will always end the function instantly, meaning you wont iterate over any of the other values in forms and form_names. This makes the for loop redundant as you could just check the first element of the list, but I presume this isn't what you want.

I suspect what you want is to return False if any of the forms don't fit the length parameters or if the username doesn't exist in users. In this case, you don't want to return True until the whole for loop has run without catching a condition that returns False.

If that assumption is correct, here's how I could write this:

def validate(form_names, forms, min_len=0, max_len=250):
    """Validates forms are valid lengths and username exists"""

    for form, form_names in zip(forms, form_names):
        if not min_len <= form <= max_len:
            return False
            if form_name == "username" and form not in users:
                return False
    return True

As you can see, we're now focused on checking for the False conditions, and if we leave the loop without coming across any invalidating conditions we know we can just return True.