wefork / wekan

The open-source Trello-like kanban (built with Meteor)
https://wekan.io
MIT License
61 stars 12 forks source link

Double slash problem on card pop-ups #49

Closed quimnuss closed 7 years ago

quimnuss commented 7 years ago

The card's pop-up links to

http://192.168.30.240/wekan//b/Gtw5WWes4kvanupHu/devel/HduAsvhiCJLPJF4Hb

Note the double slash //b, if I remove one slash, it works as expected. Also, navigating to the boards doesn't suffer from this.

I believe apache2 is at fault here, but I'm not sure. There are two similar issues: https://github.com/wekan/wekan/issues/679 and https://github.com/wekan/wekan/issues/671

I've set my root url export ROOT_URL='http://192.168.30.240/wekan'

I've tried some Rewrite Rules on apache2, but I couldn't find out the right one to use.

    RewriteEngine on
    LogLevel alert rewrite:trace6
    #RewriteRule ^/wekan$ /wekan/ [R]
    #RewriteRule ^(.*)/+$ $1 [R=301,L]

    #ProxyRequests Off
    #ProxyPreserveHost On

    ProxyPassMatch  "^/(sockjs\/.*\/websocket)$" "ws://localhost:3000/wekan/$1"
    ProxyPass /wekan http://localhost:3000/wekan
    ProxyPassReverse /wekan http://localhost:3000/wekan

a log:

[Thu Dec 15 17:21:59.925460 2016] [rewrite:trace2] [pid 10949] mod_rewrite.c(475): [client 192.168.30.83:49205] baldrik.local - - [192.168.30.240/sid#7fd0e53fa748][rid#7fd0e52f90a0/initial] init rewrite engine with requested uri /wekan/sockjs/594/w943hlqy/xhr_send, referer: http://192.168.30.240/wekan//b/Gtw5WWes4kvanupHu/devel/HduAsvhiCJLPJF4Hb
[Thu Dec 15 17:21:59.925520 2016] [rewrite:trace1] [pid 10949] mod_rewrite.c(475): [client 192.168.30.83:49205] baldrik.local - - [192.168.30.240/sid#7fd0e53fa748][rid#7fd0e52f90a0/initial] pass through /wekan/sockjs/594/w943hlqy/xhr_send, referer: http://192.168.30.240/wekan//b/Gtw5WWes4kvanupHu/devel/HduAsvhiCJLPJF4Hb
[Thu Dec 15 17:21:59.946285 2016] [rewrite:trace2] [pid 10951] mod_rewrite.c(475): [client 192.168.30.83:49181] baldrik.local - - [192.168.30.240/sid#7fd0e53fa748][rid#7fd0e52520a0/initial] init rewrite engine with requested uri /wekan/sockjs/594/w943hlqy/xhr, referer: http://192.168.30.240/wekan//b/Gtw5WWes4kvanupHu/devel/HduAsvhiCJLPJF4Hb
[Thu Dec 15 17:21:59.946314 2016] [rewrite:trace1] [pid 10951] mod_rewrite.c(475): [client 192.168.30.83:49181] baldrik.local - - [192.168.30.240/sid#7fd0e53fa748][rid#7fd0e52520a0/initial] pass through /wekan/sockjs/594/w943hlqy/xhr, referer: http://192.168.30.240/wekan//b/Gtw5WWes4kvanupHu/devel/HduAsvhiCJLPJF4Hb
[Thu Dec 15 17:22:00.048906 2016] [rewrite:trace2] [pid 10951] mod_rewrite.c(475): [client 192.168.30.83:49181] baldrik.local - - [192.168.30.240/sid#7fd0e53fa748][rid#7fd0e52fb0a0/initial] init rewrite engine with requested uri /wekan/sockjs/594/w943hlqy/xhr, referer: http://192.168.30.240/wekan//b/Gtw5WWes4kvanupHu/devel/HduAsvhiCJLPJF4Hb
[Thu Dec 15 17:22:00.048934 2016] [rewrite:trace1] [pid 10951] mod_rewrite.c(475): [client 192.168.30.83:49181] baldrik.local - - [192.168.30.240/sid#7fd0e53fa748][rid#7fd0e52fb0a0/initial] pass through /wekan/sockjs/594/w943hlqy/xhr, referer: http://192.168.30.240/wekan//b/Gtw5WWes4kvanupHu/devel/HduAsvhiCJLPJF4Hb

Somebody has a better idea?

xet7 commented 7 years ago

@quimnuss

At https://github.com/wefork/wekan/wiki/Install-from-source in "Run Wefork" section, please setup correct ROOT_URL.

I have only run Caddy in front of Wefork, not Apache or Nginx.

quimnuss commented 7 years ago

Thanks xet7 for the prompt response

That's one of the installation instructions I've followed. In my case, webkan is accessed thought /webkan because there's another webpage at /, hence the modified ROOT_URL, which I deem correct.

The rewrite rules do work, but only if I refresh the page since I suspect the supplementary backslash is added after evaluating the rules.

Maybe you don't encounter the problem because you're running webkan at /, I'm not sure.

Could the problem be related to sockjs ?

xet7 commented 7 years ago

@quimnuss

I have always used ROOT_URL like https://wekan.example.com or http://localhost that have worked for me, I have not tried running on suburl. My Apache-fu has become a little rusty after getting used to easyness of Caddy and automatic Let's Encrypt SSL.

quimnuss commented 7 years ago

Yes, if I set the config with:

  export ROOT_URL='http://192.168.30.240'

and

   ProxyPassMatch  "^/(sockjs\/.*\/websocket)$" "ws://localhost:3000/$1"
   ProxyPass / http://localhost:3000/
   ProxyPassReverse / http://localhost:3000/

It works as expected. But I'd like to place wekan at /wekan instead of '/'

quimnuss commented 7 years ago

That could be it:

https://github.com/wekan/wekan/issues/133

xet7 commented 7 years ago

@quimnuss

In the wekan#133 issue you mentioned is already some code linked that could fix this, so it only requires someone with time to get that fix as pull request to Wefork.

quimnuss commented 7 years ago

The patch is already on the kadira-flow-router since meteor uses v2.12.1 atm and it was introduced in v2.7.1

I don't know why I am having issues, but it is probably related.

quimnuss commented 7 years ago

The fix was introduced on https://github.com/wekan/wekan/commit/a78debc461944f55de246db15a1dd29353dec4ae but I don't see the changes in wefork, so I presume wefork was forked from a precedent commit? I'll check it out.

quimnuss commented 7 years ago

Sorry, i was wrong, the changes are there.

Serubin commented 7 years ago

This seems to be an issue present on nginx as well. It seems that the card links end up creating are incorrect. I think it's an issue with flow-router, which doesn't seem to be maintained any longer.

xet7 commented 7 years ago

@Serubin

Did you use Nginx settings from: https://github.com/wefork/wekan/wiki/Install-from-source

When I tried it previously it did work, if I remember correctly.

Serubin commented 7 years ago

@xet7 I tried using that standalone and it did not work. I'm using a custom config that is functionally the same. I am using the prefix /board though. I'm not sure if wekan is limited to /wekan?

I've pulled this from flowrouter: I went through and added some logging to the FlowRouter.url() function and found this output:

  var completePath = this.path.apply(this, arguments);
  // /board/b
  var basePath = this._basePath || '/';
  // /board
  var pathWithoutBase = completePath.replace(new RegExp('^' + basePath), '');
  // /b
  return Meteor.absoluteUrl(pathWithoutBase);
  // https://mydomain.net/board//b

The router should definitely be taking out the leading slash. Also, all the hrefs on the card details include that second slash. I'm thinking this is an issue with the router and not web config. Remove the slash manually and everything works fine.

This is my config - somewhat redacted if it's any help.

upstream gitlab-workhorse {
  server unix:/home/git/gitlab/tmp/sockets/gitlab-workhorse.socket fail_timeout=0;
}

# Wekan Stuff
upstream wekan_local {
  server 127.0.0.1:8123;
  keepalive 8;
}

# this section is needed to proxy web-socket connections
map $http_upgrade $connection_upgrade {
    default upgrade;
    ''      close;
}

## Redirects all HTTP traffic to the HTTPS host
server {
  ## Either remove "" from the listen line below,
  ## or delete the /etc/nginx/sites-enabled/default file. This will cause gitlab
  ## to be served if you visit any address that your server responds to, eg.
  ## the ip address of the server (http://x.x.x.x/)
  listen 0.0.0.0:80;
  listen [::]:80 ipv6only=on ;
  server_name example-url; ## Replace this with something like gitlab.example.com
  server_tokens off; ## Don't show the nginx version number, a security best practice
  location / {
    return 301 https://$http_host$request_uri;
  }

  access_log  /var/log/nginx/gitlab_access.log;
  error_log   /var/log/nginx/gitlab_error.log;

}

## HTTPS host
server {
  listen 0.0.0.0:443 ssl;
  listen [::]:443 ipv6only=on ssl ;
  server_name example-url; ## Replace this with something like gitlab.example.com
  server_tokens off; ## Don't show the nginx version number, a security best practice

  ## Strong SSL Security - REMOVED IN EXAMPLE
  ## https://raymii.org/s/tutorials/Strong_SSL_Security_On_nginx.html & https://cipherli.st/

  ## Individual nginx logs for this GitLab vhost
  access_log  /var/log/nginx/gitlab_access.log;
  error_log   /var/log/nginx/gitlab_error.log;

   merge_slashes on;

   location / {
    ## SNIPPED - Gitlab root
  }

 # Wekan config
 location /board {
    proxy_read_timeout      300;
    proxy_connect_timeout   300;
    proxy_redirect          off;

    proxy_http_version 1.1;

    proxy_set_header    X-Real-IP           $remote_addr;
    proxy_set_header    X-Forwarded-Ssl     on;
    proxy_set_header    X-Forwarded-For     $proxy_add_x_forwarded_for;
    proxy_set_header    X-Forwarded-Proto   $scheme;
    proxy_set_header Upgrade $http_upgrade; # allow websockets
    proxy_set_header Connection $connection_upgrade;
    proxy_pass http://wekan_local;
      # the root path (/) MUST NOT be cached
      if ($uri != '/board') {
            expires 30d;
      }  

  }

  error_page 404 /404.html;
  error_page 422 /422.html;
  error_page 500 /500.html;
  error_page 502 /502.html;
  location ~ ^/(404|422|500|502)\.html$ {
    root /home/git/gitlab/public;
    internal;
  }
}
xet7 commented 7 years ago

@Serubin

If you run wekan as service, then in your: /etc/systemd/system/wekan@.service file is this line, that needs to include correct address of wekan: Environment=ROOT_URL=https://example.com/board

Serubin commented 7 years ago

@xet7 I made a simple startup script that definitely covers this You can see in the example above that flowrouter is respecting the prefix, but it's adding a lead slash to it's url as if it's a base url.

cd /var/www/wekan/.build/bundle/
export PORT=8123
export HTTP_FORWARDED_COUNT=1
export MONGO_URL=mongodb://127.0.0.1:27017/admin
export ROOT_URL=https://myurl.com/board
export MAIL_URL='smtp://me@smtp.google.com:25/'

node main.js

cd -
xet7 commented 7 years ago

@Serubin

I can reproduce this bug on Nginx and Caddy webservers.

xet7 commented 7 years ago

I think this pull request is about same issue: https://github.com/kadirahq/flow-router/pull/691

I'll try to figure out how to use it.

Serubin commented 7 years ago

@xet7 Do you think it's due to FlowRouter or something within wekan? It seems like FlowRouter to me. It looks like they've discontinued development on it.

xet7 commented 7 years ago

@Serubin

Yes, that pull request is fix for bug in FlowRouter. Easiest way would be to try to use that fix.

There was update to FlowRouter git repo 13 days ago, so it does not look like discontinued. https://github.com/kadirahq/flow-router

Serubin commented 7 years ago

@xet7 Seems like the actual code hasn't been changed in about a month. The latest update was to the readme. Nothing has been merged in in almost a year.

Correct me if I'm wrong - I haven't used meteor much - but in order to use their fix they would have to merge the PR. I did try to fork my own version and fix it (which included uploading it to the meteor package site) but it seems another package that wekan uses was still pulling in the original FlowRouter and overriding my version.

Dochead was the culprit I think.

xet7 commented 7 years ago

@Serubin

Yes, fixed FlowRouter fork needs to be used in all places of Wekan.

jaimeagudo commented 7 years ago

waiting for your updates guys, https://github.com/wekan/wekan/issues/423 I wish I could help with meteor

jLouzado commented 7 years ago

Yup, seems to only be a problem when using wekan as a service.

Why is ROOT_URL in wekan@.service = http://localhost/wekan and in run-wefork.sh it's http://localhost:3000 ?

Serubin commented 7 years ago

@jLouzado It's a problem with wekan in general.

jLouzado commented 7 years ago

Sorry so do we know how to fix this yet? I'm asking because Wekan as such is pretty unusable till this gets sorted out yes?

xet7 commented 7 years ago

No.

@Serubin tried to get that FlowRouter fix https://github.com/kadirahq/flow-router/pull/691 merged, with process like forking multiple other repos that would then be hosted under Wekan GitHub organization, but was not able to make Wekan build process working correctly.

@shtefcs can you help ?

Serubin commented 7 years ago

I will post additional details shortly. I got extremely busy last week and wasn't able to work on the project further

On Mon, Feb 6, 2017, 03:46 Lauri Ojansivu notifications@github.com wrote:

No.

@Serubin https://github.com/Serubin tried to get that FlowRouter fix kadirahq/flow-router#691 https://github.com/kadirahq/flow-router/pull/691 merged, with process like forking multiple other repos that would then be hosted under Wekan GitHub organization, but was not able to make Wekan build process working correctly.

@shtefcs https://github.com/shtefcs can you help ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/wefork/wekan/issues/49#issuecomment-277617092, or mute the thread https://github.com/notifications/unsubscribe-auth/ABLWIQSGXNRuCizo9g0TjZpm-_hb-Bmxks5rZt3LgaJpZM4LOPpr .

xet7 commented 7 years ago

Moving this issue to https://github.com/wekan/wekan/issues/785 , please continue discussion there.