search

andrewlock / PwnedPasswords

An ASP.NET Core Identity validator that checks for PwnedPasswords
MIT License
103 stars 12 forks source link

Add API server sample #4

Open andrewlock opened 6 years ago

andrewlock commented 6 years ago

Refactored into several projects to share code Could potentially make it a shared project instead of an extra package. Would make using the validator and api server in the same project more complicated though.

SeanFarrow commented 6 years ago

Hi,

Three issues/concerns: Firstly we are not respecting the rate limit on the api, I would use: https://github.com/stefanprodan/AspNetCoreRateLimit and am happy to do this if you want?

Secondly, I'm just wondering why you've used a middleware and not a controller to do this--it's just a pattern I've not seen previously, although it's certainly valid.

Finally, can we split the services and there contracts out to separate assemblies, that way the EF implementation doesn't need ot know anything abou htee file service etc.

andrewlock commented 6 years ago

I don't think we want to ad the rate limit. There's up to the implementer as to how/whether they want to use a rate limit. You could add it to the sample though as an example if you like?

Adding the whole MVC stack seemed a bit overkill to just support a single endpoint, hence middleware instead. Plus that's a lot easier to package up.

We could create yet another library for the contracts alone, but then we have a lot of packages for what's meant to be a very small library. I'm already unsure whether the service implementations should actually be in a separate package, or whether we should just use a shared project and include them directly...