feature: allow channels to register with public key #382
Conversation
| def normalize_id(id): | ||
| if not id: | ||
| return id | ||
| return str(uuid.UUID(id)) |
There was a problem hiding this comment.
This is an expensive operation. Might want to have a regex for the normalized ID, so that re-normalizing a normalized one isn't done, which would be faster if its generally the case that they're already normalized. If it's rare that they're normalized, the extra regex would be slower... so whichever.
|
Pending a more efficient normalize func, this patch looks great. 👏 |
jrconlin
force-pushed
the
feature/jwt_chan
branch
from
d59cc24 to
c269640
Compare
| return id | ||
| if (len(id) == 36 and id[8] == id[13] == id[18] == id[23] == '-'): | ||
| return id.lower() | ||
| raw = id.lower().replace('-', '') |
There was a problem hiding this comment.
The - are removed so that they can be added again next?
There was a problem hiding this comment.
This is more a sanity thing. If there are misplaced dashes, strip them before we do any additional formatting. In reality, I should be stripping non-hex characters and checking for string length, but that gets me back precariously to just running the string through uuid.UUID().
|
Commit message has a semicolon after Closes that shouldn't be there, it seems to screw up multiple tools parsing the commit messages. |
jrconlin
force-pushed
the
feature/jwt_chan
branch
2 times, most recently
from
a2e7e11 to
f7bb8ef
Compare
|
This looks really good, @jrconlin! 💯 Are you planning to add |
jrconlin
force-pushed
the
feature/jwt_chan
branch
from
f7bb8ef to
61af580
Compare
| @@ -1000,7 +1000,7 @@ def process_register(self, data): | |||
| self.transport.pauseProducing() | |||
|
|
|||
| d = self.deferToThread(self.ap_settings.make_endpoint, self.ps.uaid, | |||
There was a problem hiding this comment.
Do you have an integration test for this, by chance? I don't want to block this from landing based on that, but it would be nice to see it end-to-end.
There was a problem hiding this comment.
I agree that a change like this should have an integration test.
jrconlin
force-pushed
the
feature/jwt_chan
branch
from
61af580 to
baf7e67
Compare
Channels can include a public key when registering a new subscription channel. This public key should match the public key used to send subscription updates later. NOTE: this patch changes the format of the endpoint URLs, & the content of the endpoint URL token. This change also requires that ChannelIDs be normalized to dashed format, (e.g. a lower case, dash delimited string "deadbeef-0000-0000-deca-fbad11112222") This is the default mechanism used by Firefox for UUID generation. It is STRONGLY urged that clients normalize UUIDs used for ChannelIDs and User Agent IDs. While this should not break existing clients, additional testing may be required. Closes #326
jrconlin
force-pushed
the
feature/jwt_chan
branch
from
baf7e67 to
3d15b9b
Compare
|
r+ |
feature: allow channels to register with public key
Channels can include a public key when registering a new subscription
channel. This public key should match the public key used to send
subscription updates later.
NOTE: this patch changes the format of the endpoint URLs, & the content
of the endpoint URL token. This change also requires that ChannelIDs be
normalized to dashed format, (e.g. a lower case, dash delimited string
"deadbeef-0000-0000-deca-fbad11112222") This is the default mechanism
used by Firefox for UUID generation. It is STRONGLY urged that clients
normalize UUIDs used for ChannelIDs and User Agent IDs. While this
should not break existing clients, additional testing may be required.
Closes #326
@bbangert r?