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

improv. auth perf #262

Merged
merged 5 commits into from
Jan 3, 2021
Merged

improv. auth perf #262

merged 5 commits into from
Jan 3, 2021

Conversation

Arkadax
Copy link
Contributor

@Arkadax Arkadax commented Dec 31, 2020

Bonjour
J'ai amélioré les performances lors de l'inscription des users en enlevant les hash inutiles comme le deuxieme hash a l’inscription et le hash si un utilisateur n'existe pas. J'ai aussi diminué le nombre de requêtes mysql
J'ai tout testé plusieurs fois et tout marche correctement
Je m'excuse pour le formattage comme il est pas consistent sur tout les fichiers et il y a beaucoup de lignes très longues, j'ai pas pu configurer mon éditeur.
J'ai fait ces modifications il y a plusieurs mois pour mon serveur car on avait constaté que cette partie utilisait beaucoup le serveur lors de tests de charges. Comme MineWeb redevient actif je me suis dis que c'est mieux d'en faire profiter tout le monde plutot que de garder ca pour moi et le serveur a fermé alors ca me sert a rien
J'en ai profité pour réparer le sha386 qui n'existe pas et qui fait que tous les mots de passes sont vides quand il est utilisé ce qui est très dangereux car tout le monde peut se login sur tous les comptes !!!!!
Je dev énormément avec symfony mais je ne fais jamais de contribution sur github, si j'ai fait des erreurs dans cette proposition dites moi car je voudrais après faire d'autres améliorations 😉
Bonne soirée

Copy link
Member

@nivcoo nivcoo left a comment

Choose a reason for hiding this comment

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

Je valide pour le double HASH, ensuite pour les crochets ajoutés etc je suis pas trop pour mais bon, et enfin pour l'ajout d'un paramètre dans le login pour pas faire 2 appeles pourquoi pas mais a voir si le plugin auth utilise pas directement le model pour se co

app/Controller/UserController.php Outdated Show resolved Hide resolved
@@ -79,7 +79,7 @@ public function register($data, $UtilComponent)
return $this->getLastInsertId();
}

public function login($data, $confirmEmailIsNeeded = false, $checkUUID = false, $controller)
public function login($user, $data, $confirmEmailIsNeeded = false, $checkUUID = false, $controller)
Copy link
Member

Choose a reason for hiding this comment

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

Il me semble que les requêtes depuis un model sont plus rapides donc au lieu de faire passer $user, la requête était bien dedans (Je comprends pour réduire le nombre de requêtes mais j'ai un doute si le plugin Auth utilise pas directement cette fonctionne login sans $user)

app/Controller/AppController.php Outdated Show resolved Hide resolved
@nivcoo
Copy link
Member

nivcoo commented Dec 31, 2020

Et ensuite pour le sha, bien vu

@nivcoo
Copy link
Member

nivcoo commented Dec 31, 2020

Je viens de regarder pour le pl Auth, il utilise pas donc c'est OK pour ça mais faudrait changer les fichiers que tu n'utilise pas et corrigé le problème de cpatcha et ce seront bon

@@ -36,10 +36,10 @@
class AppController extends Controller
{

var $components = array('Util', 'Module', 'Session', 'Cookie', 'Security', 'EyPlugin', 'Lang', 'Theme', 'History', 'Statistics', 'Permissions', 'Update', 'Server');
var $helpers = array('Session');
Copy link
Member

Choose a reason for hiding this comment

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

?

Copy link
Contributor Author

@Arkadax Arkadax Dec 31, 2020

Choose a reason for hiding this comment

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

Déclarer les variables avec var est déprécié j'ai mis public qui fait la même chose

Copy link
Member

Choose a reason for hiding this comment

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

Okay

@Arkadax
Copy link
Contributor Author

Arkadax commented Dec 31, 2020

C'est good j'ai fait les changements

@nivcoo
Copy link
Member

nivcoo commented Dec 31, 2020

C'est good pour moi je pense

@Eywek Eywek merged commit f537925 into MineWeb:development Jan 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants