brockallen / BrockAllen.MembershipReboot

MembershipReboot is a user identity management and authentication library.
Other
742 stars 238 forks source link

RelationalLinkedAccount claims behavior #574

Closed Winters44 closed 8 years ago

Winters44 commented 8 years ago

In my situation, I need to update a list of linked accounts once daily, and each linked account has a few (2-3) claims each. The values of the claims can change, which is the reason for the daily refresh.

The default behavior is to remove all claims then re-add them, which was going to become a problem eventually with the keys incrementing daily on 4000+ linked accounts.

The quick solution I used was to separate the linked account claim update methods to match the update/add/remove technique you used for UserClaims. Is this something you would consider to separate?

brockallen commented 8 years ago

Yes, it's not ideal. Have a look and let me know if you would like to improve on the code and we could discuss a PR.

Winters44 commented 8 years ago

Sure, why not? :) So far, I only had time to create a "dirty" quick fix.

My original goal was to edit the source as little as possible. Do you think this sort of update would fit better in the UserAccountService class? I was hesitant to make a breaking change, so I created a new RelationalUserAccountService.

brockallen commented 8 years ago

Well, if you can change the existing code so there's no breaking change, that'd be best.

Winters44 commented 8 years ago

I'll submit a PR something in the next week or 2. I think the technique will be to overload the AddOrUpdateLinkedAccount as shown below and make an additional method to update claims. When the "claims" param is null on #2, no claims are updated. When not null, claims will be overwritten. I believe this will be non-breaking.

public virtual void AddOrUpdateLinkedAccount(TAccount account, string provider, string id)

public virtual void AddOrUpdateLinkedAccount(TAccount account, string provider, string id, IEnumerable claims)