Rocket.Chat icon indicating copy to clipboard operation
Rocket.Chat copied to clipboard

Third-Party Login only works for user with "Manage Oauth Apps"-permission.

Open mbreslein-thd opened this issue 1 year ago • 11 comments

Description:

After I installed the Update to 6.6.0 (from 6.4.8) users without the "Manage Oauth Apps"-permission can't Log into apps that use RocketChat as Oauth povider. Users without that permission get this error: image

In the Logs, this message appears:

{
"level":35,
"time":"2024-02-14T10:56:11.856Z",
"pid":4621,
"hostname":"[redacted]",
"name":"API",
"method":"GET",
"url":"/api/v1/oauth-apps.get?clientId=[redacted]",
"userId":"[redacted]",
"userAgent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.3 Safari/605.1.15",
"host":"[redacted]",
"referer":"https://[redacted]/oauth/authorize?response_type=code&client_id=[redacted]&redirect_uri=[redacted]",
"remoteIP":"[redacted]",
"status":403,
"responseTime":1
} 

With a user that has the permission, the status-code is 200 instead.

Steps to reproduce:

  1. Set up a third-party app under Administration->Workspace->Third-party login
  2. Try to login in the app with a user that doesn't have the "Manage Oauth Apps"-permission. -> won't work.
  3. Configure the user to have that permission
  4. Try logging in again -> works.

Expected behavior:

What happened in 6.4.8 and previously. Users can log in using oauth without that permission.

Actual behavior:

Users need to have the permission to manage oauth apps, which isn't something they should be able to.

Server Setup Information:

  • Version of Rocket.Chat Server: 6.6.0
  • Operating System: Ubuntu 20.04.6 LTS
  • Deployment Method: "Deploy with ubuntu"
  • Number of Running Instances: 1
  • NodeJS Version: 14.18.3
  • MongoDB Version: 4.4.25

Client Setup Information

  • Desktop App or Browser Version: any
  • Operating System: any

Relevant logs:

see description.

mbreslein-thd avatar Feb 14 '24 11:02 mbreslein-thd

This line seems to be the culprit: https://github.com/RocketChat/Rocket.Chat/blob/develop/apps/meteor/app/api/server/v1/oauthapps.ts#L30

It was introduced in this commit: https://github.com/RocketChat/Rocket.Chat/commit/5bb039037015d7d4cd3dcd255ac89e63dbe5c84c

This should fix it: https://github.com/RocketChat/Rocket.Chat/pull/31771

mbreslein-thd avatar Feb 16 '24 09:02 mbreslein-thd

This issue still exists in 6.7.0.

mbreslein-thd avatar Apr 17 '24 06:04 mbreslein-thd

I'm also running into this issue. Shouldn't have to give everyone the manage outh apps permission which they can modify/delete.

AndrewChau avatar May 29 '24 01:05 AndrewChau

A fix is coming.

Thanks for your patience.

reetp avatar May 29 '24 18:05 reetp

People, we are further analyzing this and related PR as there is a security concern. Will get back with more details ASAP.

casalsgh avatar May 29 '24 20:05 casalsgh

People, we are further analyzing this and related PR as there is a security concern. Will get back with more details ASAP.

I'd like to point out that the staus quo also is a security issue because everyone that needs to log into a 3rd party app using oauth also has the permission to edit the oauth config.

mbreslein-thd avatar May 30 '24 06:05 mbreslein-thd

Yes the devs are aware of this fact.

reetp avatar May 30 '24 07:05 reetp

Any news on this? @reetp @casalsgh

mbreslein-thd avatar Jun 12 '24 10:06 mbreslein-thd

You don't need to @ people thanks.

We get notifications regardless.

Answer is nothing yet.

If it is on Gabriels list I can guarantee it isn't being ignored, but some things take time.

I believe this is pretty complicated as it doesn't just affect this, but multiple other things too.

It's with the dev team but will be competing with numerous other issues.

We'll advise when there is more news.

reetp avatar Jun 12 '24 11:06 reetp

This occurrence is not a glitch but rather an implementation to address a security loophole. Since the issue was brought to my attention by our Community Liason (Thanks John!) I shared it with applicable internal squads. Item is being analyzed in detail since we want to deliver a secure solution, might still take a while. Don't expect it to be on next release (6.10), but it might be on the one after that.

And yes, this is on my review list so tracking it.

casalsgh avatar Jun 12 '24 12:06 casalsgh

@mbreslein-thd, I'll try to help with solving the current issue. If I'm mistaken about anything, I ask @casalsgh to correct me.

The problem described in this issue initially occurred due to an incorrect solution to a security problem in the OAuth2 Authorization Flow process. During authorization, the user's browser receives all the attributes of the OAuth2 Application, including the clientSecret. To prevent this, access to the oauth-apps.get API method was restricted with the manage-oauth-apps permission. As a result, users who do not have this permission simply cannot use Rocket.Chat as an OAuth2 identity provider.

Now I'll try to describe why this security issue appeared and how it can be fixed. The solution proposed in the PR #31771 is also not correct. Creating an additional view-oauth-apps permission, which would provide access to the oauth-apps.get API method, will not resolve the original issue of accessing all attributes of the OAuth2 Application within the OAuth2 Authorization Flow.

The oauth-apps.get API method is used in Rocket.Chat in two cases:

  • When rendering the OAuth2 Application editing page in the admin section
  • When rendering the Consent Screen form during the OAuth2 Authorization Flow

For the case of rendering the Consent Screen form during the OAuth2 Authorization Flow, the call chain in the code looks as follows:

  • Route, specified on the Client Application side
  • Request to get data about the OAuth2 Application and pass it to the page rendering
  • Call to the oauth-apps.get API method and passing the clientId argument

For the case of editing the OAuth2 Application, the call chain in the code looks as follows:

  • Rendering the OAuth2 Application editing page
  • Request to get data about the OAuth2 Application and pass it to the form rendering

In the case of the Consent Screen form(here is the form template), only the name and clientId attributes of the OAuth2 Application are used. For editing the OAuth2 Application(here is the form template), all attributes are used.

To resolve the initial security issue and the current issue with broken OAuth2 Authorization Flow, two methods can be used:

  • Create an additional method to obtain a limited set of OAuth2 Application attributes for use in rendering the Consent Screen form during the OAuth2 Authorization Flow process
  • Check for the manage-oauth-apps permission in the call to the oauth-apps.get API method and return the full set of OAuth2 Application attributes only if the _id attribute is present in the request. If the clientId attribute is present, do not check for the permission (as it was before the changes that broke the OAuth2 Authorization Flow for regular users) and return a limited set of attributes

The source code for the oauth-apps.get API method can be modified as follows:

API.v1.addRoute(
    'oauth-apps.get',
    { authRequired: true, validateParams: isOauthAppsGetParams },
    {
        async get() {
            if ('_id' in this.queryParams) {
                if (!(await hasPermissionAsync(this.userId, 'manage-oauth-apps'))) {
                    return API.v1.unauthorized();
                }
            }

            const oauthApp = await OAuthApps.findOneAuthAppByIdOrClientId(this.queryParams);

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

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

            
            return API.v1.success({
                oauthApp: ('_id' in this.queryParams) ? oauthApp: {name: oauthApp.name, clientId: oauthApp.clientId},
            });
        },
    },
);

verdel avatar Jul 10 '24 22:07 verdel

Hi, @verdel . Thank you for your detailed proposal addressing the issue. Your solution to create a new method that returns only the necessary attributes during the consent screen rendering, while ensuring the security of the clientSecret, is very insightful. It looks like this approach will maintain security and restore functionality effectively. We would be delighted if you could contribute this fix to Rocket.Chat.

Please feel free to submit a pull request with your changes.

scuciatto avatar Jul 12 '24 19:07 scuciatto

@scuciatto, I have prepared a PR. Unfortunately, I am not sure if I have considered everything necessary for adding a new API method. Rocket.Chat is quite a complex product, and without deep immersion, it is difficult for me to make such changes to the code. I hope you can help me during the PR review process, and together we can resolve the issue.

As I understand it, adding a new API method will also require changes to the documentation? If I am not mistaken, the changes need to be made in the Rocket.Chat-Open-API? Who and how should initiate these changes?

verdel avatar Jul 13 '24 10:07 verdel

Thanks a lot for the contribution, @verdel. I'll ask for the code review internally. About the documentation, you are right; this is the repository. I'll check with the documentation team on how to proceed with this.

scuciatto avatar Jul 15 '24 13:07 scuciatto

@scuciatto, thank you for your time and attention to this issue. I hope we can resolve the limitation in the OAuth Authorization Flow as quickly as possible.

verdel avatar Jul 15 '24 15:07 verdel