Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

We don't seem to be checking the response status from the OpenMarket API #410

Closed
Tracked by #19317
babolivier opened this issue Oct 5, 2021 · 0 comments · Fixed by #412
Closed
Tracked by #19317

We don't seem to be checking the response status from the OpenMarket API #410

babolivier opened this issue Oct 5, 2021 · 0 comments · Fixed by #412
Assignees

Comments

@babolivier
Copy link
Contributor

In sendTextSMS:

async def sendTextSMS(
self, body: str, dest: str, source: Optional[Dict[str, str]] = None
) -> None:
"""
Sends a text message with the given body to the given MSISDN.
:param body: The message to send.
:param dest: The destination MSISDN to send the text message to.
"""
send_body = {
"mobileTerminate": {
"message": {"content": body, "type": "text"},
"destination": {
"address": dest,
},
},
}
if source:
send_body["mobileTerminate"]["source"] = {
"ton": tonFromType(source["type"]),
"address": source["text"],
}
username = self.sydent.config.sms.api_username
password = self.sydent.config.sms.api_password
b64creds = b64encode(b"%s:%s" % (username, password))
req_headers = Headers(
{
b"Authorization": [b"Basic " + b64creds],
b"Content-Type": [b"application/json"],
}
)
resp = await self.http_cli.post_json_get_nothing(
API_BASE_URL, send_body, {"headers": req_headers}
)
headers = dict(resp.headers.getAllRawHeaders())
if "Location" not in headers:
raise Exception("Got response from sending SMS with no location header")
# Nominally we should parse the URL, but we can just split on '/' since
# we only care about the last part.
parts = headers["Location"][0].split("/")
if len(parts) < 2:
raise Exception(
"Got response from sending SMS with malformed location header"
)

It looks like we do send the request but we're not actually checking if it succeeded (by checking the status code in the response). According to https://www.openmarket.com/docs/Content/apis/v4http/send-json.htm we should be checking whether the status code in the response is 202 Accepted.

It looks like we check the presence of a Location header though, which according to the doc is only present in a 202 response, but maybe that's not enough?

For extra context, I was trying to figure out why I didn't receive a text message when adding a phone number to my matrix.org account despite Sydent looking like it succeeded:

Oct  5 10:14:15 corus sydent-vectoris[16661]: 2021-10-05 10:14:15,659 - sydent.validators.msisdnvalidator - 126 - INFO - Attempting to text code [...] to 44[my phone number] (country 44) with originator {'type': 'alpha', 'text': 'Element'}
Oct  5 10:14:15 corus sydent-vectoris[16661]: 2021-10-05 10:14:15,659 - twisted - 147 - INFO - "::ffff:10.101.0.14" - - [05/Oct/2021:10:14:14 +0000] "POST /_matrix/identity/api/v1/validate/msisdn/requestToken HTTP/1.1" 200 94 "-" "Synapse/1.44.0rc2 (b=matrix-org-hotfixes,ebbd37b66)"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant