andrewlock / PwnedPasswords

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

Mention the usage of Polly in install instructions #23

Open ScholliYT opened 4 years ago

ScholliYT commented 4 years ago

I didn't knew polly was needed for this: https://github.com/andrewlock/PwnedPasswords/blob/master/samples/PwnedPasswords.Sample.NetCore3/Startup.cs#L15

SeanFarrow commented 4 years ago

Does it not get restored from NuGet? I’ll take a look at the master branch tomorrow to see what is going on.

From: Tom Stein notifications@github.com Sent: 01 September 2020 23:27 To: andrewlock/PwnedPasswords PwnedPasswords@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: [andrewlock/PwnedPasswords] Mention the usage of Polly in install instructions (#23)

I didn't knew polly was needed for this: https://github.com/andrewlock/PwnedPasswords/blob/master/samples/PwnedPasswords.Sample.NetCore3/Startup.cs#L15

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/andrewlock/PwnedPasswords/issues/23, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AALDK7UNJINJMIBGDQMBFYLSDVYLDANCNFSM4QSJUOKQ.

ScholliYT commented 4 years ago

At least for the PwnedPasswords.Client I had trouble getting it running. Things like AddTransientHttpErrorPolicy are polly specific. Now I just run without Polly required as NuGet. But I also only got in my ConfigureServices

identityBuilder.AddPwnedPasswordValidator<ApplicationUser>();

and the two package references in .csproj file.

Is the behaviour of retry and timeout implementet by the PwnedPasswords.Client by default? Without polly? Or would I need to set things up as explained in the Readme if I wanted that?

Just to clarify: I'm not trying to run the example mentioned with the link above. I was trying to implement this in my application.

SeanFarrow commented 4 years ago

The PwnedPasswordsClient constructor takes in an HttpClient and calls it’s GetAsync method to determine whether the passed in password has been pwned. It does not by itself handle retries, so you would need to use the Polly integration with HttpClientFactory for that as the sample does.

So, to use the PwnedPasswordsClient, you don’t strictly need Polly, but I always tent to use it as a matter of course when doing any HTTP work, particularly if I’m going to want to do things like retry, cache returned values and/or provide a fallback mechanism. The only exception to this is if an SDK I’m using wraps the HTTP calls, good examples of this are the ones available for AWS and Azure.

From: Tom Stein notifications@github.com Sent: 02 September 2020 01:14 To: andrewlock/PwnedPasswords PwnedPasswords@noreply.github.com Cc: Sean Farrow sean.farrow@seanfarrow.co.uk; Comment comment@noreply.github.com Subject: Re: [andrewlock/PwnedPasswords] Mention the usage of Polly in install instructions (#23)

At least for the PwnedPasswords.Client I had trouble getting it running. Things like AddTransientHttpErrorPolicy are polly specific. Now I just run without Polly required as NuGet. But I also only got in my ConfigureServices

identityBuilder.AddPwnedPasswordValidator();

and the two package references in .csproj file.

Is the behaviour of retry and timeout implementet by the PwnedPasswords.Client by default? Without polly? Or would I need to set things up as explained in the Readme if I wanted that?

Just to clarify: I'm not trying to run the example mentioned with the link above. I was trying to implement this in my application.

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/andrewlock/PwnedPasswords/issues/23#issuecomment-685200023, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AALDK7TFW4YR3IGUMRXBEG3SDWE3HANCNFSM4QSJUOKQ.

ScholliYT commented 4 years ago

Thanks for the explanation. I would appreciate to see this mentioned in the Readme. As you said the retry mechanism is really handy 😄

SeanFarrow commented 4 years ago

Yep, that’s a good shout, I’ll do this when I get a spare half hour, probably in the next few weeks.

From: Tom Stein notifications@github.com Sent: 03 September 2020 22:49 To: andrewlock/PwnedPasswords PwnedPasswords@noreply.github.com Cc: Sean Farrow sean.farrow@seanfarrow.co.uk; Comment comment@noreply.github.com Subject: Re: [andrewlock/PwnedPasswords] Mention the usage of Polly in install instructions (#23)

Thanks for the explanation. I would appreciate to see this mentioned in the Readme. As you said the retry mechanism is really handy 😄

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/andrewlock/PwnedPasswords/issues/23#issuecomment-686781388, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AALDK7UXG5X2BX6E5CIQJ5DSEAFMHANCNFSM4QSJUOKQ.