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

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

Closed
akomakom opened this issue Jul 7, 2021 · 4 comments
Assignees
Labels

Comments

@akomakom
Copy link
Contributor

akomakom commented Jul 7, 2021

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):

  • Project Version: latest

Desktop (please complete the following information):

  • OS: Ubuntu 20.04 and Mac OS Big Sur
  • Browser: N/A

Workaround

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

  • Enabling .htaccess in apache, eg:
<Directory /var/www/html/oauth>
    AllowOverride All
</Directory>
  • Added to .htaccess:
RewriteEngine on
RewriteRule ^access_token$ index.php [L]
<Files access_token>
    allow from all
</Files>
  • Changed index.php line 33 to header('Location: access_token');
@Crivaledaz
Copy link
Owner

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
Copy link

jsahner commented Nov 30, 2021

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
Copy link
Owner

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 5d0b0a0. 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
Copy link

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

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

No branches or pull requests

4 participants