brockallen / BrockAllen.MembershipReboot

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

Override validation message #641

Open JohnMcAvinue opened 8 years ago

JohnMcAvinue commented 8 years ago

I need to modify the LoginNotAllowed error_description so that a code is returned instead. From searching on here I see that I can do this with a custom command.

Here is what I have:

public class CustomErrorDescriptionCommand : ICommandHandler<GetValidationMessage> { public void Handle( GetValidationMessage cmd ) { if ( cmd.ID == MembershipRebootConstants.ValidationMessages.LoginNotAllowed ) { cmd.Message = Constants.AccountLockedErrorCode; } else { cmd.Message = null; } } }

I then register this like this:

Config.AddCommandHandler(new CustomErrorDescriptionCommand());

My breakpoints in the command handler aren't being hit and the error_description field remains the same. There are no errors from what I can see.

Any ideas on why this might be so? Also, isn't there already a command handler of this type registered by M.R so the command bus will find two command handlers?

Thanks

brockallen commented 8 years ago

Use AddCommandHandler on the UserAccountService. Looks like an oversight that the config command bus is not getting wired up.

JohnMcAvinue commented 8 years ago

Can you clarify what you mean by on the UserAccountService? I've tried adding the CommandHandler in the Constructor but that didn't work. I can't see where else it could be registered?

brockallen commented 8 years ago

https://github.com/brockallen/BrockAllen.MembershipReboot/blob/master/src/BrockAllen.MembershipReboot/AccountService/UserAccountService.cs#L132

JohnMcAvinue commented 8 years ago

Thanks but I meant at what point do I add this? I've currently done this because with using the Factory to create everything, the constructor is the only point at which I can be sure that the AddCommandHandler method will be called:

public class CustomUserAccountService : UserAccountService<CustomUser> { public CustomUserAccountService(CustomConfig config, CustomUserRepository repo) : base(config, repo) { AddCommandHandler(new CustomErrorDescriptionCommand()); } }

brockallen commented 8 years ago

Well, you'd have to intercept the creation and add it there.

If you want the config done, then send a PR :)

JohnMcAvinue commented 8 years ago

Thank you for the very kind offer but I can't share the code with you as it doesn't belong to me.

If you point me in the right direction though I can probably figure it out and stop taking up your time. We're using OWIN middleware so that's throwing me off a bit with finding where it's created & initalised.

brockallen commented 8 years ago

Thank you for the very kind offer but I can't share the code with you as it doesn't belong to me.

That's too bad that the company is ok with using "free" open source, but not willing to contribute back to help fix bugs or improve the software for others. That's not a personal attack - it's just a commentary on how businesses are behind the times and are mired in myopic legal policies. It sort of kills my motivation to help "the community".

As for pointing you in the right direction -- I was suggesting that you look at how the existing code works in the UserAccountService ctor and try to get that to merge in the commands from the config object.

JohnMcAvinue commented 8 years ago

That's a bit harsh.

I'm just a developer, even worse a contractor, and cannot open support requests to 3rd parties which costs money without going through proper channels first. I'd have to escalate this to my manager who would then make the decision and get approval for the financing.

I'm sure they wouldn't have a problem doing this and therefore supporting/rewarding you for your hard work but it could not happen without me doing my job and eliminating the possibility that I can do this myself. That is after all what they are paying me to do.

brockallen commented 8 years ago

Sure, I get that (I'm also a consultant). Personally, I don't sign contracts where I'm prohibited from contributing back to OSS projects that are being used by the company/project. Again, wasn't a personal attack on you. It's just another straw on the camel's back.

Hopefully my previous comment should be enough to help you move forward.

JohnMcAvinue commented 8 years ago

Sorry I misunderstood when you mentioned the pull request. I thought you wanted a version of our source code and not a corrected copy of M.R!

Yes I can do the latter no problem. From reading the source code I need to replicate how events on the event bus are merged in from the config in the UserAccountService. Should I create an AggregateCommandBus object in the same manner as AggregateEventBus? Any problems with that approach?

brockallen commented 8 years ago

No worries on the confusion.

Should I create an AggregateCommandBus

Yea, why not - it'd be consistent then. Then just merge the two lists -- one from the config and another from the member variable on the user account service.

JohnMcAvinue commented 8 years ago

New Pull Request on Dev branch. Sorry about that - still finding my way around Git!

brockallen commented 8 years ago

No worries. Is this now working for you with the commands in the config?

JohnMcAvinue commented 8 years ago

Yes it's working fine for me now thanks