search

ReCoded-Org / curriculum-backend-readings

Content monorepo for all the readings of the backend bootcamp curriculum developed by Re:Coded.
23 stars 4 forks source link

Module 4: Authentication and Security #2

Closed wisammechano closed 2 years ago

wisammechano commented 3 years ago

I will keep this open until I'm done with Module 4, but I would love constant review. If you have a better method, please let me know. Otherwise, if you have no changes requested, just approving it will give me the chance to ask for another review again later.

louisrli commented 3 years ago

I will review this tomorrow On Sat, 4 Sep 2021 at 19:43, Wisam Naji @.***> wrote:

I will keep this open until I'm done with Module 4, but I would love constant review. If you have a better method, please let me know. Otherwise, if you have no changes requested, just approving it will give me the chance to ask for another review again later.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/ReCoded-Org/curriculum-backend-readings/pull/2#issuecomment-913003031, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJ6G2KYT6FEDAOKKABOBZDUAJED5ANCNFSM5DNOJN7A .

wisammechano commented 3 years ago

Thank you very much @louisrli for the valuable feedback. I agree sticking to the outline is what's intended here, and to clarify, this is only the first lesson in that outline, which is a high-level introduction to what we will be tackling, why we need to secure resources, and basic implementation for the principle of authentication.

It would set the ground for the upcoming actual real-world implementation but could always be tied to this simple scenario, with more elaboration on why we need further granularity (like users table).

louisrli commented 3 years ago

i can kinda see how you're trying to set up why you need passwords but again i think you're confusing authentication and access here (authentication = logging in as a certain identity), whether by a token or username/password) and access = viewing or editing the resource

if you want to use this token thing to set up the username/password you need to emphasize that this is not a real approach for identifying as a certain person (it's not, or if it is, it's super uncommon)

in the example you give it's like "ok i'm wisam, i own lists. but i don't wanna use emails (why? i didn't really understand this either) but i wanna query all the lists, so i'm gonna have a hard-to-guess token". again i don't know any sites that really do this , where the USER is replaced by a token. in all the examples you gave, the list has a token ID, not the user who created the list

louisrli commented 2 years ago

bolding markdown headers is really not a thing, and it creates an inconsistency in the codebase. the reason h2 h3 h4 h5 h6 exist is exactly the problem that bolding them aims to solve (a consistent hierarchy where people don't make one-off things)

On Sun, Oct 3, 2021 at 7:53 PM Wisam Naji @.***> wrote:

@.**** commented on this pull request.

In module4-authentication-and-security/r1.1.1-authentication-persistence/README.md https://github.com/ReCoded-Org/curriculum-backend-readings/pull/2#discussion_r720871200 :

+

+This means that a JWT could be valid even though the user’s account has been suspended or deleted. Some solutions around this are available but they mostly require trips to the identity provider or the database, which JWT is essentially developed to minimize.

+

+So if your app has the potential to deactivate or revoke user access frequently, think twice before using JWTs.

+

+## Conclusion

+

+In this lesson, we explored authentication persistence using both server-side sessions and client-side tokens. Each has its pros and cons. Sessions have been used for more than 2 decades now, and they are quite robust. However, their limitation comes from poor portability.

+

+JWTs, on the other hand, are also robust and pretty portable. However, they come with their overhead and access longevity issues.

+

+When building your app, always measure the pros and cons of both of these approaches and take your time to decide which one is best for you.

+

+## Read more

+

+- https://www.hebergementwebs.com/news/use-of-session-cookies-vs-jwt-for-authentication

They are related to the whole topic, session, and JWTs. And I think they are helpful to include here.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ReCoded-Org/curriculum-backend-readings/pull/2#discussion_r720871200, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJ6G2NNND3KRBPDYC5MPL3UFCRDBANCNFSM5DNOJN7A .

louisrli commented 2 years ago

I don't understand this. I wouldn't dive into how encryption works or the math behind it, and for now offer ways to encrypt only. Encryption and validation are both done above, and in case the signature is incorrect, express-jwt would return invalid token error to be handled.

i'm saying that students probably don't understand what it means to encrypt things or why it's important, and there's no explanation in the module

On Sun, Oct 3, 2021 at 8:01 PM Louis Li @.***> wrote:

bolding markdown headers is really not a thing, and it creates an inconsistency in the codebase. the reason h2 h3 h4 h5 h6 exist is exactly the problem that bolding them aims to solve (a consistent hierarchy where people don't make one-off things)

On Sun, Oct 3, 2021 at 7:53 PM Wisam Naji @.***> wrote:

@.**** commented on this pull request.

In module4-authentication-and-security/r1.1.1-authentication-persistence/README.md https://github.com/ReCoded-Org/curriculum-backend-readings/pull/2#discussion_r720871200 :

+

+This means that a JWT could be valid even though the user’s account has been suspended or deleted. Some solutions around this are available but they mostly require trips to the identity provider or the database, which JWT is essentially developed to minimize.

+

+So if your app has the potential to deactivate or revoke user access frequently, think twice before using JWTs.

+

+## Conclusion

+

+In this lesson, we explored authentication persistence using both server-side sessions and client-side tokens. Each has its pros and cons. Sessions have been used for more than 2 decades now, and they are quite robust. However, their limitation comes from poor portability.

+

+JWTs, on the other hand, are also robust and pretty portable. However, they come with their overhead and access longevity issues.

+

+When building your app, always measure the pros and cons of both of these approaches and take your time to decide which one is best for you.

+

+## Read more

+

+- https://www.hebergementwebs.com/news/use-of-session-cookies-vs-jwt-for-authentication

They are related to the whole topic, session, and JWTs. And I think they are helpful to include here.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ReCoded-Org/curriculum-backend-readings/pull/2#discussion_r720871200, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJ6G2NNND3KRBPDYC5MPL3UFCRDBANCNFSM5DNOJN7A .

louisrli commented 2 years ago

Because I wasn't intending to talk about cookies as data holders. rather, simply their tied usage to session identifiers. I think another topic can be devoted to talking about cookies in general and their parameters.

i'm not saying you need to go in-depth in what cookies are. i'm pointing this out as a general issue in the module, which is that terms are being introduced with near-zero explanation.

if you don't want to take it from me (where i'm reading this from the perspective of a student), i suggest you show this to somebody who doesn't know backend and see where they get confused -- it will be in many of the same places i'm telling you that there is an introduction of a term with no explanation

On Sun, Oct 3, 2021 at 7:31 PM Wisam Naji @.***> wrote:

@.**** commented on this pull request.

In module4-authentication-and-security/r1.1.1-authentication-persistence/README.md https://github.com/ReCoded-Org/curriculum-backend-readings/pull/2#discussion_r720868321 :

+app.use(

  • sessions({
  • secret: "MY_APP_SECRET",
  • cookie: { secure: false }, // make sure to change this to true on production code
  • }) +);

  • +let listener = app.listen(5000, function () {

  • console.log("Listening on port " + listener.address().port); +}); +```

  • +In the code snippet above, we configured the session storage using app.use() and created a new session instance with the configuration below:

  • +- secret: a token that will be used to encrypt the data and cookies. It can also be a universal APP_SECRET defined in the environment variables. This must be a random secret generated by secure random generators. +- cookie: cookie parameters that will be set in the client. Because our development environment doesn't have HTTPs, we turned off secure. In a production environment, the cookie should only be sent in secure contexts so secure should be true.

Because I wasn't intending to talk about cookies as data holders. rather, simply their tied usage to session identifiers. I think another topic can be devoted to talking about cookies in general and their parameters.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ReCoded-Org/curriculum-backend-readings/pull/2#discussion_r720868321, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJ6G2J75A6MB25MWUSL45DUFCOOTANCNFSM5DNOJN7A .

louisrli commented 2 years ago

I am trying to understand your disagreement here, if it is about having roles identified by integers, then I have to disagree. Many of the roll-based access control implementations use a combination of:

you are conflating using foreign keys with making something an integer. if a novice of programming reads what you wrote, they would think that it's a good idea to use numbers to represent an enum in their database. that is my objection to this module -- you are suggesting to students that this is a good practice. using an integer-based foreign key that links to another table is not the same as using a number to represent an enum, though they happen to look similar.

the other thing is that none of the stated benefits that you listed out are actually benefits, and if you truly want to spend time discussing it, then i would be happy to hear the justifications that using integers somehow improves scalability in any semireasonable case (eg lets not consider some mission-critical facebook-level embedded software that needs to shave off nanoseconds). again, if a student reads this, they might think that using integers in place of strings for "performance reasons" would be a suitable approach

On Sun, Oct 3, 2021 at 7:22 PM Wisam Naji @.***> wrote:

@.**** commented on this pull request.

In module4-authentication-and-security/r1.2-adding-authorization-layer/README.md https://github.com/ReCoded-Org/curriculum-backend-readings/pull/2#discussion_r720867319 :

+ +- An anonymous user can watch videos and search on the website without signing up. +- To upload videos or flag content, the user needs to create an account. +- Our app needs moderators as well, who are users that can delete videos in case they violate the usage agreement, or are inappropriate. +- And finally, our app needs admins, who have the ability to make moderators by elevating other users to a moderator access level. + +From these requirements, we can distinguish our app structure as follows: + +- Some endpoints wouldn't have any authentication guard, any request can reach them. Which are endpoints to read a list of videos, watch a specific video, or search for videos. +- Certain endpoints like /upload and /flag will need an authentication guard. This is to check if we have a JWT in the request or a user in the session. If there isn't, they shouldn't be able to reach that resource. +- Endpoints like /delete would need an authorization guard. Which is a guard that contains the authentication guard, plus it also checks if the user is a moderator, before letting them complete the action. Otherwise, it shouldn't let them through. +- To elevate a user, an endpoint like /elevate would have the same authorization guard, but instead it will check if the user is an admin. + +### Implementation + +To implement this as simply as possible, we need to first define our roles as numbers. That is mainly to:

I am trying to understand your disagreement here, if it is about having roles identified by integers, then I have to disagree. Many of the roll-based access control implementations use a combination of:

  • Roles table
  • Permissions table
  • Users Table

And they are related by foreign key constraints: User.has(role) Role.has(permission)

[image: image] https://user-images.githubusercontent.com/12988551/135766430-511cb74e-fa9e-42b1-baca-ef8031fd5af6.png [image: image] https://user-images.githubusercontent.com/12988551/135766435-c6d66387-5021-4029-94b9-6d2a30be3545.png

In no case, one should type a number in a hardcoded approach, and one can mess up writing a number, they would more likely mess writing admin if that is the case. It is usually by querying the database for roles and rendering them in an options element with the role_id.

If there is any resource that strictly points this out as a wrong practice, please share.

If it is about having the roles as a code-based enum, then yes it is pointed out below that this isn't the best practice, and they need to be in a database. But I tend to simplify the implementation to make it understandable. However, I'm happy to change this to a database model with a more solid example.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ReCoded-Org/curriculum-backend-readings/pull/2#discussion_r720867319, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJ6G2LZUMA2WG32XDMAPZ3UFCNORANCNFSM5DNOJN7A .