Skip to content

Add support for client certificate selection. - #80

Merged
sta merged 1 commit into
sta:masterfrom
dpwood:client_certificates
Oct 10, 2014
Merged

Add support for client certificate selection.#80
sta merged 1 commit into
sta:masterfrom
dpwood:client_certificates

Conversation

@dpwood

@dpwood dpwood commented Oct 7, 2014

Copy link
Copy Markdown
Contributor

Added a LocalCertificateSelectionCallback property to allow a client certificate to be selected.

@sta

sta commented Oct 8, 2014

Copy link
Copy Markdown
Owner

It looks nice.

And, i want to add a little change such as the following:

    ...
    private LocalCertificateSelectionCallback 
                                    _certSelectionCallback;
    ...

    public LocalCertificateSelectionCallback ClientCertificateSelectionCallback {
      get {
        return _certSelectionCallback;
      }

      ...

How about that?

@dpwood

dpwood commented Oct 8, 2014

Copy link
Copy Markdown
Contributor Author

Sure, I actually prefer that naming.

My choice for "_userCertSelectionCallback" and
"UserCertificateSelectionCallback" was purely based on the name of the
parameter to the SSLStream constructor (userCertificateSelectionCallback).
However, I see that you have already exposed the
userCertificateValidationCallback parameter of SSLStream as
ServerCertificateValidationCallback, so your decision to name this callback
as Client... makes sense.

Thanks,
Dave

On 8 October 2014 08:10, sta notifications@github.com wrote:

It looks nice.

And, i want to add a little change such as the following:

...
private LocalCertificateSelectionCallback
                                _certSelectionCallback;
...

public LocalCertificateSelectionCallback ClientCertificateSelectionCallback {
  get {
    return _certSelectionCallback;
  }

  ...

How about that?


Reply to this email directly or view it on GitHub
#80 (comment).

@dpwood
dpwood force-pushed the client_certificates branch 2 times, most recently from 1ee8ee4 to 2dddf74 Compare October 8, 2014 11:24
@dpwood
dpwood force-pushed the client_certificates branch from 2dddf74 to 4852d3c Compare October 8, 2014 11:27
@dpwood

dpwood commented Oct 8, 2014

Copy link
Copy Markdown
Contributor Author

I've changed the names as suggested.

@sta

sta commented Oct 9, 2014

Copy link
Copy Markdown
Owner

Thx! I will merge this pull request.

And then, i will add a contributor info comment such as the following:

/*
 * Contributors:
 * - dpwood (or Dave) <your email address>
 */

So, could you tell me your email address (or something changes)?

@dpwood

dpwood commented Oct 9, 2014

Copy link
Copy Markdown
Contributor Author

David Wood dpwood@gmail.com

Many thanks,
Dave

sta added a commit that referenced this pull request Oct 10, 2014
Add support for client certificate selection.
@sta
sta merged commit f3579ec into sta:master Oct 10, 2014
sta added a commit that referenced this pull request Oct 10, 2014
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.

2 participants