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

Add password salting and minor corrections #6

Merged
merged 3 commits into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ jobs:
- name: Reuse ccache directory
uses: actions/cache@v4
with:
path: ~/.ccache
path: ~/.cache/ccache
key: '${{matrix.os}} ${{matrix.info}} ccache-dir ${{github.ref}} run-${{github.run_number}}'
restore-keys: |
${{matrix.os}} ${{matrix.info}} ccache-dir ${{github.ref}} run-'
Expand Down
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,4 @@ compile_commands.json
.ccache/
cmake-build-*
Testing/
configs/static_config.yaml
.DS_Store
7 changes: 2 additions & 5 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ add_library(${PROJECT_NAME}_objs OBJECT
src/validators/user_validators.cpp
src/validators/length_validator.hpp
src/validators/length_validator.cpp
src/utils/random.hpp
src/utils/random.cpp
src/utils/errors.hpp
src/utils/errors.cpp
src/utils/make_error.hpp
Expand Down Expand Up @@ -142,7 +144,6 @@ add_google_tests(${PROJECT_NAME}_unittest)
# Functional Tests
add_subdirectory(tests)


# Install
include(GNUInstallDirs)

Expand All @@ -152,12 +153,8 @@ if(DEFINED ENV{PREFIX})
set(CMAKE_INSTALL_PREFIX ${PREFIX_PATH})
endif()

set(CONFIG_VAR_PATH ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_SYSCONFDIR}/${PROJECT_NAME}/config_vars.yaml)
set(CONFIG_FALLBACK_PATH ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_SYSCONFDIR}/${PROJECT_NAME}/dynamic_config_fallback.json)
set(CONFIG_JWT ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_SYSCONFDIR}/${PROJECT_NAME}/jwt_config.json)

configure_file(${CMAKE_CURRENT_SOURCE_DIR}/configs/static_config.yaml.in ${CMAKE_CURRENT_SOURCE_DIR}/configs/static_config.yaml)

file(GLOB CONFIGS_FILES ${CMAKE_CURRENT_SOURCE_DIR}/configs/*.yaml ${CMAKE_CURRENT_SOURCE_DIR}/configs/*.json)

install(TARGETS ${PROJECT_NAME} DESTINATION ${CMAKE_INSTALL_BINDIR} COMPONENT ${PROJECT_NAME})
Expand Down
72 changes: 0 additions & 72 deletions configs/dynamic_config_fallback.json

This file was deleted.

4 changes: 0 additions & 4 deletions configs/static_config.yaml.in → configs/static_config.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
# yaml

config_vars: @CONFIG_VAR_PATH@

components_manager:
coro_pool:
initial_size: 500 # Preallocate 500 coroutines at startup.
Expand Down
4 changes: 3 additions & 1 deletion postgresql/schemas/db-1.sql
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ CREATE TABLE IF NOT EXISTS real_medium.users(
bio text,
image varchar(255),
password_hash varchar(255) NOT NULL,
salt varchar(255) NOT NULL,
CONSTRAINT uniq_username UNIQUE (username),
CONSTRAINT uniq_email UNIQUE (email)
);
Expand Down Expand Up @@ -77,7 +78,8 @@ CREATE TYPE real_medium.user AS (
email text,
bio TEXT,
image VARCHAR(255),
password_hash TEXT
password_hash TEXT,
salt TEXT
);

CREATE TYPE real_medium.full_article_info AS (
Expand Down
15 changes: 10 additions & 5 deletions src/db/sql.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
namespace real_medium::sql {

inline constexpr std::string_view kInsertUser = R"~(
INSERT INTO real_medium.users(username, email, password_hash)
VALUES($1, $2, $3)
INSERT INTO real_medium.users(username, email, password_hash, salt)
VALUES($1, $2, $3, $4)
RETURNING *
)~";

Expand All @@ -21,7 +21,8 @@ UPDATE real_medium.users SET
email = COALESCE($3, email),
bio = COALESCE($4, bio),
image = COALESCE($5, image),
password_hash = COALESCE($6, password_hash)
password_hash = COALESCE($6, password_hash),
salt = COALESCE($7, salt)
WHERE user_id = $1
RETURNING *
)~";
Expand Down Expand Up @@ -111,6 +112,10 @@ SELECT profile.username, profile.bio, profile.image,
FROM profile
)~";

inline constexpr std::string_view kGetSaltByEmail = R"~(
SELECT salt FROM real_medium.users WHERE email = $1
)~";

inline constexpr std::string_view KFollowingUser = R"~(
WITH profile AS (
SELECT * FROM real_medium.users WHERE user_id = $1
Expand Down Expand Up @@ -235,7 +240,7 @@ SELECT a.article_id AS articleId,
FROM real_medium.followers fl
WHERE fl.followed_user_id = a.user_id
) AS author_followed_by_user_ids,
ROW(u.user_id, u.username, u.email, u.bio, u.image, u.password_hash)::real_medium.user AS author_info
ROW(u.user_id, u.username, u.email, u.bio, u.image, u.password_hash, u.salt)::real_medium.user AS author_info
FROM real_medium.articles a
JOIN real_medium.users u ON a.user_id = u.user_id
)~"};
Expand All @@ -247,7 +252,7 @@ SELECT c.comment_id,
c.body,
c.user_id,
a.slug,
ROW(u.user_id, u.username, u.email, u.bio, u.image, u.password_hash)::real_medium.user AS author_info,
ROW(u.user_id, u.username, u.email, u.bio, u.image, u.password_hash, u.salt)::real_medium.user AS author_info,
ARRAY(
SELECT follower_user_id
FROM real_medium.followers fl
Expand Down
2 changes: 1 addition & 1 deletion src/handlers/comments/comment_delete.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ userver::formats::json::Value Handler::HandleRequestJsonThrow(
const userver::formats::json::Value& /*request_json*/,
userver::server::request::RequestContext& context) const {
auto user_id = context.GetData<std::optional<std::string>>("id");
const auto& comment_id = std::atoi(request.GetPathArg("id").c_str());
const auto& comment_id = userver::utils::FromString<int, std::string>(request.GetPathArg("id"));
const auto& slug = request.GetPathArg("slug");

const auto result_find_comment = pg_cluster_->Execute(
Expand Down
1 change: 1 addition & 0 deletions src/handlers/comments/comment_delete.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <fmt/format.h>
#include <string>
#include <string_view>
#include <userver/utils/from_string.hpp>

#include "userver/server/handlers/http_handler_base.hpp"
#include "userver/server/handlers/http_handler_json_base.hpp"
Expand Down
1 change: 0 additions & 1 deletion src/handlers/profiles/profiles.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include "userver/storages/postgres/cluster.hpp"
#include "userver/storages/postgres/component.hpp"

using namespace std;
using namespace userver::formats;
using namespace userver::server::http;
using namespace userver::server::request;
Expand Down
1 change: 0 additions & 1 deletion src/handlers/profiles/profiles_follow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include "userver/storages/postgres/cluster.hpp"
#include "userver/storages/postgres/component.hpp"

using namespace std;
using namespace userver::formats;
using namespace userver::server::http;
using namespace userver::server::request;
Expand Down
1 change: 0 additions & 1 deletion src/handlers/profiles/profiles_follow_delete.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include "userver/storages/postgres/component.hpp"
#include "utils/make_error.hpp"

using namespace std;
using namespace userver::formats;
using namespace userver::server::http;
using namespace userver::server::request;
Expand Down
7 changes: 5 additions & 2 deletions src/handlers/users/user_put.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "dto/user.hpp"
#include "models/user.hpp"
#include "utils/errors.hpp"
#include "utils/random.hpp"
#include "validators/validators.hpp"
namespace real_medium::handlers::users::put {

Expand Down Expand Up @@ -33,16 +34,18 @@ userver::formats::json::Value Handler::HandleRequestJsonThrow(
}

std::optional<std::string> password_hash = std::nullopt;
std::optional<std::string> salt = std::nullopt;
if (user_change_data.password) {
salt = utils::random::GenerateSalt();
password_hash =
userver::crypto::hash::Sha256(user_change_data.password.value());
userver::crypto::hash::Sha256(user_change_data.password.value() + salt.value());
}

const auto result = pg_cluster_->Execute(
userver::storages::postgres::ClusterHostType::kMaster,
sql::kUpdateUser.data(), user_id, user_change_data.username,
user_change_data.email, user_change_data.bio, user_change_data.image,
password_hash);
password_hash, salt);

auto user_result_data = result.AsSingleRow<real_medium::models::User>(
userver::storages::postgres::kRowTag);
Expand Down
6 changes: 4 additions & 2 deletions src/handlers/users/users.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "models/user.hpp"
#include "utils/errors.hpp"
#include "utils/make_error.hpp"
#include "utils/random.hpp"
#include "validators/validators.hpp"

namespace real_medium::handlers::users::post {
Expand Down Expand Up @@ -38,14 +39,15 @@ userver::formats::json::Value RegisterUser::HandleRequestJsonThrow(
return err.GetDetails();
}

auto salt = utils::random::GenerateSalt();
auto hash_password =
userver::crypto::hash::Sha256(user_register.password.value());
userver::crypto::hash::Sha256(user_register.password.value() + salt);
models::User result_user;
try {
auto query_result = pg_cluster_->Execute(
userver::storages::postgres::ClusterHostType::kMaster,
sql::kInsertUser.data(), user_register.username, user_register.email,
hash_password);
hash_password, salt);
result_user = query_result.AsSingleRow<models::User>(
userver::storages::postgres::kRowTag);
} catch (const userver::storages::postgres::UniqueViolation& ex) {
Expand Down
11 changes: 10 additions & 1 deletion src/handlers/users/users_login.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,17 @@ class LoginUser final : public userver::server::handlers::HttpHandlerJsonBase {
return err.GetDetails();
}

auto salt = pg_cluster_->Execute(
userver::storages::postgres::ClusterHostType::kMaster,
sql::kGetSaltByEmail.data(),
user_login.email);
if (salt.IsEmpty()) {
auto& response = request.GetHttpResponse();
response.SetStatus(userver::server::http::HttpStatus::kNotFound);
return {};
}
auto password_hash =
userver::crypto::hash::Sha256(user_login.password.value());
userver::crypto::hash::Sha256(user_login.password.value() + salt.AsSingleRow<std::string>());

auto userResult = pg_cluster_->Execute(
userver::storages::postgres::ClusterHostType::kMaster,
Expand Down
3 changes: 2 additions & 1 deletion src/models/user.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ struct User final {
std::optional<std::string> bio;
std::optional<std::string> image;
std::string password_hash;
std::string salt;

auto Introspect() {
return std::tie(id, username, email, bio, image, password_hash);
return std::tie(id, username, email, bio, image, password_hash, salt);
}
};

Expand Down
14 changes: 14 additions & 0 deletions src/utils/random.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#include "random.hpp"

#include <userver/utils/rand.hpp>

namespace real_medium::utils::random {
std::string GenerateSalt() {
const int kSaltLength = 32;
std::string salt;
for (size_t i = 0; i < kSaltLength; ++i) {
salt += userver::utils::RandRange(32, 126);
}
return salt;
}
} // namespace real_medium::utils::random
7 changes: 7 additions & 0 deletions src/utils/random.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#pragma once

#include <string>

namespace real_medium::utils::random {
std::string GenerateSalt();
} // namespace real_medium::utils::random
3 changes: 0 additions & 3 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
include(UserverTestsuite)

set(CONFIG_VARS_PATH "${CMAKE_SOURCE_DIR}/configs/config_vars_testing.yaml")
if (EXISTS "${CONFIG_VARS_PATH}")
set(PYTEST_ARGS_CONFIG_VARS "--service-config-vars=${CONFIG_VARS_PATH}")
Expand All @@ -14,6 +12,5 @@ userver_testsuite_add(
PYTEST_ARGS
--service-config=${CMAKE_SOURCE_DIR}/configs/static_config.yaml
--service-binary=${CMAKE_BINARY_DIR}/realmedium_sample
--config-fallback=${CMAKE_SOURCE_DIR}/configs/dynamic_config_fallback.json
${PYTEST_ARGS_CONFIG_VARS}
)
Loading