From 3dae19e82dc8f118e9d894f8aadd5eaadf12e27b Mon Sep 17 00:00:00 2001 From: lda Date: Tue, 21 Nov 2023 09:11:45 -0500 Subject: [PATCH] Refactor code to comply with #8 (#39) Reviewed-on: https://git.telodendria.io/Telodendria/Telodendria/pulls/39 Co-authored-by: lda Co-committed-by: lda --- Schema/LoginRequest.json | 38 +++++++++ Schema/Registration.json | 17 ++++ Schema/RequestToken.json | 21 +++++ src/Routes/RouteAdminDeactivate.c | 1 - src/Routes/RouteLogin.c | 130 +++++++--------------------- src/Routes/RouteRegister.c | 136 +++++++++--------------------- src/Routes/RouteRequestToken.c | 89 +++++++++++-------- 7 files changed, 203 insertions(+), 229 deletions(-) create mode 100644 Schema/LoginRequest.json create mode 100644 Schema/Registration.json create mode 100644 Schema/RequestToken.json diff --git a/Schema/LoginRequest.json b/Schema/LoginRequest.json new file mode 100644 index 0000000..b93dcc5 --- /dev/null +++ b/Schema/LoginRequest.json @@ -0,0 +1,38 @@ +{ + "header": "Schema\/LoginRequest.h", + "types": { + "LoginRequestType": { + "fields": { + "m.login.password": { "name": "REQUEST_TYPE_PASSWORD" } + }, + "type": "enum" + }, + "LoginRequestUserIdentifier": { + "fields": { + "type": { "type": "string" }, + "user": { "type": "string" } + }, + "type": "struct" + }, + "LoginRequest": { + "fields": { + "type": { "type": "LoginRequestType" }, + + "identifier": { "type": "object" }, + + "password": { "type": "string" }, + "address": { "type": "string" }, + "user": { "type": "string" }, + "device_id": { "type": "string" }, + "initial_device_display_name": { "type": "string" }, + "medium": { "type": "string" }, + "token": { "type": "string" }, + + "refresh_token": { "type": "boolean" } + }, + "type": "struct" + } + }, + "guard": "TELODENDRIA_SCHEMA_LOGIN_REQUEST_H" +} + diff --git a/Schema/Registration.json b/Schema/Registration.json new file mode 100644 index 0000000..f9db09c --- /dev/null +++ b/Schema/Registration.json @@ -0,0 +1,17 @@ +{ + "header": "Schema\/Registration.h", + "types": { + "RegistrationRequest": { + "fields": { + "username": { "type": "string" }, + "password": { "type": "string" }, + "device_id": { "type": "string" }, + "inhibit_login": { "type": "boolean" }, + "initial_device_display_name": { "type": "string" }, + "refresh_token": { "type": "boolean" } + }, + "type": "struct" + } + }, + "guard": "TELODENDRIA_SCHEMA_REGISTRATION_H" +} diff --git a/Schema/RequestToken.json b/Schema/RequestToken.json new file mode 100644 index 0000000..1f05393 --- /dev/null +++ b/Schema/RequestToken.json @@ -0,0 +1,21 @@ +{ + "header": "Schema\/RequestToken.h", + "types": { + "RequestToken": { + "fields": { + "client_secret": { "type": "string" }, + "send_attempt": { "type": "integer" }, + "next_link": { "type": "string" }, + "id_access_token": { "type": "string" }, + "id_server": { "type": "string" }, + + "email": { "type": "string" }, + + "country": { "type": "string" }, + "phone_number": { "type": "string" } + }, + "type": "struct" + } + }, + "guard": "TELODENDRIA_SCHEMA_REQUESTTOKEN_H" +} diff --git a/src/Routes/RouteAdminDeactivate.c b/src/Routes/RouteAdminDeactivate.c index 0cb9ac0..2a48e92 100644 --- a/src/Routes/RouteAdminDeactivate.c +++ b/src/Routes/RouteAdminDeactivate.c @@ -28,7 +28,6 @@ #include #include -#include ROUTE_IMPL(RouteAdminDeactivate, path, argp) { diff --git a/src/Routes/RouteLogin.c b/src/Routes/RouteLogin.c index 6c65a5c..f7b1d82 100644 --- a/src/Routes/RouteLogin.c +++ b/src/Routes/RouteLogin.c @@ -25,6 +25,8 @@ #include +#include + #include #include #include @@ -43,12 +45,9 @@ ROUTE_IMPL(RouteLogin, path, argp) HashMap *identifier; - char *deviceId = NULL; - char *initialDeviceDisplayName = NULL; - int refreshToken = 0; + LoginRequest loginRequest; + LoginRequestUserIdentifier userIdentifier; - char *password; - char *type; UserId *userId = NULL; Db *db = args->matrixArgs->db; @@ -57,6 +56,14 @@ ROUTE_IMPL(RouteLogin, path, argp) UserLoginInfo *loginInfo; char *fullUsername; + char *type; + char *initialDeviceDisplayName; + char *deviceId; + char *password; + int refreshToken; + + char *msg; + Config *config = ConfigLock(db); if (!config) @@ -68,6 +75,9 @@ ROUTE_IMPL(RouteLogin, path, argp) (void) path; + memset(&loginRequest, 0, sizeof(LoginRequest)); + memset(&userIdentifier, 0, sizeof(LoginRequestUserIdentifier)); + switch (HttpRequestMethodGet(args->context)) { case HTTP_GET: @@ -88,45 +98,21 @@ ROUTE_IMPL(RouteLogin, path, argp) break; } - val = HashMapGet(request, "type"); - if (!val) + if (!LoginRequestFromJson(request, &loginRequest, &msg)) { HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_MISSING_PARAM, NULL); + response = MatrixErrorCreate(M_BAD_JSON, msg); break; } - if (JsonValueType(val) != JSON_STRING) - { - HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_BAD_JSON, NULL); - break; - } - - type = JsonValueAsString(val); - if (!StrEquals(type, "m.login.password")) + if (loginRequest.type != REQUEST_TYPE_PASSWORD) { HttpResponseStatus(args->context, HTTP_BAD_REQUEST); response = MatrixErrorCreate(M_UNRECOGNIZED, NULL); break; } - val = HashMapGet(request, "identifier"); - if (!val) - { - HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_MISSING_PARAM, NULL); - break; - } - - if (JsonValueType(val) != JSON_OBJECT) - { - HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_BAD_JSON, NULL); - break; - } - - identifier = JsonValueAsObject(val); + identifier = loginRequest.identifier; val = HashMapGet(identifier, "type"); if (!val) @@ -150,23 +136,16 @@ ROUTE_IMPL(RouteLogin, path, argp) response = MatrixErrorCreate(M_UNRECOGNIZED, NULL); break; } - - val = HashMapGet(identifier, "user"); - if (!val) + if (!LoginRequestUserIdentifierFromJson(identifier, + &userIdentifier, &msg)) { HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_MISSING_PARAM, NULL); + response = MatrixErrorCreate(M_BAD_JSON, msg); break; } - if (JsonValueType(val) != JSON_STRING) - { - HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_BAD_JSON, NULL); - break; - } - userId = UserIdParse(JsonValueAsString(val), config->serverName); + userId = UserIdParse(userIdentifier.user, config->serverName); if (!userId) { HttpResponseStatus(args->context, HTTP_BAD_REQUEST); @@ -181,62 +160,12 @@ ROUTE_IMPL(RouteLogin, path, argp) response = MatrixErrorCreate(M_FORBIDDEN, NULL); break; } + + deviceId = loginRequest.device_id; - val = HashMapGet(request, "device_id"); - if (val) - { - if (JsonValueType(val) != JSON_STRING) - { - HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_BAD_JSON, NULL); - break; - } - - deviceId = JsonValueAsString(val); - } - - val = HashMapGet(request, "initial_device_display_name"); - if (val) - { - if (JsonValueType(val) != JSON_STRING) - { - HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_BAD_JSON, NULL); - break; - } - - initialDeviceDisplayName = JsonValueAsString(val); - } - - val = HashMapGet(request, "password"); - if (!val) - { - HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_MISSING_PARAM, NULL); - break; - } - - if (JsonValueType(val) != JSON_STRING) - { - HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_BAD_JSON, NULL); - break; - } - - password = JsonValueAsString(val); - - val = HashMapGet(request, "refresh_token"); - if (val) - { - if (JsonValueType(val) != JSON_BOOLEAN) - { - HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_BAD_JSON, NULL); - break; - } - - refreshToken = JsonValueAsBoolean(val); - } + initialDeviceDisplayName =loginRequest.initial_device_display_name; + password = loginRequest.password; + refreshToken = loginRequest.refresh_token; user = UserLock(db, userId->localpart); @@ -309,5 +238,8 @@ ROUTE_IMPL(RouteLogin, path, argp) JsonFree(request); ConfigUnlock(config); + LoginRequestFree(&loginRequest); + LoginRequestUserIdentifierFree(&userIdentifier); + return response; } diff --git a/src/Routes/RouteRegister.c b/src/Routes/RouteRegister.c index 3527219..c43883a 100644 --- a/src/Routes/RouteRegister.c +++ b/src/Routes/RouteRegister.c @@ -30,6 +30,8 @@ #include #include +#include + #include #include #include @@ -55,22 +57,15 @@ ROUTE_IMPL(RouteRegister, path, argp) HashMap *request = NULL; HashMap *response = NULL; - JsonValue *val; + RegistrationRequest regReq; char *kind; - - char *username = NULL; - char *password = NULL; - char *initialDeviceDisplayName = NULL; - int refreshToken = 0; - int inhibitLogin = 0; - char *deviceId = NULL; char *fullUsername; + char *msg; + char *username; Db *db = args->matrixArgs->db; - User *user = NULL; - Array *uiaFlows = NULL; int uiaResult; @@ -79,6 +74,16 @@ ROUTE_IMPL(RouteRegister, path, argp) Config *config = ConfigLock(db); + regReq.username = NULL; + regReq.password = NULL; + regReq.device_id = NULL; + regReq.initial_device_display_name = NULL; + regReq.refresh_token = 0; + regReq.inhibit_login = 0; + + + + if (!config) { Log(LOG_ERR, "Registration endpoint failed to lock configuration."); @@ -102,26 +107,23 @@ ROUTE_IMPL(RouteRegister, path, argp) response = MatrixErrorCreate(M_NOT_JSON, NULL); goto end; } - - val = HashMapGet(request, "username"); - if (val) + if (!RegistrationRequestFromJson(request, ®Req, &msg)) { - if (JsonValueType(val) != JSON_STRING) - { - HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_BAD_JSON, NULL); - goto finish; - } - username = StrDuplicate(JsonValueAsString(val)); + HttpResponseStatus(args->context, HTTP_BAD_REQUEST); + response = MatrixErrorCreate(M_NOT_JSON, msg); + goto end; + } - if (!UserValidate(username, config->serverName)) + if (regReq.username) + { + if (!UserValidate(regReq.username, config->serverName)) { HttpResponseStatus(args->context, HTTP_BAD_REQUEST); response = MatrixErrorCreate(M_INVALID_USERNAME, NULL); goto finish; } - if (UserExists(db, username)) + if (UserExists(db, regReq.username)) { HttpResponseStatus(args->context, HTTP_BAD_REQUEST); response = MatrixErrorCreate(M_USER_IN_USE, NULL); @@ -158,99 +160,44 @@ ROUTE_IMPL(RouteRegister, path, argp) /* We don't support guest accounts yet */ if (kind && !StrEquals(kind, "user")) { + msg = "Guest accounts are currently not supported"; HttpResponseStatus(args->context, HTTP_FORBIDDEN); - response = MatrixErrorCreate(M_INVALID_PARAM, NULL); + response = MatrixErrorCreate(M_INVALID_PARAM, msg); goto finish; } - val = HashMapGet(request, "password"); - if (!val) + if (!regReq.password) { + msg = "'password' field is unset"; HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_MISSING_PARAM, NULL); + response = MatrixErrorCreate(M_MISSING_PARAM, msg); goto finish; } - if (JsonValueType(val) != JSON_STRING) - { - HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_BAD_JSON, NULL); - goto finish; - } + /* All of the other fields are optional, we don't have to check + * them. */ - password = StrDuplicate(JsonValueAsString(val)); - - val = HashMapGet(request, "device_id"); - if (val) - { - if (JsonValueType(val) != JSON_STRING) - { - HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_BAD_JSON, NULL); - goto finish; - } - - deviceId = StrDuplicate(JsonValueAsString(val)); - } - - val = HashMapGet(request, "inhibit_login"); - if (val) - { - if (JsonValueType(val) != JSON_BOOLEAN) - { - HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_BAD_JSON, NULL); - goto finish; - } - - inhibitLogin = JsonValueAsBoolean(val); - } - - val = HashMapGet(request, "initial_device_display_name"); - if (val) - { - if (JsonValueType(val) != JSON_STRING) - { - HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_BAD_JSON, NULL); - goto finish; - } - - initialDeviceDisplayName = StrDuplicate(JsonValueAsString(val)); - } - - val = HashMapGet(request, "refresh_token"); - if (val) - { - if (JsonValueType(val) != JSON_BOOLEAN) - { - HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_BAD_JSON, NULL); - goto finish; - } - - refreshToken = JsonValueAsBoolean(val); - } - - user = UserCreate(db, username, password); + user = UserCreate(db, regReq.username, regReq.password); response = HashMapCreate(); - fullUsername = StrConcat(4, "@", UserGetName(user), ":", config->serverName); + fullUsername = StrConcat(4, + "@", UserGetName(user), ":", config->serverName); HashMapSet(response, "user_id", JsonValueString(fullUsername)); Free(fullUsername); HttpResponseStatus(args->context, HTTP_OK); - if (!inhibitLogin) + if (!regReq.inhibit_login) { - UserLoginInfo *loginInfo = UserLogin(user, password, deviceId, - initialDeviceDisplayName, refreshToken); + UserLoginInfo *loginInfo = UserLogin(user, regReq.password, + regReq.device_id, regReq.initial_device_display_name, + regReq.refresh_token); HashMapSet(response, "access_token", JsonValueString(loginInfo->accessToken->string)); HashMapSet(response, "device_id", JsonValueString(loginInfo->accessToken->deviceId)); - if (refreshToken) + if (regReq.refresh_token) { HashMapSet(response, "expires_in_ms", JsonValueInteger(loginInfo->accessToken->lifetime)); @@ -294,10 +241,7 @@ ROUTE_IMPL(RouteRegister, path, argp) UserUnlock(user); finish: UiaFlowsFree(uiaFlows); - Free(username); - Free(password); - Free(deviceId); - Free(initialDeviceDisplayName); + RegistrationRequestFree(®Req); JsonFree(request); } else diff --git a/src/Routes/RouteRequestToken.c b/src/Routes/RouteRequestToken.c index 76a9ee2..779b541 100644 --- a/src/Routes/RouteRequestToken.c +++ b/src/Routes/RouteRequestToken.c @@ -26,14 +26,31 @@ #include #include +#include + ROUTE_IMPL(RouteRequestToken, path, argp) { RouteArgs *args = argp; char *type = ArrayGet(path, 0); HashMap *request; HashMap *response; - JsonValue *val; - char *str; + + char *msg; + + RequestToken reqTok; + + Int64 minusOne = Int64Neg(Int64Create(0, 1)); + + reqTok.client_secret = NULL; + reqTok.next_link = NULL; + reqTok.id_access_token = NULL; + reqTok.id_server = NULL; + + reqTok.email = NULL; + reqTok.country = NULL; + reqTok.phone_number = NULL; + + reqTok.send_attempt = minusOne; if (HttpRequestMethodGet(args->context) != HTTP_POST) { @@ -48,87 +65,92 @@ ROUTE_IMPL(RouteRequestToken, path, argp) return MatrixErrorCreate(M_NOT_JSON, NULL); } - val = HashMapGet(request, "client_secret"); - if (!val || JsonValueType(val) != JSON_STRING) + if (!RequestTokenFromJson(request, &reqTok, &msg)) { HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_BAD_JSON, NULL); + response = MatrixErrorCreate(M_BAD_JSON, msg); goto finish; } - str = JsonValueAsString(val); - if (strlen(str) > 255 || StrBlank(str)) + if (!reqTok.client_secret) { + msg = "'client_secret' is not set"; HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_BAD_JSON, NULL); + response = MatrixErrorCreate(M_BAD_JSON, msg); goto finish; } - val = HashMapGet(request, "send_attempt"); - if (!val || JsonValueType(val) != JSON_INTEGER) + if (strlen(reqTok.client_secret) > 255 || StrBlank(reqTok.client_secret)) { + msg = "'client_secret' is blank or too long"; HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_BAD_JSON, NULL); + response = MatrixErrorCreate(M_BAD_JSON, msg); goto finish; } - val = HashMapGet(request, "next_link"); - if (val && JsonValueType(val) != JSON_STRING) + if (Int64Eq(reqTok.send_attempt, minusOne)) { + msg = "Invalid or inexistent 'send_attempt'"; HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_BAD_JSON, NULL); + response = MatrixErrorCreate(M_BAD_JSON, msg); goto finish; } - val = HashMapGet(request, "id_access_token"); - if (val && JsonValueType(val) != JSON_STRING) + if (!reqTok.next_link) { + msg = "'next_link' is not set"; HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_BAD_JSON, NULL); + response = MatrixErrorCreate(M_BAD_JSON, msg); goto finish; } - - val = HashMapGet(request, "id_server"); - if (val && JsonValueType(val) != JSON_STRING) + if (!reqTok.id_access_token) { + msg = "'id_access_token' is not set"; HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_BAD_JSON, NULL); + response = MatrixErrorCreate(M_BAD_JSON, msg); + goto finish; + } + if (!reqTok.id_server) + { + msg = "'id_server' is not set"; + HttpResponseStatus(args->context, HTTP_BAD_REQUEST); + response = MatrixErrorCreate(M_BAD_JSON, msg); goto finish; } if (StrEquals(type, "email")) { - val = HashMapGet(request, "email"); - if (val && JsonValueType(val) != JSON_STRING) + if (!reqTok.email) { + msg = "Type is set to 'email' yet none was set"; HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_BAD_JSON, NULL); + response = MatrixErrorCreate(M_BAD_JSON, msg); goto finish; } } else if (StrEquals(type, "msisdn")) { - val = HashMapGet(request, "country"); - if (val && JsonValueType(val) != JSON_STRING) + if (!reqTok.country) { + msg = "Type is set to 'msisdn' yet no country is set"; HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_BAD_JSON, NULL); + response = MatrixErrorCreate(M_BAD_JSON, msg); goto finish; } - str = JsonValueAsString(val); - if (strlen(str) != 2) + if (strlen(reqTok.country) != 2) { + msg = "Invalid country tag, length must be 2"; HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_BAD_JSON, NULL); + response = MatrixErrorCreate(M_BAD_JSON, msg); goto finish; } - val = HashMapGet(request, "phone_number"); - if (val && JsonValueType(val) != JSON_STRING) + if (!reqTok.phone_number) { + msg = "Type is set to 'msisdn' yet phone_number is unset"; HttpResponseStatus(args->context, HTTP_BAD_REQUEST); - response = MatrixErrorCreate(M_BAD_JSON, NULL); + response = MatrixErrorCreate(M_BAD_JSON, msg); goto finish; } } @@ -145,5 +167,6 @@ ROUTE_IMPL(RouteRequestToken, path, argp) finish: JsonFree(request); + RequestTokenFree(&reqTok); return response; }