Authentication and security improvements

[jvdoorn]

(node:67643) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'Id' of undefined
    at retrieveAccount (/Users/julian/Delta/node_modules/magister.js/lib/magister.js:559:38)
    at process._tickCallback (internal/process/next_tick.js:68:7)
(node:67643) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 2)

Occasionally an error is thrown when authenticating, after refreshing a couple times it will go away. It should probably be looked into though.

[keesvv]

Ik heb hier even kort naar gekeken: als je probeert een verkeerde gebruikersnaam of een verkeerd wachtwoord op te geven, dit is de error die je dan krijgt:

(node:7659) UnhandledPromiseRejectionWarning: AuthError: Invalid username
    at Magister.login (/home/kees/Projects/Electron/Delta/node_modules/magister.js/lib/magister.js:600:13)
    at process._tickCallback (internal/process/next_tick.js:68:7)
(node:7659) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)

Als je goed kijkt zie je dat jouw snippet een rejection id: 2 heeft en die van mij een rejection id: 1.

[keesvv]

Ik denk dat we hier try-catch blocks voor moeten maken, daar ga ik morgen wel naar kijken.

[jvdoorn]

Ik ben er op het moment mee bezig dat wachtwoorden niet meer worden opgeslagen (in plain text). Zie deze commit. Het idee is om wellicht the token op te slaan en die te gebruiken voor authenticatie (als je wilt dat je wordt onthouden). Ik weet echter niet of dat gaat lukken.

[keesvv]

Het is altijd het proberen waard! 😄 Als MagisterJS het inloggen en genereren van een token ondersteunt, dan zou dat geweldig zijn, omdat tokens niet snel kunnen worden gereversed naar het originele password.

[jvdoorn]

Ik ga kijken wat de conclusie van deze issue wordt. Mocht de token alleen maar voor korte tijd bruikbaar zijn kunnen we hem gebruiken voor logins kort na de eerste login.

In mijn branch heb ik het nu zo gedaan dat alleen je gebruikersnaam en wachtwoord worden opgeslagen. Het lijkt mij een kleine moeite als je bij iedere login je wachtwoord in moet voeren, zeker als dit de veiligheid verbeterd.

[lieuwex]

FWIW (geen idee of het uberhaupt van nut is) bij simplyHomework heb ik het vroeger zo gedaan dat hij eerst de laatste token probeerde, en als dat mislukte het opnieuw probeerde met een wachtwoord/gebruikersnaam combo. Maar dit was meer vanwege optimalisatie redenen dan beveiliging (inloggen kost toch weer wat requests, en simplyHomework heeft geen concept van ingelogd of niet, het werkte op een wat hoger niveau dan dat)
https://github.com/simplyGits/simplyHomework/blob/develop/packages/magister-binding/magister-binding.js#L190

[keesvv]

Hoi @lieuwex , bedoel je dat je eerst een token probeert en als dat niet werkt, dat je dan de gebruiker opnieuw om zijn credentials vraagt?

[keesvv]

Hoi @argetan ! Ik heb zojuist je credentials-storage branch bekeken, en het ziet er goed uit! Voor mijn gevoel lijkt de authenticatie zelfs iets soepeler te lopen. Ik ga hem mergen als je het goed vindt, en dan sluit ik deze issue!