Skip to content

Commit

Permalink
ignore invalid plugin selection choices (#9665)
Browse files Browse the repository at this point in the history
* plugins: ensure --installer/--authenticator is properly filtered

* fix windows failure in test
  • Loading branch information
alexzorin committed Apr 25, 2023
1 parent f378ec4 commit 67f14f1
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 2 deletions.
4 changes: 3 additions & 1 deletion certbot/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,13 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
* `--dns-google-credentials` now supports additional types of file-based credential, such as
[External Account Credentials](https://google.aip.dev/auth/4117) created by Workload Identity
Federation. All file-based credentials implemented by the Google Auth library are supported.
*

### Fixed

* `certbot-dns-google` no longer requires deprecated `oauth2client` library.
* Certbot will no longer try to invoke plugins which do not subclass from the proper
`certbot.interfaces.{Installer,Authenticator}` interface (e.g. `certbot -i standalone`
will now be ignored). See [GH-9664](https://github.com/certbot/certbot/issues/9664).

More details about these changes can be found on our GitHub repo.

Expand Down
3 changes: 2 additions & 1 deletion certbot/certbot/_internal/plugins/selection.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,9 @@ def pick_plugin(config: configuration.NamespaceConfig, default: Optional[str],
"https://eff.org/letsencrypt-plugins for more detail on what "
"the plugins do and how to use them.")

filtered = plugins.visible().ifaces(ifaces)
filtered = plugins.visible()

filtered = filtered.ifaces(ifaces)
filtered.init(config)
filtered.prepare()
prepared = filtered.available()
Expand Down
11 changes: 11 additions & 0 deletions certbot/certbot/_internal/tests/main_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,17 @@ def test_dryrun_installer_doesnt_restart(self, mock_sel, mock_get_cert, mock_fin
self._call('certonly --nginx -d example.com --dry-run'.split())
mock_installer.restart.assert_not_called()

@mock.patch('certbot._internal.main._report_next_steps')
@mock.patch('certbot._internal.main._report_new_cert')
@mock.patch('certbot._internal.main._find_cert')
@mock.patch('certbot._internal.main._get_and_save_cert')
def test_invalid_installer(self, mock_get_cert, mock_find_cert,
unused_report_new, unused_report_next):
mock_get_cert.return_value = mock.MagicMock()
mock_find_cert.return_value = (True, None)
self._call((f'certonly --webroot -w {tempfile.gettempdir()} ' +
'-i standalone -d example.com').split())


class FindDomainsOrCertnameTest(unittest.TestCase):
"""Tests for certbot._internal.main._find_domains_or_certname."""
Expand Down
8 changes: 8 additions & 0 deletions certbot/certbot/_internal/tests/plugins/selection_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,14 @@ def test_choose_plugin_none(self):
mock_choose.return_value = None
assert self._call() is None

def test_default_must_be_filtered(self):
# https://github.com/certbot/certbot/issues/9664
self.default = "foo"
filtered = mock.MagicMock()
self.reg.filter.return_value = filtered
self._call()
assert filtered.ifaces.call_count == 1


class ChoosePluginTest(unittest.TestCase):
"""Tests for certbot._internal.plugins.selection.choose_plugin."""
Expand Down

0 comments on commit 67f14f1

Please sign in to comment.