Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Oauth / SAML authentication should never take place in an embedded browser #1451

Closed
3 tasks done
Jenjen1324 opened this issue Jan 21, 2021 · 5 comments
Closed
3 tasks done

Comments

@Jenjen1324
Copy link

I confirm (by marking "x" in the [ ] below: [x]):


Summary

According to the OAuth 2.0 Threat Model and Security Considerations (Section 4.1.4) OAuth authentication should not take place in an embedded browser. This also happens on the mobile apps, but I'm not quite sure if this issue applies to Mattermost Server since on the web-app the authentication takes place correctly via a redirect.

Why this is a problem:

  • It's not a security vulnerability per se, but the user has to place unnecessary trust into the desktop/mobile client. The user can't verify that the login form is on the correct page and that they aren't getting phished.
  • OAuth is designed that the username/password is able to stay completely hidden from the client application. By entering the Username/Password in an embedded webview, there is a potential (not necessarily actual) risk that the application extracts these credentials.
  • UX is also impacted as the user has to log in with username/password (2FA if applicable) regardless if he already signed in to the OAuth provider via the browser. Browser-based password managers also can't be used correctly.

Environment

  • Operating System: Any
  • Mattermost Desktop App version: 4.6.1
  • Mattermost Server version: Mattermost Team Edition (GitLab Mattermost) 5.29.1 (I'm assuming all versions are affected)

Steps to reproduce

Log in via OAuth

Expected behavior

Any OAuth flow should open the system browser when logging the user in and granting permission to the client app.

Observed behavior

Login window opens within desktop app

Possible fixes

N/A

@Willyfrog
Copy link
Contributor

Hello and thanks for reportting, we totally agree and we currently have a ticket for tracking it.

I look forward to see that change implemented this year :)

@Jenjen1324
Copy link
Author

@Willyfrog that's great to hear. Just to clarify, this also affects the current GitLab authentication which is already in place. I didn't see it mentioned in the linked ticket.

@Willyfrog
Copy link
Contributor

the idea is to work on Oauth for any service, nothing especific for gitlab, google or some other. I'm afraid the linked ticket is more a reminder that we need to work on that than a full description and scope of the feature, that'll will come once we start to work on it.

@devinbinnie
Copy link
Member

We have a ticket open to deal with this: https://mattermost.atlassian.net/browse/MM-37984

@devinbinnie
Copy link
Member

devinbinnie commented Oct 16, 2023

Hey all,

I want to apologize for the lack of communication on these tickets regarding authentication. But I come with some good news: we've been working on separating the external login flow from the Desktop App, deferring to the browser like many other applications do. This should overall improve security and stability around the login process, as the current implementation requires a few hacks to make it work correctly.

As of today, we released Mattermost v9.1 which has this feature implemented, and should work with all existing Desktop App versions going back to at least v5.3.0. Going forward we will be supporting this login flow for all external providers using the Desktop App, and this should fix any issues around login flow. Your server will need to be upgraded to take advantage of this feature.

I'll be closing these tickets for now as fixed, but feel free to comment and ask questions if you have any. Thanks for your patience :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants