search

aspnet / WebHooks

[Archived] Libraries to create and consume web hooks on ASP.NET Core. Project moved to https://github.com/aspnet/AspLabs
Apache License 2.0
627 stars 439 forks source link

Remove maximum size limit on GitHubWebHook receiver key #245

Closed pranavkm closed 6 years ago

pranavkm commented 6 years ago

We limit it to 128 characters. There really isn't a good reason to have a max limit.

pranavkm commented 6 years ago

cc @dougbu \ @blowdart

dougbu commented 6 years ago

Nothing about this is specific to the GitHub receiver. For example, one common receiver configuration is to simply stick the secret key into a code query parameter. The secret key must be 32 to 128 characters long in that case.

We should remove the maximums everywhere except perhaps for the Salesforce receiver. In that one case, the sender provides the secret key and the limit ensures it was copied from the right spot. ("Perhaps" because I'm not sure that check is worthwhile either.)

dougbu commented 6 years ago

Get rid of limit on thingie in our configuration store…

dougbu commented 6 years ago

Realized (after staring at Identity's StoreOptions.MaxLengthForKeys a bit) there is a good reason to limit the size of secret keys in a WebHooks application. The keys are stored in IConfiguration and configuration stores may certainly include (say) a SQL database.

New idea: Instead of removing the maximum secret key length entirely, make the maximum a public property (somewhere) or perhaps add our first WebHookOptions property.

@blowdart do you have a preference on how this setting would be exposed?

dougbu commented 6 years ago

Updated previous comment. Since user has control of the secret keys they use and the corresponding SQL schema, database restrictions are not a good reason to keep the maximum length restrictions.

dougbu commented 6 years ago

30897e047f