X-Guardian / AdfsDsc

DSC resources for deployment and configuration of Active Directory Federation Services
MIT License
9 stars 5 forks source link

AdfsServerApplication: Add basic support for AdfsServerApplication #74

Closed watschi closed 1 year ago

watschi commented 1 year ago

Pull Request (PR) description

This PR adds basic support for AdfsServerApplication. For authentication of the Server Application only ADUserPrincipalName can be configured for now, client secrets or certificates are not configurable.

This Pull Request (PR) fixes the following issues

None

Task list


This change is Reviewable

watschi commented 1 year ago

Hi @X-Guardian,

not sure why the pipeline fails, according to a quick google search the download error can be an intermittent node and/or permission issue. The run on my forked repo went thorugh succesfully: https://dev.azure.com/fabianwendlandt0807/AdfsDsc/_build/results?buildId=10&view=results

Maybe you can retrigger your pipeline?

Thanks!

X-Guardian commented 1 year ago

Still failed after retrying. The delights of Azure DevOps! I'll try and take a look at the pipeline issue later today.

Thanks for the PR @watschi. Nice to know people are using the module.

codecov[bot] commented 1 year ago

Codecov Report

Merging #74 (7c7f093) into main (40a666b) will increase coverage by 0%. Report is 4 commits behind head on main. The diff coverage is 100%.

Impacted file tree graph

@@         Coverage Diff          @@
##           main    #74    +/-   ##
====================================
  Coverage    98%    98%            
====================================
  Files        16     17     +1     
  Lines      2146   2272   +126     
====================================
+ Hits       2110   2236   +126     
  Misses       36     36            
X-Guardian commented 1 year ago

Looks good @watschi. Have you been able to run the integration tests successfully yourself?

watschi commented 1 year ago

Hi @X-Guardian, thanks for fixing the pipeline!

I thought integration tests are running in the default test runs, apperantly not. I did push a configuration to an ADFS farm succesfully, but will run integration tests tomorrow and report the results in here after. I also noticed an oversight in the localization strings (copied from AdfsNativeClient) which I'll fix.

X-Guardian commented 1 year ago

No, the integration tests need an Active Directory, which is tricky to setup in the CI, so have always been run offline manually.

watschi commented 1 year ago

Fixed the localization strings as mentioned and was able to run the integration tests succesfully (transcript attached: adfsdsc_integration.log).

Please let me know if there's anything else I can help with!

grafik

X-Guardian commented 1 year ago

Hi @watschi, this has been released in v1.4.0

watschi commented 1 year ago

Thanks for fixing the pipeline and merging so quickly! :)