Merge branch 'master' into bcwu-fix-htmlextrafiles

Author: tdstein

.github/pull_request_template.md

@@ -1,21 +1,35 @@
-### Description
+<!--- Provide a general summary of your changes in the title. -->
 
-> Include a brief description of the problem being solved and why this
-> approach was chosen. Mention risks or shortcomings with this solution.
-> Provide relevant background information such as the associated issue, links
-> to design documents, screenshots, and performance measurements.
->
-> Even small changes deserve a little attention to detail. Put your change in
-> context.
+## Description
+<!--- Please describe the problem you are addressing and your proposed changes. -->
 
-Connected to #
+## Motivation and Context
+<!--- Even small changes deserve a little attention to detail. -->
+<!--- Why is this change required? What problem does it solve? -->
+<!--- If it fixes an open issue, please link to the issue here. -->
+<!--- Example: "Addresses Issue # 1" -->
 
-### Testing Notes / Validation Steps
+## How Has This Been Tested?
+<!--- Please describe in detail how you tested your changes. -->
+<!--- Automation should be used to verify that a change remains in place. -->
+<!--- Automated tests should be included in this pull request. -->
 
-> Explain how this change has been tested and what cases/conditions are
-> covered. Enumerate the steps someone might take to manually exercise this
-> change. Detail is important!
->
-> You can recommend one-time manual testing when someone validates your
-> changes. Rely on automation when trying to verify that a change remains in
-> place. Automated tests should be included in this PR.
+<!--- If applicable, enumerate the steps that someone can take to exercise this change manually. -->
+<!--- Please include details of your testing environment. -->
+<!--- Detail is important! -->
+
+### Screenshots (if appropriate):
+
+## Types of Changes
+<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply. -->
+- [ ] Bug fix (non-breaking change which fixes an issue)
+- [ ] New feature (non-breaking change which adds functionality)
+- [ ] Breaking change (fix or feature that would cause existing functionality to change)
+
+## Checklist:
+<!--- Go over all the following points, and put an `x` in all the boxes that apply: -->
+<!--- If you need clarification on any of these, feel free to ask. We're here to help! -->
+- [ ] I have read the **CONTRIBUTING** document.
+- [ ] My code follows the code style of this project.
+- [ ] I have updated the documentation as needed.
+- [ ] I have added tests to cover my changes.

.gitignore

@@ -2,25 +2,26 @@
 .DS_Store
 .coverage
 .vagrant
-/.vscode/
 /*.egg
 /*.egg-info
 /*.eggs
 /.conda/
 /.idea/
 /.jupyter/
 /.local/
-/.venv/
 /.pipenv-requires
+/.venv/
+/.vscode/
 /build/
 /dist/
+/docs/docs/changelog.md
+/docs/docs/index.md
 /node_modules/
 /notebooks*/
+/rsconnect-build
+/rsconnect-build-test
 /rsconnect/version.py
-htmlcov
 /tests/testdata/**/rsconnect-python/
+htmlcov
 test-home/
-/docs/docs/index.md
-/docs/docs/changelog.md
-/rsconnect-build
-/rsconnect-build-test
+venv

CHANGELOG.md

@@ -4,6 +4,16 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+
+### Added
+
+- The `--cacert` option now supports certificate files encoded in the Distinguished Encoding Rules (DER) binary format. Certificate files with DER encoding must end in a `.cer` or `.der` suffix.
+
+### Changed
+
+- The `--cacert` option now requires that Privacy Enhanced Mail (PEM) formatted certificate files end in a `.ca-bundle`, `.crt`, `.key`, or `.pem` suffix. 
+
 ## [1.14.0] - 2023-01-19
 
 ### Changed

rsconnect/api.py

@@ -18,10 +18,10 @@
 
 import re
 from warnings import warn
-from six import text_type
 import gc
 
 from . import validation
+from .certificates import read_certificate_file
 from .http_support import HTTPResponse, HTTPServer, append_to_path, CookieJar
 from .log import logger, connect_logger, cls_logged, console_logger
 from .models import AppModes
@@ -360,7 +360,7 @@ def __init__(
         url: str = None,
         api_key: str = None,
         insecure: bool = False,
-        cacert: IO = None,
+        cacert: str = None,
         ca_data: str = None,
         cookies=None,
         account=None,
@@ -415,7 +415,7 @@ def setup_remote_server(
         url: str = None,
         api_key: str = None,
         insecure: bool = False,
-        cacert: IO = None,
+        cacert: str = None,
         ca_data: str = None,
         account_name: str = None,
         token: str = None,
@@ -433,7 +433,7 @@ def setup_remote_server(
         )
 
         if cacert and not ca_data:
-            ca_data = text_type(cacert.read())
+            ca_data = read_certificate_file(cacert)
 
         server_data = ServerStore().resolve(name, url)
         if server_data.from_store:
@@ -507,7 +507,7 @@ def validate_server(
         url: str = None,
         api_key: str = None,
         insecure: bool = False,
-        cacert: IO = None,
+        cacert: str = None,
         api_key_is_required: bool = False,
         account_name: str = None,
         token: str = None,
@@ -528,7 +528,7 @@ def validate_connect_server(
         url: str = None,
         api_key: str = None,
         insecure: bool = False,
-        cacert: IO = None,
+        cacert: str = None,
         api_key_is_required: bool = False,
         **kwargs
     ):
@@ -551,7 +551,7 @@ def validate_connect_server(
 
         ca_data = None
         if cacert:
-            ca_data = text_type(cacert.read())
+            ca_data = read_certificate_file(cacert)
         api_key = api_key or self.remote_server.api_key
         insecure = insecure or self.remote_server.insecure
         if not ca_data:

rsconnect/certificates.py

@@ -0,0 +1,37 @@
+from pathlib import Path
+
+BINARY_ENCODED_FILETYPES = [".cer", ".der"]
+TEXT_ENCODED_FILETYPES = [".ca-bundle", ".crt", ".key", ".pem"]
+
+
+def read_certificate_file(location: str):
+    """Reads a certificate file from disk.
+
+    The file type (suffix) is used to determine the file encoding.
+    Assumption are made based on standard SSL practices.
+
+    Files ending in '.cer' and '.der' are assumed DER (Distinguished
+    Encoding Rules) files encoded in binary format.
+
+    Files ending in '.ca-bundle', '.crt', '.key', and '.pem' are PEM
+    (Privacy Enhanced Mail) files encoded in plain-text format.
+    """
+
+    path = Path(location)
+    suffix = path.suffix
+
+    if suffix in BINARY_ENCODED_FILETYPES:
+        with open(path, "rb") as file:
+            return file.read()
+
+    if suffix in TEXT_ENCODED_FILETYPES:
+        with open(path, "r") as file:
+            return file.read()
+
+    types = BINARY_ENCODED_FILETYPES + TEXT_ENCODED_FILETYPES
+    types = sorted(types)
+    types = [f"'{_}'" for _ in types]
+    human_readable_string = ", ".join(types[:-1]) + ", or " + types[-1]
+    raise RuntimeError(
+        f"The certificate file type is not recognized. Expected {human_readable_string}. Found '{suffix}'."
+    )

rsconnect/main.py

@@ -6,9 +6,11 @@
 import typing
 import textwrap
 import click
-from six import text_type
 from os.path import abspath, dirname, exists, isdir, join
 from functools import wraps
+
+from rsconnect.certificates import read_certificate_file
+
 from .environment import EnvironmentException
 from .exception import RSConnectException
 from .actions import (
@@ -129,7 +131,7 @@ def server_args(func):
         "--cacert",
         "-c",
         envvar="CONNECT_CA_CERTIFICATE",
-        type=click.File(),
+        type=click.Path(exists=True, file_okay=True, dir_okay=False),
         help="The path to trusted TLS CA certificates.",
     )
     @click.option("--verbose", "-v", is_flag=True, help="Print detailed messages.")
@@ -269,7 +271,7 @@ def _test_server_and_api(server, api_key, insecure, ca_cert):
     :return: a tuple containing an appropriate ConnectServer object and the username
     of the user the API key represents (or None, if no key was provided).
     """
-    ca_data = ca_cert and text_type(ca_cert.read())
+    ca_data = ca_cert and ca_cert.read()
     me = None
 
     with cli_feedback("Checking %s" % server):
@@ -312,7 +314,7 @@ def _test_rstudio_creds(server: api.PositServer):
     "--cacert",
     "-c",
     envvar="CONNECT_CA_CERTIFICATE",
-    type=click.File(),
+    type=click.Path(exists=True, file_okay=True, dir_okay=False),
     help="The path to trusted TLS CA certificates.",
 )
 @click.option(
@@ -344,7 +346,10 @@ def bootstrap(
     logger.debug("Generated JWT:\n" + bootstrap_token)
 
     logger.debug("Insecure: " + str(insecure))
-    ca_data = cacert and text_type(cacert.read())
+
+    ca_data = None
+    if cacert:
+        ca_data = read_certificate_file(cacert)
 
     with cli_feedback("", stderr=True):
         connect_server = RSConnectServer(
@@ -398,7 +403,7 @@ def bootstrap(
     "--cacert",
     "-c",
     envvar="CONNECT_CA_CERTIFICATE",
-    type=click.File(),
+    type=click.Path(exists=True, file_okay=True, dir_okay=False),
     help="The path to trusted TLS CA certificates.",
 )
 @click.option("--verbose", "-v", is_flag=True, help="Print detailed messages.")
@@ -1674,7 +1679,7 @@ def content():
     "--cacert",
     "-c",
     envvar="CONNECT_CA_CERTIFICATE",
-    type=click.File(),
+    type=click.Path(exists=True, file_okay=True, dir_okay=False),
     help="The path to trusted TLS CA certificates.",
 )
 @click.option(
@@ -1768,7 +1773,7 @@ def content_search(
     "--cacert",
     "-c",
     envvar="CONNECT_CA_CERTIFICATE",
-    type=click.File(),
+    type=click.Path(exists=True, file_okay=True, dir_okay=False),
     help="The path to trusted TLS CA certificates.",
 )
 @click.option(
@@ -1819,7 +1824,7 @@ def content_describe(name, server, api_key, insecure, cacert, guid, verbose):
     "--cacert",
     "-c",
     envvar="CONNECT_CA_CERTIFICATE",
-    type=click.File(),
+    type=click.Path(exists=True, file_okay=True, dir_okay=False),
     help="The path to trusted TLS CA certificates.",
 )
 @click.option(
@@ -1888,7 +1893,7 @@ def build():
     "--cacert",
     "-c",
     envvar="CONNECT_CA_CERTIFICATE",
-    type=click.File(),
+    type=click.Path(exists=True, file_okay=True, dir_okay=False),
     help="The path to trusted TLS CA certificates.",
 )
 @click.option(
@@ -1942,7 +1947,7 @@ def add_content_build(name, server, api_key, insecure, cacert, guid, verbose):
     "--cacert",
     "-c",
     envvar="CONNECT_CA_CERTIFICATE",
-    type=click.File(),
+    type=click.Path(exists=True, file_okay=True, dir_okay=False),
     help="The path to trusted TLS CA certificates.",
 )
 @click.option(
@@ -2005,7 +2010,7 @@ def remove_content_build(name, server, api_key, insecure, cacert, guid, all, pur
     "--cacert",
     "-c",
     envvar="CONNECT_CA_CERTIFICATE",
-    type=click.File(),
+    type=click.Path(exists=True, file_okay=True, dir_okay=False),
     help="The path to trusted TLS CA certificates.",
 )
 @click.option("--status", type=click.Choice(BuildStatus._all), help="Filter results by status of the build operation.")
@@ -2053,7 +2058,7 @@ def list_content_build(name, server, api_key, insecure, cacert, status, guid, ve
     "--cacert",
     "-c",
     envvar="CONNECT_CA_CERTIFICATE",
-    type=click.File(),
+    type=click.Path(exists=True, file_okay=True, dir_okay=False),
     help="The path to trusted TLS CA certificates.",
 )
 @click.option(
@@ -2104,7 +2109,7 @@ def get_build_history(name, server, api_key, insecure, cacert, guid, verbose):
     "--cacert",
     "-c",
     envvar="CONNECT_CA_CERTIFICATE",
-    type=click.File(),
+    type=click.Path(exists=True, file_okay=True, dir_okay=False),
     help="The path to trusted TLS CA certificates.",
 )
 @click.option(
@@ -2167,7 +2172,7 @@ def get_build_logs(name, server, api_key, insecure, cacert, guid, task_id, forma
     "--cacert",
     "-c",
     envvar="CONNECT_CA_CERTIFICATE",
-    type=click.File(),
+    type=click.Path(exists=True, file_okay=True, dir_okay=False),
     help="The path to trusted TLS CA certificates.",
 )
 @click.option(

tests/test_certificates.py

@@ -0,0 +1,40 @@
+from tempfile import NamedTemporaryFile
+from unittest import TestCase
+
+from rsconnect.certificates import read_certificate_file
+
+
+class ParseCertificateFileTestCase(TestCase):
+
+    def test_parse_certificate_file_ca_bundle(self):
+        res = read_certificate_file("tests/testdata/certificates/localhost.ca-bundle")
+        self.assertTrue(res)
+
+    def test_parse_certificate_file_cer(self):
+        res = read_certificate_file("tests/testdata/certificates/localhost.cer")
+        self.assertTrue(res)
+
+    def test_parse_certificate_file_crt(self):
+        res = read_certificate_file("tests/testdata/certificates/localhost.crt")
+        self.assertTrue(res)
+
+    def test_parse_certificate_file_der(self):
+        res = read_certificate_file("tests/testdata/certificates/localhost.der")
+        self.assertTrue(res)
+
+    def test_parse_certificate_file_key(self):
+        res = read_certificate_file("tests/testdata/certificates/localhost.key")
+        self.assertTrue(res)
+
+    def test_parse_certificate_file_pem(self):
+        res = read_certificate_file("tests/testdata/certificates/localhost.pem")
+        self.assertTrue(res)
+
+    def test_parse_certificate_file_csr(self):
+        with self.assertRaises(RuntimeError):
+            read_certificate_file("tests/testdata/certificates/localhost.csr")
+
+    def test_parse_certificate_file_invalid(self):
+        with NamedTemporaryFile() as tmpfile:
+            with self.assertRaises(RuntimeError):
+                read_certificate_file(tmpfile.name)