Crivaledaz / Mattermost-LDAP

This module provides an external LDAP authentication in Mattermost for the Team Edition (free).
MIT License
365 stars 72 forks source link

Auth fails with Mattermost Desktop 4.7.0 with "Prevented desktop from navigating to" #80

Closed akomakom closed 2 years ago

akomakom commented 3 years ago

Describe the bug

Starting in Mattermost Desktop 4.7.0, there appears to be a limited set of approved oauth URL regexes that the UI will accept. The current oauth urls fail the regex check, resulting in Prevented desktop from navigating to ....

This works fine in Chrome and Safari.

To Reproduce Steps to reproduce the behavior:

  1. Install Mattermost Desktop 4.7.0
  2. Configure Mattermost server with Mattermost-LDAP
  3. Run Mattermost Desktop from a shell
  4. Attempt to auth using the GitLab button
  5. The form is never submitted, an error message shows up in STDOUT

Expected behavior

Form should be submitted

Screenshots

$ mattermost-desktop 
11:02:46.738 › config.autostart has been configured: true
11:02:46.771 › BrowserView created for server Bla
11:02:46.775 › [Bla] Loading https://XXXXX
11:02:46.779 › couldn't show Terracotta, not ready
11:02:47.565 › [Bla] finished loading https://XXXXX
11:02:54.925 › show back button
11:02:56.594 › Prevented desktop from navigating to: https://XXXXX/oauth/index.php

Project (please complete the following information):

Desktop (please complete the following information):

Workaround

I was able to work around this by rewriting index.php to look like one of the approved custom login url strings:

Crivaledaz commented 3 years ago

Hi,

Thank you for using Mattermost-LDAP and for sharing your investigation in this detailed issue.

I confirm the latest Mattermost Desktop client (version 4.7) is expecting a restricted pool of URL for the Oauth process, and deny custom URLs, with the error Prevented desktop from navigating to:, whatever the Mattermost server version in use.

At this moment, your work around, rewriting URLs to expose Oauth on expected URLs, is the best way to bypass the restriction. Note that in the last point, the file to adapt is authorize.php.

Besides, index.php is not the only URL to rewrite. Actually, new users need to authorize the Oauth server to send their data to Mattermost. For this, there are redirected to a form on authorize.php. So the page authorize.php should also be rewritten.

On Nginx, to rewrite these URLs, you can use the following code :

    location /oauth/access_token {
      try_files $uri  /oauth/index.php;
    }

    location /oauth/authorize {
      try_files $uri /oauth/authorize.php$is_args$args;
    }

To patch the Demo, these lines should be added at line 90 in the nginx.conf. As you mentioned, the line 33 of oauth/authorize.php must be changed to header('Location: access_token'). Moreover, the parameter AuthEndpoint in the GitLab section in config.json should be changed to "http://localhost/oauth/authorize".

I will work on this issue as soon as possible to create a clean patch based on your work around. Your help is welcome, feel free to start a PR.

Thank you again for your help.

jsahner commented 3 years ago

I followed the Docker installation guide and came across this problem. It seems like 1dda144 has broken Apache, because it's missing the rewrite from oauth/access_token to oauth/index.php. Can you confirm?

Crivaledaz commented 3 years ago

Yes, my last commit has broken the Docker installation. I am very sorry.

I start to work on it last week but, I did not have the time to finish it. I hope I will find some time tomorrow to finish this change, but I am a little bit busy these days.

For the moment, if you want to use Apache, checkout commit 5d0b0a09. However, the Apache implementation does not work with the Mattermost desktop version >= 4.7. Else, you can adapt the demo to use Mattermost-LDAP with Nginx, which is working with Mattermost desktop version >= 4.7.

Since rewriting URL is necessary, I am thinking of abandoning the Apache implementation in favor of the Nginx one, because I am more comfortable with Nginx.

I will keep you inform,

Regards

WanpengQian commented 2 years ago

We should document this setting in BareMetal.md. I was getting an error for access_token not found.