Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use XDG Base Directories #96

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+67 −22
Open

Use XDG Base Directories #96

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions protonvpn_cli/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
)
# Constants
from .constants import (
CONFIG_DIR, CONFIG_FILE, PASSFILE, USER, VERSION, SPLIT_TUNNEL_FILE
CONFIG_DIR, DATA_DIR, CACHE_DIR, CONFIG_FILE, PASSFILE, USER, VERSION, SPLIT_TUNNEL_FILE
)


Expand All @@ -80,13 +80,15 @@ def cli():
"""Run user's input command."""

# Initial log values
change_file_owner(os.path.join(CONFIG_DIR, "pvpn-cli.log"))
change_file_owner(os.path.join(CACHE_DIR, "pvpn-cli.log"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be LOG_DIR?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There currently isn't a LOG_DIR as there isn't one listed in the specification. As log files are not necessary to run the program they are placed into the CACHE_DIR. I could make a separate LOG_DIR and have that be the same as CACHE_DIR to make the code clearer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I just see that XDG standard is incomplete here: https://stackoverflow.com/questions/25897836/where-should-i-write-a-user-specific-log-file-to-and-be-xdg-base-directory-comp

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The specification does state the following

The $XDG_STATE_HOME contains state data that should persist between (application) restarts, but that is not important or portable enough to the user that it should be stored in $XDG_DATA_HOME. It may contain:

  • actions history (logs, history, recently used files, …)
  • [......]

This was was already stated in the stack overflow link above as well.

Admittedly I have not defined this environment variable in my own implementation. (I only have XDG_DATA_HOME and XDG_CONFIG_HOME environment variable defined).

While it may not be set in stone, I think the XDG Base Directories is the best and most widely accepted standard that we have. Implementing this will further encourage adoption, keep $HOME directories clean and easier to maintain. This change also is somewhat trivial and provides more value than the pain of adding it.

While I understand that this seems to scatter the various files needed by protonvpn_cli among the different directories, keep in mind that this is the case with system applications as well (/etc for config files, /var/log for logs, etc....). Having the $HOME organized thus does not increase its complexity. Rather it only improves upon organization facilitating automated maintenance/archival tasks.

logger.debug("###########################")
logger.debug("### NEW PROCESS STARTED ###")
logger.debug("###########################")
logger.debug(sys.argv)
logger.debug("USER: {0}".format(USER))
logger.debug("CONFIG_DIR: {0}".format(CONFIG_DIR))
logger.debug("DATA_DIR: {0}".format(DATA_DIR))
logger.debug("CACHE_DIR: {0}".format(CACHE_DIR))

args = docopt(__doc__, version="ProtonVPN-CLI v{0}".format(VERSION))
logger.debug("Arguments\n{0}".format(str(args).replace("\n", "")))
Expand Down Expand Up @@ -180,6 +182,16 @@ def init_config_file():
logger.debug("Config Directory created")
change_file_owner(CONFIG_DIR)

if not os.path.isdir(CACHE_DIR):
os.mkdir(CACHE_DIR)
logger.debug("Cache Directory created")
change_file_owner(CACHE_DIR)

if not os.path.isdir(DATA_DIR):
os.mkdir(DATA_DIR)
logger.debug("Data Directory created")
change_file_owner(DATA_DIR)

# Warn user about reinitialization
try:
if int(get_config_value("USER", "initialized")):
Expand Down
20 changes: 10 additions & 10 deletions protonvpn_cli/connection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
)
# Constants
from .constants import (
CONFIG_DIR, TEMPLATE_FILE, OVPN_FILE, PASSFILE, CONFIG_FILE
CACHE_DIR, DATA_DIR, TEMPLATE_FILE, OVPN_FILE, PASSFILE, CONFIG_FILE
)


Expand Down Expand Up @@ -361,7 +361,7 @@ def status():
if not is_connected():
logger.debug("Disconnected")
print("Status: Disconnected")
if os.path.isfile(os.path.join(CONFIG_DIR, "iptables.backup")):
if os.path.isfile(os.path.join(DATA_DIR, "iptables.backup")):
print("[!] Kill Switch is currently active.")
logger.debug("Kill Switch active while VPN disconnected")
else:
Expand Down Expand Up @@ -416,7 +416,7 @@ def status():
last_connection = get_config_value("metadata", "connected_time")
connection_time = time.time() - int(last_connection)

if os.path.isfile(os.path.join(CONFIG_DIR, "iptables.backup")):
if os.path.isfile(os.path.join(DATA_DIR, "iptables.backup")):
killswitch_on = True
else:
killswitch_on = False
Expand Down Expand Up @@ -471,7 +471,7 @@ def openvpn_connect(servername, protocol):

print("Connecting to {0} via {1}...".format(servername, protocol.upper()))

with open(os.path.join(CONFIG_DIR, "ovpn.log"), "w+") as f:
with open(os.path.join(CACHE_DIR, "ovpn.log"), "w+") as f:
subprocess.Popen(
[
"openvpn",
Expand All @@ -484,7 +484,7 @@ def openvpn_connect(servername, protocol):
logger.debug("OpenVPN process started")
time_start = time.time()

with open(os.path.join(CONFIG_DIR, "ovpn.log"), "r") as f:
with open(os.path.join(CACHE_DIR, "ovpn.log"), "r") as f:
while True:
content = f.read()
f.seek(0)
Expand Down Expand Up @@ -557,7 +557,7 @@ def manage_dns(mode, dns_server=False):
restore: Revert changes and restore original configuration
"""

backupfile = os.path.join(CONFIG_DIR, "resolv.conf.backup")
backupfile = os.path.join(DATA_DIR, "resolv.conf.backup")
resolvconf_path = os.path.realpath("/etc/resolv.conf")

if mode == "leak_protection":
Expand Down Expand Up @@ -642,8 +642,8 @@ def manage_ipv6(mode):
restore: Revert changes and restore original configuration.
"""

ipv6_backupfile = os.path.join(CONFIG_DIR, "ipv6.backup")
ip6tables_backupfile = os.path.join(CONFIG_DIR, "ip6tables.backup")
ipv6_backupfile = os.path.join(DATA_DIR, "ipv6.backup")
ip6tables_backupfile = os.path.join(DATA_DIR, "ip6tables.backup")

if mode == "disable":

Expand Down Expand Up @@ -773,7 +773,7 @@ def manage_killswitch(mode, proto=None, port=None):
reason this will completely block access to the internet.
"""

backupfile = os.path.join(CONFIG_DIR, "iptables.backup")
backupfile = os.path.join(DATA_DIR, "iptables.backup")

if mode == "restore":
logger.debug("Restoring iptables")
Expand All @@ -797,7 +797,7 @@ def manage_killswitch(mode, proto=None, port=None):
logger.debug("Kill Switch backup exists")
manage_killswitch("restore")

with open(os.path.join(CONFIG_DIR, "ovpn.log"), "r") as f:
with open(os.path.join(CACHE_DIR, "ovpn.log"), "r") as f:
content = f.read()
device = re.search(r"(TUN\/TAP device) (.+) opened", content)
if not device:
Expand Down
45 changes: 39 additions & 6 deletions protonvpn_cli/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,44 @@
except KeyError:
USER = getpass.getuser()

CONFIG_DIR = os.path.join(os.path.expanduser("~{0}".format(USER)), ".pvpn-cli")
# Ensure backwards compatibility
home_path = os.path.expanduser("~{0}".format(USER))
home_config = os.path.join(home_path, ".pvpn-cli")
if os.path.exists(home_config):
CONFIG_DIR = home_config
DATA_DIR = home_config
CACHE_DIR = home_config

else:
# Get the config directory
xdg_config_str = os.getenv("XDG_CONFIG_HOME")
if not xdg_config_str:
xdg_config = os.path.join(home_path, ".config")
else:
xdg_config = os.path.realpath(xdg_config_str)
Comment on lines +19 to +23
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: perhaps you can simplify it by writing

xdg_config = os.path.realpath(
    os.getenv("XDG_CONFIG_HOME", os.path.join(home_path, ".config"))
)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could also use the XDG module (last updated April of 2021) to facilitate the base directory specification.

CONFIG_DIR = os.path.join(xdg_config, "pvpn-cli")

# Get the data directory
xdg_data_str = os.getenv("XDG_DATA_HOME")
if not xdg_data_str:
xdg_data = os.path.join(home_path, ".local/share")
else:
xdg_data = os.path.realpath(xdg_data_str)
DATA_DIR = os.path.join(xdg_data, "pvpn-cli")

# Get the cache directory
xdg_cache_str = os.getenv("XDG_CACHE_HOME")
if not xdg_cache_str:
xdg_cache = os.path.join(home_path, ".cache")
else:
xdg_cache = os.path.realpath(xdg_cache_str)
CACHE_DIR = os.path.join(xdg_cache, "pvpn-cli")


CONFIG_FILE = os.path.join(CONFIG_DIR, "pvpn-cli.cfg")
TEMPLATE_FILE = os.path.join(CONFIG_DIR, "template.ovpn")
SERVER_INFO_FILE = os.path.join(CONFIG_DIR, "serverinfo.json")
SPLIT_TUNNEL_FILE = os.path.join(CONFIG_DIR, "split_tunnel.txt")
OVPN_FILE = os.path.join(CONFIG_DIR, "connect.ovpn")
PASSFILE = os.path.join(CONFIG_DIR, "pvpnpass")
TEMPLATE_FILE = os.path.join(CACHE_DIR, "template.ovpn")
SERVER_INFO_FILE = os.path.join(CACHE_DIR, "serverinfo.json")
SPLIT_TUNNEL_FILE = os.path.join(DATA_DIR, "split_tunnel.txt")
OVPN_FILE = os.path.join(DATA_DIR, "connect.ovpn")
PASSFILE = os.path.join(DATA_DIR, "pvpnpass")
VERSION = "2.2.0"
8 changes: 4 additions & 4 deletions protonvpn_cli/logger.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import logging
from logging.handlers import RotatingFileHandler

from .constants import CONFIG_DIR
from .constants import CACHE_DIR


def get_logger():
Expand All @@ -13,11 +13,11 @@ def get_logger():
FORMATTER = logging.Formatter(
"%(asctime)s — %(name)s — %(levelname)s — %(funcName)s:%(lineno)d — %(message)s" # noqa
)
LOGFILE = os.path.join(CONFIG_DIR, "pvpn-cli.log")
LOGFILE = os.path.join(CACHE_DIR, "pvpn-cli.log")

# TBD, maybe /var/log is the better option
if not os.path.isdir(CONFIG_DIR):
os.mkdir(CONFIG_DIR)
if not os.path.isdir(CACHE_DIR):
os.mkdir(CACHE_DIR)

logger = logging.getLogger("protonvpn-cli")
logger.setLevel(logging.DEBUG)
Expand Down