Telegram.Bot icon indicating copy to clipboard operation
Telegram.Bot copied to clipboard

Created Secret token validator middleware

Open BlueXTX opened this issue 3 years ago • 3 comments

resolve issue #1112

BlueXTX avatar Jan 11 '23 14:01 BlueXTX

First of all, @BlueXTX thank you for your contribution! Though I have a few concerns about this change:

  • The biggest issue is ASP.NET dependencies. I'm not sure how they play out for legacy platforms, covered by .netstandard2.0 target. It might be safer to introduce this change under #if DOTNET6_OR_GREATER directive to avoid collision with older .NET frameworks.
  • I don't see a way to differentiate between controllers for this middleware. If I have other endpoints in my app, they would require X-Telegram-Bot-Api-Secret-Token header too.
  • IMO helper functions and middleware classes would greatly benefit from some level of XML documentation on them.

@tuscen WDYT?

karb0f0s avatar Jan 12 '23 10:01 karb0f0s

First of all, @BlueXTX thank you for your contribution! Though I have a few concerns about this change:

  • The biggest issue is ASP.NET dependencies. I'm not sure how they play out for legacy platforms, covered by .netstandard2.0 target. It might be safer to introduce this change under #if DOTNET6_OR_GREATER directive to avoid collision with older .NET frameworks.
  • I don't see a way to differentiate between controllers for this middleware. If I have other endpoints in my app, they would require X-Telegram-Bot-Api-Secret-Token header too.
  • IMO helper functions and middleware classes would greatly benefit from some level of XML documentation on them.

@tuscen WDYT?

  1. Microsoft.Extensions.Options and Microsoft.AspNetCore.Http.Abstractions have support for netstandard2.0.
  2. Will it be enough if I add configuration like this? app.UseWhen(context => context.Request.Path.StartsWithSegments("/YourPath"), appBuilder => { appBuilder.UseMiddleware<SecretTokenValidatorMiddleware>(); });

BlueXTX avatar Jan 12 '23 18:01 BlueXTX

I don't think we should take dependency on M.E.x libraries in the core package. It's ok to make a separate package for ASP.NET Core integration though.

tuscen avatar Jan 12 '23 20:01 tuscen

Thanks for your contribution @BlueXTX For now, I think it's simpler to do without such middleware (see #1112)

wiz0u avatar Aug 06 '24 23:08 wiz0u