Skip to content

Commit 0716e93

Browse files
Totto16Totto16
committed
feat: use distinct type for payload of SecretStorage, so that you can't use a string without manual conversion
1 parent 5228ed3 commit 0716e93

File tree

6 files changed

+129
-26
lines changed

6 files changed

+129
-26
lines changed

‎src/lobby/api.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ lobby::API::API(const std::string& api_url)
5959
auto value = m_secret_storage->load(constants::api_token_key);
6060

6161
if (value.has_value()) {
62-
if (not this->setup_authentication(value.value())) {
62+
if (not this->setup_authentication(value.value().as_string())) {
6363
throw std::runtime_error("Couldn't setup authentication");
6464
}
6565
} else {
@@ -275,7 +275,7 @@ bool lobby::API::setup_authentication(const std::string& token) {
275275
m_authentication_token = token;
276276

277277
m_client->SetBearerAuth(token);
278-
if (auto result = m_secret_storage->store(constants::api_token_key, token); result.has_value()) {
278+
if (auto result = m_secret_storage->store(constants::api_token_key, secret::Buffer{ token }); result.has_value()) {
279279
spdlog::error("API {}", result.value());
280280
}
281281

‎src/lobby/credentials/buffer.cpp‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
2+
3+
4+
#include "./buffer.hpp"
5+
6+
7+
secret::Buffer::Buffer(const std::string& data) : m_size{ data.size() } {
8+
if (m_size == 0) {
9+
return;
10+
}
11+
12+
m_data = new std::byte[m_size]; //NOLINT(cppcoreguidelines-owning-memory)
13+
std::memcpy(this->m_data, data.data(), m_size);
14+
}
15+
16+
secret::Buffer::Buffer(std::byte* data, std::size_t size) : m_size{ size } {
17+
if (m_size == 0) {
18+
return;
19+
}
20+
21+
m_data = new std::byte[m_size]; //NOLINT(cppcoreguidelines-owning-memory)
22+
std::memcpy(this->m_data, data, m_size);
23+
}
24+
25+
secret::Buffer::~Buffer() {
26+
if (m_data != nullptr && m_size != 0) {
27+
delete[] m_data;
28+
}
29+
}
30+
31+
secret::Buffer::Buffer(Buffer&& other) noexcept : m_data{ other.m_data }, m_size{ other.m_size } {
32+
other.m_size = 0;
33+
other.m_data = nullptr;
34+
}
35+
36+
37+
[[nodiscard]] std::string secret::Buffer::as_string() const {
38+
return std::string{
39+
reinterpret_cast<char*>(m_data), //NOLINT(cppcoreguidelines-pro-type-reinterpret-cast)
40+
reinterpret_cast<char*>(m_data //NOLINT(cppcoreguidelines-pro-type-reinterpret-cast)
41+
) + m_size //NOLINT(cppcoreguidelines-pro-bounds-pointer-arithmetic,coreguidelines-pro-type-reinterpret-cast)
42+
};
43+
}
44+
45+
46+
[[nodiscard]] std::byte* secret::Buffer::data() const {
47+
return m_data;
48+
}
49+
50+
[[nodiscard]] std::size_t secret::Buffer::size() const {
51+
return m_size;
52+
}

‎src/lobby/credentials/buffer.hpp‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
2+
3+
#pragma once
4+
5+
#include <cstring>
6+
#include <string>
7+
8+
#include "helper/windows.hpp"
9+
10+
namespace secret {
11+
struct Buffer {
12+
private:
13+
std::byte* m_data{ nullptr };
14+
std::size_t m_size;
15+
16+
public:
17+
OOPETRIS_GRAPHICS_EXPORTED explicit Buffer(const std::string& data);
18+
OOPETRIS_GRAPHICS_EXPORTED explicit Buffer(std::byte* data, std::size_t size);
19+
template<std::size_t A>
20+
OOPETRIS_GRAPHICS_EXPORTED explicit Buffer(std::array<std::byte, A> data) : m_size{ data.size() } {
21+
if (m_size == 0) {
22+
return;
23+
}
24+
25+
m_data = new std::byte[m_size]; //NOLINT(cppcoreguidelines-owning-memory)
26+
std::memcpy(this->m_data, data, m_size);
27+
}
28+
29+
OOPETRIS_GRAPHICS_EXPORTED ~Buffer();
30+
31+
OOPETRIS_GRAPHICS_EXPORTED Buffer(const Buffer& other) = delete;
32+
OOPETRIS_GRAPHICS_EXPORTED Buffer& operator=(const Buffer& other) = delete;
33+
34+
OOPETRIS_GRAPHICS_EXPORTED Buffer(Buffer&& other) noexcept;
35+
OOPETRIS_GRAPHICS_EXPORTED Buffer& operator=(Buffer&& other) noexcept = delete;
36+
37+
[[nodiscard]] std::string as_string() const;
38+
39+
[[nodiscard]] std::byte* data() const;
40+
41+
[[nodiscard]] std::size_t size() const;
42+
};
43+
44+
} // namespace secret

‎src/lobby/credentials/meson.build‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
graphics_src_files += files(
2+
'buffer.cpp',
3+
'buffer.hpp',
24
'secret.cpp',
35
'secret.hpp',
46
)

‎src/lobby/credentials/secret.cpp‎

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -63,13 +63,13 @@ secret::SecretStorage::SecretStorage(KeyringType type) : m_type{ type } {
6363
}
6464
}
6565

66-
secret::SecretStorage::~SecretStorage() = default;
66+
secret::SecretStorage::~SecretStorage() = default; //NOLINT(performance-trivially-destructible)
6767

6868
secret::SecretStorage::SecretStorage(SecretStorage&& other) noexcept
6969
: m_type{ other.m_type },
7070
m_ring_id{ other.m_ring_id } { }
7171

72-
[[nodiscard]] helper::expected<std::string, std::string> secret::SecretStorage::load(const std::string& key) const {
72+
[[nodiscard]] helper::expected<secret::Buffer, std::string> secret::SecretStorage::load(const std::string& key) const {
7373

7474
auto key_id = get_id_from_name(m_ring_id, key);
7575

@@ -90,19 +90,17 @@ secret::SecretStorage::SecretStorage(SecretStorage&& other) noexcept
9090
};
9191
}
9292

93-
auto result_string = std::string{
94-
static_cast<char*>(buffer),
95-
static_cast<char*>(buffer) + result //NOLINT(cppcoreguidelines-pro-bounds-pointer-arithmetic)
96-
};
93+
94+
auto result_buffer =
95+
Buffer{ reinterpret_cast<std::byte*>(buffer), //NOLINT(cppcoreguidelines-pro-type-reinterpret-cast)
96+
static_cast<std::size_t>(result) };
97+
9798
free(buffer); // NOLINT(cppcoreguidelines-no-malloc,cppcoreguidelines-owning-memory)
98-
return result_string;
99+
return result_buffer;
99100
}
100101

101-
[[nodiscard]] std::optional<std::string> secret::SecretStorage::store(
102-
const std::string& key, //NOLINT(bugprone-easily-swappable-parameters)
103-
const std::string& value,
104-
bool update
105-
) const {
102+
[[nodiscard]] std::optional<std::string>
103+
secret::SecretStorage::store(const std::string& key, const Buffer& value, bool update) const {
106104

107105
auto key_id = get_id_from_name(m_ring_id, key);
108106

@@ -113,7 +111,7 @@ secret::SecretStorage::SecretStorage(SecretStorage&& other) noexcept
113111

114112
auto full_key = get_key_name(key);
115113

116-
auto new_id = add_key(constants::key_type_user, full_key.c_str(), value.c_str(), value.size(), m_ring_id);
114+
auto new_id = add_key(constants::key_type_user, full_key.c_str(), value.data(), value.size(), m_ring_id);
117115

118116
if (new_id < 0) {
119117
return fmt::format("Error while storing a key, can't add the key: {}", strerror(errno));
@@ -235,7 +233,7 @@ secret::SecretStorage::SecretStorage(SecretStorage&& other) noexcept
235233
}
236234

237235

238-
[[nodiscard]] helper::expected<std::string, std::string> secret::SecretStorage::load(const std::string& key) const {
236+
[[nodiscard]] helper::expected<secret::Buffer, std::string> secret::SecretStorage::load(const std::string& key) const {
239237

240238
auto maybe_key_handle = get_handle_from_name(m_type, m_phProvider, key);
241239

@@ -281,15 +279,18 @@ secret::SecretStorage::SecretStorage(SecretStorage&& other) noexcept
281279

282280
NCryptFreeObject(key_handle);
283281

284-
auto result_string = std::string{ reinterpret_cast<char*>(buffer), reinterpret_cast<char*>(buffer) + bytes_needed };
282+
auto result_buffer =
283+
Buffer{ reinterpret_cast<std::byte*>(buffer), //NOLINT(cppcoreguidelines-pro-type-reinterpret-cast)
284+
static_cast<std::size_t>(bytes_needed) };
285+
285286

286287
delete[] buffer;
287288

288-
return result_string;
289+
return result_buffer;
289290
}
290291

291292
[[nodiscard]] std::optional<std::string>
292-
secret::SecretStorage::store(const std::string& key, const std::string& value, bool update) const {
293+
secret::SecretStorage::store(const std::string& key, const Buffer& value, bool update) const {
293294

294295
NCRYPT_KEY_HANDLE key_handle{};
295296

@@ -317,7 +318,7 @@ secret::SecretStorage::store(const std::string& key, const std::string& value, b
317318

318319
PBYTE buffer = new BYTE[value.size()];
319320

320-
std::memcpy(buffer, value.c_str(), value.size());
321+
std::memcpy(buffer, value.data(), value.size());
321322

322323
auto result2 =
323324
NCryptSetProperty(key_handle, constants::property_name, buffer, static_cast<DWORD>(value.size()), flags2);
@@ -403,7 +404,7 @@ secret::SecretStorage::SecretStorage(SecretStorage&& other) noexcept
403404
m_file_path{ other.m_file_path } { }
404405

405406

406-
[[nodiscard]] helper::expected<std::string, std::string> secret::SecretStorage::load(const std::string& key) const {
407+
[[nodiscard]] helper::expected<secret::Buffer, std::string> secret::SecretStorage::load(const std::string& key) const {
407408

408409
auto maybe_json_value = get_json_from_file(m_file_path);
409410

@@ -421,11 +422,13 @@ secret::SecretStorage::SecretStorage(SecretStorage&& other) noexcept
421422

422423
json_value.at(key).get_to(result);
423424

424-
return result;
425+
auto result_buffer = Buffer{ result };
426+
427+
return result_buffer;
425428
}
426429

427430
[[nodiscard]] std::optional<std::string>
428-
secret::SecretStorage::store(const std::string& key, const std::string& value, bool update) const {
431+
secret::SecretStorage::store(const std::string& key, const Buffer& value, bool update) const {
429432

430433
auto maybe_json_value = get_json_from_file(m_file_path);
431434

@@ -440,7 +443,7 @@ secret::SecretStorage::store(const std::string& key, const std::string& value, b
440443
}
441444

442445

443-
json_value.at(key) = value;
446+
json_value.at(key) = value.as_string();
444447

445448
return json::try_write_json_to_file(m_file_path, json_value, true);
446449
}

‎src/lobby/credentials/secret.hpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,10 @@
77
#include <core/helper/expected.hpp>
88
#include <core/helper/types.hpp>
99

10+
#include "./buffer.hpp"
1011
#include "helper/windows.hpp"
1112

13+
1214
#if defined(__linux__)
1315

1416
#include <keyutils.h>
@@ -49,11 +51,11 @@ namespace secret {
4951
OOPETRIS_GRAPHICS_EXPORTED SecretStorage(SecretStorage&& other) noexcept;
5052
OOPETRIS_GRAPHICS_EXPORTED SecretStorage& operator=(SecretStorage&& other) noexcept = delete;
5153

52-
OOPETRIS_GRAPHICS_EXPORTED [[nodiscard]] helper::expected<std::string, std::string> load(const std::string& key
54+
OOPETRIS_GRAPHICS_EXPORTED [[nodiscard]] helper::expected<Buffer, std::string> load(const std::string& key
5355
) const;
5456

5557
OOPETRIS_GRAPHICS_EXPORTED [[nodiscard]] std::optional<std::string>
56-
store(const std::string& key, const std::string& value, bool update = true) const;
58+
store(const std::string& key, const Buffer& value, bool update = true) const;
5759

5860
OOPETRIS_GRAPHICS_EXPORTED [[nodiscard]] std::optional<std::string> remove(const std::string& key) const;
5961
};

0 commit comments

Comments
 (0)