IdentityServer / IdentityServer3.EntityFramework

EntityFramework persistence layer for IdentityServer3
Apache License 2.0
68 stars 97 forks source link

Entities.ClientSecret to Core.Models.Secret mapping configuration #73

Closed ryandel1 closed 8 years ago

ryandel1 commented 9 years ago

idsvr's Secret type sets its Type property to "SharedSecret" in the constructor; "SharedSecret" can be thought of as a default value. This is not reflected in the EF project. The EF project's mapper configuration is set in such a way that regardless of what the constructor does with the properties, automapper will come in afterward and reset everything to the database values. This is a source of insidious bugs when the value in the db for Secret Type is null.

Unfortunately EF6 has poor support for default values using code first at this time so instead of changing the Type column to 'Required' with a default value of "SharedSecret" I propose changing the mapper configuration to the following:

Mapper.CreateMap<Entities.ClientSecret, IdentityServer3.Core.Models.Secret>(MemberList.Destination)
            .ForMember(dest => dest.Type, opt => opt.Condition(srs => !srs.IsSourceValueNull));

This will change the mapping so if the value of Type is null in the db, the mapper will ignore it and the intended "SharedSecret" value set in the constructor will persist for the newly created object.

If this is acceptable I can submit a PR, but I would also recommend changing the Type column to non-nullable with a default value of "SharedSecret" in the next schema change as well.

brockallen commented 9 years ago

Related? https://github.com/IdentityServer/IdentityServer3.EntityFramework/issues/69

wdzr commented 9 years ago

Yes,I think we should keep all fields "NOT NULL"

ryandel1 commented 9 years ago

It's not really related. I think the Clients table is set up appropriately and idsvr isn't making any assumptions on its values. Idsvr code is making an assumption that ClientSecrets.ClientSecretTypes will never be null however.

brockallen commented 8 years ago

Done on dev -- take a look, please.