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

Issue with Login API? #115

Open
mitch-b opened this issue Mar 18, 2018 · 16 comments
Open

Issue with Login API? #115

mitch-b opened this issue Mar 18, 2018 · 16 comments

Comments

@mitch-b
Copy link

mitch-b commented Mar 18, 2018

Following the README, can you help describe where this test scenario has gone awry?

https://github.com/mitch-b/synology-forbidden-error-test

I've created an ASP.NET Core Web project that tries to connect to a Synology server configured in the appsettings.json, and configured / executed in Startup.cs for demo purposes.

Even if I attempt with valid credentials, I'm getting a Forbidden 403 error. I'm on DSM 6.1.5-15254 Update 1. I tried this on an account without two factor authentication enabled, but I also tried with another account sending OtpCode as well. Still no success.

Thanks for any advice!

@matteobruni
Copy link
Contributor

Hi, you must set ports you use to login to DSM like 5000/5001. This should work, I tried your project.

@matteobruni matteobruni self-assigned this Mar 19, 2018
@mitch-b
Copy link
Author

mitch-b commented Mar 20, 2018

Ok, I’ll give that a try tomorrow. Thanks for taking a look.

@mitch-b
Copy link
Author

mitch-b commented Mar 21, 2018

You did get me past that error ... now I'm now getting:

System.Reflection.TargetInvocationException: 'Exception has been thrown by the target of an invocation.'
HttpRequestException: An error occurred while sending the request.
WinHttpException: A security error occurred

So I think it's relatively out of scope of this issue. Though doesn't lead me to much information.

I think we can close this and I'll keep working through it. Thanks!

@matteobruni
Copy link
Contributor

Are you using the SSL protocol and the relative port? If the certificate is not valid I think it's not ignored. That should be an option.

@mitch-b
Copy link
Author

mitch-b commented Mar 21, 2018

Yes, I actually was using 5001 with a LetsEncrypt SSL certificate. My research also led me to believe it was a certificate issue - but I couldn't tell why it was not valid. I (likely incorrectly) assumed that since my browser trusts it, my server running would also trust it.

I can also try just the HTTP endpoint to see if that helps any from a connectivity troubleshooting step.

I'm curious where you think an enhancement/help-wanted request could help. Is this Stack Overflow post relevant? https://stackoverflow.com/questions/42746190/https-request-fails-using-httpclient

using (var handler = new HttpClientHandler())
{
  handler.SslProtocols = SslProtocols.Tls12 | SslProtocols.Tls11 | SslProtocols.Tls;
  if(synologyConnectionSettings.IgnoreInvalidSsl) // could be added? or, !StrictSsl ?
  {
    handler.ServerCertificateCustomValidationCallback = (message, cert, chain, errors) => true;
  }
  using (var client = new HttpClient(handler))
  {
    ...
  }
}

@matteobruni
Copy link
Contributor

Yes this could be a good solution, you can create a branch from master and implement it if you want. Actually I'm implementing async methods on the dev branch and its a real mess.

@mitch-b
Copy link
Author

mitch-b commented Mar 21, 2018

Adding this link for my reference later, too. dotnet/corefx#19908

Utilize this strongly typed reference instead of creating our own validation callback to support non-Windows hosts too. 👌

@mitch-b
Copy link
Author

mitch-b commented Mar 24, 2018

So, an issue I ran into is that the DangerousAcceptAnyServerCertificateValidator property (which allows us some means of cross-platform support) is only available on .NET Core 2.0, not .NET Standard APIs.

Which leads me to think the best thing we can do is allow users to register their own singleton SynologyHttpClient : HttpClient which could have a custom handler, or uses the library default:

var synologyHttpClientHandler = new HttpClientHandler
{
    ServerCertificateCustomValidationCallback = HttpClientHandler.DangerousAcceptAnyServerCertificateValidator
};
services.AddSingleton<SynologyHttpClient>(new SynologyHttpClient(synologyHttpClientHandler));

Or, if they did not register this, they could let the library create its own when .AddSynology() service is registered in Extensions/ServiceCollectionExtension.cs:

var defaultHttpClient = new SynologyHttpClient()
{
    DefaultRequestHeaders =
    {
        ExpectContinue = false
    }
};
...
services.AddSingleton<SynologyHttpClient>(defaultHttpClient);

And it would still always override the BaseAddress in the SynologyConnection class (which would now get a SynologyHttpClient injected in the constructor).

Thoughts on the approach? This addition would make it transparent to existing consumers of the library.

For those using with .NET Framework, they could simply employ the use of ServicePointManager to accept any certificate.

mitch-b added a commit to mitch-b/Synology that referenced this issue Mar 24, 2018
Ideally, would be used to circumvent issues surfaced in DotNetDevs#115
@matteobruni
Copy link
Contributor

matteobruni commented Mar 26, 2018

I updated the dev branch with a DI for HttpClient. The code you can use after AddSynology is:
`
serviceCollection.AddScoped(provider =>
{
var handler = new HttpClientHandler();

handler.ServerCertificateCustomValidationCallback += (message, certificate, chain, errors) => true;

return handler;

});
`

Actually on macOS is not working and I can't test it on windows. I'm making a release for it, I'll fix it if its buggy.

@mitch-b
Copy link
Author

mitch-b commented Mar 26, 2018

On MacOS can you try this:

serviceCollection.AddScoped(provider =>
{
  var handler = new HttpClientHandler();
  handler.ServerCertificateCustomValidationCallback = HttpClientHandler.DangerousAcceptAnyServerCertificateValidator;
  return handler;
});

I was getting errors running anything other than the 0.6.2 Synology NuGet package. And I can't seem to test locally either ... not sure what's going on. Get some error in .NET Core:

System.TypeLoadException: 'Could not load type 'System.Runtime.InteropServices.Architecture' from assembly 'netstandard, Version=2.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51'.'

@matteobruni
Copy link
Contributor

dotnet/standard#328 (comment)

Try this

@mitch-b
Copy link
Author

mitch-b commented Mar 26, 2018

Thanks for the tip, still getting the error trying all 3 different methods suggested. I'll keep working through it though. Weird that I can use 0.6.2 but no recent versions. Instead of the NuGet package (to try and rule things out), I tried pulling code from that version and building/referencing locally, but I still got the same error. Very strange.

@mitch-b
Copy link
Author

mitch-b commented Mar 27, 2018

Oh ... I see in a18d444 that System.Runtime.InteropServices.RuntimeInformation was removed as NuGet package. I'm wondering if that was the culprit of my errors - with my error being failure to load System.Runtime.InteropServices.Architecture.

I'll try it tonight!

@matteobruni
Copy link
Contributor

Unfortunately the issue is still there. If the library is referenced as project it works fine, as a package it doesn't work, .NET Core gives runtime exception, .NET Framework build error. I'm investigating further these issues.

@mitch-b
Copy link
Author

mitch-b commented Mar 29, 2018

I wonder if it just has to do with the NuGet packaging step then. References should be fine - overall, it just requires .NET Standard 2.0.

I'll do some checking over next few days.

@matteobruni
Copy link
Contributor

I'm thinking about it too, the nupkg is configured by the .net standard 2.0 menu in visual studio and it's created by AppVeyor as always. I'll check the AppVeyor configuration, to see if I need to update something.

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

No branches or pull requests

2 participants