From 35f36fccba941ed8029ee222f7d2a5df17b42e2b Mon Sep 17 00:00:00 2001 From: Erik Michelson Date: Wed, 3 Dec 2025 21:07:56 +0100 Subject: [PATCH] fix(auth): add state parameters and PKCE support Only the OAuth2 auth strategy was using the state parameter, which should be used as described in the RFC. The other auth strategies such as GitHub, GitLab or Google were lacking the state parameter. This change adds the required state parameter as well as enabling PKCE support on providers where it's possible. Signed-off-by: Erik Michelson --- config.json.example | 13 +++++++++++++ docs/content/configuration.md | 33 ++++++++++++++++---------------- lib/config/default.js | 3 ++- lib/config/environment.js | 3 ++- lib/web/auth/dropbox/index.js | 4 +++- lib/web/auth/facebook/index.js | 4 +++- lib/web/auth/github/index.js | 4 +++- lib/web/auth/gitlab/index.js | 4 +++- lib/web/auth/google/index.js | 4 +++- lib/web/auth/mattermost/index.js | 3 ++- lib/web/auth/oauth2/index.js | 1 + public/docs/release-notes.md | 1 + 12 files changed, 53 insertions(+), 24 deletions(-) diff --git a/config.json.example b/config.json.example index 69bdb858..e9105eb2 100644 --- a/config.json.example +++ b/config.json.example @@ -106,6 +106,19 @@ "email": "change or delete this: attribute map for `email` (default: NameID)" } }, + "oauth2": { + "baseURL": "https://auth.example.com/", + "userProfileURL": "https://auth.example.com/oauth2/userinfo/", + "tokenURL": "https://auth.example.com/oauth2/token/", + "authorizationURL": "https://auth.example.com/oauth2/authorize/", + "clientID": "change-this-id", + "clientSecret": "change-this-secret", + "scope": "openid profile user", + "userProfileUsernameAttr": "preferred_username", + "userProfileEmailAttr": "email", + "userProfileDisplayNameAttr": "name", + "pkce": true + }, "imgur": { "clientID": "change this" }, diff --git a/docs/content/configuration.md b/docs/content/configuration.md index 43c9d607..adcb724d 100644 --- a/docs/content/configuration.md +++ b/docs/content/configuration.md @@ -209,22 +209,23 @@ these are rarely used for various reasons. ### OAuth2 Login -| config file | environment | **default** and example value | description | -|-------------|---------------------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| `oauth2` | | `{baseURL: ..., userProfileURL: ..., userProfileUsernameAttr: ..., userProfileDisplayNameAttr: ..., userProfileEmailAttr: ..., tokenURL: ..., authorizationURL: ..., clientID: ..., clientSecret: ..., scope: ...}` | An object detailing your OAuth2 provider. Refer to the [Mattermost](guides/auth/mattermost-self-hosted.md) or [Nextcloud](guides/auth/nextcloud.md) examples for more details! | -| | `CMD_OAUTH2_USER_PROFILE_URL` | **no default**, `https://example.com` | Where to retrieve information about a user after successful login. Needs to output JSON. (no default value) Refer to the [Mattermost](guides/auth/mattermost-self-hosted.md) or [Nextcloud](guides/auth/nextcloud.md) examples for more details on all of the `CMD_OAUTH2...` options. | -| | `CMD_OAUTH2_USER_PROFILE_USERNAME_ATTR` | **no default**, `name` | where to find the username in the JSON from the user profile URL. (no default value) | -| | `CMD_OAUTH2_USER_PROFILE_DISPLAY_NAME_ATTR` | **no default**, `display-name` | where to find the display-name in the JSON from the user profile URL. (no default value) | -| | `CMD_OAUTH2_USER_PROFILE_EMAIL_ATTR` | **no default**, `email` | where to find the email address in the JSON from the user profile URL. (no default value) | -| | `CMD_OAUTH2_USER_PROFILE_ID_ATTR` | **no default**, `user_uuid` | where to find the dedicated user ID (optional, overrides `CMD_OAUTH2_USER_PROFILE_USERNAME_ATTR`) | -| | `CMD_OAUTH2_TOKEN_URL` | **no default**, `https://example.com` | sometimes called token endpoint, please refer to the documentation of your OAuth2 provider (no default value) | -| | `CMD_OAUTH2_AUTHORIZATION_URL` | **no default**, `https://example.com` | authorization URL of your provider, please refer to the documentation of your OAuth2 provider (no default value) | -| | `CMD_OAUTH2_CLIENT_ID` | **no default**, `afae02fckafd...` | you will get this from your OAuth2 provider when you register HedgeDoc as OAuth2-client, (no default value) | -| | `CMD_OAUTH2_CLIENT_SECRET` | **no default**, `afae02fckafd...` | you will get this from your OAuth2 provider when you register HedgeDoc as OAuth2-client, (no default value) | -| | `CMD_OAUTH2_PROVIDERNAME` | **no default**, `My institution` | Optional name to be displayed at login form indicating the oAuth2 provider | -| | `CMD_OAUTH2_SCOPE` | **no default**, `openid email profile` | Scope to request for OIDC (OpenID Connect) providers. | -| | `CMD_OAUTH2_ROLES_CLAIM` | **no default**, `roles` | ID token claim, which is supposed to provide an array of strings of roles | -| | `CMD_OAUTH2_ACCESS_ROLE` | **no default**, `role/hedgedoc` | The role which should be included in the ID token roles claim to grant access | +| config file | environment | **default** and example value | description | +|-------------|---------------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `oauth2` | | `{baseURL: ..., userProfileURL: ..., userProfileUsernameAttr: ..., userProfileDisplayNameAttr: ..., userProfileEmailAttr: ..., tokenURL: ..., authorizationURL: ..., clientID: ..., clientSecret: ..., scope: ..., pkce: ...}` | An object detailing your OAuth2 provider. Refer to the [Mattermost](guides/auth/mattermost-self-hosted.md) or [Nextcloud](guides/auth/nextcloud.md) examples for more details! | +| | `CMD_OAUTH2_USER_PROFILE_URL` | **no default**, `https://example.com` | Where to retrieve information about a user after successful login. Needs to output JSON. (no default value) Refer to the [Mattermost](guides/auth/mattermost-self-hosted.md) or [Nextcloud](guides/auth/nextcloud.md) examples for more details on all of the `CMD_OAUTH2...` options. | +| | `CMD_OAUTH2_USER_PROFILE_USERNAME_ATTR` | **no default**, `name` | where to find the username in the JSON from the user profile URL. (no default value) | +| | `CMD_OAUTH2_USER_PROFILE_DISPLAY_NAME_ATTR` | **no default**, `display-name` | where to find the display-name in the JSON from the user profile URL. (no default value) | +| | `CMD_OAUTH2_USER_PROFILE_EMAIL_ATTR` | **no default**, `email` | where to find the email address in the JSON from the user profile URL. (no default value) | +| | `CMD_OAUTH2_USER_PROFILE_ID_ATTR` | **no default**, `user_uuid` | where to find the dedicated user ID (optional, overrides `CMD_OAUTH2_USER_PROFILE_USERNAME_ATTR`) | +| | `CMD_OAUTH2_TOKEN_URL` | **no default**, `https://example.com` | sometimes called token endpoint, please refer to the documentation of your OAuth2 provider (no default value) | +| | `CMD_OAUTH2_AUTHORIZATION_URL` | **no default**, `https://example.com` | authorization URL of your provider, please refer to the documentation of your OAuth2 provider (no default value) | +| | `CMD_OAUTH2_CLIENT_ID` | **no default**, `afae02fckafd...` | you will get this from your OAuth2 provider when you register HedgeDoc as OAuth2-client, (no default value) | +| | `CMD_OAUTH2_CLIENT_SECRET` | **no default**, `afae02fckafd...` | you will get this from your OAuth2 provider when you register HedgeDoc as OAuth2-client, (no default value) | +| | `CMD_OAUTH2_PROVIDERNAME` | **no default**, `My institution` | Optional name to be displayed at login form indicating the oAuth2 provider | +| | `CMD_OAUTH2_SCOPE` | **no default**, `openid email profile` | Scope to request for OIDC (OpenID Connect) providers. | +| | `CMD_OAUTH2_ROLES_CLAIM` | **no default**, `roles` | ID token claim, which is supposed to provide an array of strings of roles | +| | `CMD_OAUTH2_ACCESS_ROLE` | **no default**, `role/hedgedoc` | The role which should be included in the ID token roles claim to grant access | +| | `CMD_OAUTH2_PKCE` | **`false`**, `true` | Whether to use PKCE auth. Defaults to false since not every OAuth2 provider supports it. | !!! info If you are using a [CA not trusted by Node.js](https://github.com/nodejs/node/issues/4175) (like Let's Encrypt e.g) for diff --git a/lib/config/default.js b/lib/config/default.js index 3110119b..19346db4 100644 --- a/lib/config/default.js +++ b/lib/config/default.js @@ -104,7 +104,8 @@ module.exports = { tokenURL: undefined, clientID: undefined, clientSecret: undefined, - scope: undefined + scope: undefined, + pkce: false }, facebook: { clientID: undefined, diff --git a/lib/config/environment.js b/lib/config/environment.js index 6b9f8312..393928b5 100644 --- a/lib/config/environment.js +++ b/lib/config/environment.js @@ -115,7 +115,8 @@ module.exports = { clientSecret: process.env.CMD_OAUTH2_CLIENT_SECRET, scope: process.env.CMD_OAUTH2_SCOPE, rolesClaim: process.env.CMD_OAUTH2_ROLES_CLAIM, - accessRole: process.env.CMD_OAUTH2_ACCESS_ROLE + accessRole: process.env.CMD_OAUTH2_ACCESS_ROLE, + pkce: toBooleanConfig(process.env.CMD_OAUTH2_PKCE) }, dropbox: { clientID: process.env.CMD_DROPBOX_CLIENTID, diff --git a/lib/web/auth/dropbox/index.js b/lib/web/auth/dropbox/index.js index c35f04e3..569887df 100644 --- a/lib/web/auth/dropbox/index.js +++ b/lib/web/auth/dropbox/index.js @@ -12,7 +12,9 @@ passport.use(new DropboxStrategy({ apiVersion: '2', clientID: config.dropbox.clientID, clientSecret: config.dropbox.clientSecret, - callbackURL: config.serverURL + '/auth/dropbox/callback' + callbackURL: config.serverURL + '/auth/dropbox/callback', + state: true, + pkce: true }, passportGeneralCallback)) dropboxAuth.get('/auth/dropbox', function (req, res, next) { diff --git a/lib/web/auth/facebook/index.js b/lib/web/auth/facebook/index.js index acf566eb..162cf6a8 100644 --- a/lib/web/auth/facebook/index.js +++ b/lib/web/auth/facebook/index.js @@ -12,7 +12,9 @@ const facebookAuth = module.exports = Router() passport.use(new FacebookStrategy({ clientID: config.facebook.clientID, clientSecret: config.facebook.clientSecret, - callbackURL: config.serverURL + '/auth/facebook/callback' + callbackURL: config.serverURL + '/auth/facebook/callback', + state: true, + pkce: true }, passportGeneralCallback)) facebookAuth.get('/auth/facebook', function (req, res, next) { diff --git a/lib/web/auth/github/index.js b/lib/web/auth/github/index.js index c7f7e5d1..81ad77bd 100644 --- a/lib/web/auth/github/index.js +++ b/lib/web/auth/github/index.js @@ -12,7 +12,9 @@ const githubAuth = module.exports = Router() passport.use(new GithubStrategy({ clientID: config.github.clientID, clientSecret: config.github.clientSecret, - callbackURL: config.serverURL + '/auth/github/callback' + callbackURL: config.serverURL + '/auth/github/callback', + pkce: true, + state: true }, passportGeneralCallback)) githubAuth.get('/auth/github', function (req, res, next) { diff --git a/lib/web/auth/gitlab/index.js b/lib/web/auth/gitlab/index.js index 11579bd1..b9445da6 100644 --- a/lib/web/auth/gitlab/index.js +++ b/lib/web/auth/gitlab/index.js @@ -14,7 +14,9 @@ passport.use(new GitlabStrategy({ clientID: config.gitlab.clientID, clientSecret: config.gitlab.clientSecret, scope: config.gitlab.scope, - callbackURL: config.serverURL + '/auth/gitlab/callback' + callbackURL: config.serverURL + '/auth/gitlab/callback', + pkce: true, + state: true }, passportGeneralCallback)) gitlabAuth.get('/auth/gitlab', function (req, res, next) { diff --git a/lib/web/auth/google/index.js b/lib/web/auth/google/index.js index 0262dedf..51ffaee0 100644 --- a/lib/web/auth/google/index.js +++ b/lib/web/auth/google/index.js @@ -12,7 +12,9 @@ passport.use(new GoogleStrategy({ clientID: config.google.clientID, clientSecret: config.google.clientSecret, callbackURL: config.serverURL + '/auth/google/callback', - userProfileURL: 'https://www.googleapis.com/oauth2/v3/userinfo' + userProfileURL: 'https://www.googleapis.com/oauth2/v3/userinfo', + pkce: true, + state: true }, passportGeneralCallback)) googleAuth.get('/auth/google', function (req, res, next) { diff --git a/lib/web/auth/mattermost/index.js b/lib/web/auth/mattermost/index.js index 2f15c812..071fd57e 100644 --- a/lib/web/auth/mattermost/index.js +++ b/lib/web/auth/mattermost/index.js @@ -16,7 +16,8 @@ const mattermostStrategy = new OAuthStrategy({ tokenURL: config.mattermost.baseURL + '/oauth/access_token', clientID: config.mattermost.clientID, clientSecret: config.mattermost.clientSecret, - callbackURL: config.serverURL + '/auth/mattermost/callback' + callbackURL: config.serverURL + '/auth/mattermost/callback', + state: true }, passportGeneralCallback) mattermostStrategy.userProfile = (accessToken, done) => { diff --git a/lib/web/auth/oauth2/index.js b/lib/web/auth/oauth2/index.js index b0ffa5e8..0ce38dde 100644 --- a/lib/web/auth/oauth2/index.js +++ b/lib/web/auth/oauth2/index.js @@ -138,6 +138,7 @@ passport.use(new OAuth2CustomStrategy({ callbackURL: config.serverURL + '/auth/oauth2/callback', userProfileURL: config.oauth2.userProfileURL, scope: config.oauth2.scope, + pkce: config.oauth2.pkce, state: true }, passportGeneralCallback)) diff --git a/public/docs/release-notes.md b/public/docs/release-notes.md index 9f7b6f78..dcca9b55 100644 --- a/public/docs/release-notes.md +++ b/public/docs/release-notes.md @@ -16,6 +16,7 @@ - Force kill the server after a timeout when waiting for the realtime server to close connections on shutdown - Secure iframes with `credentialless` and `sandbox` attributes - Fix regexes for `[time=...]`, `[name=...]` and `[color=...]` shortcodes in lists +- Use `state` parameter for OAuth2 flows and PKCE where applicable ## 1.10.3 2025-04-09