brockallen / BrockAllen.MembershipReboot

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

Allow non-unique certificates #645

Closed mirecg closed 7 years ago

mirecg commented 8 years ago

Hi, I need to allow for multiple accounts to have the same certificate. This is not allowed in your current implementation, where the UserAccountValidator handles the CertificateAddedEvent and checks for existence of other account with the same certificate. I do not see any chance to override this. The UserAccountValidator is hardcoded in the UserAccountService, so I cannot replace or nor to override this behavior. It would be nice to have an option to supress this built-in UserAccountValidator using some configuration option.

brockallen commented 8 years ago

Can you explain the use case for multiple certs for one user?

mirecg commented 8 years ago

Hi, not multiple certificates for one user, this is allowed. But multiple users to have the same certificate. We have a customer where multiple users share the same certificate for login to our system. This was allowed in our currently used identity model. But we are migrating to this MembershipReboot identity model. This is killer requirement for us. We use certificate as the second authentication factor, so there's no identification problem when authenticating user by certificate as a second factor, because user is already identified by username/password.

brockallen commented 8 years ago

I'd have to think about this. It's certainly not a hard change... but it might affect current uses. Maybe a setting that would say this is allowed?

mirecg commented 8 years ago

The setting controlling the uniqueness of the certificate is sufficient.

mirecg commented 8 years ago

Hi, how do you see this, when are you going to incorporate this setting into release? Without it I will need to go with own version of code which would be my last resort.

brockallen commented 8 years ago

I don't have time. You can propose a PR.

mirecg commented 8 years ago

hmm, the same on my side As I can see there are already several PRs waiting for quite a long time. This will not be the solution ...

mirecg commented 8 years ago

Hi, I've finaly created a PR for this issue. Be the meantime I've found simmilar issue with the phone uniqueness. Would mind allowing non-unique mobile phones? The reason is the same as for the certificates. In our existing solution the customers have several accounts with same certificate and with the same phone number. I would open another issue for this and I would create another PR with simmilar solution, as having configuration property PhoneNumberIsUnique.

brockallen commented 7 years ago

Given that I don't foresee making this change, I'll close this issue. Thanks.