2
\$\begingroup\$

I am trying to make a basic auth password protected folder for Joomla. This script will simply ask for username and password to secure administrator folder in Joomla and creates .htaccess and .pass_file.

I want you to review it and offer suggestions so that I can do it more efficiently and learn to manage code.

#!/usr/bin/env python
import os
import pwd
import sys

def ok():
    os.chdir("/home/" + username);
    os.system('htpasswd -c .pass_file '+ user); 
    os.chdir("/home/" + username + "/public_html/administrator/");
    os.system('echo AuthUserFile /home/' + username + '/.pass_file > .htaccess');
    os.system('echo AuthName Restricted >> .htaccess');
    os.system('echo AuthType Basic >> .htaccess');
    os.system('echo require valid-user >> .htaccess');      
    print "=" *40;  
    quit(); 

username = raw_input("Enter your cpanel username: ");
try:
    pwd.getpwnam(username);
except KeyError:
    print "User not found.";
    quit();
print "=" * 40;
user = raw_input("Enter the username you want to create for authentication: ");
print "=" * 40;
print "Your username will be "+ user +" for authentication";
print "=" * 40;
filename="/home/"+username+"/public_html/administrator/.htaccess";
if os.path.isfile(filename):
    yes = set(['yes','y', 'ye', ''])
    no = set(['no','n'])
    response = raw_input("Filename exists. Overwrite??");
    ans = response.lower();
    if ans in no:
        print "Sorry we didn't secure the administrator folder. :(";
        quit();
    elif ans in yes:
        ok();
    else:
        sys.stdout.write("Please respond with 'yes' or 'no'. Quiting...");  
quit();
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

My suggestions would be:

  • Rewrite ok() using python I/O rather than running echo in a subprocess. We're passing in arguments because they are no longer going to be global variables after some of the edits below.

    def ok(passfile, htaccess, user):
        os.system("htpasswd -c %s %s" % (passfile, user))
        with open(htaccess, 'w') as f:
            f.write('AuthUserFile %s\n' % (passfile))
            f.write('AuthName Restricted\n')
            f.write('AuthType Basic\n')
            f.write('require valid-user\n')
        print "=" *40
        quit()
    
  • Consider using the python module https://pypi.python.org/pypi/htpasswd instead of calling htpasswd in a subprocess. For example:

    import htpasswd
    
    with htpasswd.Basic("/path/to/user.db") as userdb:
    try:
        userdb.add("bob", "password")
    except htpasswd.basic.UserExists, e:
        print e
    
  • Take all the top level lines outside any function and put them in a main() routine at the top of the file (after the imports). Like so (I'm also doing some tweaks here to make things more portable and pythonic):

    #!/usr/bin/env python
    import os
    import pwd
    import sys
    
    def main(args):
        username = raw_input("Enter your cpanel username: ");
        try:
            pwd.getpwnam(username);
        except KeyError:
            print "User not found.";
            quit();
        print "=" * 40;
        user = raw_input("Enter the username you want to create for authentication: ");
        print "=" * 40;
        print "Your username will be "+ user +" for authentication";
        print "=" * 40;
    
        # Note we're building up our file paths without ever using '/' or 
        # '\'. By letting python build our paths for us, they'll be portable
        homedir = os.getenv("HOME")
        passfile = os.path.join(homedir, ".pass_file")
        htaccess = os.path.join(homedir, "public_html", "administrator", ".htaccess")
    
        # If htaccess already exists, we ask user whether to overwrite. 
        # If the user says "no", we say sorry and call quit().
        # If the user says "yes", we break out of the loop and call ok().
        # Otherwise, we keep telling the user to answer 'yes' or 'no' 
        # until they do.
        #
        # If htaccess does not exist, I think we want to create it. The 
        # original code did not address this case.
        if os.path.isfile(htaccess):
            while True
                ans = raw_input("File %s exists. Overwrite??" % htaccess);
                # re.findall() will return a list of regexp matches. If there
                # are no matches, the list is empty and the if evaluates to 
                # False. Note the regexp handles mixed case and abbreviations.
                if re.findall('[Nn][Oo]?', ans):
                    print "Sorry we didn't secure the administrator folder. :(";
                    quit();
                elif re.findall('[Yy]?[Ee]?[Ss]?', ans):
                    break
                else:
                    sys.stdout.write("Please respond with 'yes' or 'no'.");  
        ok(passfile, htaccess, user)  
        quit();
    

    Then at the bottom of the file,

    if __name__ == '__main__':
        main(sys.argv)
    

    By using a main() routine and calling it from the bottom of the file if __name__ contains the string '__main__', you can import this file into other files and reuse the code by calling the functions. With lines at the top level of the file, outside functions, those lines would get run if you ever imported the file. Following this convention consistently makes debugging and code reuse much easier.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.