ynh-vpnclient-loadcubefile.sh script isn't working anymore

[SohKa]

Hello !

I did several attempts to install a cube from scratch without success. At each attempt, the cube's installation ended up with a non-functional vpnclient application. The script is failing at the step 15-configure_vpnclient.log :

809  loading actions map namespace 'yunohost'
961  extra parameter classes loaded: ['comment', 'ask', 'password', 'required', 'pattern']
962  initializing base actions map parser for cli
970  registering new callback action 'yunohost.utils.packages.ynh_packages_version' to ['-v', '--version']
1676 'yunohost app addaccess' is deprecated and will be removed in the future
1681 acquiring lock...
1845 lock has been acquired
2056 loading python module yunohost.app took 0.209s
2057 processing action [18563.1]: yunohost.app.addaccess with args={'apps': ['vpnclient'], 'users': ['myusername']}
2065 /!\ Packagers ! This app is using the legacy permission system. Please use the new helpers ynh_permission_{create,url,update,delete} and the 'visitors' group to manage permissions.
2245 initialize authenticator 'as-root' with: uri='ldapi://%2Fvar%2Frun%2Fslapd%2Fldapi', base_dn='dc=yunohost,dc=org', user_rdn='gidNumber=0+uidNumber=0,cn=peercred,cn=external,cn=auth'
2299 The permission database has been resynchronized
4178 SSOwat configuration generated
4252 No default hook for action 'post_app_addaccess' in /usr/share/yunohost/hooks/
4255 No custom hook for action 'post_app_addaccess' in /etc/yunohost/hooks.d/
4257 No default hook for action 'post_app_removeaccess' in /usr/share/yunohost/hooks/
4258 No custom hook for action 'post_app_removeaccess' in /etc/yunohost/hooks.d/
4261 Permission 'vpnclient.main' updated
4281 To view the log of the operation 'Update accesses for permission 'vpnclient'', use the command 'yunohost log display 20200402-121901-user_permission_update-vpnclient'
4303 action [18563.1] executed in 2.244s
4304 lock has been released
allowed_users: 
  vpnclient: myusername
853  loading actions map namespace 'yunohost'
1002 extra parameter classes loaded: ['comment', 'ask', 'password', 'required', 'pattern']
1003 initializing base actions map parser for cli
1011 registering new callback action 'yunohost.utils.packages.ynh_packages_version' to ['-v', '--version']
1655 acquiring lock...
1866 lock has been acquired
2077 loading python module yunohost.app took 0.209s
2078 processing action [18581.1]: yunohost.app.setting with args={'app': 'vpnclient', 'delete': False, 'value': '1', 'key': 'service_enabled'}
2134 action [18581.1] executed in 0.054s
2135 lock has been released
[ERROR] SSO login failed

The reason is, since this change has been merged in the SSOwat repository, the grep command at this line can't find the Logout string any more (although the curl command is successful). Thus the grep command gives the return code 1 and this check echoes the message [ERROR] SSO login failed and exits the script (so the upload of the cube file and the configuration of the IPv6 prefix are skipped).

We should maybe find a better way to check that the curl command succeeded than grepping the output. As it juts happened, the content of the web page can change again.

I'll think of a solution, but I'm not really sure on what to rely to do this check.

In consequence, I think it is not possible any more to install cubes without manually modifying the code.

[SohKa]

Just an idea :

What do we want exactly to achieve with this code ? To get the session cookie.

So we can maybe just test if the file ${tmpdir}/cookies doesn't exist or is empty. If this is the case, we echo the error message and exit from the script (with 1 as return code).

Something like this :

# make sure the cookies file doesn't exit to not get a false positive at the test
rm -f ${tmpdir}/cookies || true

# the curl command is the same as in the original code beside the additional -s and -o /dev/null parameters
curl -s -o /dev/null -kLe "https://${ynh_domain}/yunohost/sso/" --data-urlencode "user=${ynh_user}" --data-urlencode "password=${ynh_password}" "https://${ynh_domain}/yunohost/sso/" --resolve "${ynh_domain}:443:127.0.0.1" -c "${tmpdir}/cookies"

# test if the curl command failed, if the file doesn't exist or if the file is empty
if [ $? -ne 0 ] || [ ! -f "${tmpdir}/cookies" ] || [ ! -s "${tmpdir}/cookies" ]; then
  echo "[ERROR] SSO login failed" >&2
  exit 1
fi

Is this test robust enough ?

[alexAubin]

Didn't dig deep to check, but naively that looks kinda legit ? Did you test it ?

[SohKa]

Not with a full install. I'm going to launch one with this patch. I'll keep you updated this evening about the result.

[alexAubin]

Cheers

Don't hesitate to make a pull request and harass us, given that it breaks the install and reload of cube file, we should try to fix it asap ...

ping @pitchum @keomabrun

[SohKa]

Ok, will do ! If the test works, I'll create the PR right away

[pitchum]

What do we want exactly to achieve with this code ? To get the session cookie.

More precisely, the goal is probably to make sure that the authentication was successful.

A better check would be to look for set-cookie: SSOwAuthUser= header in the response with "curl -i" or to look for this inside the generated cookie file.
After a successful authentication the cookie file looks like this:

#HttpOnly_.domain.net    TRUE    /       TRUE    1586466033      SSOwAuthExpire  1586458833.604
#HttpOnly_.domain.net    TRUE    /       TRUE    1586466033      SSOwAuthHash    blahblah
#HttpOnly_.domain.net    TRUE    /       TRUE    1586466033      SSOwAuthUser    alice

If you have time to prepare a PR @SohKa it would be appreciated. Otherwise I'll do it next week-end.

[SohKa]

You're right. I've been a bit paranoid as I was thinking that the cookie variable name SSOwAuthUser could also be changed in the future. I suppose it's safe to assume it won't be the case (I don't see any reason why).

Instead of curl -i, I would use curl -D - to only get the http-header (and not the whole web page).

The code would look like this :

curl -D - -skLe "https://${ynh_domain}/yunohost/sso/" --data-urlencode "user=${ynh_user}" --data-urlencode "password=${ynh_password}" "https://${ynh_domain}/yunohost/sso/" --resolve "${ynh_domain}:443:127.0.0.1" -o /dev/null -c "${tmpdir}/cookies" 2> /dev/null | grep -q "set-cookie: SSOwAuthUser=${ynh_user}"

if [ $? -ne 0 ]; then
  echo "[ERROR] SSO login failed" >&2
  exit 1
fi

So basically, it's the same logic as the original one.

I'll redo a full reinstallation for testing this.

Edit : I've added the username to the grep command
Edit2 : My microsd card seems to be dead... I'll finish the test and the PR tomorrow.