Printf debugging for MSISDN validation#11882
Printf debugging for MSISDN validation#11882DMRobertson merged 5 commits intomatrix-org-hotfixesfrom
Conversation
f45ebb4 to
574b4ff
Compare
synapse/rest/client/account.py
Outdated
| # Didn't like the sound of logging `client_secret`, but the spec says it is | ||
| # "A unique string generated by the client, and used to identify the validation | ||
| # attempt." I.e. something to facilitate deduplication. I don't think it's a | ||
| # sensitive secret per se. |
There was a problem hiding this comment.
It's not so much used to deduplicate but rather to make sure the next request(s) come from the client that started the process. I'm not sure what amount of harm can be done if someone gets a hold of someone else's client secret, but my paranoid self would prefer being cautious and not logging it if we don't need it.
There was a problem hiding this comment.
Thanks, I didn't grok that it was to be sent in another request.
synapse/rest/client/account.py
Outdated
| threepid_send_requests.labels(type="msisdn", reason="add_threepid").observe( | ||
| send_attempt | ||
| ) | ||
| logger.info("MSISDN %s is already in use by %s", msisdn, existing_user_id) |
There was a problem hiding this comment.
This doesn't look like it's in the right place?
There was a problem hiding this comment.
no, looks like a copy paste fail. thanks.
|
I'm assuming the CI failing might fixed by #11906 |
babolivier
left a comment
There was a problem hiding this comment.
CI failing isn't related to this PR, let's not block it on that.
I propose committing this, running it on m.org's master for a bit, then reverting once we're sufficiently confident in the results.