search

esek / ekorre

🐿 E-hemsidans backend i node skriven med Typescript, GraphQL via Apollo och Prisma
https://api.esek.se
GNU Affero General Public License v3.0
6 stars 2 forks source link

Separera refresh tokens som en HTTP Only cookie för att minska risken för XSS - [merged] #62

Closed esek-macapar closed 2 years ago

esek-macapar commented 3 years ago

In GitLab by @afroborg on Aug 17, 2021, 22:38

Merges fix/refresh-token -> master

esek-macapar commented 3 years ago

In GitLab by @afroborg on Aug 17, 2021, 22:38

requested review from @blennster

esek-macapar commented 3 years ago

In GitLab by @afroborg on Aug 17, 2021, 22:45

added 1 commit

Compare with previous version

esek-macapar commented 3 years ago

In GitLab by @afroborg on Aug 17, 2021, 22:53

added 1 commit

Compare with previous version

esek-macapar commented 3 years ago

In GitLab by @ginger51011 on Aug 30, 2021, 10:44

Commented on src/resolvers/auth.resolver.ts line 39

Här tappar jag mig lite, en kommentar här hade varit bra!

esek-macapar commented 3 years ago

In GitLab by @ginger51011 on Aug 30, 2021, 10:46

Commented on src/schemas/auth.graphql line 9

Stämmer detta? Om ja, skulle en felaktig inloggning numera ge False som svar (isåfall måste det ändras i EsekWikiAuth och espresso-MR

esek-macapar commented 3 years ago

In GitLab by @ginger51011 on Aug 30, 2021, 10:48

Commented on src/auth.ts line 57

Vet ej om vi sagt något om styling här, och lint kanske godkänner, men detta gör rätt ont i ögonen att läsa, även om man vet isch vad som händer

esek-macapar commented 3 years ago

In GitLab by @afroborg on Aug 31, 2021, 12:21

Commented on src/schemas/auth.graphql line 9

Exakt, min tanke är att inloggnings-flödet ser ut såhär:

  1. En mutation mot login där man får true / false som response (om true får du även en e-refresh-token cookie som är httpOnly viket innebär att den inte går att läsa client side)
  2. Gör en query anrop mot refreshToken med din e-refresh-token cookie där du får accessToken och User
  3. Varje ish 60 min gör du ett nytt refresh anrop för att uppdatera accessToken.
  4. Använd accessToken för alla andra anrop

På så sätt behöver vi inte spara accessToken (den som faktiskt är viktig) i varken localStorage eller i en cookie och är på så sätt skyddade mot XSS osv. Istället kan man ha den i någon state som sätts var 60e minut eller när man laddar om sidan.

Ni får säga vad ni tycker men det känns säkrast enligt mig?

esek-macapar commented 3 years ago

In GitLab by @afroborg on Aug 31, 2021, 12:23

Commented on src/resolvers/auth.resolver.ts line 39

changed this line in version 4 of the diff

esek-macapar commented 3 years ago

In GitLab by @afroborg on Aug 31, 2021, 12:23

Commented on src/auth.ts line 57

changed this line in version 4 of the diff

esek-macapar commented 3 years ago

In GitLab by @afroborg on Aug 31, 2021, 12:23

added 1 commit

Compare with previous version

esek-macapar commented 3 years ago

In GitLab by @afroborg on Aug 31, 2021, 12:23

Commented on src/auth.ts line 57

Bröt ut Record<string unkonwn> till en egen typ vilket gjorde det lite snyggare men vet inte hur man kan göra det mycket snyggare och fortfarande dynamiskt :/

esek-macapar commented 3 years ago

In GitLab by @ginger51011 on Aug 31, 2021, 13:40

Commented on src/schemas/auth.graphql line 9

Tycker detta låter väldigt bra, säkrare än vad vi kört tidigare om inte annat! För min del är det bara att mergea, men tror det är bra om @blennster tar sig en titt också. Tar gärna emot reviews på !31

esek-macapar commented 3 years ago

In GitLab by @blennster on Sep 1, 2021, 21:06

Jag gillar detta, har en fundering dock. Om vi nu säger att någon vill skriva något annat som ska använda sig av ekorre, behöver de då använda cookies?

Jag tänker att ifall vi hostar något själva och servern ska nå så kan det kanske vara bra att tillåta att server har en egen nyckel så den kan nå direkt och inte behöva gå via cookies?

Vet inte hur görbart detta är (eller nödvändigt för den delen) men jag tycker detta bör mergas. Vi behöver anpassa inlogg på ekollon dock.

Bra jobbat!

esek-macapar commented 3 years ago

In GitLab by @blennster on Sep 1, 2021, 21:06

approved this merge request

esek-macapar commented 3 years ago

In GitLab by @afroborg on Sep 2, 2021, 13:17

Det är en bra fråga, första tanken är att vi bygger ut ett permissions-system framöver där vi kan lägga till api-token auth för vissa applikationer som bara kräver access till vissa delar av api:et?

esek-macapar commented 3 years ago

In GitLab by @afroborg on Sep 2, 2021, 13:17

mentioned in commit b53e09aba3be9c9814ec49b23cf7ef63a3e98639