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

Develop NOT for production #26

Closed
wants to merge 4 commits into from
Closed

Conversation

Shnoulle
Copy link
Contributor

There are some difference between same domain and other domain

  • Other domain : webserver are broken : because SSOWat don't redirect to the good domain (this domain don't have /yunohost currently) ynh bug or this install bug ?
  • Then activate LDAP for user 1 seems OK , but Other domain : unable to use primary user #25 broken (LS bug ? mis-feature ?)

=> Currently user 1 must not be a ynh user : solution create a fake one : done here

Shnoulle and others added 3 commits February 18, 2017 20:10
- Fix YunoHost-Apps#17
- Here get a specific commit (specific command action for plugin)
[fix] partially YunoHost-Apps#25 using an alt user for super-super-admin
[dev] Add a activatePlugin (to manage ls via a ynh plugin (when i done it)
[dev] Little fix on installer (warning when testing files in patch directory)
[dev] Using LimeSurvey install command for installing
@@ -56,7 +56,12 @@ return array(
// LimeSurvey developers: Set this to 2 to additionally display STRICT PHP error messages and get full access to standard templates
'debug'=>0,
'debugsql'=>0, // Set this to 1 to enanble sql logging, only active when debug = 2
'enableLdap'=>true
/* yunohost part */
'auth_webserver_user_map' => array( '{{ admin }}'=>'ynh_admin'),// Primary user as ynh admin
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

admin choosen when installing : super-super admin via webserver only #25

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to call this user ynh_admin (and not {{ admin }} ) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do some test : if you remove user access (1st user) from LS : then you don't have a user who can set super-admin ...

The issue with LS is

  1. Only the 1st super-admin (userid==1) can set other super-admin
  2. The 1st super-admin (userid==1) can always log by DBAuth
  3. The 1st super admin can not be removed (from LS)
  4. The 1st super-admin have always all the rights on all ...

Then : if you set 'ynh-user1' to super-admin at start, play a little, via yunohots : remove this admin, set other admin : the new admin don't are really a real super-admin ....

It's really the point where i didn't know how it's best to fix ....

'enableLdap'=>true
/* yunohost part */
'auth_webserver_user_map' => array( '{{ admin }}'=>'ynh_admin'),// Primary user as ynh admin
'auth_webserver_autocreate_user'=>1,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix #24

(2,'AuditLog',0),
(3,'oldUrlCompat',0),
(4,'ExportR',0),
(5,'Authwebserver',1),
(6,'extendedStartPage',0),
(7,'ExportSTATAxml',0),
(8,'QuickMenu',0),
(9,'AuthLDAP',0);
(9,'AuthLDAP',1);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix 'another domain' or AuthWebserver not default with another domain (allow ldap)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I need to test. If I remember, with AuthLDAP the user is obliged to login twice no ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but there are some issue with Auth webserver if LS are not a subdirectory of ynh primary server. Last time i test it ....

@@ -2,7 +2,7 @@
"name": "LimeSurvey",
"id": "limesurvey",
"packaging_format": 1,
"version": "2.62.2",
"version": "2.62.2.1",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wait for 2.62.3 for final release : false version

update

---
application/commands/ActivatePluginCommand.php | 70 ++++++++++++++++++++++++++
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To move DB direct access system to ls system (with a new ynh plugin (i work on it). Think we can manage really more easily ls via ynh after.

sudo mkdir -p "$DEST"
sudo chown $AS_USER: "$DEST"
sudo chown $AS_USER: "$DEST"
if [ "$(echo ${SOURCE_FILE##*.})" == "gz" ]; then
ynh_exec_as "$AS_USER" tar xf $SOURCE_FILE -C "$DEST" --strip-components 1
elif [ "$(echo ${SOURCE_FILE##*.})" == "bz2" ]; then
ynh_exec_as "$AS_USER" tar xjf $SOURCE_FILE -C "$DEST" --strip-components 1
elif [ "$(echo ${SOURCE_FILE##*.})" == "zip" ]; then
mkdir -p "/tmp/$SOURCE_FILE"
ynh_exec_as "$AS_USER" unzip -q $SOURCE_FILE -d "/tmp/$SOURCE_FILE"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I try with zip : but seems there are an issue here : user don't have the right on /tmp/$SOURCE_FILE not ls related and not really needed

# Apply patches
if [ -f ${PKG_DIR}/patches/$SOURCE_ID-*.patch ]; then
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before : have an error shown here, now allow multiple patch file without bad sentence in command

@Shnoulle
Copy link
Contributor Author

Shnoulle commented Apr 1, 2017

@zamentur que pense tu des modifications ici ? Est ce que je continu dans ce sens sur l'utilisateur numéro 1 ou bien est ce que je revient sur le principe que l'utilisateur 1 reste celui de ynh.

En fait : ici c'est la mise à jour que j'aurais plus de mal à gérer.

Denis

Copy link
Contributor

@zamentur zamentur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this pr is cool !
The upgrade script is missing, but we can wrote it later.
I need to test the LDAP part.

# Set permissions
ynh_set_default_perm $local_path
sudo chmod -R u+w $local_path/tmp
sudo chmod -R u+w $local_path/upload
sudo chmod -R u+w $local_path/application/config/
#~ sudo chmod -R u+w $local_path/application/config/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without write options the ynh_admin can't change the configuration of LS.
What was the idea to remove this ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oups, unsure , maybe a bad removing during testing ... but have another idea here : allow to load user-config from another file (in user directory). Must be reviewed.


sudo yunohost app addaccess $app -u $admin
ynh_sso_access "/index.php?r=admin,/index.php?r=plugins,/scripts"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember why I have put a protection on /index.php?r=plugins,/scripts
May be it's historical (with the last version of the package)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/script : really not needed, about plugins : specific : ls have issue here (plugins/index) but some plugin must allow public access to plugins/

(2,'AuditLog',0),
(3,'oldUrlCompat',0),
(4,'ExportR',0),
(5,'Authwebserver',1),
(6,'extendedStartPage',0),
(7,'ExportSTATAxml',0),
(8,'QuickMenu',0),
(9,'AuthLDAP',0);
(9,'AuthLDAP',1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I need to test. If I remember, with AuthLDAP the user is obliged to login twice no ?

@zamentur
Copy link
Contributor

zamentur commented Apr 1, 2017

tested, works well.
Configuration seems to be updatable.

About ynh_admin, I don't know I am not sure to understand why it's needed, but you have a better knowledge on LS than me.
I think it could be strange for a user to connect with his username and see "ynh_admin" as the user connected !

@Shnoulle
Copy link
Contributor Author

Shnoulle commented Apr 1, 2017

Yes : ynh_admin are really the issue here .....

2 options :

  1. The first admin set :
    • disallow remove rights via ynh (must be fixed)
  2. Use a false admin (like here)
    • can be updated in ynh part.

It's the choice, i can do 2, but not sure for 1 (know less ynh than LS ).

And see : #25 the super-super-admin can not come from LDAP .....

@Shnoulle
Copy link
Contributor Author

PS : i think i improve LS to allow multiple super-super-admin ;)

@Shnoulle
Copy link
Contributor Author

Shnoulle commented Jun 3, 2017

I think it could be strange for a user to connect with his username and see "ynh_admin" as the user connected !

After more thinking : you're right . Best seems to have a dedicated user for 1st admin (else you can break system if you remove the rights of the first user (tested with old ynh).

I review next week, update to last version, make a multiple download system for other needed plugins .

@Shnoulle
Copy link
Contributor Author

Must review all :(

@Shnoulle Shnoulle closed this Jun 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants