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.

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

feature: Augment default truststore by default, optionally limit to custom CA certs #2057

Merged
merged 8 commits into from
Jun 17, 2024

Conversation

slowjoe007
Copy link
Contributor

Continuation of PR #1937, considering the changes proposed in that PR.

Description

The change in feature #1863 did not cover the CLI feature of Bruno, bru. This feature adds that part and introduces a new command line option to bru CLI to control the way custom CA certificates are handled: replace or extend the default truststore.

The new CLI option is called --ignore-truststore and documented in both synopsis and readme.
This new option only is in effect, if a custom CA certificate is specified via option --cacert.

image

image

At the same time, this change inverts the default handling of specifying custom CA certificates in the GUI. Previously, the default truststore was replaced completely, if custom CA certificates had been specified. Now specifying custom CA certificates results in augmenting the default truststore: the setting "Keep default truststore" is selected by default.

Contribution Checklist:

@slowjoe007
Copy link
Contributor Author

Hi @mjhcorporate and @helloanoop, here are the promised changes (see #1937). I created a new PR for this.
The logic of extending or replacing the truststore is inverted now. This should be easier to consume.

@slowjoe007 slowjoe007 changed the title feat: Augment default truststore by default, optionally limit to custom CA certs feature: Augment default truststore by default, optionally limit to custom CA certs May 1, 2024
@slowjoe007
Copy link
Contributor Author

The failing test of auth/inherit also fails in branch main. This error is not related to the changes made here.

auth/inherit auth/inherit Bearer Auth 200 (401 Unauthorized) - 175 ms
   ✕ assert: res.status: 200
      expected 401 to equal 200
   ✕ assert: res.body.message: Authentication successful
      expected 'Unauthorized' to equal 'Authentication successful'

@slowjoe007
Copy link
Contributor Author

@helloanoop, please, consider adding this PR for one of the next releases. It is done for 3 weeks. I just merged the branch with the latest updates of main, so the PR should be ready for merging.

@slowjoe007
Copy link
Contributor Author

Did the rebase exercise again. Would be great to have this missing feature of the CLI component considered for the next release, @helloanoop.
Thank you!

@helloanoop
Copy link
Contributor

Thanks @slowjoe007 !

@lohit-1 Please you prioritise this for the upcoming release.

@helloanoop helloanoop requested a review from lohxt1 June 17, 2024 06:12
@helloanoop helloanoop merged commit b01f2e0 into usebruno:main Jun 17, 2024
3 checks passed
@helloanoop
Copy link
Contributor

Merged!

Thanks for taking care of this @slowjoe007 !
Thanks for the review @lohit-1

jwetzell pushed a commit to jwetzell/bruno that referenced this pull request Aug 2, 2024
…ustom CA certs (usebruno#2057)

* feat: Allow default truststore extension on bru CLI

* feat: augment default truststore by default, optionally limit to custom CA certs

* feat: augment default truststore by default, optionally limit to custom CA certs

* feat: Allow default truststore extension on bru CLI

* feat: augment default truststore by default, optionally limit to custom CA certs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants