Skip to content

chore: Passport Nextcloud OAuth #40591

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

Merged
yash-rajpal merged 3 commits into from
May 18, 2026
Merged

chore: Passport Nextcloud OAuth #40591

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3eefaf9
nextcloud OAuth
yash-rajpal May 17, 2026
cf5800a
Merge branch 'feat/phishing-resistant-mfa' into passport-nextcloud-oauth
yash-rajpal May 18, 2026
72f66bd
add auto publish fields
yash-rajpal May 18, 2026
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
2 changes: 2 additions & 0 deletions apps/meteor/app/custom-oauth/server/customOAuth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,8 @@ export class CustomOAuthStrategy extends Strategy {
try {
const result = JSON.parse(typeof body === 'string' ? body : body.toString());
const normalizedIdentity = this.normalizeIdentity(result);
//Nextcloud URL needed on addWebdavServer
normalizedIdentity.serverURL = this.serverURL;
return done(null, normalizedIdentity);
} catch (e) {
return done(new Error(`Failed to parse identity from ${this.name} at ${this.identityPath}. ${e}`));
Expand Down
45 changes: 27 additions & 18 deletions apps/meteor/app/nextcloud/server/lib.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
import type { OauthConfig } from '@rocket.chat/core-typings';
import type { OAuthConfiguration } from '@rocket.chat/core-typings';
import { Meteor } from 'meteor/meteor';
import _ from 'underscore';

import { CustomOAuth } from '../../custom-oauth/server/custom_oauth_server';
import { settings } from '../../settings/server';
import { addPassportCustomOAuth } from '../../../server/lib/oauth/addPassportCustomOAuth';
import { settings } from '../../settings/server/cached';

const config: OauthConfig = {
serverURL: '',
const NEXTCLOUD_PATHS = {
tokenPath: '/index.php/apps/oauth2/api/v1/token',
tokenSentVia: 'header',
tokenSentVia: 'header' as OAuthConfiguration['tokenSentVia'],
authorizePath: '/index.php/apps/oauth2/authorize',
identityPath: '/ocs/v2.php/cloud/user?format=json',
scope: 'openid',
Expand All @@ -18,20 +16,31 @@ const config: OauthConfig = {
},
};

const Nextcloud = new CustomOAuth('nextcloud', config);
function configureNextcloudOAuth(): void {
const enabled = settings.get<boolean>('Accounts_OAuth_Nextcloud');
if (!enabled) {
return;

@cubic-dev-ai cubic-dev-ai Bot May 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The disabled branch returns without unregistering previously configured Nextcloud Passport auth, so OAuth can remain active after the setting is turned off.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/app/nextcloud/server/lib.ts, line 18:

<comment>The disabled branch returns without unregistering previously configured Nextcloud Passport auth, so OAuth can remain active after the setting is turned off.</comment>

<file context>
@@ -1,37 +1,42 @@
+function configureNextcloudOAuth(): void {
+	const enabled = settings.get<boolean>('Accounts_OAuth_Nextcloud');
+	if (!enabled) {
+		return;
+	}
 
</file context>

}

const fillServerURL = _.debounce((): void => {
const nextcloudURL = settings.get<string>('Accounts_OAuth_Nextcloud_URL');
if (!nextcloudURL) {
if (nextcloudURL === undefined) {
return fillServerURL();
}
const serverURL = settings.get<string>('Accounts_OAuth_Nextcloud_URL')?.trim().replace(/\/*$/, '');
const clientId = settings.get<string>('Accounts_OAuth_Nextcloud_id');
const clientSecret = settings.get<string>('Accounts_OAuth_Nextcloud_secret');

if (!serverURL || !clientId || !clientSecret) {
return;
}
config.serverURL = nextcloudURL.trim().replace(/\/*$/, '');
return Nextcloud.configure(config);
}, 1000);

addPassportCustomOAuth('nextcloud', {

@cubic-dev-ai cubic-dev-ai Bot May 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Re-registering Nextcloud OAuth on every settings change adds duplicate Express routes, because addPassportCustomOAuth appends oAuthRouter.get(...) handlers on each call.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/app/nextcloud/server/lib.ts, line 29:

<comment>Re-registering Nextcloud OAuth on every settings change adds duplicate Express routes, because `addPassportCustomOAuth` appends `oAuthRouter.get(...)` handlers on each call.</comment>

<file context>
@@ -1,37 +1,42 @@
-	return Nextcloud.configure(config);
-}, 1000);
+
+	addPassportCustomOAuth('nextcloud', {
+		...NEXTCLOUD_PATHS,
+		serverURL,
</file context>

...NEXTCLOUD_PATHS,
serverURL,
clientId,
clientSecret,
});
Comment on lines +19 to +38

@coderabbitai coderabbitai Bot May 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Inspect addPassportCustomOAuth implementation:"
fd -i 'addPassportCustomOAuth.ts' | while read -r file; do
  echo "== $file =="
  sed -n '1,240p' "$file"
done

echo
echo "Search for existing OAuth teardown helpers:"
rg -n -C3 'passport\.unuse|removePassportCustomOAuth|unregister.*OAuth|disable.*OAuth' --type=ts

Repository: RocketChat/Rocket.Chat

Length of output: 3176

Call passport.unuse('nextcloud') before early returns to clear stale strategies.

When Nextcloud OAuth is disabled or credentials become incomplete, the function returns without deregistering the strategy. Since addPassportCustomOAuth only runs on valid config paths and it calls passport.unuse() internally, disabling or blanking credentials leaves the old strategy active in passport.

Add passport.unuse('nextcloud') before both early returns (when !enabled and when credentials are missing).

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/meteor/app/nextcloud/server/lib.ts` around lines 15 - 34, In
configureNextcloudOAuth, ensure stale Passport strategies are removed before
early returns by calling passport.unuse('nextcloud') when Nextcloud OAuth is
disabled and again when required credentials are missing; locate the
configureNextcloudOAuth function and add passport.unuse('nextcloud') immediately
before both return statements (the branches checking !enabled and the missing
serverURL/clientId/clientSecret) so that addPassportCustomOAuth/NEXTCLOUD_PATHS
is only relied on to register the strategy when config is valid.

}

Meteor.startup(() => {
settings.watch('Accounts_OAuth_Nextcloud_URL', () => fillServerURL());
settings.watchMultiple(
['Accounts_OAuth_Nextcloud', 'Accounts_OAuth_Nextcloud_URL', 'Accounts_OAuth_Nextcloud_id', 'Accounts_OAuth_Nextcloud_secret'],
configureNextcloudOAuth,
);
});
Loading