Skip to content

Conversation

@sampaiodiego
Copy link
Member

@sampaiodiego sampaiodiego commented Nov 10, 2022

Proposed changes (including videos or screenshots)

The main change of this PR is to consider a workspace registered only if it has both Cloud_Workspace_Client_Id and Cloud_Workspace_Client_Secret settings saved on the server's cache here.

The problem was it was checking only for Cloud_Workspace_Client_Id but was also using the secret to perform requests afterwards, and due to some race conditions on how settings are saved in cache, it was happening some times the clientId was in cache but the secret was not, so now with this additional check it will prevent those cases.

The other changes are due to TS conversion.

Issue(s)

ARCH-84

Steps to test or reproduce

Further comments

@sampaiodiego sampaiodiego added this to the 5.3.1 milestone Nov 10, 2022
@sampaiodiego sampaiodiego requested a review from a team November 10, 2022 14:02
@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: needs QA labels Nov 10, 2022
@lgtm-com
Copy link

lgtm-com bot commented Nov 10, 2022

This pull request introduces 1 alert when merging 4524d82 into f357af4 - view on LGTM.com

new alerts:

  • 1 for Missing await

@lgtm-com
Copy link

lgtm-com bot commented Nov 10, 2022

This pull request introduces 1 alert when merging 3047c87 into f357af4 - view on LGTM.com

new alerts:

  • 1 for Missing await

@sampaiodiego sampaiodiego requested a review from a team as a code owner November 10, 2022 17:22
ggazzo
ggazzo previously approved these changes Nov 10, 2022
@ggazzo ggazzo merged commit 29e7b37 into develop Nov 11, 2022
@ggazzo ggazzo deleted the check-cloud-client-secret branch November 11, 2022 13:12
bkrith pushed a commit to bkrith/Rocket.Chat that referenced this pull request Nov 12, 2022
@ggazzo ggazzo mentioned this pull request Nov 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stat: ready to merge PR tested and approved waiting for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants