py_vapid: do not enforce dot in sub claim email host part #90
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
While developing our django application, we had `admin@localhost` configured as our site contact address, which was automatically used by our webpush lib. While testing, py_vapid errored complaining that the sub claim wasn't there, even though it was. `mailto:admin@localhost` wasn't considered as a valid sub claim. I propose not enforcing having a dot in the regex, so people don't have to get frustrated figuring that out like me. Although I would understand if you would say that a `localhost` host shouldn't be considered valid in a sub claim. We mitigated by changing our default developing contact address.
8 tasks
|
I feel like I should push back on this one a little bit, but not for the reason you're saying. VAPID is there mostly for Ops folk to have a way to contact the origin if there's a problem, so having something reasonably valid is kind of important. RFC8292 notes that I took a fairly lazy route on this one, and it's kinda come back to haunt me, so, going to file a proper issue and see about resolving that. |
|
Thanks for explaining! :) |
|
No worries. Thanks for pointing out the issue! |
jrconlin
added a commit
that referenced
this pull request
Mar 20, 2021
While far from perfect, this is a little better about checking the sorts of values passed in the `sub` claim. It checks for ipv6 like as well as localhost in mailto and https forms. (Yeah, you'll still need to supply `https` since that's part of the RFC. Plus, letsEncrypt is a thing, so there's that.) I've also introduced a new option `--no-strict` which turns off `sub` checks. It has to be present, but what the value is can be up to you. I'll note that this kinda ruins what VAPID is supposed to be about. The `sub` is there so that if there's a problem with your subscription, Ops can reach out to you to help fix it rather than just straight up block you. Closes: #90
jrconlin
added a commit
that referenced
this pull request
Mar 29, 2021
While far from perfect, this is a little better about checking the sorts of values passed in the `sub` claim. It checks for ipv6 like as well as localhost in mailto and https forms. (Yeah, you'll still need to supply `https` since that's part of the RFC. Plus, letsEncrypt is a thing, so there's that.) I've also introduced a new option `--no-strict` which turns off `sub` checks. It has to be present, but what the value is can be up to you. I'll note that this kinda ruins what VAPID is supposed to be about. The `sub` is there so that if there's a problem with your subscription, Ops can reach out to you to help fix it rather than just straight up block you. Closes: #90
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While developing our django application, we had
admin@localhostconfigured as our site contact address, which was automatically used by our webpush lib. While testing, py_vapid errored complaining that the sub claim wasn't there, even though it was.mailto:admin@localhostwasn't considered as a valid sub claim.I propose not enforcing having a dot in the regex, so people don't have to get frustrated figuring that out like me.
Although I would understand if you would say that a
localhosthost shouldn't be considered valid in a sub claim. We mitigated by changing our default developing contact address.