[PR] Adding auth

[LuchoTurtle]

This PR adds authentication which allows people to filter for items they created.
Closes:

• Auth! #37
• Broken Build Badge #63
• Coverage not at 100% #65

[LuchoTurtle]

This should be done.
All the test pass and authentication seems to be working properly.

[nelsonic]

@LuchoTurtle the "Todo" in #37 was to "lift-and-shift" from: dwyl/phoenix-chat-example#120 ...
That added optional auth. i.e. people could still try the app without authenticating.
Adding |> validate_required([:person_email, :text]) to the item changeset means that it's no longer optional. unless you are capturing their email address. But then why are we complicating this? 🤷‍♂️

[LuchoTurtle]

@nelsonic I knew that an id was returned from auth_plug but I still decided to use person_email because I thought it would be more eligible for beginners. It would also be easier for these to see which "emails" referred to the todo list (instead of having an id).
I was aware that it would "complicate a bit" but I did it in good faith so it would be easier for people to see which todos referred to whom.

[nelsonic]

We don't want to store email addresses in a random app tutorial app [or any other non-auth app for that matter].
if we deploy this to Fly.io without encrypting the email addresses then we have to think about how to keep those email addresses [highly personally identifiable information] private. or we're breaking the GDPR law.
see: https://www.linkedin.com/pulse/gdpr-you-collect-email-addresses-personal-data-dr-adrian-ioannou/

A person_id from authdemo is not personally identifiable because it's a system generated id
that gives a hacker no other info about the person and cannot be used maliciously without hacking our auth app.

This isn't a question of "good faith" ...
it's a question of doing something that wasn't/isn't required
and ultimately makes the app a target for data theft and us liable.
It's not something we would do in practice so we shouldn't do it in in a tutorial
where a beginner might get the idea that it's "OK" to store email addresses like this.

As much as I want to just ignore this, and get back to my work, I cannot.
We cannot write tutorials that ignore best practice [and the Law] for data privacy/security.
Emails are appropriately encrypted in the auth App:
https://github.com/dwyl/auth/blob/bc1f8a9ca08ff26a4f7a50ef2e71bae4985d33d1/lib/auth/person.ex#L14-L15

Our MVP uses the person_id and has no other personally identifiable information.
This is the appropriate Separation of concerns.

Please make your own life a lot simpler and try not over-think it. 🙏

[LuchoTurtle]

The auth version was added as a bonus and was meant to be run on localhost, never wanted it to actually be deployed unencrypted online. Perhaps I didn't make that clear.

I'll change it.

[nelsonic]

PR is assigned to me. I am reviewing now. I will be leaving comments and making the relevant updates on localhost. If you want to follow along, please join Zoom. Please don't push any commits to the branch as they will cause merge-conflicts. 🙏 (thanks!)

[nelsonic]

git checkout 'auth'
git pull
mix deps.get
mix test

ompiling 10 files (.ex)
Generated app app
......warning: variable "item" is unused (if the variable is not meant to be used, prefix it with an underscore)
  test/app_web/controllers/item_controller_test.exs:85: AppWeb.ItemControllerTest."test clear completed clears the completed items"/1

warning: variable "items" is unused (if the variable is not meant to be used, prefix it with an underscore)
  test/app_web/controllers/item_controller_test.exs:91: AppWeb.ItemControllerTest."test clear completed clears the completed items"/1

warning: variable "item" is unused (if the variable is not meant to be used, prefix it with an underscore)
  test/app_web/controllers/item_controller_test.exs:98: AppWeb.ItemControllerTest."test clear completed clears the completed items in public (person_id=0)"/1

warning: variable "items" is unused (if the variable is not meant to be used, prefix it with an underscore)
  test/app_web/controllers/item_controller_test.exs:104: AppWeb.ItemControllerTest."test clear completed clears the completed items in public (person_id=0)"/1

warning: key :givenName will be overridden in map
  test/app_web/controllers/item_controller_test.exs:149

....09:26:46.686 [error] ERROR: AUTH_API_KEY Environment Variable is not set


  1) test get /logout with valid JWT (AppWeb.AuthControllerTest)
     test/app_web/controllers/auth_controller_test.exs:21
     ** (FunctionClauseError) no function clause matching in Regex.run/3

     The following arguments were given to Regex.run/3:

         # 1
         ~r/^(.*)\/(.*)\/(.*)$/

         # 2
         nil

         # 3
         []

     Attempted function clauses (showing 1 out of 1):

         def run(%Regex{} = regex, string, options) when is_binary(string)

     code: jwt = AuthPlug.Token.generate_jwt!(data)
     stacktrace:
       (elixir 1.14.1) lib/regex.ex:335: Regex.run/3
       (auth_plug 1.5.1) lib/token.ex:52: AuthPlug.Token.client_secret/0
       (auth_plug 1.5.1) lib/token.ex:86: AuthPlug.Token.generate_jwt!/1
       test/app_web/controllers/auth_controller_test.exs:31: (test)

warning: module attribute @completed_attrs was set but never used
  test/app_web/controllers/item_controller_test.exs:9

09:26:46.843 request_id=FzJ2Otal0L_oqbcAAHqH [error] ERROR: AUTH_API_KEY Environment Variable is not set
09:26:46.849 [error] ERROR: AUTH_API_KEY Environment Variable is not set

... more similar failing tests ....

  9) test update item redirects when data is valid (AppWeb.ItemControllerTest)
     test/app_web/controllers/item_controller_test.exs:70
     ** (FunctionClauseError) no function clause matching in Regex.run/3

     The following arguments were given to Regex.run/3:

         # 1
         ~r/^(.*)\/(.*)\/(.*)$/

         # 2
         nil

         # 3
         []

     Attempted function clauses (showing 1 out of 1):

         def run(%Regex{} = regex, string, options) when is_binary(string)

     code: conn = setup_conn(conn)
     stacktrace:
       (elixir 1.14.1) lib/regex.ex:335: Regex.run/3
       (auth_plug 1.5.1) lib/token.ex:52: AuthPlug.Token.client_secret/0
       (auth_plug 1.5.1) lib/token.ex:86: AuthPlug.Token.generate_jwt!/1
       test/app_web/controllers/item_controller_test.exs:173: AppWeb.ItemControllerTest.setup_conn/1
       test/app_web/controllers/item_controller_test.exs:71: (test)

........
Finished in 0.3 seconds (0.2s async, 0.1s sync)
34 tests, 9 failures

Randomized with seed 570274

My reading of the README.md in it's current state is that there is no reference to AUTH_API_KEY Environment Variable:
dwyl/phoenix-todo-list-tutorial/blob/auth/README.md

This will impact the next person's ability to run the finished project on their localhost ... 💔

[nelsonic]

Created .env_sample file with AUTH_API_KEY so that tests always pass on localhost: 4238996

Now tidying up these warnings:

mix test
warning: variable "item" is unused (if the variable is not meant to be used, prefix it with an underscore)
  test/app_web/controllers/item_controller_test.exs:85: AppWeb.ItemControllerTest."test clear completed clears the completed items"/1

warning: variable "items" is unused (if the variable is not meant to be used, prefix it with an underscore)
  test/app_web/controllers/item_controller_test.exs:91: AppWeb.ItemControllerTest."test clear completed clears the completed items"/1

warning: variable "item" is unused (if the variable is not meant to be used, prefix it with an underscore)
  test/app_web/controllers/item_controller_test.exs:98: AppWeb.ItemControllerTest."test clear completed clears the completed items in public (person_id=0)"/1

warning: variable "items" is unused (if the variable is not meant to be used, prefix it with an underscore)
  test/app_web/controllers/item_controller_test.exs:104: AppWeb.ItemControllerTest."test clear completed clears the completed items in public (person_id=0)"/1

warning: key :givenName will be overridden in map
  test/app_web/controllers/item_controller_test.exs:149

..........warning: module attribute @completed_attrs was set but never used
  test/app_web/controllers/item_controller_test.exs:9

........................
Finished in 0.7 seconds (0.6s async, 0.1s sync)
34 tests, 0 failures

Randomized with seed 76640

[nelsonic]

OK no more warnings:

mix c
..................................
Finished in 0.6 seconds (0.5s async, 0.07s sync)
34 tests, 0 failures

Randomized with seed 910810
----------------
COV    FILE                                        LINES RELEVANT   MISSED
100.0% lib/app/todo.ex                               112        7        0
100.0% lib/app/todo/item.ex                           19        2        0
100.0% lib/app_web/controllers/auth_controller.       14        2        0
100.0% lib/app_web/controllers/error_html.ex          19        1        0
100.0% lib/app_web/controllers/error_json.ex          15        1        0
 97.8% lib/app_web/controllers/item_controller.      123       45        1
100.0% lib/app_web/controllers/item_html.ex           41       13        0
  0.0% lib/app_web/gettext.ex                         24        0        0
100.0% lib/app_web/router.ex                          35        8        0
[TOTAL]  98.7%
----------------
Generating report...

But still missing that crucial last line of code coverage to reach 100% ... 💭
Going to add the missing test now. 🧑‍💻

[LuchoTurtle]

But still missing that crucial last line of code coverage to reach 100% ... 💭

This fix might entail tinkering with the endpoints at router.ex and changing a few tests. I had to deal with this because of #65 when working on dwyl/phoenix-papertrail-demo#3

[nelsonic]

Final thing is to update the GIF in the README.md ...

"It's the little details that are vital.
Little things make big things happen."
~ John Wooden

[nelsonic]

Then I started re-reading auth.md and see code blocks like this:

case Map.has_key?(conn.assigns, :person) do
  false ->
    items = Todo.list_items()
    changeset = Todo.change_item(item)

    render(conn, "index.html",
      items: items,
      changeset: changeset,
      editing: item,
      filter: Map.get(params, "filter", "all")
    )

  true ->
    person_id = Map.get(conn.assigns.person, :id)

    items = Todo.list_items(person_id)
    changeset =

    render(conn, "index.html",
      items: Todo.list_items(person_id),
      changeset: Todo.change_item(item),
      editing: item,
      filter: Map.get(params, "filter", "all")
    )
end

DRYed to:

render(conn, "index.html",
  items: Todo.list_items(Useful.get_in_default(conn, [:assigns, :person, :id], 0)),
  changeset: Todo.change_item(item),
  editing: item,
  filter: Map.get(params, "filter", "all")
)

[nelsonic]

@LuchoTurtle great work. Thanks. 🎉