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

Added support for including a client certificate #1972

Merged

Conversation

lindsve
Copy link
Collaborator

@lindsve lindsve commented Jun 27, 2023

Fixes

This pull request fixes #1971

Description

This PR includes an extension of the WitsmlClient that accepts a X509Certificate client certificate for certificate validation when communicating with a WITSML server.

Due to the added arguments to the WitsmlClient constructor, I've refactored it to accept a WitsmlClientOptions object instead. The previous constructor has been marked as obsolete to move usage over to the new constructor (I removed the Obsolete attribute since it broke the build which seem to be set up to break on warnings)

I've also refactored the WitsmlClient file a bit and extracted some of the content to their own files.

Type of change

  • Bugfix
  • New feature (non-breaking change which adds functionality)
  • Enhancement of existing functionality
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Impacted Areas in Application

  • Frontend
  • API
  • WITSML
  • Other (please describe)

Checklist:

Communication

  • I have made corresponding changes to the documentation
  • PR affects application security

Code quality

  • I have self-reviewed my code
  • No new warnings are generated

Test coverage

  • New code is covered by passing tests

Further comments

janburak
janburak previously approved these changes Jun 28, 2023
Copy link
Contributor

@janburak janburak left a comment

Choose a reason for hiding this comment

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

  • Read issue and PR
  • Pulled branch and manually tested
  • Verify that PR resolves issue
  • Reviewed the code

…or has been updated and a significant feature has been added
@lindsve
Copy link
Collaborator Author

lindsve commented Jun 28, 2023

@janburak I need you to take a look at it again since I pushed a new commit :) I bumped the nuget package version from 2.7->2.8 since the WitsmlClient constructor has been updated and the certificate support is significant enough to warrant bump in package minor version

@lindsve lindsve requested a review from janburak June 28, 2023 10:26
@lindsve lindsve merged commit 5d5eaab into equinor:main Jun 28, 2023
1 check passed
@lindsve lindsve deleted the added_support_for_client_certificate_validation branch June 28, 2023 10:27
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.

💡Add support for including a client certificate for certificate validation
2 participants