youmy001 / apine-framework Goto Github PK

The latest changes to the authentication that allow old and new passwords does not take into account that we may log in with our email address. Line 233 currently reads

if ((ApineUserFactory::is_name_exist($user_name) || ApineUserFactory::is_email_exist($user_name)) && ApineUserFactory::create_by_name($user_name)->get_register_date() < "2015-09-04") {

and should probably read something in those lines:

if ((ApineUserFactory::is_name_exist($user_name) && ApineUserFactory::create_by_name($user_name)->get_register_date() < "2015-09-04") || (ApineUserFactory::is_email_exist($user_name) && ApineUserFactory::create_by_email_address($user_name)->get_register_date() < "2015-09-04")) {

In the class ApineUser, the method "get_register_date" refers to a non-existant constant, APP_DATE_FORMAT. Data format should be defined in the config.

The method array_diff_assoc on line 352 in lib/core/Liste.php throws a notice when at least one of the two compared objects contain an array. That is because array_diff_assoc then treats the object as a multidimentional array, which it does not support. It then gives the following:

Notice: Array to string conversion in lib/core/Liste.php at line 352

A possible fix would be to serialize the objects before passing them into an array_diff, then unserializing the result, like this:

$diff = array_map('unserialize', array_diff(array_map('serialize', $pageids), array_map('serialize', $parentpage)));

The default user is defined to be of type 70 in the database, but this type does not exist in the code. The codified type for a default user is 65, and the type for an admin is 77. Because of that, a newly created user is assigned a type that does not correspond to anything, therefore can't be used.

The constructor of lib/file/File.php uses the method fread without being enclosed in a try/catch, which means whenever it tries to read a file that doesn't exist, it crashes the whole script.

The installer add the reference to a directory "resources/language" for the locales which does not exists. It should instead add "resources/languages" as the default locales directory.

The user authentication could use some customization outside of the "ApineUser" class. For example, a website administrator may want to authenticate users using the email insteal of the username, or even use both.

The actual groups are very useful for permission management, but it would be better to integrate it as a list of groups, to use more complex permissions. The method "get_group" could be renamed as "get_groups" and return a "Liste" of groups, and "set_group" could become "add_group".

When I call ApineUser::has_group() it always returns false even if users have a group assigned by default during registration. Currently the only way to fix that is to call ApineUser::get_group() beforehand.

Many modules in the framework currently implies the request come from the default http and https ports. When testing with custom ports the application will fail to generate valid links and return 404 response.

Plausible addition point would be inside Request and URLHelper classes.

In "alone" installation, a password is written in [] even if no password is written.

ApineFileImage does not follow the same structure as ApineFile, and the functions used in ApineFileImage do not work anymore with its parent. Therefore, the class should use a rewrite in my opinion.

In the class ApineUserFactory, the method "create_by_group" contains a bad SQL query. Normally, "SELECT ID from apine_users where group=$group" would work. In this case, the keyword "group" in reserved in MySQL and is therefore not considered a column name, but that specific keyword and thus the query fails. The query should be replaced by "SELECT ID from apine_userswheregroup=$group".