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

mbreslein-thd commented 7 months ago

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:

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:

Set up a third-party app under Administration->Workspace->Third-party login
Try to login in the app with a user that doesn't have the "Manage Oauth Apps"-permission. -> won't work.
Configure the user to have that permission
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 commented 7 months ago

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 commented 5 months ago

This issue still exists in 6.7.0.

AndrewChau commented 4 months ago

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

reetp commented 4 months ago

A fix is coming.

Thanks for your patience.

casalsgh commented 4 months ago

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

mbreslein-thd commented 4 months ago

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.

reetp commented 4 months ago

Yes the devs are aware of this fact.

mbreslein-thd commented 3 months ago

Any news on this? @reetp @casalsgh

reetp commented 3 months ago

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.

casalsgh commented 3 months ago

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.

verdel commented 2 months ago

@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},
            });
        },
    },
);

scuciatto commented 2 months ago

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.

verdel commented 2 months ago

@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?

scuciatto commented 2 months ago

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.

verdel commented 2 months ago

@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.

RocketChat / Rocket.Chat