I just can't get other how awful this code is.

It's like every worst pratice possible in one place.


And it get worst and worst everytime you reread it 😐

Everything is a pure disaster.

> If you had to name THE most horrible part of this code, what would it be? 👀😑


@bram @DarckCrystale je vous assure que j’ai vu des CTO faire du code comme ça en prod (directement sur la prod d’ailleurs), et littéralement gueuler sur les devs qui osaient dire quoi que ce soit

@bram I'm not a JavaScript guy really, but am I correct in reading there that they used an incredibly awkward "if" statement that's always true to return failure instead of just an "else"?

@bram see, that's the horrifying one to me. The other bits are mainly someone not thinking things through or considering scale.

That statement indicates that this person has just completely missed the basic fundamental concepts of what they're doing.

@mike you've probably missed it since you aren't familiar with Javascript but here : the whole content of the user table (and probably the whole database) is accessible to every web browser that load this page. With all the password being stored in plain text.

Also you can probably execute whatever sql you want so it's a free for all database 😐

That the worst part for me. By far.

@bram oh wow yeah - I got to the point where I realised I could disable JS and create my own "logged in" cookie and that the password db had to be plaintext ... But yeah haha there really are more levels to the awful every glance.

@mrjack @bram This is a SQL query. In this case you get the content of all rows in the table "users"

@owp @bram I know, i just tried to reply to the answer :)

@athoune @bram what about leaking **all** users and their passwords to the client? And allowing to execute SQL queries?

@bram l'API qui accepte juste du SQL, quel carnage.

Même le mot de passe en clair c'est quedalle à côté.

@milia ouais, c'est juste la db free for all à *tout le monde*.

Je sais même pas comment on peut envisager ça.

@milia @bram un stagiaire/alternant total newbie qui se fait sa première XP dessus et un chef de projet/encadrant qui s'en fout royalement de vérifier comment c'est fait du moment que ça fonctionne ;)

@bram et après un second coup d’oeil je viens de remarquer que l'appel à l'API est synchrone aussi ...

Il y a genre tout qui va mal dans à peine 40 lignes tss.

Ah non, my bad : l'indentation est respecté. C'est déjà ça.

@bram La pire partie étant le commentaire initial "Put this in a different file" (avec le gros sous-entendu : incluons le fichier dans la page via un <script> qui ne résoudrait strictement rien au problème...

Le reste me fait saigner des yeux à chaque lecture...

@Soubi je t'avoue que j'ai un peu plus de mal avec le fait que la db (ou au moins la table user, j'imagine même pas que ça soit une views) soit exposée en free for all à tous les navigateurs.

Avec les mots de passe en clair dedans.

@bram moi, regardant rapidement : bouah, c'est pas si mal.
Wait c'est quoi ce “if ("true" === "true")” ?
Bizarre ce “else if” avec la condition précédente inversée (enfin, c'est pas tout à fait la condition opposée mais bon).
*relis authenticateUser* oh god

the best part is the todo on top.
someone has their priorities sorted out.

- selecting the whole table then looping through it
- with plain text passwords
- via an interface which takes SQL queries as the input
- Using prop on the cookie to store login status
- that "if" statement...
- and most heinous of all - inconsistent quotes for strings 😋


I'm very confused about the if "true"==="true" part??

@bram I don't particularly know javascript and even I can tell that's bad.

@bram “<script>”

It starts awful and just keeps giving more awful

@bram Sometimes you struggle with imposter syndrome. And sometimes you find stuff like this.

I might add unnecessarily complex code, I might do security-check client-side, I might store user credentials in unencrypted plaintext, I might expose of all that data to any random visitor, and I might even expose a free sql access to any client, but at least I know that $.click() is deprecated in favour of $.on().

I feel like a real trustworthy professional now 🕶

@bram L’API qui accepte n’importe quel SQL. Tous les utilisateurs/mdp chargés dans le navigateur. Les mots de passe en clair. Le "true" === "true" return false. Cette boucle digne d’un code C. Le cookie de connexion.

how is it even possible to be so wrong on so many levels

@bram For my money the worst part is that the slapdash way it does everything makes it weirdly hard to spot just how terrible a thing it's doing in that slapdash way.

@bram La requête SQL en javascript, je dis 👌

@bram le apiService.sql() pour moi, deja parce que c'est au debut et c'esmt assez hilarant. Trust the front side, we have cookies.

Bah, au moins il met les ";" partout où il faut 🤷

@bram It's a joke ! The SQL is inside the web Browser ! No hash password ! I don't care of best practices when the security sucks.

@merwin I tend to include security in the best practice ^^'

And my god this is so awful yes :|

Amazing. It's like some horrific poem, each time I read it I find another layer of shit.

@bram That has to be a joke, there is no way someone that stupid got hired to make an intranet.

The worst part of this code is that you can literally get the login data of every user on the intranet with 0 effort from your browser and then login as whoever you want.

@robots people have sadly come up and told me they've seen similar things in the field :[

Sign in to participate in the conversation

This is a mastodon instance for social justice activists, LGBTQIA+ people, and activists in general See the Goals and technical details, and Rules and privacy policy pages for more information