add suggestions and notes

Author: Xronophobe

chapter_19_spiking_custom_auth.asciidoc

@@ -91,6 +91,7 @@ but this is just a fun toy project so let's give it a go.
 To get this Magic Links project set up, the first thing I did was take a look at existing Python and Django authentication
 packages, like https://docs.allauth.org/en/latest/[django-allauth]
 and https://github.com/omab/python-social-auth[python-social-auth],
+// CSANAD: python-social-auth looks abandoned
 but both of them looked overcomplicated for this stage
 (and besides, it'll be more fun to code our own!).
 
@@ -109,6 +110,11 @@ and convince yourself that it really does work.
 
 ==== Starting a Branch for the Spike
 
+// CSANAD: although we introduced to expression "spike" in the 17th chapter, I
+// would still mention more explicitly that this is a quick and dirty hacking
+// around, which is not production code, and this is the reason we are not
+// writing tests just yet - we still need to see what and how to test.
+
 ((("spiking and de-spiking", "branching your VCS")))
 ((("Git", "creating branches")))
 This spike is going to be a bit more involved than the last one,
@@ -182,12 +188,20 @@ The login in theory will be something like this:
 
 - When someone wants to log in, we generate a unique secret token for them,
   store it in the database linked to their email, and send it to them.
+// CSANAD: although it should be obvious there is no "database linked to their
+// email", still, I suggest different order of words for clarity:
+// "store it linked to their email in the database, (...)"
+// or
+// "store it linked in the database to their email, (...)"
 
 - The user then checks their email,
   which will have a link to a URL that includes that token.
 
 - When they click that link, we check whether the token exists in the database,
   and if so, they are logged in as the associated user.
+// CSANAD: what do you think about rephrasing it a little:
+// "...and if so, we authorize/approve the request to authenticate/log in as the
+// associated user"
 
 // https://docs.djangoproject.com/en/5.0/topics/auth/customizing/
 
@@ -334,7 +348,9 @@ TIP: If you want to use Gmail as well,
 ((("", startref="DFemail18")))
 ((("", startref="SDemail18")))
 
-
+// CSANAD: I was only able to make it work with 2FA enabled and the
+// app-specific password set. If I have time, I'll try again with allowing
+// less secure apps too.
 
 ==== Another Secret, Another Environment Variable
 
@@ -461,6 +477,14 @@ OK so we've done the first half of "generate and recognise unique tokens":
 
 Before we can move on to recognising them and making the login work end-to-end though,
 we need to sort out user authentication in Django.
+// CSANAD: not sure if it's clear for the readers why we need to sort out the
+// authn first. Maybe we could just rephrase:
+//
+// "In order to move on to recognising them and making the login work end-to-end,
+// we need to sort out user authentication in Django.
+//
+// This way we aren't just stating what needs to be done first, but also the
+// reason behind it.
 The first thing we'll need is a user model.
 I took a dive into the
 https://docs.djangoproject.com/en/5.0/topics/auth/customizing[Django
@@ -523,6 +547,12 @@ class ListUserManager(BaseUserManager):
 ----
 ====
 
+// CSANAD: ListUserManager has to be defined before ListUser, since its
+// reference to ListUser isn't evaluated until `create_user` is called. This is
+// not the case the other way around, ListUser's reference to ListUserManager
+// is instantiated in the class definition. Maybe we could leave a note about
+// this?
+
 
 No need to worry about what a model manager is at this stage;
 for now we just need it because we need it, and it works.
@@ -608,6 +638,9 @@ def login(request):
 
 The `authenticate()` function invokes Django's authentication framework,
 which we configure using a "custom authentication backend",
+// CSANAD: why the quote marks? If it's because this is a reference to the
+// official Django docs, we could just make it a link instead to
+// https://docs.djangoproject.com/en/5.1/topics/auth/customizing/#writing-an-authentication-backend
 whose job it is to validate the UID and return a user with the right email.
 
 We could have done this stuff directly in the view,
@@ -971,6 +1004,8 @@ what information you want to track about users,
 from explicitly recording first name and last namefootnote:[
 A decision which you'll find prominent Django maintainers
 saying they now regret.  Not everyone has a first name and a last name.]
+// CSANAD: also, profile-related information has no business in auth-related
+// models.
 to forcing you to use a username.
 I'm a great believer in not storing information about users
 unless you absolutely must,