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

Allow https metadata without credentials #67

Closed
wants to merge 1 commit into from
Closed

Allow https metadata without credentials #67

wants to merge 1 commit into from

Conversation

Gabee01
Copy link

@Gabee01 Gabee01 commented Sep 14, 2017

@@ -77,6 +77,8 @@ private string GetMetadata(out Version edmxVersion)
}
}

ServicePointManager.ServerCertificateValidationCallback += new System.Net.Security.RemoteCertificateValidationCallback((s, ce, ch, ssl) => true);
Copy link
Contributor

@AlanWong-MS AlanWong-MS Sep 15, 2017

Choose a reason for hiding this comment

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

Is this meant for test/development environment only, or have you encountered issues outside of the development process?

In either case, we should add some guards inside the lambda. We should add a boolean such as isDevelopment which returns true as presented in your code. We should also add validation when isDevelopment == false. I would also recommend using more descriptive variable names. Below is my comprehensive suggestion:

ServicePointManager.ServerCertificateValidationCallback +=
    new System.Net.Security.RemoteCertificateValidationCallback((sender, certificate, chain, sslPolicyErrors) =>
    {
        if (isDevelopment)
        {
            return true;
        }

        if (sslPolicyErrors == System.Net.Security.SslPolicyErrors.None)
        {
            return true;
        }

        // Do not allow this client to communicate with unauthenticated servers.
        return false;
    });

The isDevelopment would be a represented as a checkbox in the UI that you can add in ODataConnectedService/src/Views/AdvancedSettings.xaml. You would have to bind that checkbox with the boolean.

Copy link
Contributor

@AlanWong-MS AlanWong-MS left a comment

Choose a reason for hiding this comment

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

Great start. Let's add some validations to it.

@OData OData deleted a comment from msftclas Sep 26, 2017
@nla-brandonjames
Copy link

He has deleted this repository

@AlanWong-MS
Copy link
Contributor

Closing this PR due to contributor's repo deletion.

@AlanWong-MS AlanWong-MS closed this Nov 8, 2017
@onyangofred
Copy link

Just open the link in Internet explorer, will prompt for username and password. Then save the credentials. Try again in visual studio. It works!!!

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.

8 participants