Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ASI-614] Bull queue to expire user session tokens #1905

Merged
merged 10 commits into from
Nov 15, 2021

Conversation

csjiang
Copy link
Contributor

@csjiang csjiang commented Sep 23, 2021

Prevent replay attacks by expiring user session tokens with a daily cron
and Bull queue in content node.

See also:

ASI-614

Description

STs get generated when you go to client and start a new session. But tokens don’t get cleared in current architecture. So there’s possibility of replay attacks - session from 2y ago could be leveraged if someone got ahold of DB and made requests on behalf of a user. So we want to clean up sessions after 2 weeks.

See ASI-614 design doc on Notion for additional details.

Tests

  • added tests for SessionManager.deleteSessions and DBManager.deleteSessionTokensFromDB.
  • E2E tested in dev environment

How will this change be monitored?

  • CN logs

@csjiang
Copy link
Contributor Author

csjiang commented Sep 24, 2021

monitoring - prob not needed. test on staging.

@csjiang csjiang force-pushed the asi-614-session-expiration-queue branch 7 times, most recently from d4339a2 to 4399716 Compare September 29, 2021 04:30
@csjiang csjiang marked this pull request as ready for review September 29, 2021 04:33
@csjiang csjiang force-pushed the asi-614-session-expiration-queue branch from 4399716 to a6d6654 Compare September 29, 2021 15:01
creator-node/test/dbManager.test.js Outdated Show resolved Hide resolved
creator-node/test/dbManager.test.js Outdated Show resolved Hide resolved
@csjiang csjiang force-pushed the asi-614-session-expiration-queue branch from a6d6654 to c5b5699 Compare September 29, 2021 16:06
@csjiang csjiang force-pushed the asi-614-session-expiration-queue branch from c5b5699 to 5bd7694 Compare September 29, 2021 18:20
@csjiang csjiang added content-node Content Node (previously known as Creator Node) javascript Pull requests that update Javascript code labels Sep 29, 2021
@csjiang csjiang force-pushed the asi-614-session-expiration-queue branch 3 times, most recently from 23f6bf0 to cbc8d62 Compare October 26, 2021 21:29
Copy link
Contributor

@dmanjunath dmanjunath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! Small asks but i think we can get it merged today or tomorrow!

creator-node/src/dbManager.js Outdated Show resolved Hide resolved
creator-node/src/dbManager.js Outdated Show resolved Hide resolved
creator-node/test/dbManager.test.js Outdated Show resolved Hide resolved
creator-node/test/dbManager.test.js Show resolved Hide resolved
Prevent replay attacks by expiring user session tokens with a daily cron
and Bull queue in content node.

See also:

[ASI-614][1]

[1]: https://linear.app/audius/issue/ASI-614
* Use Redis MULTI feature to batch handle transactions in
SessionExpirationQueue
* Implement retry for DB bulk session delete call in SessionManager before rollback

See also:

[ASI-614][1]

[1]: https://linear.app/audius/issue/ASI-614
i
* add tests for SessionManager
* add tests for DBManager

See also:

[ASI-614][1]

[1]: https://linear.app/audius/issue/ASI-614
@csjiang csjiang force-pushed the asi-614-session-expiration-queue branch from 0c8ee6c to fc636ba Compare October 28, 2021 19:27
@csjiang
Copy link
Contributor Author

csjiang commented Oct 28, 2021

good to go out after next client release

@raymondjacobson raymondjacobson removed their request for review November 9, 2021 08:12
@csjiang csjiang merged commit c538f27 into master Nov 15, 2021
@csjiang csjiang deleted the asi-614-session-expiration-queue branch November 15, 2021 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content-node Content Node (previously known as Creator Node) javascript Pull requests that update Javascript code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants