Skip to content

feat(oidc): provide better logging and output for the user when oidc attempt is made #6737

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.

Sign up for GitHub

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

Closed
wants to merge 7 commits into from
+1,753 −25
Closed

feat(oidc): provide better logging and output for the user when oidc attempt is made #6737

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
23d7229
feat(oidc): perhaps this will provide better logging and output for t…
perfectra1n Aug 21, 2025
f3c32af
feat(oidc): provide better logging and output for the user
perfectra1n Aug 21, 2025
527d5b6
feat(oidc): provide better logging and output for the user
perfectra1n Aug 21, 2025
d0b5826
feat(oidc): add lots of more logging and guardrails
perfectra1n Aug 22, 2025
8f9b566
feat(oidc): try to provide better logging output for oidc issues/errors
perfectra1n Aug 22, 2025
c1961db
feat(oidc): even more logging and output
perfectra1n Aug 22, 2025
e160f9a
feat(oidc): add even more logging, and try to add some more useful ca…
perfectra1n Aug 23, 2025
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
33 changes: 32 additions & 1 deletion apps/server/src/app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,39 @@
app.use(sessionParser);
app.use(favicon(path.join(assetsDir, "icon.ico")));

if (openID.isOpenIDEnabled())
if (openID.isOpenIDEnabled()) {
// Always log OAuth initialization for better debugging
log.info('OAuth: Initializing OAuth authentication middleware');

// Test OAuth connectivity on startup for non-Google providers
const issuerUrl = config.MultiFactorAuthentication.oauthIssuerBaseUrl;
const isCustomProvider = issuerUrl &&
issuerUrl !== "" &&
issuerUrl !== "https://accounts.google.com";

if (isCustomProvider) {
// For non-Google providers, verify connectivity
openID.testOAuthConnectivity().then(result => {
if (result.success) {
log.info('OAuth: Provider connectivity verified successfully');
} else {
log.error(`OAuth: Provider connectivity check failed: ${result.error}`);
log.error('OAuth: Authentication may not work. Please verify:');
log.error(' 1. The OAuth provider URL is correct');
log.error(' 2. Network connectivity between Trilium and the OAuth provider');
log.error(' 3. Any firewall or proxy settings');
}
}).catch(err => {
log.error(`OAuth: Connectivity test error: ${err.message || err}`);
});
}

// Register OAuth middleware
app.use(auth(openID.generateOAuthConfig()));

// Add OAuth error logging middleware AFTER auth middleware
app.use(openID.oauthErrorLogger);

Check failure

Code scanning / CodeQL

Missing rate limiting High

This route handler performs
authorization
Loading
, but is not rate-limited.

Copilot Autofix

AI 3 months ago

To fix the issue, we should add rate-limiting middleware to the application in order to prevent abuse of the authentication/authorization chain, including the openID.oauthErrorLogger middleware. The best practice is to use the popular express-rate-limit package. You should import this package near the top of your file, configure a sensible rate limit (for example, 100 requests per 15 minutes), and use app.use(limiter) immediately before the authentication middlewares, so that all upstream authentication attempts (and error loggers) will be protected. This approach ensures that expensive operations in the authentication chain are not abused by high-frequency requests. All changes are to be done in the shown region of apps/server/src/app.ts, including the import statement, definition/configuration of the limiter, and the application of the middleware.

Suggested changeset 2
apps/server/src/app.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/apps/server/src/app.ts b/apps/server/src/app.ts
--- a/apps/server/src/app.ts
+++ b/apps/server/src/app.ts
@@ -1,3 +1,4 @@
+import RateLimit from "express-rate-limit";
 import express from "express";
 import path from "path";
 import favicon from "serve-favicon";
@@ -144,6 +145,13 @@
             });
         }
         
+        // Apply rate limiter before all authentication and error logging middleware
+        const limiter = RateLimit({
+            windowMs: 15 * 60 * 1000, // 15 minutes
+            max: 100, // limit each IP to 100 requests per windowMs
+        });
+        app.use(limiter);
+
         // Register OAuth middleware
         app.use(auth(openID.generateOAuthConfig()));
         
EOF
@@ -1,3 +1,4 @@
import RateLimit from "express-rate-limit";
import express from "express";
import path from "path";
import favicon from "serve-favicon";
@@ -144,6 +145,13 @@
});
}

// Apply rate limiter before all authentication and error logging middleware
const limiter = RateLimit({
windowMs: 15 * 60 * 1000, // 15 minutes
max: 100, // limit each IP to 100 requests per windowMs
});
app.use(limiter);

// Register OAuth middleware
app.use(auth(openID.generateOAuthConfig()));

apps/server/package.json
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/apps/server/package.json b/apps/server/package.json
--- a/apps/server/package.json
+++ b/apps/server/package.json
@@ -4,7 +4,8 @@
   "description": "The server-side component of TriliumNext, which exposes the client via the web, allows for sync and provides a REST API for both internal and external use.",
   "private": true,
   "dependencies": {
-    "better-sqlite3": "12.2.0"
+    "better-sqlite3": "12.2.0",
+    "express-rate-limit": "^8.0.1"
   },
   "devDependencies": {
     "@electron/remote": "2.1.3",
EOF
@@ -4,7 +4,8 @@
"description": "The server-side component of TriliumNext, which exposes the client via the web, allows for sync and provides a REST API for both internal and external use.",
"private": true,
"dependencies": {
"better-sqlite3": "12.2.0"
"better-sqlite3": "12.2.0",
"express-rate-limit": "^8.0.1"
},
"devDependencies": {
"@electron/remote": "2.1.3",
This fix introduces these dependencies
Package Version Security advisories
express-rate-limit (npm) 8.0.1 None
Copilot is powered by AI and may make mistakes. Always verify output.
}

await assets.register(app);
routes.register(app);
Expand Down
4 changes: 2 additions & 2 deletions apps/server/src/services/encryption/open_id_encryption.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,14 @@ function verifyOpenIDSubjectIdentifier(subjectIdentifier: string) {
return false;
}

const salt = sql.getValue("SELECT salt FROM user_data;");
const salt = sql.getValue<string>("SELECT salt FROM user_data;");
if (salt == undefined) {
console.log("Salt undefined");
return undefined;
}

const givenHash = myScryptService
.getSubjectIdentifierVerificationHash(subjectIdentifier)
.getSubjectIdentifierVerificationHash(subjectIdentifier, salt)
?.toString("base64");
if (givenHash === undefined) {
console.log("Sub id hash undefined!");
Expand Down
Loading