Regression: OAuth Login (#28303)

Co-authored-by: Rodrigo Nascimento <234261+rodrigok@users.noreply.github.com>
Co-authored-by: Guilherme Gazzo <5263975+ggazzo@users.noreply.github.com>

apps/meteor/app/api/server/helpers/deprecationWarning.ts

@@ -1,20 +1,24 @@
 import { API } from '../api';
 import { apiDeprecationLogger } from '../../../lib/server/lib/deprecationWarningLogger';
 
-export function deprecationWarning<T>({
-	endpoint,
-	versionWillBeRemoved = '5.0',
-	response,
-}: {
+type DeprecationWarningParams<T> = {
 	endpoint: string;
 	versionWillBeRemoved?: string;
 	response: T;
-}): T {
-	const warningMessage = `The endpoint "${endpoint}" is deprecated and will be removed after version ${versionWillBeRemoved}`;
-	apiDeprecationLogger.warn(warningMessage);
+	warningMessage?: string | ((props: Omit<DeprecationWarningParams<T>, 'warningMessage'>) => string);
+};
+export function deprecationWarning<T>({
+	endpoint,
+	versionWillBeRemoved = '6.0',
+	response,
+	warningMessage = `The endpoint "${endpoint}" is deprecated and will be removed on version ${versionWillBeRemoved}`,
+}: DeprecationWarningParams<T>): T {
+	const warning = typeof warningMessage === 'function' ? warningMessage({ endpoint, versionWillBeRemoved, response }) : warningMessage;
+
+	apiDeprecationLogger.warn(warning);
 	if (process.env.NODE_ENV === 'development') {
 		return {
-			warning: warningMessage,
+			warning,
 			...response,
 		};
 	}

apps/meteor/app/api/server/v1/oauthapps.ts

@@ -4,6 +4,7 @@ import { OAuthApps } from '@rocket.chat/models';
 import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission';
 import { API } from '../api';
 import { addOAuthApp } from '../../../oauth2-server-config/server/admin/functions/addOAuthApp';
+import { deprecationWarning } from '../helpers/deprecationWarning';
 
 API.v1.addRoute(
 	'oauth-apps.list',
@@ -23,19 +24,24 @@ API.v1.addRoute(
 
 API.v1.addRoute(
 	'oauth-apps.get',
-	{ authRequired: true },
+	{ authRequired: true, validateParams: isOauthAppsGetParams },
 	{
 		async get() {
-			if (!isOauthAppsGetParams(this.queryParams)) {
-				return API.v1.failure('At least one of the query parameters "clientId" or "appId" is required.');
-			}
-
 			const oauthApp = await OAuthApps.findOneAuthAppByIdOrClientId(this.queryParams);
 
 			if (!oauthApp) {
 				return API.v1.failure('OAuth app not found.');
 			}
-
+			if ('appId' in this.queryParams) {
+				return API.v1.success(
+					deprecationWarning({
+						endpoint: 'oauth-apps.get',
+						warningMessage: ({ versionWillBeRemoved, endpoint }) =>
+							`appId get parameter from "${endpoint}" is deprecated and will be removed after version ${versionWillBeRemoved}. Use _id instead.`,
+						response: { oauthApp },
+					}),
+				);
+			}
 			return API.v1.success({
 				oauthApp,
 			});

apps/meteor/client/views/admin/oauthApps/EditOauthAppWithData.tsx

@@ -2,23 +2,21 @@ import { Box, Button, ButtonGroup, Skeleton, Throbber, InputBox } from '@rocket.
 import { useEndpoint, useToastMessageDispatch, useTranslation } from '@rocket.chat/ui-contexts';
 import { useQuery } from '@tanstack/react-query';
 import type { ReactElement } from 'react';
-import React, { useCallback, useMemo } from 'react';
+import React, { useCallback } from 'react';
 
 import EditOauthApp from './EditOauthApp';
 
 const EditOauthAppWithData = ({ _id, ...props }: { _id: string }): ReactElement => {
 	const t = useTranslation();
 
-	const params = useMemo(() => ({ appId: _id }), [_id]);
-
 	const getOauthApps = useEndpoint('GET', '/v1/oauth-apps.get');
 
 	const dispatchToastMessage = useToastMessageDispatch();
 
 	const { data, isLoading, error, refetch } = useQuery(
-		['oauth-apps', params],
+		['oauth-apps', _id],
 		async () => {
-			const oauthApps = await getOauthApps(params);
+			const oauthApps = await getOauthApps({ _id });
 			return oauthApps;
 		},
 		{

apps/meteor/client/views/oauth/components/AuthorizationFormPage.tsx

@@ -42,11 +42,13 @@ const AuthorizationFormPage = ({ oauthApp, redirectUri, user }: AuthorizationFor
 
 	const submitRef = useRef<HTMLButtonElement>(null);
 
+	const hasAuthorized = user.oauth?.authorizedClients?.includes(oauthApp.clientId);
+
 	useEffect(() => {
-		if (user.oauth?.authorizedClients?.includes(oauthApp.clientId)) {
+		if (hasAuthorized) {
 			submitRef.current?.click();
 		}
-	}, [oauthApp.clientId, user]);
+	}, [oauthApp.clientId, hasAuthorized]);
 
 	return (
 		<Layout>

apps/meteor/server/models/raw/OAuthApps.ts

@@ -9,10 +9,11 @@ export class OAuthAppsRaw extends BaseRaw<IOAuthApps> implements IOAuthAppsModel
 		super(db, 'oauth_apps', trash);
 	}
 
-	findOneAuthAppByIdOrClientId(props: { clientId: string } | { appId: string }): Promise<IOAuthApps | null> {
+	findOneAuthAppByIdOrClientId(props: { clientId: string } | { appId: string } | { _id: string }): Promise<IOAuthApps | null> {
 		return this.findOne({
+			...('_id' in props && { _id: props._id }),
 			...('appId' in props && { _id: props.appId }),
-			...('clientId' in props && { _id: props.clientId }),
+			...('clientId' in props && { clientId: props.clientId }),
 		});
 	}
 }

packages/model-typings/src/models/IOAuthAppsModel.ts

@@ -3,5 +3,12 @@ import type { IOAuthApps } from '@rocket.chat/core-typings';
 import type { IBaseModel } from './IBaseModel';
 
 export interface IOAuthAppsModel extends IBaseModel<IOAuthApps> {
-	findOneAuthAppByIdOrClientId(props: { clientId: string } | { appId: string }): Promise<IOAuthApps | null>;
+	findOneAuthAppByIdOrClientId(
+		props:
+			| { clientId: string }
+			| { appId: string }
+			| {
+					_id: string;
+			  },
+	): Promise<IOAuthApps | null>;
 }

packages/rest-typings/src/v1/oauthapps.ts

@@ -5,7 +5,7 @@ const ajv = new Ajv({
 	coerceTypes: true,
 });
 
-export type OauthAppsGetParams = { clientId: string } | { appId: string };
+export type OauthAppsGetParams = { clientId: string } | { appId: string } | { _id: string };
 
 const oauthAppsGetParamsSchema = {
 	oneOf: [
@@ -29,6 +29,16 @@ const oauthAppsGetParamsSchema = {
 			required: ['appId'],
 			additionalProperties: false,
 		},
+		{
+			type: 'object',
+			properties: {
+				appId: {
+					type: 'string',
+				},
+			},
+			required: ['_id'],
+			additionalProperties: false,
+		},
 	],
 };