Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
7 changes: 7 additions & 0 deletions .changeset/tall-scissors-boil.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@rocket.chat/meteor": patch
"@rocket.chat/core-typings": patch
"@rocket.chat/rest-typings": patch
---

Add OpenAPI support for the Rocket.Chat oauth-apps.get API endpoints by migrating to a modern chained route definition syntax and utilizing shared AJV schemas for validation to enhance API documentation and ensure type safety through response validation.
2 changes: 2 additions & 0 deletions apps/meteor/app/api/server/definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,8 @@ export type TypedThis<TOptions extends TypedOptions, TPath extends string = ''>
bodyParams: TOptions['body'] extends ValidateFunction<infer Body> ? Body : never;

requestIp?: string;
route: string;
response: Response;
Comment on lines +306 to +307
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It was missing in the typedThis, so I added it.

};

type PromiseOrValue<T> = T | Promise<T>;
Expand Down
100 changes: 80 additions & 20 deletions apps/meteor/app/api/server/v1/oauthapps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { OAuthApps } from '@rocket.chat/models';
import {
ajv,
isUpdateOAuthAppParams,
isOauthAppsGetParams,
isDeleteOAuthAppParams,
validateUnauthorizedErrorResponse,
validateBadRequestErrorResponse,
Expand Down Expand Up @@ -63,30 +62,87 @@ const oauthAppsListEndpoints = API.v1.get(
},
);

API.v1.addRoute(
type OauthAppsGetParams = { clientId: string } | { appId: string } | { _id: string };

const oauthAppsGetParamsSchema = {
oneOf: [
{
type: 'object',
properties: {
_id: {
type: 'string',
},
},
required: ['_id'],
additionalProperties: false,
},
{
type: 'object',
properties: {
clientId: {
type: 'string',
},
},
required: ['clientId'],
additionalProperties: false,
},
{
type: 'object',
properties: {
appId: {
type: 'string',
},
},
required: ['appId'],
additionalProperties: false,
},
],
};

const isOauthAppsGetParams = ajv.compile<OauthAppsGetParams>(oauthAppsGetParamsSchema);

const oauthAppsGetEndpoints = API.v1.get(
'oauth-apps.get',
{ authRequired: true, validateParams: isOauthAppsGetParams },
{
async get() {
const isOAuthAppsManager = await hasPermissionAsync(this.userId, 'manage-oauth-apps');
authRequired: true,
query: isOauthAppsGetParams,
response: {
400: validateBadRequestErrorResponse,
401: validateUnauthorizedErrorResponse,
200: ajv.compile<{ oauthApp: IOAuthApps }>({
type: 'object',
properties: {
oauthApp: { anyOf: [{ $ref: '#/components/schemas/IOAuthApps' }, { type: 'null' }] },
success: {
type: 'boolean',
enum: [true],
},
},
required: ['oauthApp', 'success'],
additionalProperties: false,
}),
},
},

async function action() {
const isOAuthAppsManager = await hasPermissionAsync(this.userId, 'manage-oauth-apps');

const oauthApp = await OAuthApps.findOneAuthAppByIdOrClientId(
this.queryParams,
!isOAuthAppsManager ? { projection: { clientSecret: 0 } } : {},
);
const oauthApp = await OAuthApps.findOneAuthAppByIdOrClientId(
this.queryParams,
!isOAuthAppsManager ? { projection: { clientSecret: 0 } } : {},
);

if (!oauthApp) {
return API.v1.failure('OAuth app not found.');
}
if (!oauthApp) {
return API.v1.failure('OAuth app not found.');
}

if ('appId' in this.queryParams) {
apiDeprecationLogger.parameter(this.route, 'appId', '7.0.0', this.response);
}
if ('appId' in this.queryParams) {
apiDeprecationLogger.parameter(this.route, 'appId', '7.0.0', this.response);
}

return API.v1.success({
oauthApp,
});
},
return API.v1.success({
oauthApp,
});
},
);

Expand Down Expand Up @@ -187,11 +243,15 @@ type OauthAppsCreateEndpoints = ExtractRoutesFromAPI<typeof oauthAppsCreateEndpo

type OauthAppsListEndpoints = ExtractRoutesFromAPI<typeof oauthAppsListEndpoints>;

export type OAuthAppsEndpoints = OauthAppsCreateEndpoints | OauthAppsListEndpoints;
type OauthAppsGetEndpoints = ExtractRoutesFromAPI<typeof oauthAppsGetEndpoints>;

export type OAuthAppsEndpoints = OauthAppsCreateEndpoints | OauthAppsListEndpoints | OauthAppsGetEndpoints;

declare module '@rocket.chat/rest-typings' {
// eslint-disable-next-line @typescript-eslint/naming-convention, @typescript-eslint/no-empty-interface
interface Endpoints extends OauthAppsCreateEndpoints {}
// eslint-disable-next-line @typescript-eslint/naming-convention, @typescript-eslint/no-empty-interface
interface Endpoints extends OauthAppsListEndpoints {}
// eslint-disable-next-line @typescript-eslint/naming-convention, @typescript-eslint/no-empty-interface
interface Endpoints extends OauthAppsGetEndpoints {}
}
9 changes: 6 additions & 3 deletions apps/meteor/tests/end-to-end/api/oauthapps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -286,8 +286,9 @@ describe('[OAuthApps]', () => {
.expect(400)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('errorType', 'error-invalid-params');
expect(res.body).to.have.property('error');
expect(res.body.error).to.include('must be string').and.include('must match exactly one schema in oneOf [invalid-params]');
expect(res.body.error).to.include('must be string').and.include('must match exactly one schema in oneOf');
Comment on lines +289 to +291
Copy link
Copy Markdown
Contributor Author

@ahmed-n-abdeltwab ahmed-n-abdeltwab Aug 1, 2025

Choose a reason for hiding this comment

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

The error was must match exactly one schema in oneOf but didn’t include [invalid-params], so I added a separate assertion for errorType instead.

});
});

Expand All @@ -311,8 +312,9 @@ describe('[OAuthApps]', () => {
.expect(400)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('errorType', 'error-invalid-params');
expect(res.body).to.have.property('error');
expect(res.body.error).to.include('must be string').and.include('must match exactly one schema in oneOf [invalid-params]');
expect(res.body.error).to.include('must be string').and.include('must match exactly one schema in oneOf');
});
});

Expand All @@ -336,8 +338,9 @@ describe('[OAuthApps]', () => {
.expect(400)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('errorType', 'error-invalid-params');
expect(res.body).to.have.property('error');
expect(res.body.error).to.include('must be string').and.include('must match exactly one schema in oneOf [invalid-params]');
expect(res.body.error).to.include('must be string').and.include('must match exactly one schema in oneOf');
});
});

Expand Down
2 changes: 1 addition & 1 deletion packages/core-typings/src/IOAuthApps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ export interface IOAuthApps {
name: string;
active: boolean;
clientId: string;
clientSecret: string;
clientSecret?: string;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The IOAuthApps used in findOneAuthAppByIdOrClientId doesn’t include clientSecret, so I made it optional in the interface

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

packages/models/src/models/OAuthApps.ts
findOneAuthAppByIdOrClientId(
		props: { clientId: string } | { appId: string } | { _id: string },
		options?: FindOptions<IOAuthApps>,
	): Promise<IOAuthApps | null> {
		return this.findOne(
			{
				...('_id' in props && { _id: props._id }),
				...('appId' in props && { _id: props.appId }),
				...('clientId' in props && { clientId: props.clientId }),
			},
			options,
		);
	}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Without this change, AJV would throw an error saying clientSecret is required, even though it's not returned

redirectUri: string;
_createdAt: Date;
_createdBy: {
Expand Down
1 change: 0 additions & 1 deletion packages/rest-typings/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,6 @@ export * from './v1/licenses';
export * from './v1/omnichannel';
export * from './v1/oauthapps';
export * from './v1/oauthapps/UpdateOAuthAppParamsPOST';
export * from './v1/oauthapps/OAuthAppsGetParamsGET';
export * from './v1/oauthapps/DeleteOAuthAppParamsDELETE';
export * from './helpers/PaginatedRequest';
export * from './helpers/PaginatedResult';
Expand Down
7 changes: 0 additions & 7 deletions packages/rest-typings/src/v1/oauthapps.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,9 @@
import type { IOAuthApps } from '@rocket.chat/core-typings';

import type { DeleteOAuthAppParams } from './oauthapps/DeleteOAuthAppParamsDELETE';
import type { OauthAppsGetParams } from './oauthapps/OAuthAppsGetParamsGET';
import type { UpdateOAuthAppParams } from './oauthapps/UpdateOAuthAppParamsPOST';

export type OAuthAppsEndpoint = {
'/v1/oauth-apps.get': {
GET: (params: OauthAppsGetParams) => {
oauthApp: IOAuthApps;
};
};

'/v1/oauth-apps.update': {
POST: (params: UpdateOAuthAppParams) => IOAuthApps | null;
};
Expand Down
40 changes: 0 additions & 40 deletions packages/rest-typings/src/v1/oauthapps/OAuthAppsGetParamsGET.ts

This file was deleted.

Loading