Skip to content

Commit 0f5f121

Browse files
AlexTugarevroboquat
AlexTugarev
authored and
roboquat
committed
[server] sanitize logging
1 parent f27efb7 commit 0f5f121

13 files changed

+30
-35
lines changed

‎components/server/ee/src/prebuilds/bitbucket-app.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ export class BitbucketApp {
5454
console.warn(`Ignoring unsupported bitbucket event: ${req.header("X-Event-Key")}`);
5555
}
5656
} catch (err) {
57-
console.error(`Couldn't handle request.`, err, { headers: req.headers, reqBody: req.body });
57+
console.error(`Couldn't handle request.`, err, { headers: req.headers });
5858
} finally {
5959
// we always respond with OK, when we received a valid event.
6060
res.sendStatus(200);
@@ -189,7 +189,7 @@ function toData(body: BitbucketPushHook): ParsedRequestData | undefined {
189189
gitCloneUrl: body.repository.links.html.href + ".git",
190190
};
191191
if (!result.commitHash || !result.repoUrl) {
192-
console.error("Bitbucket push event: unexpected request body.", body);
192+
console.error("Bitbucket push event: unexpected request body.");
193193
throw new Error("Unexpected request body.");
194194
}
195195
return result;

‎components/server/ee/src/prebuilds/bitbucket-server-app.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ export class BitbucketServerApp {
5252
console.warn(`Ignoring unsupported BBS event.`, { headers: req.headers });
5353
}
5454
} catch (err) {
55-
console.error(`Couldn't handle request.`, err, { headers: req.headers, reqBody: req.body });
55+
console.error(`Couldn't handle request.`, err, { headers: req.headers });
5656
} finally {
5757
// we always respond with OK, when we received a valid event.
5858
res.sendStatus(200);

‎components/server/ee/src/prebuilds/github-app.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -442,9 +442,9 @@ export class GithubApp {
442442
prebuildStartPromise.catch((err) => log.error(err, "Error while starting prebuild", { contextURL }));
443443
return prebuildStartPromise;
444444
} else {
445-
log.debug({ userId: user.id }, `Not running prebuild, the user did not enable it for this context`, {
445+
log.debug({ userId: user.id }, `Not running prebuild, the user did not enable it for this context`, null, {
446446
contextURL,
447-
user,
447+
userId: user.id,
448448
project,
449449
});
450450
return;

‎components/server/ee/src/prebuilds/github-enterprise-app.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ export class GitHubEnterpriseApp {
4545
try {
4646
user = await this.findUser({ span }, payload, req);
4747
} catch (error) {
48-
log.error("Cannot find user.", error, { req });
48+
log.error("Cannot find user.", error, {});
4949
}
5050
if (!user) {
5151
res.statusCode = 401;

‎components/server/ee/src/prebuilds/gitlab-app.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ export class GitLabApp {
4343
try {
4444
user = await this.findUser({ span }, context, req);
4545
} catch (error) {
46-
log.error("Cannot find user.", error, { req });
46+
log.error("Cannot find user.", error, {});
4747
}
4848
if (!user) {
4949
res.statusCode = 503;

‎components/server/src/auth/auth-provider-service.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ export class AuthProviderService {
4949
protected toAuthProviderParams = (oap: AuthProviderEntry) =>
5050
<AuthProviderParams>{
5151
...oap,
52+
// HINT: host is expected to be lower case
5253
host: oap.host.toLowerCase(),
5354
verified: oap.status === "verified",
5455
builtin: false,

‎components/server/src/auth/authenticator.ts

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ export class Authenticator {
7575
for (const { authProvider } of hostContexts) {
7676
const authCallbackPath = authProvider.authCallbackPath;
7777
if (req.url.startsWith(authCallbackPath)) {
78-
log.info(`Auth Provider Callback. Path: ${authCallbackPath}`, { req });
78+
log.info(`Auth Provider Callback. Path: ${authCallbackPath}`);
7979
await authProvider.callback(req, res, next);
8080
return;
8181
}
@@ -104,26 +104,25 @@ export class Authenticator {
104104
const host: string = req.query.host?.toString() || "";
105105
const authProvider = host && (await this.getAuthProviderForHost(host));
106106
if (!host || !authProvider) {
107-
log.info({ sessionId: req.sessionID }, `Bad request: missing parameters.`, { req, "login-flow": true });
107+
log.info({ sessionId: req.sessionID }, `Bad request: missing parameters.`, { "login-flow": true });
108108
res.redirect(this.getSorryUrl(`Bad request: missing parameters.`));
109109
return;
110110
}
111111
if (this.config.disableDynamicAuthProviderLogin && !authProvider.params.builtin) {
112-
log.info({ sessionId: req.sessionID }, `Auth Provider is not allowed.`, { req, ap: authProvider.info });
112+
log.info({ sessionId: req.sessionID }, `Auth Provider is not allowed.`, { ap: authProvider.info });
113113
res.redirect(this.getSorryUrl(`Login with ${authProvider.params.host} is not allowed.`));
114114
return;
115115
}
116116
if (!req.session) {
117117
increaseLoginCounter("failed", authProvider.info.host);
118-
log.info({}, `No session.`, { req, "login-flow": true });
118+
log.info({}, `No session.`, { "login-flow": true });
119119
res.redirect(this.getSorryUrl(`No session found. Please refresh the browser.`));
120120
return;
121121
}
122122

123123
if (!authProvider.info.verified && !(await this.isInSetupMode())) {
124124
increaseLoginCounter("failed", authProvider.info.host);
125125
log.info({ sessionId: req.sessionID }, `Login with "${host}" is not permitted.`, {
126-
req,
127126
"login-flow": true,
128127
ap: authProvider.info,
129128
});
@@ -153,7 +152,7 @@ export class Authenticator {
153152
async deauthorize(req: express.Request, res: express.Response, next: express.NextFunction) {
154153
const user = req.user;
155154
if (!req.isAuthenticated() || !User.is(user)) {
156-
log.info({ sessionId: req.sessionID }, `User is not authenticated.`, { req });
155+
log.info({ sessionId: req.sessionID }, `User is not authenticated.`);
157156
res.redirect(this.getSorryUrl(`Not authenticated. Please login.`));
158157
return;
159158
}
@@ -163,7 +162,7 @@ export class Authenticator {
163162
const authProvider = host && (await this.getAuthProviderForHost(host));
164163

165164
if (!host || !authProvider) {
166-
log.warn({ sessionId: req.sessionID }, `Bad request: missing parameters.`, { req });
165+
log.warn({ sessionId: req.sessionID }, `Bad request: missing parameters.`);
167166
res.redirect(this.getSorryUrl(`Bad request: missing parameters.`));
168167
return;
169168
}
@@ -174,7 +173,6 @@ export class Authenticator {
174173
} catch (error) {
175174
next(error);
176175
log.error({ sessionId: req.sessionID }, `Failed to disconnect a provider.`, error, {
177-
req,
178176
host,
179177
userId: user.id,
180178
});
@@ -188,13 +186,13 @@ export class Authenticator {
188186

189187
async authorize(req: express.Request, res: express.Response, next: express.NextFunction) {
190188
if (!req.session) {
191-
log.info({}, `No session.`, { req, "authorize-flow": true });
189+
log.info({}, `No session.`, { "authorize-flow": true });
192190
res.redirect(this.getSorryUrl(`No session found. Please refresh the browser.`));
193191
return;
194192
}
195193
const user = req.user;
196194
if (!req.isAuthenticated() || !User.is(user)) {
197-
log.info({ sessionId: req.sessionID }, `User is not authenticated.`, { req, "authorize-flow": true });
195+
log.info({ sessionId: req.sessionID }, `User is not authenticated.`, { "authorize-flow": true });
198196
res.redirect(this.getSorryUrl(`Not authenticated. Please login.`));
199197
return;
200198
}
@@ -204,14 +202,13 @@ export class Authenticator {
204202
const override = req.query.override === "true";
205203
const authProvider = host && (await this.getAuthProviderForHost(host));
206204
if (!returnTo || !host || !authProvider) {
207-
log.info({ sessionId: req.sessionID }, `Bad request: missing parameters.`, { req, "authorize-flow": true });
205+
log.info({ sessionId: req.sessionID }, `Bad request: missing parameters.`, { "authorize-flow": true });
208206
res.redirect(this.getSorryUrl(`Bad request: missing parameters.`));
209207
return;
210208
}
211209

212210
if (!authProvider.info.verified && user.id !== authProvider.info.ownerId) {
213211
log.info({ sessionId: req.sessionID }, `Authorization with "${host}" is not permitted.`, {
214-
req,
215212
"authorize-flow": true,
216213
ap: authProvider.info,
217214
});

‎components/server/src/auth/generic-auth-provider.ts

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ export class GenericAuthProvider implements AuthProvider {
216216
expiryDate,
217217
});
218218
} catch (error) {
219-
log.error(`(${this.strategyName}) Failed to refresh token!`, { error, token });
219+
log.error(`(${this.strategyName}) Failed to refresh token!`, { error });
220220
throw error;
221221
}
222222
}
@@ -252,7 +252,6 @@ export class GenericAuthProvider implements AuthProvider {
252252
log.error(`(${this.strategyName}) Failed to fetch from "configURL"`, {
253253
error,
254254
configURL,
255-
accessToken,
256255
});
257256
throw new Error("Error while reading user profile.");
258257
}
@@ -274,14 +273,13 @@ export class GenericAuthProvider implements AuthProvider {
274273
log.error(`(${this.strategyName}) Failed to call "fetchAuthUserSetup"`, {
275274
error,
276275
configFn,
277-
accessToken,
278276
});
279277
throw new Error("Error with the Auth Provider Configuration.");
280278
}
281279
try {
282280
return await promise;
283281
} catch (error) {
284-
log.error(`(${this.strategyName}) Failed to run "configFn"`, { error, configFn, accessToken });
282+
log.error(`(${this.strategyName}) Failed to run "configFn"`, { error, configFn });
285283
throw new Error("Error while reading user profile.");
286284
}
287285
};
@@ -313,14 +311,13 @@ export class GenericAuthProvider implements AuthProvider {
313311
const clientInfo = getRequestingClientInfo(request);
314312
const cxt = LogContext.from({ user: request.user });
315313
if (response.headersSent) {
316-
log.warn(cxt, `(${strategyName}) Callback called repeatedly.`, { request, clientInfo });
314+
log.warn(cxt, `(${strategyName}) Callback called repeatedly.`, { clientInfo });
317315
return;
318316
}
319317
log.info(cxt, `(${strategyName}) OAuth2 callback call. `, {
320318
clientInfo,
321319
authProviderId,
322320
requestUrl: request.originalUrl,
323-
request,
324321
});
325322

326323
const isAlreadyLoggedIn = request.isAuthenticated() && User.is(request.user);
@@ -330,7 +327,7 @@ export class GenericAuthProvider implements AuthProvider {
330327
log.warn(
331328
cxt,
332329
`(${strategyName}) User is already logged in. No auth info provided. Redirecting to dashboard.`,
333-
{ request, clientInfo },
330+
{ clientInfo },
334331
);
335332
response.redirect(this.config.hostUrl.asDashboard().toString());
336333
return;
@@ -341,20 +338,20 @@ export class GenericAuthProvider implements AuthProvider {
341338
if (!authFlow) {
342339
increaseLoginCounter("failed", this.host);
343340

344-
log.error(cxt, `(${strategyName}) No session found during auth callback.`, { request, clientInfo });
341+
log.error(cxt, `(${strategyName}) No session found during auth callback.`, { clientInfo });
345342
response.redirect(this.getSorryUrl(`Please allow Cookies in your browser and try to log in again.`));
346343
return;
347344
}
348345

349346
if (authFlow.host !== this.host) {
350347
increaseLoginCounter("failed", this.host);
351348

352-
log.error(cxt, `(${strategyName}) Host does not match.`, { request, clientInfo });
349+
log.error(cxt, `(${strategyName}) Host does not match.`, { clientInfo });
353350
response.redirect(this.getSorryUrl(`Host does not match.`));
354351
return;
355352
}
356353

357-
const defaultLogPayload = { authFlow, clientInfo, authProviderId, request };
354+
const defaultLogPayload = { authFlow, clientInfo, authProviderId };
358355

359356
// check OAuth2 errors
360357
const callbackParams = new URL(`https://anyhost${request.originalUrl}`).searchParams;

‎components/server/src/bitbucket-server/bitbucket-server-auth-provider.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ export class BitbucketServerAuthProvider extends GenericAuthProvider {
7171
currentScopes: BitbucketServerOAuthScopes.ALL,
7272
};
7373
} catch (error) {
74-
log.error(`(${this.strategyName}) Reading current user info failed`, error, { accessToken, error });
74+
log.error(`(${this.strategyName}) Reading current user info failed`, error, { error });
7575
throw error;
7676
}
7777
};

‎components/server/src/bitbucket/bitbucket-auth-provider.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ export class BitbucketAuthProvider extends GenericAuthProvider {
8585
currentScopes,
8686
};
8787
} catch (error) {
88-
log.error(`(${this.strategyName}) Reading current user info failed`, error, { accessToken, error });
88+
log.error(`(${this.strategyName}) Reading current user info failed`, error, { error });
8989
throw error;
9090
}
9191
};

‎components/server/src/express/ws-connection-handler.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ export class WsConnectionHandler implements Disposable {
113113
ws.on("error", (err: any) => {
114114
if (err.code !== "ECONNRESET" && err.code !== "EPIPE") {
115115
// exclude very common errors
116-
log.warn("websocket error, closing.", err, { ws, req });
116+
log.warn("websocket error, closing.", err);
117117
}
118118
ws.close(); // ws should trigger close() itself on any socket error. We do this just to be sure.
119119
});

‎components/server/src/express/ws-layer.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ export class WsLayerImpl implements WsLayer {
5151
try {
5252
return fn(ws, req, next);
5353
} catch (err) {
54-
log.error(err, { ws, req });
54+
log.error(err);
5555
return next(err);
5656
}
5757
}
@@ -60,7 +60,7 @@ export class WsLayerImpl implements WsLayer {
6060
try {
6161
return this.next(ws, req);
6262
} catch (err) {
63-
log.error(err, { ws, req });
63+
log.error(err);
6464
}
6565
}
6666

‎components/server/src/github/github-auth-provider.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ export class GitHubAuthProvider extends GenericAuthProvider {
127127
currentScopes,
128128
};
129129
} catch (error) {
130-
log.error(`(${this.strategyName}) Reading current user info failed`, error, { accessToken, error });
130+
log.error(`(${this.strategyName}) Reading current user info failed`, error, { error });
131131
throw error;
132132
}
133133
};

0 commit comments

Comments
 (0)