Update Dependencies > Fix Failing Tests #83

nelsonic · 2020-07-14T09:46:33Z

3 of our dependencies are out-of-date because we published new versions of fields and auth_plugand a new version ofplug_cowboy` was released. https://libraries.io/hex/auth

Todo

Update dependencies before working on RBAC Research and Writeup RBAC Systems #82 so we have a clean slate.
Fix failing tests ⬇️ Update Dependencies > Fix Failing Tests #83 (comment)

The text was updated successfully, but these errors were encountered:

nelsonic · 2020-07-14T10:16:49Z

auth_plug had a breaking change where AuthPlug.session_options/0 was removed in https://github.com/dwyl/auth_plug/pull/20/files#diff-1560d1759954bb80cc81c6769976610fL68

So now we need to a bit of refactoring in auth to not invoke the unavailable function.

th0mas · 2020-07-14T11:02:49Z

Oops! Was there a particular reason why `Auth` was getting its session configuration from `auth-plug`?

…

On 14 Jul 2020, at 11:17, Nelson ***@***.***> wrote: auth_plug had a breaking change where AuthPlug.session_options/0 was removed in https://github.com/dwyl/auth_plug/pull/20/files#diff-1560d1759954bb80cc81c6769976610fL68 <https://github.com/dwyl/auth_plug/pull/20/files#diff-1560d1759954bb80cc81c6769976610fL68> <https://user-images.githubusercontent.com/194400/87414288-79bdd500-c5c3-11ea-8a4f-bbf060525c34.png> So now we need to a bit of refactoring in auth to not invoke the unavailable function. <https://user-images.githubusercontent.com/194400/87413611-6c541b00-c5c2-11ea-86b8-820eb8c07ff9.png> — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#83 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACFLAIDFPOKHGBXMADYZWJLR3QWCDANCNFSM4OZK7XDQ>.

nelsonic · 2020-07-14T11:35:34Z

@th0mas not really. it was just a way of keeping the auth App consistent with any app using auth_plug. ♻️
I figured that by using auth_plug in auth I would immediately know if there was a breaking change. 💭
In this case, there is, so now we get to do a bit more refactoring. 🔍 😉
The starting point is: dwyl/auth_plug_example#4
I'm happy to do this as it's a low-hanging fruit (chore) that I have all the environment variables setup for.
I will assign the PR(s) to you though. 👍

nelsonic · 2020-07-15T15:59:01Z

After I made the same update to the lib/auth_web/endpoint.ex as in https://github.com/dwyl/auth_plug_example/pull/6/files#r454414361 the project compiles.
Unfortunately, 12 tests are failing with a similar-looking error:

  1) test delete apikey deletes chosen apikey (AuthWeb.ApikeyControllerTest)
     test/auth_web/controllers/apikey_controller_test.exs:204
     ** (ArgumentError) session not fetched, call fetch_session/2
     code: conn = AuthPlug.create_jwt_session(conn, person)
     stacktrace:
       (plug 1.10.3) lib/plug/conn.ex:1568: Plug.Conn.get_session/1
       (plug 1.10.3) lib/plug/conn.ex:1727: Plug.Conn.put_session/2
       (auth_plug 1.2.0) lib/auth_plug.ex:92: AuthPlug.create_session/3
       test/auth_web/controllers/apikey_controller_test.exs:206: (test)

Looking into it now.

nelsonic · 2020-07-16T09:24:23Z

It's not just the tests that fail on localhost, when I run the App it compiles but then Google Auth Fails:

If I downgrade auth_plug 1.2.0 => 1.1.1 and (and revert the change in endpoint.ex)
then re-run the tests they all pass (as expected).
But the Google Auth still fails:

🤦

From the stack trace I suspect this is a Cowboy issue (using the incorrect version of TLS).
So that's something else I will need to look at once the tests are passing with auth_plug@v1.2.0. 🙄

Note: I want to get onto RBAC ASAP as I know it's blocking progress in dwyl/smart-home-auth-server#1
but I want to have a solid foundation (known working state) before adding anything new.

nelsonic · 2020-07-16T23:18:03Z

Still getting the error ... but this looks like a promising line of enquiry: https://elixirforum.com/t/erlang-v-23-0-and-httpoison-error/31615/7

nelsonic · 2020-07-17T10:02:15Z

On heroku the error is even less helpful/friendly:

nelsonic · 2020-07-17T10:42:04Z

Ok after updating all the underlying dependencies, and making a few tweeks to the code,
we are back at a known state with Auth.

However I still need to update to the latest version of auth_plug again and continue debugging that.

nelsonic · 2020-07-17T11:15:20Z

OK, back to auth_plug@1.2.0 and debugging the error:

 12) test admin/2 show welcome page (AuthWeb.AuthControllerTest)
     test/auth_web/controllers/auth_controller_test.exs:16
     ** (ArgumentError) session not fetched, call fetch_session/2
     code: conn = AuthPlug.create_jwt_session(conn, Map.merge(data, %{id: person.id}))
     stacktrace:
       (plug 1.10.3) lib/plug/conn.ex:1568: Plug.Conn.get_session/1
       (plug 1.10.3) lib/plug/conn.ex:1727: Plug.Conn.put_session/2
       (auth_plug 1.2.0) lib/auth_plug.ex:92: AuthPlug.create_session/3
       test/auth_web/controllers/auth_controller_test.exs:25: (test)

.......

Finished in 17.8 seconds
1 property, 45 tests, 12 failures

The session needs to be started at the beginning of the request lifecycle to avoid seeing the error:

** (ArgumentError) session not fetched, call fetch_session/2

That's why we had:

auth/lib/auth_web/endpoint.ex

Line 40 in 946f6ba

plug Plug.Session, AuthPlug.session_options()

I'm going to keep digging. ⏳

nelsonic · 2020-07-17T11:20:57Z

Ok, after a bit of digging, it turns out a new helper method was added to Plug.Test which we can use:
init_test_session/2 discovered via: phoenixframework/phoenix#861 (comment)

If we update a test from:

test "lists all apikeys", %{conn: conn} do
  person = Auth.Person.get_person_by_email(@email)
  conn = AuthPlug.create_jwt_session(conn, %{email: @email, id: person.id})
  conn = get(conn, Routes.apikey_path(conn, :index))
  assert html_response(conn, 200) =~ "Auth API Keys"
end

To:

test "lists all apikeys", %{conn: conn} do
  person = Auth.Person.get_person_by_email(@email)
  conn = conn
  |> init_test_session(foo: "bar") # auth/issues/83
  |> AuthPlug.create_jwt_session(%{email: @email, id: person.id})
  |> get(Routes.apikey_path(conn, :index))
  assert html_response(conn, 200) =~ "Auth API Keys"
end

It passes!! 🎉
It's an extra line of code (in each of the 12 tests) but I'm fine with that because it reduces the number of steps to setup auth_plug as noted in: https://github.com/dwyl/auth_plug/pull/20/files#diff-04c6e90faac2675aa89e2176d2eec7d8

Going to make the changes to all the failing tests and see how far I get. 🤞

nelsonic · 2020-07-17T11:40:29Z

Applied the update to all 12 tests across 2 files. Works: 🙌

…(comment) ...)

… (comment)

th0mas · 2020-07-17T11:44:49Z

init_test_session is how we setup the session in auth_plug as well:

    setup %{endpoint: endpoint} do
      test_conn =
        conn(:get, "endpoint")
        |> init_test_session(%{})

      {:ok, conn: test_conn}

th0mas · 2020-07-17T11:47:11Z

To reduce LOC in tests, you can move init_test_session/1 to the conn_case test helper, this is the strategy used in dwyl/smart_home_auth:

https://github.com/dwyl/smart-home-auth-server/blob/420ff63a009a0dbed7a17b23ea9f05ef85fbed9e/test/support/conn_case.ex#L43

nelsonic · 2020-07-17T11:47:32Z

@th0mas yeah, seen that now. ✅
https://github.com/dwyl/auth_plug/pull/20/files#diff-a8c2fcf60912e310ee9939d699ab98d4

nelsonic · 2020-07-17T11:52:00Z

@th0mas putting init_test_session/1 in the conn_case is much nicer. Thanks! 👍

@th0mas

… as suggested by @th0mas in #83 (comment)

#83

…#83

nelsonic · 2020-07-17T12:11:55Z

@th0mas if you don't mind, please review/merg the PR: #84 Thanks! 🌻

Update Dependencies issue #83

th0mas · 2020-07-17T12:14:10Z

Done & merged

nelsonic added enhancement New feature or enhancement of existing functionality chore a tedious but necessary task often paying technical debt T25m Time Estimate 25 Minutes labels Jul 14, 2020

nelsonic self-assigned this Jul 14, 2020

nelsonic added the in-progress An issue or pull request that is being worked on by the assigned person label Jul 14, 2020

nelsonic mentioned this issue Jul 14, 2020

Update to Latest Version of auth_plug dwyl/auth_plug_example#4

Closed

4 tasks

nelsonic mentioned this issue Jul 16, 2020

[Epic] Roles, Permissions and Grants #31

Closed

1 task

nelsonic changed the title ~~Update Dependencies~~ Update Dependencies > Fix Failing Tests Jul 16, 2020

nelsonic mentioned this issue Jul 16, 2020

MatchError Google OAuth Fails due to TLS Version in Cowboy 🤦 dwyl/elixir-auth-google-demo#3

Closed

This was referenced Jul 16, 2020

Fix TLS Error issue #3 dwyl/elixir-auth-google-demo#4

Merged

Update httpoison to 1.7.0 to avoid TLS Error dwyl/elixir-auth-google#38

Closed

Update elixir-auth-github and Phoenix dependencies to latest versions dwyl/elixir-auth-github-demo#3

Closed

nelsonic added a commit that referenced this issue Jul 17, 2020

update dependencies to latest versions for #83 (tests fail! see: #83 …

02ee20d

…(comment) ...)

nelsonic added a commit that referenced this issue Jul 17, 2020

update tests to use Plug.Test.init_test_session/2 as described in #83…

a0ea2b5

… (comment)

nelsonic added a commit that referenced this issue Jul 17, 2020

update to auth_plug v1.2.0 #83

1d54de7

nelsonic added a commit that referenced this issue Jul 17, 2020

move Plug.Test.init_test_session/2 invocation to ConnCase test helper…

631c4c8

… as suggested by @th0mas in #83 (comment)

nelsonic added a commit that referenced this issue Jul 17, 2020

bump version in mix.exs to 1.2.4 to publish new version (prior to RBAC)

3954b1d

#83

nelsonic added a commit that referenced this issue Jul 17, 2020

simplify remaining tests to use init_test_session/2 in ConnCase helper …

fea04e3

…#83

nelsonic mentioned this issue Jul 17, 2020

Update Dependencies issue #83 #84

Merged

3 tasks

th0mas added a commit that referenced this issue Jul 17, 2020

Merge pull request #84 from dwyl/update-dependencies-issue#83

fafc5e0

Update Dependencies issue #83

th0mas closed this as completed Jul 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Dependencies > Fix Failing Tests #83

Update Dependencies > Fix Failing Tests #83

nelsonic commented Jul 14, 2020 •

edited

Loading

nelsonic commented Jul 14, 2020

th0mas commented Jul 14, 2020 via email

nelsonic commented Jul 14, 2020

nelsonic commented Jul 15, 2020

nelsonic commented Jul 16, 2020

nelsonic commented Jul 16, 2020

nelsonic commented Jul 17, 2020

nelsonic commented Jul 17, 2020

nelsonic commented Jul 17, 2020

nelsonic commented Jul 17, 2020

nelsonic commented Jul 17, 2020

th0mas commented Jul 17, 2020

th0mas commented Jul 17, 2020

nelsonic commented Jul 17, 2020

nelsonic commented Jul 17, 2020

nelsonic commented Jul 17, 2020

th0mas commented Jul 17, 2020

Update Dependencies > Fix Failing Tests #83

Update Dependencies > Fix Failing Tests #83

Comments

nelsonic commented Jul 14, 2020 • edited Loading

Todo

nelsonic commented Jul 14, 2020

th0mas commented Jul 14, 2020 via email

nelsonic commented Jul 14, 2020

nelsonic commented Jul 15, 2020

nelsonic commented Jul 16, 2020

nelsonic commented Jul 16, 2020

nelsonic commented Jul 17, 2020

nelsonic commented Jul 17, 2020

nelsonic commented Jul 17, 2020

nelsonic commented Jul 17, 2020

nelsonic commented Jul 17, 2020

th0mas commented Jul 17, 2020

th0mas commented Jul 17, 2020

nelsonic commented Jul 17, 2020

nelsonic commented Jul 17, 2020

nelsonic commented Jul 17, 2020

th0mas commented Jul 17, 2020

nelsonic commented Jul 14, 2020 •

edited

Loading