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

feat: Store client certificate paths in collection settings as relative to collection and display them the UI. #2421

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pietrygamat
Copy link
Contributor

@pietrygamat pietrygamat commented Jun 6, 2024

Description

The UI for collection level certificates does not reveal which file is used for which domains, so the only way to make sure you are using correct keys is to remove and re-add them. Also, the paths to key and cert files are kept in bruno.json file as absolute paths, making it unsuitable for version control and sharing between users with various setups.

With this change, the paths picked with file picker are relative to current collection and displayed in the Client Certificates view of collection settings.

Screenshot from 2024-06-06 20-14-53

Note, the absolute paths are still supported, so collections using old format are not affected.

fixes #2420

Contribution Checklist:

  • The pull request only addresses one issue or adds one feature.
  • The pull request does not introduce any breaking changes
  • I have added screenshots or gifs to help explain the change if applicable.
  • I have read the contribution guidelines.
  • Create an issue and link to the pull request.

Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.

Publishing to New Package Managers

Please see here for more information.

@pietrygamat pietrygamat marked this pull request as draft June 7, 2024 09:21
@pietrygamat pietrygamat marked this pull request as ready for review June 12, 2024 19:30
@pietrygamat
Copy link
Contributor Author

@sanjai0py @helloanoop consider this one when merging of #2336 . Thanks.

@lohxt1 lohxt1 added the pr-qol label Jun 19, 2024
@helloanoop
Copy link
Contributor

@pietrygamat Can you fix the conflicts?
Otherwise this is GTG

@pietrygamat pietrygamat force-pushed the feature/cert-paths branch 4 times, most recently from c663bf5 to db577da Compare June 21, 2024 09:42
@pietrygamat
Copy link
Contributor Author

@helloanoop done.
As a sidenote: from my testing, when running bruno on Windows the path.relative produces wrong results (i.e. posix implementation is fired even though win32 one should). I am not sure it's my build issue, or electron issue.

I work around this by using:

      if (isWindowsOS()) {
        relativePath = slash(path.win32.relative(root, e.files[0].path));
      } else {
        relativePath = path.posix.relative(root, e.files[0].path);
      }

I think there are a couple more places in the application, that this may cause problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Store client certificate paths in collection settings as relative to collection
3 participants