Skip to content

Commit 20ac548

Browse files
committed
refactor: First full pass of logging fixes
1 parent 874878c commit 20ac548

File tree

13 files changed

+125
-82
lines changed

13 files changed

+125
-82
lines changed

common/include/common/IAuthenticationManager.hpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,8 @@
55

66
// Standard imports
77
#include <string>
8-
98
namespace SDMS {
10-
9+
struct LogContext;
1110
/**
1211
* Interface class for managing authenticating
1312
*
@@ -26,7 +25,7 @@ class IAuthenticationManager {
2625
* Increments the number of times that the key has been accessed, this is
2726
*useful information when deciding if a key should be purged.
2827
**/
29-
virtual void incrementKeyAccessCounter(const std::string &public_key) = 0;
28+
virtual void incrementKeyAccessCounter(const std::string &public_key, LogContext log_context) = 0;
3029

3130
/**
3231
* Will return true if the public key is known. This is also dependent on the
@@ -39,7 +38,7 @@ class IAuthenticationManager {
3938
* - SESSION
4039
* - PERSISTENT
4140
**/
42-
virtual bool hasKey(const std::string &pub_key) const = 0;
41+
virtual bool hasKey(const std::string &pub_key, LogContext log_context) const = 0;
4342

4443
/**
4544
* Will get the unique id or throw an error
@@ -49,7 +48,7 @@ class IAuthenticationManager {
4948
* - SESSION
5049
* - PERSISTENT - user or repo
5150
**/
52-
virtual std::string getUID(const std::string &pub_key) const = 0;
51+
virtual std::string getUID(const std::string &pub_key, LogContext log_context) const = 0;
5352

5453
/**
5554
* Purge keys if needed

common/source/operators/AuthenticationOperator.cpp

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,14 @@
44

55
// Local public includes
66
#include "common/TraceException.hpp"
7+
#include "common/DynaLog.hpp"
78

89
// Standard includes
910
#include <any>
1011
#include <iostream>
12+
#include <boost/uuid/uuid.hpp>
13+
#include <boost/uuid/uuid_generators.hpp>
14+
#include <boost/uuid/uuid_io.hpp>
1115

1216
namespace SDMS {
1317

@@ -25,17 +29,22 @@ void AuthenticationOperator::execute(IMessage &message) {
2529
if (message.exists(MessageAttribute::KEY) == 0) {
2630
EXCEPT(1, "'KEY' attribute not defined.");
2731
}
32+
// 🔹 Generate correlation ID for this request
33+
boost::uuids::random_generator generator;
34+
boost::uuids::uuid uuid = generator();
2835

36+
LogContext log_context;
37+
log_context.correlation_id = boost::uuids::to_string(uuid);
2938
m_authentication_manager->purge();
3039

3140
std::string key = std::get<std::string>(message.get(MessageAttribute::KEY));
3241

3342
std::string uid = "anon";
34-
if (m_authentication_manager->hasKey(key)) {
35-
m_authentication_manager->incrementKeyAccessCounter(key);
43+
if (m_authentication_manager->hasKey(key, log_context)) {
44+
m_authentication_manager->incrementKeyAccessCounter(key, log_context);
3645

3746
try {
38-
uid = m_authentication_manager->getUID(key);
47+
uid = m_authentication_manager->getUID(key, log_context);
3948
} catch (const std::exception& e) {
4049
// Log the exception to help diagnose authentication issues
4150
std::cerr << "[AuthenticationOperator] Failed to get UID for key: "

common/tests/unit/test_OperatorFactory.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "common/MessageFactory.hpp"
1111
#include "common/OperatorFactory.hpp"
1212
#include "common/OperatorTypes.hpp"
13+
#include "common/DynaLog.hpp"
1314

1415
// Third party includes
1516
#include <google/protobuf/stubs/common.h>
@@ -38,15 +39,15 @@ class DummyAuthManager : public IAuthenticationManager {
3839
/**
3940
* Methods only available via the interface
4041
**/
41-
virtual void incrementKeyAccessCounter(const std::string &pub_key) final {
42+
virtual void incrementKeyAccessCounter(const std::string &pub_key, LogContext log_context) final {
4243
++m_counters.at(pub_key);
4344
}
4445

45-
virtual bool hasKey(const std::string &pub_key) const {
46+
virtual bool hasKey(const std::string &pub_key, LogContext log_context) const {
4647
return m_counters.count(pub_key);
4748
}
4849
// Just assume all keys map to the anon_uid
49-
virtual std::string getUID(const std::string &) const {
50+
virtual std::string getUID(const std::string &, LogContext log_context) const {
5051
return "authenticated_uid";
5152
}
5253

core/server/AuthMap.cpp

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,8 @@ size_t AuthMap::size(const PublicKeyType pub_key_type) const {
168168
}
169169

170170
void AuthMap::incrementKeyAccessCounter(const PublicKeyType pub_key_type,
171-
const std::string &public_key) {
171+
const std::string &public_key,
172+
LogContext log_context) {
172173
if (pub_key_type == PublicKeyType::TRANSIENT) {
173174
lock_guard<mutex> lock(m_trans_clients_mtx);
174175
if (m_trans_auth_clients.count(public_key)) {
@@ -183,7 +184,8 @@ void AuthMap::incrementKeyAccessCounter(const PublicKeyType pub_key_type,
183184
}
184185

185186
bool AuthMap::hasKey(const PublicKeyType pub_key_type,
186-
const std::string &public_key) const {
187+
const std::string &public_key,
188+
LogContext log_context) const {
187189
if (pub_key_type == PublicKeyType::TRANSIENT) {
188190
lock_guard<mutex> lock(m_trans_clients_mtx);
189191
return m_trans_auth_clients.count(public_key) > 0;
@@ -203,7 +205,7 @@ bool AuthMap::hasKey(const PublicKeyType pub_key_type,
203205
try {
204206
DatabaseAPI db(m_db_url, m_db_user, m_db_pass);
205207
std::string uid;
206-
if (db.uidByPubKey(public_key, uid)) {
208+
if (db.uidByPubKey(public_key, uid, log_context)) {
207209
return true;
208210
}
209211
} catch (const std::exception& e) {
@@ -217,9 +219,10 @@ bool AuthMap::hasKey(const PublicKeyType pub_key_type,
217219
}
218220

219221
std::string AuthMap::getUID(const PublicKeyType pub_key_type,
220-
const std::string &public_key) const {
222+
const std::string &public_key,
223+
LogContext log_context) const {
221224

222-
std::string uid = getUIDSafe(pub_key_type, public_key);
225+
std::string uid = getUIDSafe(pub_key_type, public_key, log_context);
223226

224227
if (uid.empty()) {
225228
if (pub_key_type == PublicKeyType::TRANSIENT) {
@@ -238,7 +241,8 @@ std::string AuthMap::getUID(const PublicKeyType pub_key_type,
238241
}
239242

240243
std::string AuthMap::getUIDSafe(const PublicKeyType pub_key_type,
241-
const std::string &public_key) const {
244+
const std::string &public_key,
245+
LogContext log_context) const {
242246
if (pub_key_type == PublicKeyType::TRANSIENT) {
243247
lock_guard<mutex> lock(m_trans_clients_mtx);
244248
if (m_trans_auth_clients.count(public_key)) {
@@ -261,7 +265,7 @@ std::string AuthMap::getUIDSafe(const PublicKeyType pub_key_type,
261265
// Check database for user keys
262266
DatabaseAPI db(m_db_url, m_db_user, m_db_pass);
263267
std::string uid;
264-
if (db.uidByPubKey(public_key, uid)) {
268+
if (db.uidByPubKey(public_key, uid, log_context)) {
265269
return uid;
266270
}
267271
}

core/server/AuthMap.hpp

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
// Local common includes
1010
#include "common/IAuthenticationManager.hpp"
11+
#include "common/DynaLog.hpp"
1112

1213
// Standard includes
1314
#include <map>
@@ -113,13 +114,15 @@ class AuthMap {
113114
*does not exist. Best to call hasKey first.
114115
**/
115116
std::string getUID(const PublicKeyType pub_key_type,
116-
const std::string &public_key) const;
117+
const std::string &public_key,
118+
LogContext log_context) const;
117119

118120
/**
119121
* Safe version that returns empty string if key not found
120122
**/
121123
std::string getUIDSafe(const PublicKeyType pub_key_type,
122-
const std::string &public_key) const;
124+
const std::string &public_key,
125+
LogContext log_context) const;
123126

124127
/**
125128
* Will return the number of keys of the provided type. Does not currently
@@ -128,7 +131,8 @@ class AuthMap {
128131
size_t size(const PublicKeyType pub_key_type) const;
129132

130133
bool hasKey(const PublicKeyType pub_key_type,
131-
const std::string &public_key) const;
134+
const std::string &public_key,
135+
LogContext log_context) const;
132136

133137
/***********************************************************************************
134138
* Manipulators
@@ -138,7 +142,8 @@ class AuthMap {
138142
* Increase the recorded times the the public key has been accessed by one.
139143
**/
140144
void incrementKeyAccessCounter(const PublicKeyType pub_key_type,
141-
const std::string &public_key);
145+
const std::string &public_key,
146+
LogContext log_context);
142147

143148
/**
144149
* Adds the key to the AuthMap object

core/server/AuthenticationManager.cpp

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
// Common includes
66
#include "common/TraceException.hpp"
7+
#include "common/DynaLog.hpp"
78

89
// Standard includes
910
#include <iostream>
@@ -69,46 +70,47 @@ void AuthenticationManager::purge(const PublicKeyType pub_key_type) {
6970
}
7071

7172
void AuthenticationManager::incrementKeyAccessCounter(
72-
const std::string &public_key) {
73+
const std::string &public_key,
74+
LogContext log_context) {
7375
std::lock_guard<std::mutex> lock(m_lock);
74-
if (m_auth_mapper.hasKey(PublicKeyType::TRANSIENT, public_key)) {
76+
if (m_auth_mapper.hasKey(PublicKeyType::TRANSIENT, public_key, log_context)) {
7577
m_auth_mapper.incrementKeyAccessCounter(PublicKeyType::TRANSIENT,
76-
public_key);
77-
} else if (m_auth_mapper.hasKey(PublicKeyType::SESSION, public_key)) {
78-
m_auth_mapper.incrementKeyAccessCounter(PublicKeyType::SESSION, public_key);
78+
public_key, log_context);
79+
} else if (m_auth_mapper.hasKey(PublicKeyType::SESSION, public_key, log_context)) {
80+
m_auth_mapper.incrementKeyAccessCounter(PublicKeyType::SESSION, public_key, log_context);
7981
}
8082
// Ignore persistent cases because counter does nothing for them
8183
}
8284

83-
bool AuthenticationManager::hasKey(const std::string &public_key) const {
85+
bool AuthenticationManager::hasKey(const std::string &public_key, LogContext log_context) const {
8486
std::lock_guard<std::mutex> lock(m_lock);
8587

86-
if (m_auth_mapper.hasKey(PublicKeyType::TRANSIENT, public_key)) {
88+
if (m_auth_mapper.hasKey(PublicKeyType::TRANSIENT, public_key, log_context)) {
8789
return true;
8890
}
8991

90-
if (m_auth_mapper.hasKey(PublicKeyType::SESSION, public_key)) {
92+
if (m_auth_mapper.hasKey(PublicKeyType::SESSION, public_key, log_context)) {
9193
return true;
9294
}
9395

94-
if (m_auth_mapper.hasKey(PublicKeyType::PERSISTENT, public_key)) {
96+
if (m_auth_mapper.hasKey(PublicKeyType::PERSISTENT, public_key, log_context)) {
9597
return true;
9698
}
9799

98100
return false;
99101
}
100102

101-
std::string AuthenticationManager::getUID(const std::string &public_key) const {
103+
std::string AuthenticationManager::getUID(const std::string &public_key, LogContext log_context) const {
102104
std::lock_guard<std::mutex> lock(m_lock);
103105

104-
if (m_auth_mapper.hasKey(PublicKeyType::TRANSIENT, public_key)) {
105-
return m_auth_mapper.getUID(PublicKeyType::TRANSIENT, public_key);
106+
if (m_auth_mapper.hasKey(PublicKeyType::TRANSIENT, public_key, log_context)) {
107+
return m_auth_mapper.getUID(PublicKeyType::TRANSIENT, public_key, log_context);
106108
}
107-
if (m_auth_mapper.hasKey(PublicKeyType::SESSION, public_key)) {
108-
return m_auth_mapper.getUID(PublicKeyType::SESSION, public_key);
109+
if (m_auth_mapper.hasKey(PublicKeyType::SESSION, public_key, log_context)) {
110+
return m_auth_mapper.getUID(PublicKeyType::SESSION, public_key, log_context);
109111
}
110-
if (m_auth_mapper.hasKey(PublicKeyType::PERSISTENT, public_key)) {
111-
return m_auth_mapper.getUID(PublicKeyType::PERSISTENT, public_key);
112+
if (m_auth_mapper.hasKey(PublicKeyType::PERSISTENT, public_key, log_context)) {
113+
return m_auth_mapper.getUID(PublicKeyType::PERSISTENT, public_key, log_context);
112114
}
113115

114116
EXCEPT(1, "Unrecognized public_key during execution of getUID.");
@@ -122,9 +124,10 @@ void AuthenticationManager::addKey(const PublicKeyType &pub_key_type,
122124
}
123125

124126
bool AuthenticationManager::hasKey(const PublicKeyType &pub_key_type,
125-
const std::string &public_key) const {
127+
const std::string &public_key,
128+
LogContext log_context) const {
126129
std::lock_guard<std::mutex> lock(m_lock);
127-
return m_auth_mapper.hasKey(pub_key_type, public_key);
130+
return m_auth_mapper.hasKey(pub_key_type, public_key, log_context);
128131
}
129132

130133
void AuthenticationManager::migrateKey(const PublicKeyType &from_type,
@@ -150,21 +153,21 @@ void AuthenticationManager::clearAllNonPersistentKeys() {
150153
m_auth_mapper.clearAllNonPersistentKeys();
151154
}
152155

153-
std::string AuthenticationManager::getUIDSafe(const std::string &public_key) const {
156+
std::string AuthenticationManager::getUIDSafe(const std::string &public_key, LogContext log_context) const {
154157
std::lock_guard<std::mutex> lock(m_lock);
155158

156159
// Try each key type in order
157-
std::string uid = m_auth_mapper.getUIDSafe(PublicKeyType::TRANSIENT, public_key);
160+
std::string uid = m_auth_mapper.getUIDSafe(PublicKeyType::TRANSIENT, public_key, log_context);
158161
if (!uid.empty()) {
159162
return uid;
160163
}
161164

162-
uid = m_auth_mapper.getUIDSafe(PublicKeyType::SESSION, public_key);
165+
uid = m_auth_mapper.getUIDSafe(PublicKeyType::SESSION, public_key, log_context);
163166
if (!uid.empty()) {
164167
return uid;
165168
}
166169

167-
uid = m_auth_mapper.getUIDSafe(PublicKeyType::PERSISTENT, public_key);
170+
uid = m_auth_mapper.getUIDSafe(PublicKeyType::PERSISTENT, public_key, log_context);
168171
if (!uid.empty()) {
169172
return uid;
170173
}

core/server/AuthenticationManager.hpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ class AuthenticationManager : public IAuthenticationManager {
5454
*allotted purge time frame. If the count is above one then the session key
5555
*not be purged.
5656
**/
57-
virtual void incrementKeyAccessCounter(const std::string &public_key) final;
57+
virtual void incrementKeyAccessCounter(const std::string &public_key, LogContext log_context) final;
5858

5959
/**
6060
* This will purge all keys of a particular type that have expired.
@@ -79,15 +79,15 @@ class AuthenticationManager : public IAuthenticationManager {
7979
* - SESSION
8080
* - PERSISTENT
8181
**/
82-
virtual bool hasKey(const std::string &pub_key) const final;
82+
virtual bool hasKey(const std::string &pub_key, LogContext log_context) const final;
8383

8484
void addKey(const PublicKeyType &pub_key_type, const std::string &public_key,
8585
const std::string &uid);
8686

8787
/**
8888
* Check if a specific key exists in a specific map type
8989
**/
90-
bool hasKey(const PublicKeyType &pub_key_type, const std::string &public_key) const;
90+
bool hasKey(const PublicKeyType &pub_key_type, const std::string &public_key, LogContext log_context) const;
9191

9292
/**
9393
* Migrate a key from one type to another
@@ -121,13 +121,13 @@ class AuthenticationManager : public IAuthenticationManager {
121121
* - SESSION
122122
* - PERSISTENT
123123
**/
124-
virtual std::string getUID(const std::string &pub_key) const final;
124+
virtual std::string getUID(const std::string &pub_key, LogContext log_context) const final;
125125

126126
/**
127127
* Safe version that returns empty string if key not found
128128
* instead of throwing an exception
129129
**/
130-
std::string getUIDSafe(const std::string &pub_key) const;
130+
std::string getUIDSafe(const std::string &pub_key, LogContext log_context) const;
131131
};
132132

133133
} // namespace Core

core/server/Condition.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,26 @@
44

55
// Standard includes
66
#include <iostream>
7+
#include <boost/uuid/uuid.hpp>
8+
#include <boost/uuid/uuid_generators.hpp>
9+
#include <boost/uuid/uuid_io.hpp>
710

811
namespace SDMS {
912
namespace Core {
1013

1114
void Promote::enforce(AuthMap &auth_map, const std::string &public_key) {
1215
if (auth_map.hasKeyType(m_promote_from, public_key)) {
1316
size_t access_count = auth_map.getAccessCount(m_promote_from, public_key);
17+
boost::uuids::random_generator generator;
18+
boost::uuids::uuid uuid = generator();
19+
20+
LogContext log_context;
21+
log_context.correlation_id = boost::uuids::to_string(uuid);
22+
1423
if (access_count >= m_transient_to_session_count_threshold) {
1524
// Convert transient key to session key if has been accessed more than the
1625
// threshold
17-
std::string uid = auth_map.getUID(m_promote_from, public_key);
26+
std::string uid = auth_map.getUID(m_promote_from, public_key, log_context);
1827
auth_map.addKey(m_promote_to, public_key, uid);
1928
}
2029
// Remove expired short lived transient key

0 commit comments

Comments
 (0)