feat: switch over to passport.js #314

coderbyheart · 2021-08-24T23:08:46Z

This adds username/password based authentication,
using a cookie when accessing secure routes

See ./docs/authentication.md for description of the implementation.

CAPTCHA will be handled in a separate PR: #335

Includes changes to host it on CleverCloud, so this PR can be reviewed on a working instance.

ghost · 2021-08-26T21:22:38Z

Changes to your CodeSee Architecture Map:

Legend

deammer · 2021-08-30T15:29:54Z

package.json

+    "@sinclair/typebox": "0.20.5",
+    "@types/graphql-depth-limit": "1.1.2",
+    "@types/lodash": "4.14.172",
+    "@types/supertest": "2.0.11",
+    "ajv": "8.6.2",
+    "ajv-formats": "2.1.1",
+    "apollo-server": "3.3.0",
+    "apollo-server-express": "3.3.0",
+    "bcrypt": "5.0.1",
+    "body-parser": "1.19.0",
+    "compression": "1.7.4",
+    "cookie-parser": "1.4.5",
+    "cors": "2.8.5",
+    "dotenv": "10.0.0",
+    "express": "4.17.1",
+    "graphql": "15.5.1",
+    "graphql-depth-limit": "1.1.0",
+    "graphql-import-node": "0.0.4",
+    "graphql-scalars": "1.10.0",
+    "lodash": "4.17.21",
+    "nodemon": "2.0.12",
+    "passport": "0.4.1",
+    "passport-cookie": "1.0.9",
+    "pg": "8.7.1",
+    "prettier-plugin-organize-imports": "2.3.3",
+    "reflect-metadata": "0.1.13",
+    "sequelize": "6.6.2",
+    "sequelize-typescript": "2.1.0",
+    "supertest": "6.1.6",
+    "uuid": "8.3.2"


praise: thank you for pinning every dependency. I hope it wasn't too much of a pain.

Only a little, not all of them are on the latest release (e.g. sequelize-typescript)

deammer · 2021-08-30T15:33:08Z

src/authenticateRequest.ts

+  i: number
+  /** user is admin */
+  a: boolean
+  /** user hash */
+  c: string


question (non-blocking): are those fields named that way to match the way passport-cookie formats its data?

No, but we encode this in the cookie sent to the browser, and there is a size limit, so I want to keep it as small as possible (while still using JSON).

deammer · 2021-08-30T15:36:18Z

src/models/user_account.ts

  @Column
-  public auth0Id!: string
+  public username!: string


question (non-blocking): why require a username instead of an email?

Yes, good point. I started this switch without emails, but I realize we need them anyway (for password reset), so I will remove the username field and turn it into an email field.

deammer · 2021-08-30T15:43:32Z

src/routes/login.ts

+    if (user === null) {
+      // Penalize
+      await new Promise((resolve) => setTimeout(resolve, penaltySeconds * 1000))
+      return response.status(401).end()
+    }
+    if (!bcrypt.compareSync(valid.value.password, user.passwordHash)) {
+      // Penalize
+      await new Promise((resolve) => setTimeout(resolve, penaltySeconds * 1000))
+      return response.status(401).end()
+    }


though (non-blocking): this means that if I type the wrong username or password, I'll have to wait 10 seconds until I see an error message, right? I think that could become annoying from a UX perspective. I wonder if we could instead throttle access to that endpoint based on the user's IP (not necessarily in this PR though)

Yes, a more advanced feature of this would be remembering the user, however, the point here is to slow down attackers, which most likely will switch IP addresses. I think a better solution will be to implement a CAPTCHA, like https://friendlycaptcha.com/.

I like the CAPTCHA solution 👍

coderbyheart · 2021-08-30T21:15:08Z

@deammer I realize that I've build the authentication API outside of GraphQL, as REST. Do you feel that is an issue? They are pretty independent of each other, any way (and were before).

deammer · 2021-08-31T01:22:45Z

@deammer I realize that I've build the authentication API outside of GraphQL, as REST. Do you feel that is an issue? They are pretty independent of each other, any way (and were before).

I feel like that's not a problem. The previous /profile endpoint was REST as well and we didn't have any issues.

coderbyheart · 2021-09-06T22:51:10Z

I've asked friendlyCaptcha if they would provide us with a free business license, because only that one uses EU-only endpoints.

coderbyheart · 2021-09-14T08:21:33Z

I have to figure out the problem with sequelize before merging.

This adds username/password based authentication, using a cookie when accessing secure routes

@see https://www.clever-cloud.com/blog/engineering/2020/06/24/deploy-cellar-s3-static-site/

REACT_APP_VERSION is not used and maintained right now

So Chrome will include it in cross-site requests

Update the DevContainer definition so users are able to run `npm test`.

coderbyheart force-pushed the passport.js branch 2 times, most recently from eb7c444 to 06765c0 Compare August 26, 2021 21:20

coderbyheart force-pushed the passport.js branch 3 times, most recently from 04c6f50 to 2438aaa Compare August 29, 2021 22:23

deammer reviewed Aug 30, 2021

View reviewed changes

deammer mentioned this pull request Aug 30, 2021

The shipment tracker should be hosted in the EU on servers owned by a European company 🇪🇺 #311

Closed

9 tasks

coderbyheart force-pushed the passport.js branch 2 times, most recently from 74bd298 to 185de9f Compare August 30, 2021 21:12

coderbyheart force-pushed the passport.js branch 3 times, most recently from 2c39069 to e64eaec Compare September 5, 2021 22:45

coderbyheart marked this pull request as ready for review September 6, 2021 22:51

coderbyheart force-pushed the passport.js branch 3 times, most recently from 8ac8bcb to 265e69f Compare September 9, 2021 20:19

coderbyheart mentioned this pull request Sep 9, 2021

Enable FriendlyCaptcha, sponsored by friendlycaptcha.com #335

Closed

2 tasks

coderbyheart force-pushed the passport.js branch from 58e70ba to 1ffb1e2 Compare September 9, 2021 21:43

coderbyheart mentioned this pull request Sep 9, 2021

Set up Clever Cloud deployment pipeline #300

Closed

coderbyheart force-pushed the passport.js branch from d185570 to 080fd5d Compare September 13, 2021 18:47

deammer approved these changes Sep 14, 2021

View reviewed changes

feat: switch over to passport.js

3969cb0

This adds username/password based authentication, using a cookie when accessing secure routes

coderbyheart added 27 commits September 15, 2021 09:12

build: downgrade node version so it works on clever cloud

31511e8

build: add script to deploy to Clever Cloud cellar

dc3af6d

@see https://www.clever-cloud.com/blog/engineering/2020/06/24/deploy-cellar-s3-static-site/

build(clevercloud): install rclone

f40ffd1

docs(frontend): node.js 16 is fine

6e2ff92

build: use local rclone

22e6709

build: use rclone from local folder

7bc2460

build: use only bucket name

fc53c96

build: echo progress

386b5d9

fix(frontend): only hardcode env vars for development

296730e

REACT_APP_VERSION is not used and maintained right now

fix(frontend): use only one server URL env var

13caeee

fix: ensure Access-Control-Allow-Origin has no trailing slash

e3b7f93

fix(passwords): do not enforce special characters

1c79ec3

fix: sent cookies with sameSite=none

e3d6a09

So Chrome will include it in cross-site requests

feat: remove UserProfileContext, add more auth pages

29bdaf5

fix: include credentials with log-in, too

e2e5989

fix: remove unused LoginButton

9bf8ebd

fix(logout): reload to /

a8f954c

feat: include version string

cf7d2df

build(frontend): fail on errors

5157cc9

ci(cd): change to correct folder

31713a0

fix: print running version

83755fe

fix: version middleware must come before routes

21f42c3

fix: logout url has colon

8680b84

fix: remove log statement

6e145ea

fix(env): remove dotenve so order of imports is not important

1405c2b

docs(devcontainer): update dev container to run tests

d113d3a

Update the DevContainer definition so users are able to run `npm test`.

test(authentication): use correct cookie attributes

abbc315

coderbyheart merged commit 550af17 into saga Sep 15, 2021

coderbyheart deleted the passport.js branch September 15, 2021 19:05

coderbyheart mentioned this pull request Sep 15, 2021

new auth forms need some love #345

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: switch over to passport.js #314

feat: switch over to passport.js #314

coderbyheart commented Aug 24, 2021 •

edited

Loading

ghost commented Aug 26, 2021 •

edited by ghost

Loading

deammer Aug 30, 2021

coderbyheart Aug 30, 2021

deammer Aug 30, 2021

coderbyheart Aug 30, 2021

deammer Aug 30, 2021

coderbyheart Aug 30, 2021

deammer Aug 30, 2021

coderbyheart Aug 30, 2021

deammer Aug 31, 2021

coderbyheart commented Aug 30, 2021

deammer commented Aug 31, 2021

coderbyheart commented Sep 6, 2021

coderbyheart commented Sep 14, 2021

feat: switch over to passport.js #314

feat: switch over to passport.js #314

Conversation

coderbyheart commented Aug 24, 2021 • edited Loading

ghost commented Aug 26, 2021 • edited by ghost Loading

Changes to your CodeSee Architecture Map:

Legend

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coderbyheart commented Aug 30, 2021

deammer commented Aug 31, 2021

coderbyheart commented Sep 6, 2021

coderbyheart commented Sep 14, 2021

coderbyheart commented Aug 24, 2021 •

edited

Loading

ghost commented Aug 26, 2021 •

edited by ghost

Loading