duosecurity / duo_api_csharp

Other
32 stars 31 forks source link

Restructured and updated project #10

Closed Atrejoe closed 1 year ago

Atrejoe commented 2 years ago

Made solution with separated examples and unit tests Replaced VS Test with Xunit test Upgraded to .Net Framework 4.8

AaronAtDuo commented 2 years ago

@Atrejoe Sorry for getting to this so late. I'm definitely looking to clean up and modernize this repo a bit; it's been neglected for a while. Do yo think it would be worthwhile migrating off .NET Framework entirely, and targeting .NET Standard (like duo_universal_csharp)?

Atrejoe commented 2 years ago

For libraries in general: certainly. This should not be a big chore though.

I do not have my current code with me, but I think I have switched to: https://github.com/duosecurity/duo_universal_csharp

markcouvaras commented 2 years ago

Hi there, sorry to ask this on here but I was wondering if anyone knew how to call JSONPagingApiCall using 'v2/logs/authentication' ? I don't know how to use the offset parameter as it is no longer an int. Would be nice to update the examples file to show to use in the V2 api?

If anyone can help, it would greatly appreciated.

Atrejoe commented 1 year ago

I rebased the branch.

indeed DuoApiMethods.c was part of my other feature branch. When you look at our copy, you'll also see a port to .Net Standard & XUnit, and a branch to make the library more user friendly by having convenience methods for calling the DUO api. (see DuoApiMethods.cs)

image

yizshi commented 1 year ago

Hi - @Atrejoe Thank you for rebasing! There are just couple things I want to ask before I merge this.

  1. In the attempt I tried to run this locally as well as in github action. I notice VS2019 as well as dotnet could not find the tests. I have it working after I added xunit.runner.visualstudio.2.4.5 from nuget. I think it might be worth to add it in this diff too.
  2. After I got the test running. I notice one test could not pass. TestApiCall.TestJsonUnparseableResponse. I am suspect this is a because the unit tests are try to listen to a losthost that already in use for another test. I am still looking into how to get it fix. But if you could get it fix before me, that would fantastic :P.
Atrejoe commented 1 year ago

I have added xunit.runner.visualstudio, I didn't need it in VS2022, but dotnet did. Please note that this is currently still a .Net framework library. It may worth it to port the library to .Net standard and then iron out these issues.

This issue with TestApiCall.TestJsonUnparseableResponse may be a concurrency issue. Multiple tests use the same instance on the TestServer. When run concurrently these tests they change the handler then perform the tests. In concurrent scenario's it may be that the handler get changed before performing the actual test.

I have another issue with TestPostParameterSigningCustomDate which indicates Bad Date!. This may be due to unexpectyed time zone handling:

Expected value: Mon, 11 Nov 2013 22:34:00 +0000 my actual value: Mon, 11 Nov 2013 22:34:00 +0100

A UTC dat is passed in as a parameter, but then in DateToRFC822 the offset is determined using TimeZoneInfo.Local, this may need to be TimeZoneInfo.UTC dependant on the type of time passed in:

            TimeSpan offsetTime= default;
            switch (date.Kind)
            {
                case DateTimeKind.Unspecified:
                    //Don't know yet
                    break;
                case DateTimeKind.Utc:
                    {
                        offsetTime = TimeZoneInfo.Utc.GetUtcOffset(date);
                        break;
                    }
                case DateTimeKind.Local:
                    {
                        offsetTime = TimeZoneInfo.Local.GetUtcOffset(date);
                        break;
                    }
            };

            int offset = offsetTime.Hours;

I haven't looked at the purpose of the method, so I do not know for sure though.

AaronAtDuo commented 1 year ago

FWIW we do (long term) want to port it to .NET standard. We have a short-term need to get to .NET 4.8 framework ASAP though, so we're working on getting this merged. We really appreciate you working on this with us.