Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 5 additions & 0 deletions .changeset/little-steaks-itch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@rocket.chat/meteor': patch
---

Fixes incorrect permission checks on workspace registration status, aligning the API and UI hooks with manage-cloud access.
5 changes: 5 additions & 0 deletions .changeset/selfish-jeans-rest.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@rocket.chat/meteor': patch
---

Prevents custom status being saved in local storage as `undefined` and breaking the UI when accessing it
16 changes: 9 additions & 7 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -655,24 +655,26 @@ jobs:
# List loaded images
docker images

- name: Run federation integration tests with pre-built image
- name: Start federation services
working-directory: ./ee/packages/federation-matrix
env:
ROCKETCHAT_IMAGE: ghcr.io/${{ needs.release-versions.outputs.lowercase-repo }}/rocket.chat:${{ needs.release-versions.outputs.gh-docker-tag }}-amd64
ENTERPRISE_LICENSE_RC1: ZAikY+LLaal7mT6RNYxpyWEmMQyucrl50/7pYBXqHczc90j+RLwF+T0xuCT2pIpKMC5DxcZ1TtkV6MYJk5whrwmap+mQ0FV+VpILJlL0i4T21K4vMfzZXTWm/pzcAy2fMTUNH+mUA9HTBD6lYYh40KnbGXPAd80VbZk0MO/WbWBm2dOT0YCwfvlRyurRqkDAQrftLaffzCNUsMKk0fh+MKs73UDHZQDp1yvs7WoGpPu5ZVi5mTBOt3ZKVz5KjGfClLwJptFPmW1w6nKelAiJBDPpjcX1ylfjxpnBoixko7uN52zlyaeoAYwfRcdDLnZ8k0Ou6tui/vTQUXjGIjHw2AhMaKwonn4E9LYpuA1KEXt08qJL5J3ZtjSCV1T+A9Z3zFhhLgp5dxP/PPUbxDn/P8XKp7nXM9duIfcCMlnea7V8ixEyCHwwvKQaXVVidcsUGtB8CwS0GlsAEBLOzqMehuQUK2rdQ4WgEz3AYveikeVvSzgBHvyXsxssWAThc0Mht0eEJqdDhUB2QeZ2WmPsaSSD639Z4WgjSUoR0zh8bfqepH+2XRcUryXe2yN+iU+3POzi9wfg0k65MxXT8pBg3PD5RHnR8oflEP0tpZts33JiBhYRxX3MKplAFm4dMuphTsDJTh+e534pT7IPuZF79QSVaLEWZfVVVb7nGFtmMwA=
QASE_TESTOPS_JEST_API_TOKEN: ${{ secrets.QASE_TESTOPS_JEST_API_TOKEN }}
PR_NUMBER: ${{ github.event.number }}
run: yarn test:integration --image "${ROCKETCHAT_IMAGE}"
run: yarn test:integration --start-containers-only --image "${ROCKETCHAT_IMAGE}"

- name: Show rc server logs if tests failed
if: failure()
- name: Run federation integration tests
working-directory: ./ee/packages/federation-matrix
run: docker compose -f docker-compose.test.yml logs rc1-prebuilt
env:
QASE_TESTOPS_JEST_API_TOKEN: ${{ secrets.QASE_TESTOPS_JEST_API_TOKEN }}
PR_NUMBER: ${{ github.event.number }}
run: yarn test:integration --ci

- name: Show hs server logs if tests failed
- name: Show federation integration tests logs
if: failure()
working-directory: ./ee/packages/federation-matrix
run: docker compose -f docker-compose.test.yml logs hs1
run: yarn test:integration --logs

- name: Show mongo logs if tests failed
if: failure()
Expand Down
4 changes: 2 additions & 2 deletions apps/meteor/app/api/server/v1/cloud.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { isCloudConfirmationPollProps, isCloudCreateRegistrationIntentProps, isC

import { CloudWorkspaceRegistrationError } from '../../../../lib/errors/CloudWorkspaceRegistrationError';
import { SystemLogger } from '../../../../server/lib/logger/system';
import { hasRoleAsync } from '../../../authorization/server/functions/hasRole';
import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission';
import { getCheckoutUrl } from '../../../cloud/server/functions/getCheckoutUrl';
import { getConfirmationPoll } from '../../../cloud/server/functions/getConfirmationPoll';
import {
Expand Down Expand Up @@ -88,7 +88,7 @@ API.v1.addRoute(
{ authRequired: true },
{
async get() {
if (!(await hasRoleAsync(this.userId, 'admin'))) {
if (!(await hasPermissionAsync(this.userId, 'manage-cloud'))) {
return API.v1.forbidden();
}

Expand Down
4 changes: 2 additions & 2 deletions apps/meteor/app/meteor-accounts-saml/server/lib/Utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ export class SAMLUtils {
const buffer = Buffer.from(base64Data, 'base64');
zlib.inflateRaw(buffer, (err, decoded) => {
if (err) {
this.log(`Error while inflating. ${err}`);
this.log({ msg: 'Error while inflating.', err });
return reject(errorCallback(err));
}

Expand Down Expand Up @@ -426,7 +426,7 @@ export class SAMLUtils {
const attributeList = new Map();
for (const attributeName of userDataMap.attributeList) {
if (profile[attributeName] === undefined) {
this.log(`SAML user profile is missing the attribute ${attributeName}.`);
this.log({ msg: 'SAML user profile is missing the attribute.', attribute: attributeName });
continue;
}
attributeList.set(attributeName, profile[attributeName]);
Expand Down
8 changes: 4 additions & 4 deletions apps/meteor/app/push/server/fcm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ async function fetchWithRetry(url: string, _removeToken: () => void, options: Ex
}

const error: FCMError = await response.json();
logger.error('sendFCM error', error);
logger.error({ msg: 'sendFCM error', err: error });

return response;
}
Expand Down Expand Up @@ -164,7 +164,7 @@ export const sendFCM = function ({ userTokens, notification, _removeToken, optio
return;
}

logger.debug('sendFCM', tokens, notification);
logger.debug({ msg: 'sendFCM', tokens, notification });

const messages = getFCMMessagesFromPushData(tokens, notification);
const headers = {
Expand All @@ -181,7 +181,7 @@ export const sendFCM = function ({ userTokens, notification, _removeToken, optio
const url = `https://fcm.googleapis.com/v1/projects/${options.gcm.projectNumber}/messages:send`;

for (const fcmRequest of messages) {
logger.debug('sendFCM message', fcmRequest);
logger.debug({ msg: 'sendFCM message', request: fcmRequest });

const removeToken = () => {
const { token } = fcmRequest.message;
Expand All @@ -191,7 +191,7 @@ export const sendFCM = function ({ userTokens, notification, _removeToken, optio
const response = fetchWithRetry(url, removeToken, { method: 'POST', headers, body: JSON.stringify(fcmRequest) });

response.catch((err) => {
logger.error('sendFCM error', err);
logger.error({ msg: 'sendFCM error', err });
});
}
};
8 changes: 4 additions & 4 deletions apps/meteor/app/push/server/methods.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,19 +100,19 @@ export const pushUpdate = async (options: PushUpdateOptions): Promise<Omit<IApps
).deletedCount;

if (removed) {
logger.debug(`Removed ${removed} existing app items`);
logger.debug({ msg: 'Removed existing app items', removed });
}
}

logger.debug('updated', doc);
logger.debug({ msg: 'Push token updated', doc });

// Return the doc we want to use
return doc;
};

Meteor.methods<ServerMethods>({
async 'raix:push-update'(options) {
logger.debug('Got push token from app:', options);
logger.debug({ msg: 'Got push token from app', options });

check(options, {
id: Match.Optional(String),
Expand All @@ -137,7 +137,7 @@ Meteor.methods<ServerMethods>({
throw new Meteor.Error(403, 'Forbidden access');
}

logger.debug(`Settings userId "${this.userId}" for app:`, id);
logger.debug({ msg: 'Setting userId for app', userId: this.userId, appId: id });
const found = await AppsTokens.updateOne({ _id: id }, { $set: { userId: this.userId } });

return !!found;
Expand Down
6 changes: 3 additions & 3 deletions apps/meteor/app/utils/lib/templateVarHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export const templateVarHandler = function (variable: string, object: Record<str

if (match == null) {
if (!object.hasOwnProperty(variable)) {
logger?.debug(`user does not have attribute: ${variable}`);
logger?.debug({ msg: 'User does not have attribute', attribute: variable });
return;
}
return object[variable];
Expand All @@ -20,12 +20,12 @@ export const templateVarHandler = function (variable: string, object: Record<str
const tmplAttrName = match[1];

if (!object.hasOwnProperty(tmplAttrName)) {
logger?.debug(`user does not have attribute: ${tmplAttrName}`);
logger?.debug({ msg: 'User does not have attribute', attribute: tmplAttrName });
return;
}

const attrVal = object[tmplAttrName];
logger?.debug(`replacing template var: ${tmplVar} with value: ${attrVal}`);
logger?.debug({ msg: 'Replacing template var', templateVar: tmplVar, value: attrVal });
tmpVariable = tmpVariable.replace(tmplVar, attrVal);
match = templateRegex.exec(variable);
}
Expand Down
68 changes: 68 additions & 0 deletions apps/meteor/client/hooks/useRegistrationStatus.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import { mockAppRoot } from '@rocket.chat/mock-providers';
import { renderHook, waitFor } from '@testing-library/react';

import { useRegistrationStatus } from './useRegistrationStatus';

describe('useRegistrationStatus', () => {
it('should not call API and return error state when user does not have manage-cloud permission', async () => {
const mockGetRegistrationStatus = jest.fn();

const { result } = renderHook(() => useRegistrationStatus(), {
wrapper: mockAppRoot().withEndpoint('GET', '/v1/cloud.registrationStatus', mockGetRegistrationStatus).withJohnDoe().build(),
});

await waitFor(() => {
expect(result.current.isError).toBe(true);
});

expect(result.current.canViewRegistrationStatus).toBe(false);
expect(result.current.isRegistered).toBeFalsy();
expect(mockGetRegistrationStatus).not.toHaveBeenCalled();
});

it('should call API and return isRegistered as true and canViewRegistrationStatus as true when workspace is registered and user has manage-cloud permission', async () => {
const mockGetRegistrationStatus = jest.fn().mockResolvedValue({
registrationStatus: {
workspaceRegistered: true,
},
});

const { result } = renderHook(() => useRegistrationStatus(), {
wrapper: mockAppRoot()
.withEndpoint('GET', '/v1/cloud.registrationStatus', mockGetRegistrationStatus)
.withPermission('manage-cloud')
.withJohnDoe()
.build(),
});

await waitFor(() => {
expect(mockGetRegistrationStatus).toHaveBeenCalled();
});

expect(result.current.isRegistered).toBe(true);
expect(result.current.canViewRegistrationStatus).toBe(true);
});

it('should call API, return isRegistered as false and canViewRegistrationStatus as true when workspace is not registered and user has manage-cloud permission', async () => {
const mockGetRegistrationStatus = jest.fn().mockResolvedValue({
registrationStatus: {
workspaceRegistered: false,
},
});

const { result } = renderHook(() => useRegistrationStatus(), {
wrapper: mockAppRoot()
.withEndpoint('GET', '/v1/cloud.registrationStatus', mockGetRegistrationStatus)
.withPermission('manage-cloud')
.withJohnDoe()
.build(),
});

await waitFor(() => {
expect(mockGetRegistrationStatus).toHaveBeenCalled();
});

expect(result.current.isRegistered).toBe(false);
expect(result.current.canViewRegistrationStatus).toBe(true);
});
});
11 changes: 8 additions & 3 deletions apps/meteor/client/hooks/useRegistrationStatus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,28 @@ import type { UseQueryResult } from '@tanstack/react-query';
import { useQuery } from '@tanstack/react-query';

type useRegistrationStatusReturnType = {
canViewRegistrationStatus: boolean;
isRegistered?: boolean;
} & UseQueryResult<OperationResult<'GET', '/v1/cloud.registrationStatus'>>;

export const useRegistrationStatus = (): useRegistrationStatusReturnType => {
const getRegistrationStatus = useEndpoint('GET', '/v1/cloud.registrationStatus');
const canViewregistrationStatus = usePermission('manage-cloud');
const canViewRegistrationStatus = usePermission('manage-cloud');

const queryResult = useQuery({
queryKey: ['getRegistrationStatus'],
queryFn: () => {
if (!canViewregistrationStatus) {
if (!canViewRegistrationStatus) {
throw new Error('unauthorized api call');
}
return getRegistrationStatus();
},
staleTime: Infinity,
});

return { isRegistered: !queryResult.isPending && queryResult.data?.registrationStatus?.workspaceRegistered, ...queryResult };
return {
canViewRegistrationStatus,
isRegistered: !queryResult.isPending && queryResult.data?.registrationStatus?.workspaceRegistered,
...queryResult,
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ type EditStatusModalProps = {
const EditStatusModal = ({ onClose, userStatus, userStatusText }: EditStatusModalProps): ReactElement => {
const allowUserStatusMessageChange = useSetting('Accounts_AllowUserStatusMessageChange');
const dispatchToastMessage = useToastMessageDispatch();
const [customStatus, setCustomStatus] = useLocalStorage<string | undefined>('Local_Custom_Status', '');
const initialStatusText = customStatus || userStatusText;
const [customStatus, setCustomStatus] = useLocalStorage<string>('Local_Custom_Status', '');
const initialStatusText = customStatus || userStatusText || '';

const t = useTranslation();
const modalId = useId();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ const SubscriptionPage = () => {
const showLicense = useShowLicense();
const router = useRouter();
const { data: enterpriseData } = useIsEnterprise();
const { isRegistered } = useRegistrationStatus();
const { canViewRegistrationStatus } = useRegistrationStatus();
const { data: licensesData, isLoading: isLicenseLoading } = useLicenseWithCloudAnnouncement({ loadValues: true });
const syncLicenseUpdate = useWorkspaceSync();
const invalidateLicenseQuery = useInvalidateLicense();
Expand Down Expand Up @@ -108,7 +108,7 @@ const SubscriptionPage = () => {
<Page bg='tint'>
<PageHeaderNoShadow title={t('Subscription')}>
<ButtonGroup>
{isRegistered && (
{canViewRegistrationStatus && (
<Button loading={syncLicenseUpdate.isPending} icon='reload' onClick={() => handleSyncLicenseUpdate()}>
{t('Sync_license_update')}
</Button>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ const VersionCard = ({ serverInfo }: VersionCardProps): ReactElement => {
const formatDate = useFormatDate();

const { data: licenseData, isPending, refetch: refetchLicense } = useLicense({ loadValues: true });
const { isRegistered } = useRegistrationStatus();
const { isRegistered, canViewRegistrationStatus } = useRegistrationStatus();

const { license, limits } = licenseData || {};
const isAirgapped = license?.information?.offline;
Expand Down Expand Up @@ -82,7 +82,7 @@ const VersionCard = ({ serverInfo }: VersionCardProps): ReactElement => {
action: () => void;
label: ReactNode;
} = useMemo(() => {
if (!isRegistered) {
if (canViewRegistrationStatus && !isRegistered) {
return {
action: () => {
const handleModalClose = (): void => {
Expand All @@ -107,7 +107,7 @@ const VersionCard = ({ serverInfo }: VersionCardProps): ReactElement => {
if (isOverLimits) {
return { path: '/admin/subscription', label: t('Manage_subscription') };
}
}, [isRegistered, versionStatus, isOverLimits, t, setModal, refetchLicense]);
}, [canViewRegistrationStatus, isRegistered, versionStatus, isOverLimits, t, setModal, refetchLicense]);

const actionItems = useMemo(() => {
return (
Expand Down Expand Up @@ -154,19 +154,30 @@ const VersionCard = ({ serverInfo }: VersionCardProps): ReactElement => {
</Trans>
),
},
isRegistered
? {
icon: 'check',
label: t('Workspace_registered'),
}
: {
danger: true,
icon: 'warning',
label: t('Workspace_not_registered'),
},
canViewRegistrationStatus &&
(isRegistered
? {
icon: 'check',
label: t('Workspace_registered'),
}
: {
danger: true,
icon: 'warning',
label: t('Workspace_not_registered'),
}),
].filter(Boolean) as VersionActionItem[]
).sort((a) => (a.danger ? -1 : 1));
}, [isOverLimits, t, isAirgapped, versions, versionStatus?.label, versionStatus?.expiration, formatDate, isRegistered]);
}, [
isOverLimits,
t,
isAirgapped,
versions,
versionStatus?.label,
versionStatus?.expiration,
formatDate,
canViewRegistrationStatus,
isRegistered,
]);

if (isPending && !licenseData) {
return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export class AutoCloseOnHoldSchedulerClass {
throw new Error('AutoCloseOnHoldScheduler is not running');
}

this.logger.debug(`Scheduling room ${roomId} to be closed in ${timeout} seconds`);
this.logger.debug({ msg: 'Scheduling room to be closed', roomId, timeoutSeconds: timeout });
await this.unscheduleRoom(roomId);

const jobName = `${SCHEDULER_NAME}-${roomId}`;
Expand All @@ -60,13 +60,13 @@ export class AutoCloseOnHoldSchedulerClass {
if (!this.running) {
throw new Error('AutoCloseOnHoldScheduler is not running');
}
this.logger.debug(`Unscheduling room ${roomId}`);
this.logger.debug({ msg: 'Unscheduling room', roomId });
const jobName = `${SCHEDULER_NAME}-${roomId}`;
await this.scheduler.cancel({ name: jobName });
}

private async executeJob({ attrs: { data } }: any = {}): Promise<void> {
this.logger.debug(`Executing job for room ${data.roomId}`);
this.logger.debug({ msg: 'Executing job for room', roomId: data.roomId });
const { roomId, comment } = data;

const [room, user] = await Promise.all([LivechatRooms.findOneById(roomId), this.getSchedulerUser()]);
Expand Down
Loading
Loading