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

ca cert support #5498

Merged
merged 5 commits into from
Dec 9, 2022
Merged

ca cert support #5498

merged 5 commits into from
Dec 9, 2022

Conversation

jackkav
Copy link
Contributor

@jackkav jackkav commented Dec 7, 2022

changelog(Improvements): Added support for setting CA certificates in workspace settings

image

questions

  • should we store the file path of the file contents?
    • client certs are paths also, and are shared only on gitsync
  • should we store it in the workspace or make a new model?
    • new model is more flexible
  • should we support team sync and/or git sync?
    • client certs don't currently support team sync, and git sync with paths does work but doesn't really make sense without support for relative paths and path validation.
  • should we combine the root certificate with the ca cert ourselves or just let the user combine them?

Related discussion: #3617
Related issue: #675
Related PR: #3203

Closes: INS-320
Closes: INS-322
Closes: INS-1669

@jackkav jackkav marked this pull request as ready for review December 8, 2022 17:10
@jackkav jackkav self-assigned this Dec 9, 2022
@jackkav jackkav changed the title spike: ca cert support ca cert support Dec 9, 2022
@jackkav jackkav added N-discussion Needs: Discussion N-help Needs: Help labels Dec 9, 2022
@ErikKringen
Copy link

should we combine the root certificate with the ca cert ourselves or just let the user combine them?

I think it would be most beneficial to take in the CA exactly as given and leave it up to the user to merge as needed.

@joepdegroot
Copy link

No strong preference on this. Agree with Erik to leave it to the user to merge.

Maybe good to also have a checkbox option to enable/disable the default/system CAs, to prevent the server certificate to be verified by those by accident...?

Copy link
Member

@filfreire filfreire left a comment

Choose a reason for hiding this comment

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

LGTM, didn't test, hoping for the best

@jackkav jackkav requested review from filfreire and a team December 9, 2022 16:18
@jackkav jackkav enabled auto-merge (squash) December 9, 2022 16:18
@jackkav jackkav merged commit bd61bcf into Kong:develop Dec 9, 2022
@jackkav jackkav deleted the spike/ca-cert-support branch December 9, 2022 16:31
pavkout pushed a commit to pavkout/insomnia that referenced this pull request Jan 18, 2023
* exploration of new data model

* POC second pass

* fix

* fix types

* improve layout
@wioxjk
Copy link

wioxjk commented Oct 16, 2024

Sorry for necroing this - but is it possible to import a custom RootCA globally instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
N-discussion Needs: Discussion N-help Needs: Help
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants