vl222cu / 1DV450_vl222cu_RegistrationApp

0 stars 0 forks source link

Kommentar #3

Closed 1dv450 closed 9 years ago

1dv450 commented 9 years ago

Hejsan!

Din ena peer gav bra feedback på ett farligt säkerhetshål du bör fixa till. Kolla över hur du satt dina before_action så du skyddar de metoder du behöver. Du får inte skydda de som man måste kunna komma åt som oinloggad men vissa är helt öppna och bör säkras upp med kontroll.

routingen kan diskuteras lite. Jag kan ha lite svårt med att ha GET-förfrågningar kopplade mot deletemetoder. En GET på user/:id skulle jag naturligt tro ger mig informationen om en användare. Men det kommer vi ju jobba mer med i och med API-delen där vi försöker designa vårt API enligt REST-principerna.

Annars ser det bra ut tycker. Man känner ju igen lite kod från bok och föreläsningar ;) Som sagt blir det lite DRY med en User och en Admin men det kan jag ändå köpa och kan i vissa fall vara en uppdelning man vill ha. Visst skulle man kunna göra lite refactoring och samla liknande kod i en gemensam del iofs.

vl222cu commented 9 years ago

Hej,

Ska kika över mina before_action inom kort och ordna till säkerhetshålet :) Du har absolut rätt ang GET mot deletemetoder, ska ju vara POST nu när jag tänker efter som vi fått lära oss från tidigare kurser med Mats bl a :) Kan kika på det också :) Ja, min kod kan förbättras på många håll nu när jag har fått gå igenom både Rasmus och Martins appar så får ta med mig det till framtida RoR-kodande :) Ska jag skicka bekräftelse här när jag känner att jag är klar med det jag behöver fixa till?

1dv450 commented 9 years ago

:+1: Tja eller till och med DELETE om vi tänker restful...Det räcker med att du stänger denna issue så ser jag det i mitt flöde

vl222cu commented 9 years ago

OK, låter bra, jobbar på det nu! :+1: :)