Moved Models and Public classes to Fido2NetLib.Models project #103

abergs · 2019-07-24T12:19:00Z

I've made some major refactoring and moved a lot of the Objects namespace to a separate project, including the "public" objects such as Options, RawResponse classes. This is to help consumers to avoid getting unnecessary dependencies on the System.IdentityModel.tokens.Jwt package.

I noticed that package is a concern when implementing this is in a http://asp.net non-core webapp. I wanted to use the OptionsClasses when sending JSON to clients, but the dependency on Tokens.JWT 5.* actually broke AspNet which requires 4.*

The problem could have been solved by me (as the consumer) by writing my own POCO-classes and mapping etc but that is a lot of boilerplate. I believe this approach will benefit everyone else on the OWIN/Asp.net stack

This PR is a W.I.P. Before merging we should consider renaming the Configuration class To Fido2Configuration since it is not a child class anymore.

All namespaces have been kept as-is. However Options.Create have been made public instead of internal and as wel as some property setters

abergs · 2019-07-24T12:57:35Z

@MeikTranel Do you have any insights to share regarding splitting a library into two nuget packages but still have a easy development flow?

The way I see this being consumed is:

Flow:

Developers install Fido2.Models into their ASP.Net project running full framework
Developers install Fido2 into their Backend project.
Developers populate data in models (from Fido2.Models) and calls function in Backend with said models.
Backend uses Fido2 to attest.

End result:

Aspnet-project depends on Fido2.Models package
Backend depends on Fido2 package
Fido2 depends on Fido2.Models package

All this to avoid introducing the System.IdentityModel.tokens.Jwt version 5.* in aspnet projects.

While we work on the library, it would be preferable to have Fido2 depend on Fido2.Models project instead of package to make sure intellisense and f5 debugging is seamless.

Any insights on this sort of setup?

abergs · 2019-07-24T15:37:28Z

I solved the project structure with a conditional in the csproj in 7d1258e.

I also added some ok-ish build scripts for publishing nugets.

Wondering if we should rename the new project to just Models before merging, thoughts @aseigler?

MeikTranel

There's nothing fundamentally wrong, but i would really reconsider just using the same version. I could help this weekend (there's simply no way i will be able to boot up my home workstation during the heatwave today 🤣)

MeikTranel · 2019-07-25T06:30:33Z

Models/Fido2NetLib.Models.csproj

+  <PropertyGroup>
+    <PackageId>Fido2.Models</PackageId>
+    <VersionPrefix>1.0.0</VersionPrefix>
+    <VersionSuffix>preview5</VersionSuffix>


Are you sure you want these packages to have isolated versioning flows? Don't want you want FIDO2 and FIDO2.Models to have the same package version etc.?

The should have the same version. Currently building a .ps1 script to sync it up and release them.

MeikTranel · 2019-07-25T06:31:40Z

Src/Fido2NetLib.csproj

  <ItemGroup>
+    <PackageReference Include="Fido2.Models" Version="1.0.0-preview5" />


If we were to release both packages as one deployment object but two artifacts this packagereference would not be required.

I'm intrigued, could you explain more?

With new csprojs doing "dotnet pack" on a csproj will automatically pack all projects referenced (ProjectReference-Item) as well while adding nuspec dependencies to them simultaneously (unless specifically disabled of course).
With a shared Version variable this means that dotnet pack on FIDO2 will pack both Fido2.nupkg and Fido2.Models.nupkg with the versions and dependencies automagically set.

Sounds great!

abergs · 2019-07-25T19:03:38Z

I believe my assumption to move dependency to different projects would allow side-by-side assemblys is wrong and there is by defualt no side by side. This is what I thought would allow multiple versions of System.identitymodel.token.jwt to co-exist.

Project A (asp.net full framework)

dependent on System.IdentityModel.Tokens.Jwt 4.*
dependent on Fido2.Models
dependent on Project B

Project B

dependent on Fido2

Fido2

dependent on Fido2.Models
dependent on System.IdentityModel.Tokens.Jwt 5.*

Asp.net full framework has a dependency on Jwt 4.*, so if we want to offer Fido2 to those (We should) we need to come up with a solution.

An idea I have is to condition our dependency on Identitymodel to either 4* or 5* depending on targeting framework. This comes with the price that we need to re-implement the two parts (AndroidSafetyNet and MDS) of our code that is using JWT because of the breaking changes in JWT, dependent on targeting framework.

Other ways (maybe?) could be to guide consumers to step 2 or 3 here: https://michaelscodingspot.com/how-to-resolve-net-reference-and-nuget-package-version-conflicts/

abergs · 2019-07-25T19:35:01Z

Or - crazy idea - we source the parts for validating a token as we've done with other projects. The code is MIT.

Parsing: https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/blob/b0867057c1b98ead823eed37869b5656ed2ba695/src/System.IdentityModel.Tokens.Jwt/JwtSecurityToken.cs

ValidateToken:
https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/blob/e69d0c77909334ddba19ac15a6d993adcc77e458/src/System.IdentityModel.Tokens.Jwt/JwtSecurityTokenHandler.cs

Do you think this would be doable @aseigler ?

MeikTranel · 2019-07-26T08:38:29Z

Or - crazy idea - we source the parts for validating a token as we've done with other projects. The code is MIT.

Parsing: https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/blob/b0867057c1b98ead823eed37869b5656ed2ba695/src/System.IdentityModel.Tokens.Jwt/JwtSecurityToken.cs

ValidateToken:
https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/blob/e69d0c77909334ddba19ac15a6d993adcc77e458/src/System.IdentityModel.Tokens.Jwt/JwtSecurityTokenHandler.cs

Do you think this would be doable @aseigler ?

Is JWT-Communication a center pillar of your current implementation? Or is just a flavor of usage - is another alternative usage possible?
If its the latter i would just do "Fido2" and "Fido2.JWT" wherein JWT has all the types required to configure Fido for usage with JWT.

abergs · 2019-07-26T08:44:58Z

It's unfortunately used to verify AndroidSafetyNet and the whole MDS chain so moving it to another package would not be recommended.

I made a quick attempt to depend target NET471 and then depend on the JWT 4.* package, but then it seems like we loose the ECDsaSecurityKey class.

I'm running out of ideas. If possible i think sourcing is the most suitable way to support asp.net full framework clients - although I'm not sure how feasible that is.

MeikTranel · 2019-07-26T09:03:49Z

One thing is sure tho. Sourcing types from microsoft packages is bound for disaster if you're making it an exchange type... you can not for all that is holy expose this type to the public interface of your package.

abergs · 2019-07-26T09:05:07Z

@MeikTranel It would only be kept internal to parse the incoming JWT bytes. Still feels like a last resort.

Tratcher · 2019-07-26T15:50:19Z

but the dependency on Tokens.JWT 5.* actually broke AspNet which requires 4.*

Microsoft.OWIN 4.0 also moved to Tokens.JWT 5.x. Are you still targeting Microsoft.OWIN 3.x? 3.x is out of support.
https://www.nuget.org/packages/Microsoft.Owin.Security.ActiveDirectory/

aseigler · 2019-07-27T00:31:18Z

I can look into moving the JWT stuff to a different package if it would help, but I don't know what would be most compatible with what you're talking about doing.

abergs · 2019-07-29T07:20:14Z

What @Tratcher is saying is interesting though. The App I'm testing with has Owin-packages 4 installed... but still breaks down.

@aseigler Yeah lets hold that until we know if asp.net could be made compatible.

abergs · 2019-07-29T12:36:53Z

@Tratcher It seems like that SaaS app was partly running on owin4.0 with bindingRedirect hell as well as a dependency on IdentityServer3 packages that required Tokens.JWT 4.*. Once that was cleared up it worked fine. Thank you for helping out! 🥇

While it's a bit of a shame we can't support Owin < 4.0 out of the box there seems to be OK workarounds.

MeikTranel · 2019-07-29T19:06:28Z

I'm about to through you a PR on this branch.

…cts and confusion between domain models

Also fixed the documentation URL in the net46x warning Suppressed warnings in sourced libs Globalized supported targetframeworks as well as version variables etc.

MeikTranel reviewed Jul 25, 2019

View reviewed changes

abergs mentioned this pull request Jul 29, 2019

Nuget package is broken #110

Closed

abergs marked this pull request as ready for review July 31, 2019 10:19

abergs force-pushed the models-refactor branch from aa3affb to f169419 Compare July 31, 2019 10:29

Anders Åberg added 13 commits July 31, 2019 13:37

Moved Models and Public classes to Fido2NetLib.Models project

5d387e5

Allow arguments to buildRelease.ps1

9a0d125

Conditionally use nuget package when in release mode

05087c1

Added version information

fbe4ea4

removed old dashboard

113376e

Fixed broken package name

ddccb02

Added buildscripts and bumped versions

fe6d7ca

Changed to published nuget package

c6db6ab

Change foldername to Models

9924eff

Added Models to buildRelease script so that CI can test it

1d88a3e

Fixed some issues with moving the project

7dc191a

Prefix Configuration and User with Fido2 to create less naming confli…

b42fabf

…cts and confusion between domain models

Bumped version

024f5a8

Anders Åberg and others added 7 commits July 31, 2019 13:37

Added run-all build script

b2f499c

Bumped version dependency

9469fa7

Released preview7

4d05517

Shuffled around props to reduce complexity and increase flexibility

e1d4055

Also fixed the documentation URL in the net46x warning Suppressed warnings in sourced libs Globalized supported targetframeworks as well as version variables etc.

Fixed buildscripts

0403874

Fixed buildWarnings/Errors

9d649bc

Upgraded to netcoreapp2.2 and removed JWT from Demo

aec005c

abergs force-pushed the models-refactor branch from eaecc53 to aec005c Compare July 31, 2019 11:37

abergs merged commit e723dda into master Jul 31, 2019

abergs deleted the models-refactor branch March 24, 2020 16:00

abergs added breaking change Indicate that something is a breaking change enhancement Enhancements or general improvements labels May 28, 2020

abergs mentioned this pull request May 28, 2020

Release version 2.0.0 #175

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Moved Models and Public classes to Fido2NetLib.Models project #103

Moved Models and Public classes to Fido2NetLib.Models project #103

abergs commented Jul 24, 2019

abergs commented Jul 24, 2019 •

edited

Loading

abergs commented Jul 24, 2019 •

edited

Loading

MeikTranel left a comment

MeikTranel Jul 25, 2019

abergs Jul 25, 2019

MeikTranel Jul 25, 2019

abergs Jul 25, 2019

MeikTranel Jul 26, 2019 •

edited

Loading

abergs Jul 26, 2019

abergs commented Jul 25, 2019 •

edited

Loading

abergs commented Jul 25, 2019

MeikTranel commented Jul 26, 2019

abergs commented Jul 26, 2019

MeikTranel commented Jul 26, 2019

abergs commented Jul 26, 2019

Tratcher commented Jul 26, 2019

aseigler commented Jul 27, 2019

abergs commented Jul 29, 2019 •

edited

Loading

abergs commented Jul 29, 2019 •

edited

Loading

MeikTranel commented Jul 29, 2019

		<ItemGroup>
		<PackageReference Include="Fido2.Models" Version="1.0.0-preview5" />

Moved Models and Public classes to Fido2NetLib.Models project #103

Moved Models and Public classes to Fido2NetLib.Models project #103

Conversation

abergs commented Jul 24, 2019

abergs commented Jul 24, 2019 • edited Loading

abergs commented Jul 24, 2019 • edited Loading

MeikTranel left a comment

Choose a reason for hiding this comment

MeikTranel Jul 25, 2019

Choose a reason for hiding this comment

abergs Jul 25, 2019

Choose a reason for hiding this comment

MeikTranel Jul 25, 2019

Choose a reason for hiding this comment

abergs Jul 25, 2019

Choose a reason for hiding this comment

MeikTranel Jul 26, 2019 • edited Loading

Choose a reason for hiding this comment

abergs Jul 26, 2019

Choose a reason for hiding this comment

abergs commented Jul 25, 2019 • edited Loading

abergs commented Jul 25, 2019

MeikTranel commented Jul 26, 2019

abergs commented Jul 26, 2019

MeikTranel commented Jul 26, 2019

abergs commented Jul 26, 2019

Tratcher commented Jul 26, 2019

aseigler commented Jul 27, 2019

abergs commented Jul 29, 2019 • edited Loading

abergs commented Jul 29, 2019 • edited Loading

MeikTranel commented Jul 29, 2019

abergs commented Jul 24, 2019 •

edited

Loading

abergs commented Jul 24, 2019 •

edited

Loading

MeikTranel Jul 26, 2019 •

edited

Loading

abergs commented Jul 25, 2019 •

edited

Loading

abergs commented Jul 29, 2019 •

edited

Loading

abergs commented Jul 29, 2019 •

edited

Loading