From 89cccb5f962b1470d8fe4adb64d703b716c7d707 Mon Sep 17 00:00:00 2001 From: ning Date: Tue, 12 Jun 2018 16:00:20 +0800 Subject: [PATCH 01/39] Move context files folders with the same name As per the standard phoenix 1.3.0 structure. http://phoenixframework.org/blog/phoenix-1-3-0-released --- lib/cadet/{ => accounts}/accounts.ex | 0 lib/cadet/{ => assessments}/assessments.ex | 0 lib/cadet/{ => course}/course.ex | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename lib/cadet/{ => accounts}/accounts.ex (100%) rename lib/cadet/{ => assessments}/assessments.ex (100%) rename lib/cadet/{ => course}/course.ex (100%) diff --git a/lib/cadet/accounts.ex b/lib/cadet/accounts/accounts.ex similarity index 100% rename from lib/cadet/accounts.ex rename to lib/cadet/accounts/accounts.ex diff --git a/lib/cadet/assessments.ex b/lib/cadet/assessments/assessments.ex similarity index 100% rename from lib/cadet/assessments.ex rename to lib/cadet/assessments/assessments.ex diff --git a/lib/cadet/course.ex b/lib/cadet/course/course.ex similarity index 100% rename from lib/cadet/course.ex rename to lib/cadet/course/course.ex From 4a9d700ae99e48b9b4e3cf0ca87e9ac629c949e4 Mon Sep 17 00:00:00 2001 From: ning Date: Tue, 12 Jun 2018 18:12:36 +0800 Subject: [PATCH 02/39] Change user fields to include NUSNET ID --- lib/cadet/accounts/authorization.ex | 2 +- lib/cadet/accounts/form/registration.ex | 11 ++++--- lib/cadet/accounts/user.ex | 11 ++++--- lib/cadet/factory.ex | 3 +- .../20180612084218_add_ivle_to_accounts.exs | 12 ++++++++ priv/repo/seeds.exs | 14 ++++----- .../cadet/accounts/form/registration_test.exs | 3 +- test/cadet/accounts/user_test.exs | 8 ++--- test/cadet/accounts_test.exs | 30 ++++++++++--------- 9 files changed, 54 insertions(+), 40 deletions(-) create mode 100644 priv/repo/migrations/20180612084218_add_ivle_to_accounts.exs diff --git a/lib/cadet/accounts/authorization.ex b/lib/cadet/accounts/authorization.ex index fb3493ea1..2850afbd0 100644 --- a/lib/cadet/accounts/authorization.ex +++ b/lib/cadet/accounts/authorization.ex @@ -1,7 +1,7 @@ defmodule Cadet.Accounts.Authorization do @moduledoc """ The User entity represents a user. - It stores basic information such as first name, last name, and e-mail. + It stores basic information such as name, NUSNET ID, and e-mail. Each user is associated to one `role` which determines the access level of the user. """ diff --git a/lib/cadet/accounts/form/registration.ex b/lib/cadet/accounts/form/registration.ex index e697439ea..3ca965401 100644 --- a/lib/cadet/accounts/form/registration.ex +++ b/lib/cadet/accounts/form/registration.ex @@ -2,7 +2,7 @@ defmodule Cadet.Accounts.Form.Registration do @moduledoc """ The Accounts.Form entity represents an entry from an accounts form. A registration form contains the same information as the User and Authorization - entity, including first name, last name, e-mail, password and password + entity, including name, NUSNET ID, e-mail, password and password confirmation. """ use Ecto.Schema @@ -10,21 +10,20 @@ defmodule Cadet.Accounts.Form.Registration do import Ecto.Changeset embedded_schema do - field(:first_name, :string) - field(:last_name, :string) + field(:name, :string) + field(:nusnet_id, :string) field(:email, :string) field(:password, :string) field(:password_confirmation, :string) end - @required_fields ~w(first_name email password password_confirmation)a - @optional_fields ~w(last_name)a + @required_fields ~w(name nusnet_id email password password_confirmation)a @email_format ~r/^[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,4}$/ def changeset(registration, params \\ %{}) do registration - |> cast(params, @required_fields ++ @optional_fields) + |> cast(params, @required_fields) |> validate_required(@required_fields) |> validate_format(:email, @email_format) |> validate_length(:password, min: 8) diff --git a/lib/cadet/accounts/user.ex b/lib/cadet/accounts/user.ex index 8a2791241..c1dcc102e 100644 --- a/lib/cadet/accounts/user.ex +++ b/lib/cadet/accounts/user.ex @@ -1,7 +1,7 @@ defmodule Cadet.Accounts.User do @moduledoc """ The User entity represents a user. - It stores basic information such as first name, last name, and e-mail. + It stores basic information such as name, NUSNET ID, and e-mail. Each user is associated to one `role` which determines the access level of the user. """ @@ -10,19 +10,18 @@ defmodule Cadet.Accounts.User do alias Cadet.Accounts.Role schema "users" do - field(:first_name, :string) - field(:last_name, :string) + field(:name, :string) + field(:nusnet_id, :string) field(:role, Role) timestamps() end - @required_fields ~w(first_name role)a - @optional_fields ~w(last_name)a + @required_fields ~w(name nusnet_id role)a def changeset(user, params \\ %{}) do user - |> cast(params, @required_fields ++ @optional_fields) + |> cast(params, @required_fields) |> validate_required(@required_fields) |> validate_inclusion(:role, Role.__valid_values__()) end diff --git a/lib/cadet/factory.ex b/lib/cadet/factory.ex index 33cad1673..83c600c5c 100644 --- a/lib/cadet/factory.ex +++ b/lib/cadet/factory.ex @@ -15,7 +15,8 @@ defmodule Cadet.Factory do def user_factory do %User{ - first_name: "John Smith", + name: "John Smith", + nusnet_id: "E345678", role: :student } end diff --git a/priv/repo/migrations/20180612084218_add_ivle_to_accounts.exs b/priv/repo/migrations/20180612084218_add_ivle_to_accounts.exs new file mode 100644 index 000000000..bb291b63d --- /dev/null +++ b/priv/repo/migrations/20180612084218_add_ivle_to_accounts.exs @@ -0,0 +1,12 @@ +defmodule Cadet.Repo.Migrations.AddIvleToAccounts do + use Ecto.Migration + + def change do + alter table(:users) do + remove(:first_name) + remove(:last_name) + add(:name, :string) + add(:nusnet_id, :string) + end + end +end diff --git a/priv/repo/seeds.exs b/priv/repo/seeds.exs index 7de8c74fe..7f8161226 100644 --- a/priv/repo/seeds.exs +++ b/priv/repo/seeds.exs @@ -14,18 +14,18 @@ import Cadet.Factory if Application.get_env(:cadet, :environment) == :dev do seeded_users = [ %{ - first_name: "Test", - last_name: "Student", + name: "TestStudent", + nusnet_id: "E012345", role: :student }, %{ - first_name: "Test", - last_name: "Staff", + name: "TestStaff", + nusnet_id: "E123456", role: :staff }, %{ - first_name: "Test", - last_name: "Admin", + name: "TestAdmin", + nusnet_id: "E234567", role: :admin } ] @@ -34,7 +34,7 @@ if Application.get_env(:cadet, :environment) == :dev do user = insert(:user, attr) insert(:email, %{ - uid: attr.first_name <> attr.last_name <> "@test.com", + uid: attr.name <> "@test.com", token: Pbkdf2.hash_pwd_salt("password"), user: user }) diff --git a/test/cadet/accounts/form/registration_test.exs b/test/cadet/accounts/form/registration_test.exs index 7fcf621f3..80a3e45f8 100644 --- a/test/cadet/accounts/form/registration_test.exs +++ b/test/cadet/accounts/form/registration_test.exs @@ -5,7 +5,8 @@ defmodule Cadet.Accounts.Form.RegistrationTest do valid_changesets Registration do %{ - first_name: "happy", + name: "happy", + nusnet_id: "e853820", email: "some@gmail.com", password: "mypassword", password_confirmation: "mypassword" diff --git a/test/cadet/accounts/user_test.exs b/test/cadet/accounts/user_test.exs index 16630fe5e..8c6cb30af 100644 --- a/test/cadet/accounts/user_test.exs +++ b/test/cadet/accounts/user_test.exs @@ -4,12 +4,12 @@ defmodule Cadet.Accounts.UserTest do alias Cadet.Accounts.User valid_changesets User do - %{first_name: "happy people", role: :admin} - %{first_name: "happy", last_name: "people", role: :student} + %{name: "happy people", nusnet_id: "e123456", role: :admin} + %{name: "happy", nusnet_id: "e438492", role: :student} end invalid_changesets User do - %{last_name: "people", role: :student} - %{first_name: "happy", last_name: "people", role: :avenger} + %{name: "people", role: :student} + %{name: "happy", nusnet_id: "e8493201", role: :avenger} end end diff --git a/test/cadet/accounts_test.exs b/test/cadet/accounts_test.exs index 6303ed307..064a94161 100644 --- a/test/cadet/accounts_test.exs +++ b/test/cadet/accounts_test.exs @@ -6,21 +6,21 @@ defmodule Cadet.AccountsTest do test "create user" do {:ok, user} = Accounts.create_user(%{ - first_name: "happy", - last_name: "user", + name: "happy user", + nusnet_id: "e948329", role: :student }) - assert user.first_name == "happy" - assert user.last_name == "user" + assert user.name == "happy user" + assert user.nusnet_id == "e948329" assert user.role == :student end test "invalid create user" do {:error, changeset} = Accounts.create_user(%{ - first_name: "happy", - last_name: "user", + name: "happy user", + nusnet_id: "e483921", role: :unknown }) @@ -28,9 +28,9 @@ defmodule Cadet.AccountsTest do end test "get existing user" do - user = insert(:user, first_name: "Teddy") + user = insert(:user, name: "Teddy") result = Accounts.get_user(user.id) - assert result.first_name == "Teddy" + assert result.name == "Teddy" end test "get unknown user" do @@ -91,21 +91,22 @@ defmodule Cadet.AccountsTest do test "valid registration" do attrs = %{ - first_name: "Test", - last_name: "Name", + name: "Test Name", + nusnet_id: "e948203", email: "test@gmail.com", password: "somepassword", password_confirmation: "somepassword" } assert {:ok, user} = Accounts.register(attrs, :student) - assert user.first_name == "Test" - assert user.last_name == "Name" + assert user.name == "Test Name" + assert user.nusnet_id == "e948203" end test "register using invalid email format" do attrs = %{ - first_name: "Test", + name: "Test", + nusnet_id: "e948293", email: "testgmail.com", password: "somepassword", password_confirmation: "somepassword" @@ -117,7 +118,8 @@ defmodule Cadet.AccountsTest do test "register password confirmation does not match" do attrs = %{ - first_name: "Test", + name: "Test", + nusnet_id: "e839182", email: "test@gmail.com", password: "somepassword2", password_confirmation: "somepassword" From fac912629b2dd3aa2e2b5e7a30c16d566a5aa649 Mon Sep 17 00:00:00 2001 From: ning Date: Wed, 13 Jun 2018 11:58:57 +0800 Subject: [PATCH 03/39] Change /auth to take ivle_token (was email, pw) --- lib/cadet/accounts/form/login.ex | 7 +-- lib/cadet_web/controllers/auth_controller.ex | 24 ++++++-- test/cadet/accounts/form/login_test.exs | 6 +- .../controllers/auth_controller_test.exs | 61 +------------------ 4 files changed, 28 insertions(+), 70 deletions(-) diff --git a/lib/cadet/accounts/form/login.ex b/lib/cadet/accounts/form/login.ex index 3a31ca654..f76edda27 100644 --- a/lib/cadet/accounts/form/login.ex +++ b/lib/cadet/accounts/form/login.ex @@ -1,18 +1,17 @@ defmodule Cadet.Accounts.Form.Login do @moduledoc """ The Accounts.Form entity represents an entry from an accounts form. - A login form comprises of e-mail and password + A login form comprises of an IVLE authentication token. """ use Ecto.Schema import Ecto.Changeset embedded_schema do - field(:email, :string) - field(:password, :string) + field(:ivle_token, :string) end - @required_fields ~w(email password)a + @required_fields ~w(ivle_token)a def changeset(login, params \\ %{}) do login diff --git a/lib/cadet_web/controllers/auth_controller.ex b/lib/cadet_web/controllers/auth_controller.ex index 3ab6cf416..bb79d082e 100644 --- a/lib/cadet_web/controllers/auth_controller.ex +++ b/lib/cadet_web/controllers/auth_controller.ex @@ -73,7 +73,12 @@ defmodule CadetWeb.AuthController do summary("Obtain access and refresh tokens to authenticate user.") description( - "When accessing resources, pass the access token in the Authorization HTTP header using the Bearer schema: `Authorization: Bearer `. The access token expires 1 hour after issuance while the refresh token expires 1 year after issuance. When access token expires, the refresh token can be used to obtain a new access token." + "Get a set of access and refresh tokens, using the authentication token " <> + "from IVLE. When accessing resources, pass the access token in the " <> + "Authorization HTTP header using the Bearer schema: `Authorization: " <> + "Bearer `. The access token expires 1 hour after issuance while " <> + "the refresh token expires 1 year after issuance. When access token " <> + "expires, the refresh token can be used to obtain a new access token. " ) consumes("application/json") @@ -133,8 +138,7 @@ defmodule CadetWeb.AuthController do login( Schema.new do properties do - email(:string, "Email of user", required: true) - password(:string, "Password of user", required: true) + ivle_token(:string, "IVLE authentication token", required: true) end end ) @@ -142,7 +146,19 @@ defmodule CadetWeb.AuthController do required(:login) - example(%{login: %{email: "TestAdmin@test.com", password: "password"}}) + example(%{ + login: %{ + ivle_token: + "058DA4D1692CEA834A9311G704BA438P9BA2E1829D3N1B5F39F25556FBDB2B" <> + "0FA7B08361C77A75127908704BF2CIDC034F7N4B1217441412B0E3CB5B544E" <> + "EBP2ED8D0D2ABAF2F6A021B7F4GE5F648F64E02B3E36B1V755CC776EEAE38C" <> + "D58D46D1493426C4BC17F276L4E74C835C2C5338C01APFF1DE580D3D559A9A" <> + "7FB3013A0FE7DED7ADC45654ABB5C170460F308F42UECF2D76F2CCC0B21B1F" <> + "IE5B5892D398F4670658V87A6DBA1E16F64AEEB8PD51B1FD7C858F8BECE8G4" <> + "E62DD0EB54F761C1F6T0290FABC27AEB1B707FB4BD1B466C32CE08FDAEB25B" <> + "D9B6F3D75CE9A086ACBD72641EBCC1E3A3A7WA82FDFA8D" + } + }) end, Tokens: swagger_schema do diff --git a/test/cadet/accounts/form/login_test.exs b/test/cadet/accounts/form/login_test.exs index e074f2cc2..d2dd7fedb 100644 --- a/test/cadet/accounts/form/login_test.exs +++ b/test/cadet/accounts/form/login_test.exs @@ -4,11 +4,11 @@ defmodule Cadet.Accounts.LoginTest do alias Cadet.Accounts.Form.Login valid_changesets Login do - %{email: "some@gmail.com", password: "somepassword"} + %{ivle_token: "T0K3N"} end invalid_changesets Login do - %{email: "", password: "somepassword"} - %{email: "some@gmail.com", password: ""} + %{ivle_token: ""} + %{} end end diff --git a/test/cadet_web/controllers/auth_controller_test.exs b/test/cadet_web/controllers/auth_controller_test.exs index 622c4584e..da46898da 100644 --- a/test/cadet_web/controllers/auth_controller_test.exs +++ b/test/cadet_web/controllers/auth_controller_test.exs @@ -20,71 +20,14 @@ defmodule CadetWeb.AuthControllerTest do assert response(conn, 400) end - test "blank email", %{conn: conn} do + test "blank token", %{conn: conn} do conn = post(conn, "/v1/auth", %{ - "login" => %{"email" => "", "password" => "password"} + "login" => %{"ivle_token" => ""} }) assert response(conn, 400) end - - test "blank password", %{conn: conn} do - conn = - post(conn, "/v1/auth", %{ - "login" => %{"email" => "test@test.com", "password" => ""} - }) - - assert response(conn, 400) - end - - test "email not found", %{conn: conn} do - conn = - post(conn, "/v1/auth", %{ - "login" => %{"email" => "unknown@test.com", "password" => "password"} - }) - - assert response(conn, 403) - end - - test "invalid password", %{conn: conn} do - test_password = "password" - user = insert(:user) - - email = - insert(:email, %{ - token: Pbkdf2.hash_pwd_salt(test_password), - user: user - }) - - conn = - post(conn, "/v1/auth", %{ - "login" => %{"email" => email.uid, "password" => "password2"} - }) - - assert response(conn, 403) - end - - test "valid email and password", %{conn: conn} do - test_password = "password" - user = insert(:user) - - email = - insert(:email, %{ - token: Pbkdf2.hash_pwd_salt(test_password), - user: user - }) - - conn = - post(conn, "/v1/auth", %{ - "login" => %{ - "email" => email.uid, - "password" => test_password - } - }) - - assert json_response(conn, 200) - end end describe "POST /auth/refresh" do From 2a2d8ba4a26bc291efd0808f26a3e612f6e303cc Mon Sep 17 00:00:00 2001 From: ning Date: Wed, 13 Jun 2018 12:55:02 +0800 Subject: [PATCH 04/39] Add HTTPoison dependency for requests to IVLE --- README.md | 6 +++++- mix.exs | 33 +++++++++++++++++---------------- mix.lock | 50 +++++++++++++++++++++++++------------------------- 3 files changed, 47 insertions(+), 42 deletions(-) diff --git a/README.md b/README.md index 16d757b73..7abbc93de 100644 --- a/README.md +++ b/README.md @@ -21,10 +21,14 @@ Install Elixir dependencies mix deps.get -Initialise Development Database +Initialise development database mix ecto.setup +Setup environment variables + + export IVLE_KEY="aB1de9g" + Run the server in your local machine mix cadet.server diff --git a/mix.exs b/mix.exs index 5b9fbe7c0..98afc8385 100644 --- a/mix.exs +++ b/mix.exs @@ -41,30 +41,31 @@ defmodule Cadet.Mixfile do # Type `mix help deps` for examples and options. defp deps do [ - {:phoenix, "~> 1.3.0"}, - {:phoenix_pubsub, "~> 1.0"}, - {:phoenix_ecto, "~> 3.2"}, - {:postgrex, ">= 0.0.0"}, - {:phoenix_html, "~> 2.10"}, - {:gettext, "~> 0.11"}, + {:arc, "~> 0.8.0"}, + {:arc_ecto, "~> 0.7.0"}, + {:comeonin, "~> 4.0"}, {:cowboy, "~> 1.0"}, {:dotenv, "~> 2.0.0"}, {:ecto_enum, "~> 1.0"}, + {:ex_json_schema, "~> 0.5"}, {:ex_machina, "~> 2.1"}, - {:pbkdf2_elixir, "~> 0.12"}, - {:comeonin, "~> 4.0"}, + {:gettext, "~> 0.11"}, {:guardian, "~> 1.0"}, {:guardian_db, "~> 1.0"}, - {:phoenix_live_reload, "~> 1.0", only: :dev}, - {:credo, "~> 0.8", only: [:dev, :test], runtime: false}, - {:excoveralls, "~> 0.8", only: :test}, - {:dialyxir, "~> 1.0.0-rc.2", only: [:dev, :test], runtime: false}, - {:arc, "~> 0.8.0"}, - {:arc_ecto, "~> 0.7.0"}, + {:httpoison, "~> 1.0", override: true}, + {:pbkdf2_elixir, "~> 0.12"}, + {:phoenix, "~> 1.3.0"}, + {:phoenix_ecto, "~> 3.2"}, + {:phoenix_html, "~> 2.10"}, + {:phoenix_pubsub, "~> 1.0"}, + {:phoenix_swagger, "~> 0.8"}, + {:postgrex, ">= 0.0.0"}, {:timex, "~> 3.0"}, {:timex_ecto, "~> 3.0"}, - {:phoenix_swagger, "~> 0.8"}, - {:ex_json_schema, "~> 0.5"}, + {:credo, "~> 0.8", only: [:dev, :test], runtime: false}, + {:dialyxir, "~> 1.0.0-rc.2", only: [:dev, :test], runtime: false}, + {:excoveralls, "~> 0.8", only: :test}, + {:phoenix_live_reload, "~> 1.0", only: :dev}, {:pre_commit, "~> 0.3.4", only: [:dev, :test]} ] end diff --git a/mix.lock b/mix.lock index 7105ac397..a102934cf 100644 --- a/mix.lock +++ b/mix.lock @@ -3,52 +3,52 @@ "arc_ecto": {:hex, :arc_ecto, "0.7.0", "428ed7ea4ed0be8996d8fc6e9f651d3e26fe9c9063f26cbf6731c865369845d3", [:mix], [{:arc, "~> 0.8.0", [hex: :arc, repo: "hexpm", optional: false]}, {:ecto, "~> 2.0", [hex: :ecto, repo: "hexpm", optional: false]}], "hexpm"}, "base64url": {:hex, :base64url, "0.0.1", "36a90125f5948e3afd7be97662a1504b934dd5dac78451ca6e9abf85a10286be", [:rebar], [], "hexpm"}, "bunt": {:hex, :bunt, "0.2.0", "951c6e801e8b1d2cbe58ebbd3e616a869061ddadcc4863d0a2182541acae9a38", [:mix], [], "hexpm"}, - "certifi": {:hex, :certifi, "2.0.0", "a0c0e475107135f76b8c1d5bc7efb33cd3815cb3cf3dea7aefdd174dabead064", [:rebar3], [], "hexpm"}, + "certifi": {:hex, :certifi, "2.3.1", "d0f424232390bf47d82da8478022301c561cf6445b5b5fb6a84d49a9e76d2639", [:rebar3], [{:parse_trans, "3.2.0", [hex: :parse_trans, repo: "hexpm", optional: false]}], "hexpm"}, "combine": {:hex, :combine, "0.10.0", "eff8224eeb56498a2af13011d142c5e7997a80c8f5b97c499f84c841032e429f", [:mix], [], "hexpm"}, - "comeonin": {:hex, :comeonin, "4.0.3", "4e257dcb748ed1ca2651b7ba24fdbd1bd24efd12482accf8079141e3fda23a10", [:mix], [{:argon2_elixir, "~> 1.2", [hex: :argon2_elixir, repo: "hexpm", optional: true]}, {:bcrypt_elixir, "~> 0.12.1 or ~> 1.0", [hex: :bcrypt_elixir, repo: "hexpm", optional: true]}, {:pbkdf2_elixir, "~> 0.12", [hex: :pbkdf2_elixir, repo: "hexpm", optional: true]}], "hexpm"}, + "comeonin": {:hex, :comeonin, "4.1.1", "c7304fc29b45b897b34142a91122bc72757bc0c295e9e824999d5179ffc08416", [:mix], [{:argon2_elixir, "~> 1.2", [hex: :argon2_elixir, repo: "hexpm", optional: true]}, {:bcrypt_elixir, "~> 0.12.1 or ~> 1.0", [hex: :bcrypt_elixir, repo: "hexpm", optional: true]}, {:pbkdf2_elixir, "~> 0.12", [hex: :pbkdf2_elixir, repo: "hexpm", optional: true]}], "hexpm"}, "connection": {:hex, :connection, "1.0.4", "a1cae72211f0eef17705aaededacac3eb30e6625b04a6117c1b2db6ace7d5976", [:mix], [], "hexpm"}, "cowboy": {:hex, :cowboy, "1.1.2", "61ac29ea970389a88eca5a65601460162d370a70018afe6f949a29dca91f3bb0", [:rebar3], [{:cowlib, "~> 1.0.2", [hex: :cowlib, repo: "hexpm", optional: false]}, {:ranch, "~> 1.3.2", [hex: :ranch, repo: "hexpm", optional: false]}], "hexpm"}, "cowlib": {:hex, :cowlib, "1.0.2", "9d769a1d062c9c3ac753096f868ca121e2730b9a377de23dec0f7e08b1df84ee", [:make], [], "hexpm"}, - "credo": {:hex, :credo, "0.9.1", "f021affa11b32a94dc2e807a6472ce0914289c9132f99644a97fc84432b202a1", [:mix], [{:bunt, "~> 0.2.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:poison, ">= 0.0.0", [hex: :poison, repo: "hexpm", optional: false]}], "hexpm"}, - "db_connection": {:hex, :db_connection, "1.1.2", "2865c2a4bae0714e2213a0ce60a1b12d76a6efba0c51fbda59c9ab8d1accc7a8", [:mix], [{:connection, "~> 1.0.2", [hex: :connection, repo: "hexpm", optional: false]}, {:poolboy, "~> 1.5", [hex: :poolboy, repo: "hexpm", optional: true]}, {:sbroker, "~> 1.0", [hex: :sbroker, repo: "hexpm", optional: true]}], "hexpm"}, - "decimal": {:hex, :decimal, "1.4.1", "ad9e501edf7322f122f7fc151cce7c2a0c9ada96f2b0155b8a09a795c2029770", [:mix], [], "hexpm"}, + "credo": {:hex, :credo, "0.9.3", "76fa3e9e497ab282e0cf64b98a624aa11da702854c52c82db1bf24e54ab7c97a", [:mix], [{:bunt, "~> 0.2.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:poison, ">= 0.0.0", [hex: :poison, repo: "hexpm", optional: false]}], "hexpm"}, + "db_connection": {:hex, :db_connection, "1.1.3", "89b30ca1ef0a3b469b1c779579590688561d586694a3ce8792985d4d7e575a61", [:mix], [{:connection, "~> 1.0.2", [hex: :connection, repo: "hexpm", optional: false]}, {:poolboy, "~> 1.5", [hex: :poolboy, repo: "hexpm", optional: true]}, {:sbroker, "~> 1.0", [hex: :sbroker, repo: "hexpm", optional: true]}], "hexpm"}, + "decimal": {:hex, :decimal, "1.5.0", "b0433a36d0e2430e3d50291b1c65f53c37d56f83665b43d79963684865beab68", [:mix], [], "hexpm"}, "dialyxir": {:hex, :dialyxir, "1.0.0-rc.2", "1e68d42870c50dac013e8184254f232c32fa4164a4cdc001d723389cdee27b35", [:mix], [], "hexpm"}, "dotenv": {:hex, :dotenv, "2.0.0", "87e77a94e54c20eca49989876395e26878b109f016f9fd9c42b65bb9c59bca3f", [:mix], [], "hexpm"}, - "ecto": {:hex, :ecto, "2.2.6", "3fd1067661d6d64851a0d4db9acd9e884c00d2d1aa41cc09da687226cf894661", [:mix], [{:db_connection, "~> 1.1", [hex: :db_connection, repo: "hexpm", optional: true]}, {:decimal, "~> 1.2", [hex: :decimal, repo: "hexpm", optional: false]}, {:mariaex, "~> 0.8.0", [hex: :mariaex, repo: "hexpm", optional: true]}, {:poison, "~> 2.2 or ~> 3.0", [hex: :poison, repo: "hexpm", optional: true]}, {:poolboy, "~> 1.5", [hex: :poolboy, repo: "hexpm", optional: false]}, {:postgrex, "~> 0.13.0", [hex: :postgrex, repo: "hexpm", optional: true]}, {:sbroker, "~> 1.0", [hex: :sbroker, repo: "hexpm", optional: true]}], "hexpm"}, + "ecto": {:hex, :ecto, "2.2.10", "e7366dc82f48f8dd78fcbf3ab50985ceeb11cb3dc93435147c6e13f2cda0992e", [:mix], [{:db_connection, "~> 1.1", [hex: :db_connection, repo: "hexpm", optional: true]}, {:decimal, "~> 1.2", [hex: :decimal, repo: "hexpm", optional: false]}, {:mariaex, "~> 0.8.0", [hex: :mariaex, repo: "hexpm", optional: true]}, {:poison, "~> 2.2 or ~> 3.0", [hex: :poison, repo: "hexpm", optional: true]}, {:poolboy, "~> 1.5", [hex: :poolboy, repo: "hexpm", optional: false]}, {:postgrex, "~> 0.13.0", [hex: :postgrex, repo: "hexpm", optional: true]}, {:sbroker, "~> 1.0", [hex: :sbroker, repo: "hexpm", optional: true]}], "hexpm"}, "ecto_enum": {:hex, :ecto_enum, "1.1.0", "d44fe2ce6e1c0e907e7c3b6456a69e0f1d662348d8b4e2a662ba312223d8ff62", [:mix], [{:ecto, ">= 2.0.0", [hex: :ecto, repo: "hexpm", optional: false]}, {:mariaex, ">= 0.0.0", [hex: :mariaex, repo: "hexpm", optional: true]}, {:postgrex, ">= 0.0.0", [hex: :postgrex, repo: "hexpm", optional: true]}], "hexpm"}, "ex_json_schema": {:hex, :ex_json_schema, "0.5.7", "14a1bcd716e432badb19e5f54fd0f3f0be47b34d1b5957944702be90d66a6cf6", [:mix], [], "hexpm"}, - "ex_machina": {:hex, :ex_machina, "2.1.0", "4874dc9c78e7cf2d429f24dc3c4005674d4e4da6a08be961ffccc08fb528e28b", [:mix], [{:ecto, "~> 2.1", [hex: :ecto, repo: "hexpm", optional: true]}], "hexpm"}, - "excoveralls": {:hex, :excoveralls, "0.8.0", "99d2691d3edf8612f128be3f9869c4d44b91c67cec92186ce49470ae7a7404cf", [:mix], [{:exjsx, ">= 3.0.0", [hex: :exjsx, repo: "hexpm", optional: false]}, {:hackney, ">= 0.12.0", [hex: :hackney, repo: "hexpm", optional: false]}], "hexpm"}, - "exjsx": {:hex, :exjsx, "4.0.0", "60548841e0212df401e38e63c0078ec57b33e7ea49b032c796ccad8cde794b5c", [:mix], [{:jsx, "~> 2.8.0", [hex: :jsx, repo: "hexpm", optional: false]}], "hexpm"}, - "file_system": {:hex, :file_system, "0.2.2", "7f1e9de4746f4eb8a4ca8f2fbab582d84a4e40fa394cce7bfcb068b988625b06", [:mix], [], "hexpm"}, - "gettext": {:hex, :gettext, "0.13.1", "5e0daf4e7636d771c4c71ad5f3f53ba09a9ae5c250e1ab9c42ba9edccc476263", [:mix], [], "hexpm"}, - "guardian": {:hex, :guardian, "1.0.1", "db0fbaf571c3b874785818b7272eaf5f1ed97a2f9b1f8bc5dc8b0fb8f8f7bb06", [:mix], [{:jose, "~> 1.8", [hex: :jose, repo: "hexpm", optional: false]}, {:phoenix, "~> 1.0 or ~> 1.2 or ~> 1.3", [hex: :phoenix, repo: "hexpm", optional: true]}, {:plug, "~> 1.3.3 or ~> 1.4", [hex: :plug, repo: "hexpm", optional: true]}, {:poison, "~> 2.2 or ~> 3.0", [hex: :poison, repo: "hexpm", optional: false]}, {:uuid, ">= 1.1.1", [hex: :uuid, repo: "hexpm", optional: false]}], "hexpm"}, + "ex_machina": {:hex, :ex_machina, "2.2.0", "fec496331e04fc2db2a1a24fe317c12c0c4a50d2beb8ebb3531ed1f0d84be0ed", [:mix], [{:ecto, "~> 2.1", [hex: :ecto, repo: "hexpm", optional: true]}], "hexpm"}, + "excoveralls": {:hex, :excoveralls, "0.9.1", "14fd20fac51ab98d8e79615814cc9811888d2d7b28e85aa90ff2e30dcf3191d6", [:mix], [{:hackney, ">= 0.12.0", [hex: :hackney, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm"}, + "file_system": {:hex, :file_system, "0.2.5", "a3060f063b116daf56c044c273f65202e36f75ec42e678dc10653056d3366054", [:mix], [], "hexpm"}, + "gettext": {:hex, :gettext, "0.15.0", "40a2b8ce33a80ced7727e36768499fc9286881c43ebafccae6bab731e2b2b8ce", [:mix], [], "hexpm"}, + "guardian": {:hex, :guardian, "1.1.0", "36c1ea356a1bac02bc120c3f91f4f0259c5aa0ee92cee0efe8def5d7401f1921", [:mix], [{:jose, "~> 1.8", [hex: :jose, repo: "hexpm", optional: false]}, {:phoenix, "~> 1.0 or ~> 1.2 or ~> 1.3", [hex: :phoenix, repo: "hexpm", optional: true]}, {:plug, "~> 1.3.3 or ~> 1.4", [hex: :plug, repo: "hexpm", optional: true]}, {:poison, "~> 2.2 or ~> 3.0", [hex: :poison, repo: "hexpm", optional: false]}, {:uuid, ">= 1.1.1", [hex: :uuid, repo: "hexpm", optional: false]}], "hexpm"}, "guardian_db": {:hex, :guardian_db, "1.1.0", "45ab94206cce38f7443dc27de6dc52966ccbdeff65ca1b1f11a6d8f3daceb556", [:mix], [{:ecto, "~> 2.2", [hex: :ecto, repo: "hexpm", optional: false]}, {:guardian, "~> 1.0", [hex: :guardian, repo: "hexpm", optional: false]}, {:postgrex, "~> 0.13", [hex: :postgrex, repo: "hexpm", optional: true]}], "hexpm"}, - "hackney": {:hex, :hackney, "1.10.1", "c38d0ca52ea80254936a32c45bb7eb414e7a96a521b4ce76d00a69753b157f21", [:rebar3], [{:certifi, "2.0.0", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "5.1.0", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "1.0.1", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "1.0.2", [hex: :mimerl, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "1.1.1", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}], "hexpm"}, - "httpoison": {:hex, :httpoison, "0.13.0", "bfaf44d9f133a6599886720f3937a7699466d23bb0cd7a88b6ba011f53c6f562", [:mix], [{:hackney, "~> 1.8", [hex: :hackney, repo: "hexpm", optional: false]}], "hexpm"}, - "idna": {:hex, :idna, "5.1.0", "d72b4effeb324ad5da3cab1767cb16b17939004e789d8c0ad5b70f3cea20c89a", [:rebar3], [{:unicode_util_compat, "0.3.1", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm"}, + "hackney": {:hex, :hackney, "1.12.1", "8bf2d0e11e722e533903fe126e14d6e7e94d9b7983ced595b75f532e04b7fdc7", [:rebar3], [{:certifi, "2.3.1", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "5.1.1", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "1.0.1", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "1.0.2", [hex: :mimerl, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "1.1.1", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}], "hexpm"}, + "httpoison": {:hex, :httpoison, "1.2.0", "2702ed3da5fd7a8130fc34b11965c8cfa21ade2f232c00b42d96d4967c39a3a3", [:mix], [{:hackney, "~> 1.8", [hex: :hackney, repo: "hexpm", optional: false]}], "hexpm"}, + "idna": {:hex, :idna, "5.1.1", "cbc3b2fa1645113267cc59c760bafa64b2ea0334635ef06dbac8801e42f7279c", [:rebar3], [{:unicode_util_compat, "0.3.1", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm"}, + "jason": {:hex, :jason, "1.0.0", "0f7cfa9bdb23fed721ec05419bcee2b2c21a77e926bce0deda029b5adc716fe2", [:mix], [{:decimal, "~> 1.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm"}, "jose": {:hex, :jose, "1.8.4", "7946d1e5c03a76ac9ef42a6e6a20001d35987afd68c2107bcd8f01a84e75aa73", [:mix, :rebar3], [{:base64url, "~> 0.0.1", [hex: :base64url, repo: "hexpm", optional: false]}], "hexpm"}, - "jsx": {:hex, :jsx, "2.8.3", "a05252d381885240744d955fbe3cf810504eb2567164824e19303ea59eef62cf", [:mix, :rebar3], [], "hexpm"}, "metrics": {:hex, :metrics, "1.0.1", "25f094dea2cda98213cecc3aeff09e940299d950904393b2a29d191c346a8486", [:rebar3], [], "hexpm"}, - "mime": {:hex, :mime, "1.1.0", "01c1d6f4083d8aa5c7b8c246ade95139620ef8effb009edde934e0ec3b28090a", [:mix], [], "hexpm"}, + "mime": {:hex, :mime, "1.3.0", "5e8d45a39e95c650900d03f897fbf99ae04f60ab1daa4a34c7a20a5151b7a5fe", [:mix], [], "hexpm"}, "mimerl": {:hex, :mimerl, "1.0.2", "993f9b0e084083405ed8252b99460c4f0563e41729ab42d9074fd5e52439be88", [:rebar3], [], "hexpm"}, + "parse_trans": {:hex, :parse_trans, "3.2.0", "2adfa4daf80c14dc36f522cf190eb5c4ee3e28008fc6394397c16f62a26258c2", [:rebar3], [], "hexpm"}, "pbkdf2_elixir": {:hex, :pbkdf2_elixir, "0.12.3", "6706a148809a29c306062862c803406e88f048277f6e85b68faf73291e820b84", [:mix], [], "hexpm"}, - "phoenix": {:hex, :phoenix, "1.3.0", "1c01124caa1b4a7af46f2050ff11b267baa3edb441b45dbf243e979cd4c5891b", [:mix], [{:cowboy, "~> 1.0", [hex: :cowboy, repo: "hexpm", optional: true]}, {:phoenix_pubsub, "~> 1.0", [hex: :phoenix_pubsub, repo: "hexpm", optional: false]}, {:plug, "~> 1.3.3 or ~> 1.4", [hex: :plug, repo: "hexpm", optional: false]}, {:poison, "~> 2.2 or ~> 3.0", [hex: :poison, repo: "hexpm", optional: false]}], "hexpm"}, + "phoenix": {:hex, :phoenix, "1.3.3", "bafb5fa408d202e8d9f739e781bdb908446a2c1c1e00797c1158918ed55566a4", [:mix], [{:cowboy, "~> 1.0", [hex: :cowboy, repo: "hexpm", optional: true]}, {:phoenix_pubsub, "~> 1.0", [hex: :phoenix_pubsub, repo: "hexpm", optional: false]}, {:plug, "~> 1.3.3 or ~> 1.4", [hex: :plug, repo: "hexpm", optional: false]}, {:poison, "~> 2.2 or ~> 3.0", [hex: :poison, repo: "hexpm", optional: false]}], "hexpm"}, "phoenix_ecto": {:hex, :phoenix_ecto, "3.3.0", "702f6e164512853d29f9d20763493f2b3bcfcb44f118af2bc37bb95d0801b480", [:mix], [{:ecto, "~> 2.1", [hex: :ecto, repo: "hexpm", optional: false]}, {:phoenix_html, "~> 2.9", [hex: :phoenix_html, repo: "hexpm", optional: true]}, {:plug, "~> 1.0", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm"}, - "phoenix_html": {:hex, :phoenix_html, "2.10.4", "d4f99c32d5dc4918b531fdf163e1fd7cf20acdd7703f16f5d02d4db36de803b7", [:mix], [{:plug, "~> 1.0", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm"}, - "phoenix_live_reload": {:hex, :phoenix_live_reload, "1.1.3", "1d178429fc8950b12457d09c6afec247bfe1fcb6f36209e18fbb0221bdfe4d41", [:mix], [{:file_system, "~> 0.2.1 or ~> 0.3", [hex: :file_system, repo: "hexpm", optional: false]}, {:phoenix, "~> 1.0 or ~> 1.2 or ~> 1.3", [hex: :phoenix, repo: "hexpm", optional: false]}], "hexpm"}, + "phoenix_html": {:hex, :phoenix_html, "2.11.2", "86ebd768258ba60a27f5578bec83095bdb93485d646fc4111db8844c316602d6", [:mix], [{:plug, "~> 1.5", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm"}, + "phoenix_live_reload": {:hex, :phoenix_live_reload, "1.1.5", "8d4c9b1ef9ca82deee6deb5a038d6d8d7b34b9bb909d99784a49332e0d15b3dc", [:mix], [{:file_system, "~> 0.2.1 or ~> 0.3", [hex: :file_system, repo: "hexpm", optional: false]}, {:phoenix, "~> 1.0 or ~> 1.2 or ~> 1.3", [hex: :phoenix, repo: "hexpm", optional: false]}], "hexpm"}, "phoenix_pubsub": {:hex, :phoenix_pubsub, "1.0.2", "bfa7fd52788b5eaa09cb51ff9fcad1d9edfeb68251add458523f839392f034c1", [:mix], [], "hexpm"}, "phoenix_swagger": {:hex, :phoenix_swagger, "0.8.0", "8fde6eefa9bab9af5517aca62f04997db87f5a82a499a52800b141ad220dfcb6", [:mix], [{:ex_json_schema, "~> 0.5", [hex: :ex_json_schema, repo: "hexpm", optional: true]}, {:plug, "~> 1.4", [hex: :plug, repo: "hexpm", optional: false]}, {:poison, "~> 2.2 or ~> 3.0", [hex: :poison, repo: "hexpm", optional: false]}], "hexpm"}, - "plug": {:hex, :plug, "1.4.3", "236d77ce7bf3e3a2668dc0d32a9b6f1f9b1f05361019946aae49874904be4aed", [:mix], [{:cowboy, "~> 1.0.1 or ~> 1.1", [hex: :cowboy, repo: "hexpm", optional: true]}, {:mime, "~> 1.0", [hex: :mime, repo: "hexpm", optional: false]}], "hexpm"}, + "plug": {:hex, :plug, "1.6.0", "90d338a44c8cd762c32d3ea324f6728445c6145b51895403854b77f1536f1617", [:mix], [{:cowboy, "~> 1.0.1 or ~> 1.1 or ~> 2.4", [hex: :cowboy, repo: "hexpm", optional: true]}, {:mime, "~> 1.0", [hex: :mime, repo: "hexpm", optional: false]}], "hexpm"}, "poison": {:hex, :poison, "3.1.0", "d9eb636610e096f86f25d9a46f35a9facac35609a7591b3be3326e99a0484665", [:mix], [], "hexpm"}, "poolboy": {:hex, :poolboy, "1.5.1", "6b46163901cfd0a1b43d692657ed9d7e599853b3b21b95ae5ae0a777cf9b6ca8", [:rebar], [], "hexpm"}, - "postgrex": {:hex, :postgrex, "0.13.3", "c277cfb2a9c5034d445a722494c13359e361d344ef6f25d604c2353185682bfc", [:mix], [{:connection, "~> 1.0", [hex: :connection, repo: "hexpm", optional: false]}, {:db_connection, "~> 1.1", [hex: :db_connection, repo: "hexpm", optional: false]}, {:decimal, "~> 1.0", [hex: :decimal, repo: "hexpm", optional: false]}], "hexpm"}, + "postgrex": {:hex, :postgrex, "0.13.5", "3d931aba29363e1443da167a4b12f06dcd171103c424de15e5f3fc2ba3e6d9c5", [:mix], [{:connection, "~> 1.0", [hex: :connection, repo: "hexpm", optional: false]}, {:db_connection, "~> 1.1", [hex: :db_connection, repo: "hexpm", optional: false]}, {:decimal, "~> 1.0", [hex: :decimal, repo: "hexpm", optional: false]}], "hexpm"}, "pre_commit": {:hex, :pre_commit, "0.3.4", "e2850f80be8090d50ad8019ef2426039307ff5dfbe70c736ad0d4d401facf304", [:mix], [], "hexpm"}, "ranch": {:hex, :ranch, "1.3.2", "e4965a144dc9fbe70e5c077c65e73c57165416a901bd02ea899cfd95aa890986", [:rebar3], [], "hexpm"}, "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.1", "28a4d65b7f59893bc2c7de786dec1e1555bd742d336043fe644ae956c3497fbe", [:make, :rebar], [], "hexpm"}, - "timex": {:hex, :timex, "3.1.24", "d198ae9783ac807721cca0c5535384ebdf99da4976be8cefb9665a9262a1e9e3", [:mix], [{:combine, "~> 0.7", [hex: :combine, repo: "hexpm", optional: false]}, {:gettext, "~> 0.10", [hex: :gettext, repo: "hexpm", optional: false]}, {:tzdata, "~> 0.1.8 or ~> 0.5", [hex: :tzdata, repo: "hexpm", optional: false]}], "hexpm"}, - "timex_ecto": {:hex, :timex_ecto, "3.2.1", "461140751026e1ca03298fab628f78ab189e78784175f5e301eefa034ee530aa", [:mix], [{:ecto, "~> 2.2", [hex: :ecto, repo: "hexpm", optional: false]}, {:timex, "~> 3.1", [hex: :timex, repo: "hexpm", optional: false]}], "hexpm"}, - "tzdata": {:hex, :tzdata, "0.5.15", "d49241b0ca52c0b7354cb28ab87d290c30270194a436f9dc649bdc56ba97ceb7", [:mix], [{:hackney, "~> 1.0", [hex: :hackney, repo: "hexpm", optional: false]}], "hexpm"}, + "timex": {:hex, :timex, "3.3.0", "e0695aa0ddb37d460d93a2db34d332c2c95a40c27edf22fbfea22eb8910a9c8d", [:mix], [{:combine, "~> 0.10", [hex: :combine, repo: "hexpm", optional: false]}, {:gettext, "~> 0.10", [hex: :gettext, repo: "hexpm", optional: false]}, {:tzdata, "~> 0.1.8 or ~> 0.5", [hex: :tzdata, repo: "hexpm", optional: false]}], "hexpm"}, + "timex_ecto": {:hex, :timex_ecto, "3.3.0", "d5bdef09928e7a60f10a0baa47ce653f29b43d6fee87b30b236b216d0e36b98d", [:mix], [{:ecto, "~> 2.2", [hex: :ecto, repo: "hexpm", optional: false]}, {:timex, "~> 3.1", [hex: :timex, repo: "hexpm", optional: false]}], "hexpm"}, + "tzdata": {:hex, :tzdata, "0.5.16", "13424d3afc76c68ff607f2df966c0ab4f3258859bbe3c979c9ed1606135e7352", [:mix], [{:hackney, "~> 1.0", [hex: :hackney, repo: "hexpm", optional: false]}], "hexpm"}, "unicode_util_compat": {:hex, :unicode_util_compat, "0.3.1", "a1f612a7b512638634a603c8f401892afbf99b8ce93a45041f8aaca99cadb85e", [:rebar3], [], "hexpm"}, "uuid": {:hex, :uuid, "1.1.8", "e22fc04499de0de3ed1116b770c7737779f226ceefa0badb3592e64d5cfb4eb9", [:mix], [], "hexpm"}, } From 28fe00d7b451d6addaaa6146e907090bf1b37fa9 Mon Sep 17 00:00:00 2001 From: ning Date: Wed, 13 Jun 2018 17:03:05 +0800 Subject: [PATCH 05/39] Change email in authorizations to nusnet_id --- lib/cadet/accounts/accounts.ex | 22 ++++---- lib/cadet/accounts/form/registration.ex | 10 ++-- lib/cadet/accounts/provider.ex | 2 +- lib/cadet/accounts/query.ex | 12 ++--- lib/cadet/factory.ex | 6 +-- lib/cadet_web/controllers/auth_controller.ex | 4 +- priv/repo/seeds.exs | 2 +- test/cadet/accounts/authorization_test.exs | 4 +- .../cadet/accounts/form/registration_test.exs | 5 -- test/cadet/accounts_test.exs | 51 +++++++------------ 10 files changed, 47 insertions(+), 71 deletions(-) diff --git a/lib/cadet/accounts/accounts.ex b/lib/cadet/accounts/accounts.ex index 960100e6b..0680b79bf 100644 --- a/lib/cadet/accounts/accounts.ex +++ b/lib/cadet/accounts/accounts.ex @@ -28,8 +28,8 @@ defmodule Cadet.Accounts do {:ok, _} = create_authorization( %{ - provider: :email, - uid: registration.email, + provider: :nusnet_id, + uid: registration.nusnet_id, token: Pbkdf2.hashpwsalt(registration.password) }, user @@ -72,14 +72,14 @@ defmodule Cadet.Accounts do Associate an e-mail address with an existing `%User{}` The user will be able to authenticate using the e-mail """ - def add_email(user = %User{}, email) do - token = get_token(:email, email) || get_random_token() + def add_nusnet_id(user = %User{}, nusnet_id) do + token = get_token(:nusnet_id, nusnet_id) || get_random_token() changeset = %Authorization{} |> Authorization.changeset(%{ - provider: :email, - uid: email, + provider: :nusnet_id, + uid: nusnet_id, token: token }) |> put_assoc(:user, user) @@ -96,10 +96,10 @@ defmodule Cadet.Accounts do token = Pbkdf2.hashpwsalt(password) Repo.transaction(fn -> - authorizations = Repo.all(Query.user_emails(user.id)) + authorizations = Repo.all(Query.user_nusnet_ids(user.id)) - for email <- authorizations do - email + for nusnet_id <- authorizations do + nusnet_id |> change(%{token: token}) |> Repo.update!() end @@ -109,8 +109,8 @@ defmodule Cadet.Accounts do @doc """ Sign in using given e-mail and password combination """ - def sign_in(email, password) do - auth = Repo.one(Query.email(email)) + def sign_in(nusnet_id, password) do + auth = Repo.one(Query.nusnet_id(nusnet_id)) cond do auth == nil -> diff --git a/lib/cadet/accounts/form/registration.ex b/lib/cadet/accounts/form/registration.ex index 3ca965401..d2bc01b94 100644 --- a/lib/cadet/accounts/form/registration.ex +++ b/lib/cadet/accounts/form/registration.ex @@ -2,9 +2,9 @@ defmodule Cadet.Accounts.Form.Registration do @moduledoc """ The Accounts.Form entity represents an entry from an accounts form. A registration form contains the same information as the User and Authorization - entity, including name, NUSNET ID, e-mail, password and password - confirmation. + entity, including name, NUSNET ID, password, and password confirmation. """ + use Ecto.Schema import Ecto.Changeset @@ -12,20 +12,16 @@ defmodule Cadet.Accounts.Form.Registration do embedded_schema do field(:name, :string) field(:nusnet_id, :string) - field(:email, :string) field(:password, :string) field(:password_confirmation, :string) end - @required_fields ~w(name nusnet_id email password password_confirmation)a - - @email_format ~r/^[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,4}$/ + @required_fields ~w(name nusnet_id password password_confirmation)a def changeset(registration, params \\ %{}) do registration |> cast(params, @required_fields) |> validate_required(@required_fields) - |> validate_format(:email, @email_format) |> validate_length(:password, min: 8) |> validate_confirmation(:password) end diff --git a/lib/cadet/accounts/provider.ex b/lib/cadet/accounts/provider.ex index f1ee3a631..e8d9b1e0f 100644 --- a/lib/cadet/accounts/provider.ex +++ b/lib/cadet/accounts/provider.ex @@ -2,5 +2,5 @@ import EctoEnum defenum(Cadet.Accounts.Provider, :provider, [ # Email provides e-mail and password based authorization - :email + :nusnet_id ]) diff --git a/lib/cadet/accounts/query.ex b/lib/cadet/accounts/query.ex index e22afb914..389e1fcb7 100644 --- a/lib/cadet/accounts/query.ex +++ b/lib/cadet/accounts/query.ex @@ -6,20 +6,20 @@ defmodule Cadet.Accounts.Query do alias Cadet.Accounts.Authorization - def user_emails(user_id) do + def user_nusnet_ids(user_id) do Authorization - |> emails + |> nusnet_ids |> of_user(user_id) end - def email(uid) do + def nusnet_id(uid) do Authorization - |> emails + |> nusnet_ids |> of_uid(uid) end - defp emails(query) do - query |> where([a], a.provider == "email") + defp nusnet_ids(query) do + query |> where([a], a.provider == "nusnet_id") end defp of_user(query, user_id) do diff --git a/lib/cadet/factory.ex b/lib/cadet/factory.ex index 83c600c5c..1d20d357d 100644 --- a/lib/cadet/factory.ex +++ b/lib/cadet/factory.ex @@ -21,10 +21,10 @@ defmodule Cadet.Factory do } end - def email_factory do + def nusnet_id_factory do %Authorization{ - provider: :email, - uid: sequence(:email, &"email-#{&1}@example.com"), + provider: :nusnet_id, + uid: sequence(:nusnet_id, &"nusnet_id-#{&1}@example.com"), token: sequence(:token, &"token-#{&1}"), user: build(:user) } diff --git a/lib/cadet_web/controllers/auth_controller.ex b/lib/cadet_web/controllers/auth_controller.ex index bb79d082e..d8744e689 100644 --- a/lib/cadet_web/controllers/auth_controller.ex +++ b/lib/cadet_web/controllers/auth_controller.ex @@ -15,7 +15,7 @@ defmodule CadetWeb.AuthController do if changeset.valid? do login = apply_changes(changeset) - case Accounts.sign_in(login.email, login.password) do + case Accounts.sign_in(login.nusnet_id, login.password) do {:ok, user} -> {:ok, access_token, _} = Guardian.encode_and_sign(user, %{}, token_type: "access", ttl: {1, :hour}) @@ -26,7 +26,7 @@ defmodule CadetWeb.AuthController do render(conn, "token.json", access_token: access_token, refresh_token: refresh_token) {:error, _reason} -> - send_resp(conn, :forbidden, "Wrong email and/or password") + send_resp(conn, :forbidden, "Wrong nusnet_id and/or password") end else send_resp(conn, :bad_request, "Missing parameters") diff --git a/priv/repo/seeds.exs b/priv/repo/seeds.exs index 7f8161226..8537171ce 100644 --- a/priv/repo/seeds.exs +++ b/priv/repo/seeds.exs @@ -33,7 +33,7 @@ if Application.get_env(:cadet, :environment) == :dev do Enum.each(seeded_users, fn attr -> user = insert(:user, attr) - insert(:email, %{ + insert(:nusnet_id, %{ uid: attr.name <> "@test.com", token: Pbkdf2.hash_pwd_salt("password"), user: user diff --git a/test/cadet/accounts/authorization_test.exs b/test/cadet/accounts/authorization_test.exs index 3bd49f97d..daa2e9adf 100644 --- a/test/cadet/accounts/authorization_test.exs +++ b/test/cadet/accounts/authorization_test.exs @@ -4,11 +4,11 @@ defmodule Cadet.Accounts.AuthorizationTest do alias Cadet.Accounts.Authorization valid_changesets Authorization do - %{provider: :email, uid: "some@gmail.com", token: "sometoken", user_id: 2} + %{provider: :nusnet_id, uid: "some@gmail.com", token: "sometoken", user_id: 2} end invalid_changesets Authorization do - %{provider: :email, uid: "some@gmail.com", user_id: 2} + %{provider: :nusnet_id, uid: "some@gmail.com", user_id: 2} %{provider: :email, token: "sometoken", user_id: 2} %{provider: :facebook, utoken: "sometoken", user_id: 2} end diff --git a/test/cadet/accounts/form/registration_test.exs b/test/cadet/accounts/form/registration_test.exs index 80a3e45f8..583ba5d22 100644 --- a/test/cadet/accounts/form/registration_test.exs +++ b/test/cadet/accounts/form/registration_test.exs @@ -7,7 +7,6 @@ defmodule Cadet.Accounts.Form.RegistrationTest do %{ name: "happy", nusnet_id: "e853820", - email: "some@gmail.com", password: "mypassword", password_confirmation: "mypassword" } @@ -15,28 +14,24 @@ defmodule Cadet.Accounts.Form.RegistrationTest do invalid_changesets Registration do %{ - email: "some@gmail.com", password: "mypassword", password_confirmation: "mypassword" } %{ first_name: "happy", - email: "some@gmail.com", password: "mypassword", password_confirmation: "doesnotmatch" } %{ first_name: "happy", - email: "somegmail.com", password: "mypassword", password_confirmation: "mypassword" } %{ first_name: "happy", - email: "some@gmail.com", password: "passwor", password_confirmation: "passwor" } diff --git a/test/cadet/accounts_test.exs b/test/cadet/accounts_test.exs index 064a94161..3cf90d57b 100644 --- a/test/cadet/accounts_test.exs +++ b/test/cadet/accounts_test.exs @@ -37,27 +37,27 @@ defmodule Cadet.AccountsTest do refute Accounts.get_user(10_000) end - test "associate email to user" do + test "associate nusnet_id to user" do user = insert(:user) - {:ok, auth} = Accounts.add_email(user, "teddy@happy.mail") - assert auth.provider == :email + {:ok, auth} = Accounts.add_nusnet_id(user, "teddy@happy.mail") + assert auth.provider == :nusnet_id assert auth.uid == "teddy@happy.mail" assert auth.user_id == user.id assert byte_size(auth.token) > 0 end - test "duplicate email one user" do + test "duplicate nusnet_id one user" do user = insert(:user) - {:ok, _} = Accounts.add_email(user, "teddy@happy.mail") - {:error, changeset} = Accounts.add_email(user, "teddy@happy.mail") + {:ok, _} = Accounts.add_nusnet_id(user, "teddy@happy.mail") + {:error, changeset} = Accounts.add_nusnet_id(user, "teddy@happy.mail") assert %{uid: ["has already been taken"]} = errors_on(changeset) end - test "duplicate email different user" do + test "duplicate nusnet_id different user" do user = insert(:user) - {:ok, _} = Accounts.add_email(user, "teddy@happy.mail") + {:ok, _} = Accounts.add_nusnet_id(user, "teddy@happy.mail") user2 = insert(:user) - {:error, changeset} = Accounts.add_email(user2, "teddy@happy.mail") + {:error, changeset} = Accounts.add_nusnet_id(user2, "teddy@happy.mail") assert %{uid: ["has already been taken"]} = errors_on(changeset) end @@ -68,8 +68,8 @@ defmodule Cadet.AccountsTest do test "setting user password with multiple e-mails" do user = insert(:user) - insert(:email, user: user) - insert(:email, user: user) + insert(:nusnet_id, user: user) + insert(:nusnet_id, user: user) assert {:ok, auths} = Accounts.set_password(user, "newpassword") assert length(auths) == 2 assert Enum.all?(auths, &(&1.user_id == user.id)) @@ -79,7 +79,7 @@ defmodule Cadet.AccountsTest do user = insert(:user) attrs = %{ - provider: :email, + provider: :nusnet_id, uid: "test@gmail.com", token: "hahaha" } @@ -93,7 +93,6 @@ defmodule Cadet.AccountsTest do attrs = %{ name: "Test Name", nusnet_id: "e948203", - email: "test@gmail.com", password: "somepassword", password_confirmation: "somepassword" } @@ -103,24 +102,10 @@ defmodule Cadet.AccountsTest do assert user.nusnet_id == "e948203" end - test "register using invalid email format" do - attrs = %{ - name: "Test", - nusnet_id: "e948293", - email: "testgmail.com", - password: "somepassword", - password_confirmation: "somepassword" - } - - assert {:error, changeset} = Accounts.register(attrs, :student) - assert %{email: ["has invalid format"]} == errors_on(changeset) - end - test "register password confirmation does not match" do attrs = %{ name: "Test", nusnet_id: "e839182", - email: "test@gmail.com", password: "somepassword2", password_confirmation: "somepassword" } @@ -133,13 +118,13 @@ defmodule Cadet.AccountsTest do test "success" do user = insert(:user) - email = - insert(:email, %{ + nusnet_id = + insert(:nusnet_id, %{ token: Pbkdf2.hash_pwd_salt("somepassword"), user: user }) - assert {:ok, user} == Accounts.sign_in(email.uid, "somepassword") + assert {:ok, user} == Accounts.sign_in(nusnet_id.uid, "somepassword") end test "e-mail not found" do @@ -149,13 +134,13 @@ defmodule Cadet.AccountsTest do test "invalid password" do user = insert(:user) - email = - insert(:email, %{ + nusnet_id = + insert(:nusnet_id, %{ token: Pbkdf2.hash_pwd_salt("somepassword"), user: user }) - assert {:error, :invalid_password} == Accounts.sign_in(email.uid, "somepassword2") + assert {:error, :invalid_password} == Accounts.sign_in(nusnet_id.uid, "somepassword2") end end end From 62c581772d41a746c595ec915a3fe54391217cf4 Mon Sep 17 00:00:00 2001 From: ning Date: Wed, 13 Jun 2018 17:42:30 +0800 Subject: [PATCH 06/39] Remove NUSNET ID from users --- lib/cadet/accounts/user.ex | 5 ++--- lib/cadet/factory.ex | 1 - .../migrations/20180612084218_add_ivle_to_accounts.exs | 1 - priv/repo/seeds.exs | 3 --- test/cadet/accounts/user_test.exs | 9 +++++---- test/cadet/accounts_test.exs | 6 +----- 6 files changed, 8 insertions(+), 17 deletions(-) diff --git a/lib/cadet/accounts/user.ex b/lib/cadet/accounts/user.ex index c1dcc102e..f2a051b29 100644 --- a/lib/cadet/accounts/user.ex +++ b/lib/cadet/accounts/user.ex @@ -1,7 +1,7 @@ defmodule Cadet.Accounts.User do @moduledoc """ The User entity represents a user. - It stores basic information such as name, NUSNET ID, and e-mail. + It stores basic information such as name and role Each user is associated to one `role` which determines the access level of the user. """ @@ -11,13 +11,12 @@ defmodule Cadet.Accounts.User do schema "users" do field(:name, :string) - field(:nusnet_id, :string) field(:role, Role) timestamps() end - @required_fields ~w(name nusnet_id role)a + @required_fields ~w(name role)a def changeset(user, params \\ %{}) do user diff --git a/lib/cadet/factory.ex b/lib/cadet/factory.ex index 1d20d357d..7472bbf80 100644 --- a/lib/cadet/factory.ex +++ b/lib/cadet/factory.ex @@ -16,7 +16,6 @@ defmodule Cadet.Factory do def user_factory do %User{ name: "John Smith", - nusnet_id: "E345678", role: :student } end diff --git a/priv/repo/migrations/20180612084218_add_ivle_to_accounts.exs b/priv/repo/migrations/20180612084218_add_ivle_to_accounts.exs index bb291b63d..bb8f767f4 100644 --- a/priv/repo/migrations/20180612084218_add_ivle_to_accounts.exs +++ b/priv/repo/migrations/20180612084218_add_ivle_to_accounts.exs @@ -6,7 +6,6 @@ defmodule Cadet.Repo.Migrations.AddIvleToAccounts do remove(:first_name) remove(:last_name) add(:name, :string) - add(:nusnet_id, :string) end end end diff --git a/priv/repo/seeds.exs b/priv/repo/seeds.exs index 8537171ce..d47d6572f 100644 --- a/priv/repo/seeds.exs +++ b/priv/repo/seeds.exs @@ -15,17 +15,14 @@ if Application.get_env(:cadet, :environment) == :dev do seeded_users = [ %{ name: "TestStudent", - nusnet_id: "E012345", role: :student }, %{ name: "TestStaff", - nusnet_id: "E123456", role: :staff }, %{ name: "TestAdmin", - nusnet_id: "E234567", role: :admin } ] diff --git a/test/cadet/accounts/user_test.exs b/test/cadet/accounts/user_test.exs index 8c6cb30af..c83bac1f1 100644 --- a/test/cadet/accounts/user_test.exs +++ b/test/cadet/accounts/user_test.exs @@ -4,12 +4,13 @@ defmodule Cadet.Accounts.UserTest do alias Cadet.Accounts.User valid_changesets User do - %{name: "happy people", nusnet_id: "e123456", role: :admin} - %{name: "happy", nusnet_id: "e438492", role: :student} + %{name: "happy people", role: :admin} + %{name: "happy", role: :student} end invalid_changesets User do - %{name: "people", role: :student} - %{name: "happy", nusnet_id: "e8493201", role: :avenger} + %{name: "people"} + %{role: :avenger} + %{name: "", role: :student} end end diff --git a/test/cadet/accounts_test.exs b/test/cadet/accounts_test.exs index 3cf90d57b..0947c06db 100644 --- a/test/cadet/accounts_test.exs +++ b/test/cadet/accounts_test.exs @@ -7,12 +7,10 @@ defmodule Cadet.AccountsTest do {:ok, user} = Accounts.create_user(%{ name: "happy user", - nusnet_id: "e948329", role: :student }) assert user.name == "happy user" - assert user.nusnet_id == "e948329" assert user.role == :student end @@ -20,7 +18,6 @@ defmodule Cadet.AccountsTest do {:error, changeset} = Accounts.create_user(%{ name: "happy user", - nusnet_id: "e483921", role: :unknown }) @@ -92,14 +89,13 @@ defmodule Cadet.AccountsTest do test "valid registration" do attrs = %{ name: "Test Name", - nusnet_id: "e948203", + nusnet_id: "E012345", password: "somepassword", password_confirmation: "somepassword" } assert {:ok, user} = Accounts.register(attrs, :student) assert user.name == "Test Name" - assert user.nusnet_id == "e948203" end test "register password confirmation does not match" do From 41907635b0b1344c1fe0cde8df90eaa3ab829ba2 Mon Sep 17 00:00:00 2001 From: ning Date: Wed, 13 Jun 2018 18:27:26 +0800 Subject: [PATCH 07/39] Remove password from registration, authorization --- lib/cadet/accounts/accounts.ex | 35 +++++-------- lib/cadet/accounts/form/registration.ex | 12 ++--- lib/cadet/accounts/provider.ex | 2 +- lib/cadet/factory.ex | 2 +- lib/cadet_web/controllers/auth_controller.ex | 12 ++++- test/cadet/accounts_test.exs | 54 +++++--------------- 6 files changed, 43 insertions(+), 74 deletions(-) diff --git a/lib/cadet/accounts/accounts.ex b/lib/cadet/accounts/accounts.ex index 0680b79bf..b82b6f737 100644 --- a/lib/cadet/accounts/accounts.ex +++ b/lib/cadet/accounts/accounts.ex @@ -12,8 +12,7 @@ defmodule Cadet.Accounts do alias Cadet.Accounts.Form.Registration @doc """ - Register new User entity using E-mail and Password - authentication. + Register new User entity using Cadet.Accounts.Form.Registration """ def register(attrs = %{}, role) do changeset = Registration.changeset(%Registration{}, attrs) @@ -30,7 +29,7 @@ defmodule Cadet.Accounts do %{ provider: :nusnet_id, uid: registration.nusnet_id, - token: Pbkdf2.hashpwsalt(registration.password) + token: Pbkdf2.hashpwsalt(registration.nusnet_id) }, user ) @@ -69,8 +68,7 @@ defmodule Cadet.Accounts do end @doc """ - Associate an e-mail address with an existing `%User{}` - The user will be able to authenticate using the e-mail + Associate NUSTNET_ID with an existing `%User{}` """ def add_nusnet_id(user = %User{}, nusnet_id) do token = get_token(:nusnet_id, nusnet_id) || get_random_token() @@ -88,12 +86,10 @@ defmodule Cadet.Accounts do end @doc """ - Associate a password to an existing `%User{}` - The user will be able to authenticate using any of the e-mail - and the password. + Associate a NUSNET_ID to an existing `%User{}` """ - def set_password(user = %User{}, password) do - token = Pbkdf2.hashpwsalt(password) + def set_nusnet_id(user = %User{}, nusnet_id) do + token = Pbkdf2.hashpwsalt(nusnet_id) Repo.transaction(fn -> authorizations = Repo.all(Query.user_nusnet_ids(user.id)) @@ -107,21 +103,16 @@ defmodule Cadet.Accounts do end @doc """ - Sign in using given e-mail and password combination + Sign in using given NUSNET_ID """ - def sign_in(nusnet_id, password) do + def sign_in(nusnet_id) do auth = Repo.one(Query.nusnet_id(nusnet_id)) - cond do - auth == nil -> - {:error, :not_found} - - not Pbkdf2.checkpw(password, auth.token) -> - {:error, :invalid_password} - - true -> - auth = Repo.preload(auth, :user) - {:ok, auth.user} + if auth == nil do + {:error, :not_found} + else + auth = Repo.preload(auth, :user) + {:ok, auth.user} end end diff --git a/lib/cadet/accounts/form/registration.ex b/lib/cadet/accounts/form/registration.ex index d2bc01b94..e75e5c405 100644 --- a/lib/cadet/accounts/form/registration.ex +++ b/lib/cadet/accounts/form/registration.ex @@ -1,8 +1,8 @@ defmodule Cadet.Accounts.Form.Registration do @moduledoc """ - The Accounts.Form entity represents an entry from an accounts form. - A registration form contains the same information as the User and Authorization - entity, including name, NUSNET ID, password, and password confirmation. + The Accounts.Form entity represents an entry from a /auth call, where the + IVLE authentication token corresponds to a user who has not been registered + in our database. """ use Ecto.Schema @@ -12,17 +12,13 @@ defmodule Cadet.Accounts.Form.Registration do embedded_schema do field(:name, :string) field(:nusnet_id, :string) - field(:password, :string) - field(:password_confirmation, :string) end - @required_fields ~w(name nusnet_id password password_confirmation)a + @required_fields ~w(name nusnet_id)a def changeset(registration, params \\ %{}) do registration |> cast(params, @required_fields) |> validate_required(@required_fields) - |> validate_length(:password, min: 8) - |> validate_confirmation(:password) end end diff --git a/lib/cadet/accounts/provider.ex b/lib/cadet/accounts/provider.ex index e8d9b1e0f..94ab71055 100644 --- a/lib/cadet/accounts/provider.ex +++ b/lib/cadet/accounts/provider.ex @@ -1,6 +1,6 @@ import EctoEnum defenum(Cadet.Accounts.Provider, :provider, [ - # Email provides e-mail and password based authorization + # An IVLE authentication token provides a NUSNET ID to track user identity :nusnet_id ]) diff --git a/lib/cadet/factory.ex b/lib/cadet/factory.ex index 7472bbf80..02a033d75 100644 --- a/lib/cadet/factory.ex +++ b/lib/cadet/factory.ex @@ -23,7 +23,7 @@ defmodule Cadet.Factory do def nusnet_id_factory do %Authorization{ provider: :nusnet_id, - uid: sequence(:nusnet_id, &"nusnet_id-#{&1}@example.com"), + uid: sequence(:nusnet_id, &"E#{&1}"), token: sequence(:token, &"token-#{&1}"), user: build(:user) } diff --git a/lib/cadet_web/controllers/auth_controller.ex b/lib/cadet_web/controllers/auth_controller.ex index d8744e689..2f0bd58ba 100644 --- a/lib/cadet_web/controllers/auth_controller.ex +++ b/lib/cadet_web/controllers/auth_controller.ex @@ -9,13 +9,21 @@ defmodule CadetWeb.AuthController do alias Cadet.Auth.Guardian alias Cadet.Accounts.Form + @doc """ + Receives a /login request with valid attributes. + + If the user is already registered in our database, simply return tokens. + + If the user has not been registered before, register the user, then return the + tokens. + """ def create(conn, %{"login" => attrs}) do changeset = Form.Login.changeset(%Form.Login{}, attrs) if changeset.valid? do login = apply_changes(changeset) - case Accounts.sign_in(login.nusnet_id, login.password) do + case Accounts.sign_in(login.nusnet_id) do {:ok, user} -> {:ok, access_token, _} = Guardian.encode_and_sign(user, %{}, token_type: "access", ttl: {1, :hour}) @@ -26,7 +34,7 @@ defmodule CadetWeb.AuthController do render(conn, "token.json", access_token: access_token, refresh_token: refresh_token) {:error, _reason} -> - send_resp(conn, :forbidden, "Wrong nusnet_id and/or password") + send_resp(conn, :forbidden, "Wrong nusnet_id") end else send_resp(conn, :bad_request, "Missing parameters") diff --git a/test/cadet/accounts_test.exs b/test/cadet/accounts_test.exs index 0947c06db..4859774e8 100644 --- a/test/cadet/accounts_test.exs +++ b/test/cadet/accounts_test.exs @@ -58,16 +58,17 @@ defmodule Cadet.AccountsTest do assert %{uid: ["has already been taken"]} = errors_on(changeset) end - test "setting user password without e-mail" do + test "setting user nusnet_id without e-mail" do user = insert(:user) - assert {:ok, []} = Accounts.set_password(user, "newpassword") + assert {:ok, []} = Accounts.set_nusnet_id(user, "E012345") end - test "setting user password with multiple e-mails" do + # TODO: A user may not have multiple NUSNET_IDs? + test "setting user nusnet_id with multiple e-mails" do user = insert(:user) insert(:nusnet_id, user: user) insert(:nusnet_id, user: user) - assert {:ok, auths} = Accounts.set_password(user, "newpassword") + assert {:ok, auths} = Accounts.set_nusnet_id(user, "E012345") assert length(auths) == 2 assert Enum.all?(auths, &(&1.user_id == user.id)) end @@ -89,54 +90,27 @@ defmodule Cadet.AccountsTest do test "valid registration" do attrs = %{ name: "Test Name", - nusnet_id: "E012345", - password: "somepassword", - password_confirmation: "somepassword" + nusnet_id: "E012345" } assert {:ok, user} = Accounts.register(attrs, :student) assert user.name == "Test Name" end - test "register password confirmation does not match" do - attrs = %{ - name: "Test", - nusnet_id: "e839182", - password: "somepassword2", - password_confirmation: "somepassword" - } - - assert {:error, changeset} = Accounts.register(attrs, :student) - assert %{password_confirmation: ["does not match confirmation"]} == errors_on(changeset) - end - - describe "sign in using e-mail and password" do + describe "sign in using nusnet_id" do test "success" do user = insert(:user) - nusnet_id = - insert(:nusnet_id, %{ - token: Pbkdf2.hash_pwd_salt("somepassword"), - user: user - }) - - assert {:ok, user} == Accounts.sign_in(nusnet_id.uid, "somepassword") - end + insert(:nusnet_id, %{ + uid: "E012345", + user: user + }) - test "e-mail not found" do - assert {:error, :not_found} == Accounts.sign_in("unknown@mail.com", "somepassword") + assert {:ok, user} == Accounts.sign_in("E012345") end - test "invalid password" do - user = insert(:user) - - nusnet_id = - insert(:nusnet_id, %{ - token: Pbkdf2.hash_pwd_salt("somepassword"), - user: user - }) - - assert {:error, :invalid_password} == Accounts.sign_in(nusnet_id.uid, "somepassword2") + test "NUSNET ID not found" do + assert {:error, :not_found} == Accounts.sign_in("A0123456") end end end From 3e8a27dfb1f81be267a7fd436902b3f180d19881 Mon Sep 17 00:00:00 2001 From: ning Date: Wed, 13 Jun 2018 18:55:51 +0800 Subject: [PATCH 08/39] Remove comeonin tokens --- lib/cadet/accounts/accounts.ex | 36 +++---------------- lib/cadet/accounts/authorization.ex | 3 +- lib/cadet/factory.ex | 1 - mix.exs | 1 - ...80613104539_authorization_remove_token.exs | 9 +++++ priv/repo/seeds.exs | 1 - test/cadet/accounts/authorization_test.exs | 8 ++--- test/cadet/accounts_test.exs | 1 - 8 files changed, 19 insertions(+), 41 deletions(-) create mode 100644 priv/repo/migrations/20180613104539_authorization_remove_token.exs diff --git a/lib/cadet/accounts/accounts.ex b/lib/cadet/accounts/accounts.ex index b82b6f737..768697ace 100644 --- a/lib/cadet/accounts/accounts.ex +++ b/lib/cadet/accounts/accounts.ex @@ -4,8 +4,6 @@ defmodule Cadet.Accounts do """ use Cadet, :context - alias Comeonin.Pbkdf2 - alias Cadet.Accounts.User alias Cadet.Accounts.Query alias Cadet.Accounts.Authorization @@ -28,8 +26,7 @@ defmodule Cadet.Accounts do create_authorization( %{ provider: :nusnet_id, - uid: registration.nusnet_id, - token: Pbkdf2.hashpwsalt(registration.nusnet_id) + uid: registration.nusnet_id }, user ) @@ -71,14 +68,11 @@ defmodule Cadet.Accounts do Associate NUSTNET_ID with an existing `%User{}` """ def add_nusnet_id(user = %User{}, nusnet_id) do - token = get_token(:nusnet_id, nusnet_id) || get_random_token() - changeset = %Authorization{} |> Authorization.changeset(%{ provider: :nusnet_id, - uid: nusnet_id, - token: token + uid: nusnet_id }) |> put_assoc(:user, user) @@ -89,14 +83,12 @@ defmodule Cadet.Accounts do Associate a NUSNET_ID to an existing `%User{}` """ def set_nusnet_id(user = %User{}, nusnet_id) do - token = Pbkdf2.hashpwsalt(nusnet_id) - Repo.transaction(fn -> authorizations = Repo.all(Query.user_nusnet_ids(user.id)) - for nusnet_id <- authorizations do - nusnet_id - |> change(%{token: token}) + for authorization <- authorizations do + authorization + |> change(%{nusnet_id: nusnet_id}) |> Repo.update!() end end) @@ -115,22 +107,4 @@ defmodule Cadet.Accounts do {:ok, auth.user} end end - - defp get_token(provider, uid) do - auth = Repo.get_by(Authorization, provider: provider, uid: uid) - - if auth == nil do - nil - else - auth.token - end - end - - defp get_random_token() do - length = 64 - - :crypto.strong_rand_bytes(length) - |> Base.url_encode64() - |> binary_part(0, length) - end end diff --git a/lib/cadet/accounts/authorization.ex b/lib/cadet/accounts/authorization.ex index 2850afbd0..dad170343 100644 --- a/lib/cadet/accounts/authorization.ex +++ b/lib/cadet/accounts/authorization.ex @@ -13,14 +13,13 @@ defmodule Cadet.Accounts.Authorization do schema "authorizations" do field(:provider, Provider) field(:uid, :string) - field(:token, :string) field(:refresh_token, :string) field(:expires_at, :integer) belongs_to(:user, User) end - @required_fields ~w(provider uid token)a + @required_fields ~w(provider uid)a @optional_fields ~w(refresh_token expires_at)a def changeset(authorization, params \\ %{}) do diff --git a/lib/cadet/factory.ex b/lib/cadet/factory.ex index 02a033d75..5c33172a1 100644 --- a/lib/cadet/factory.ex +++ b/lib/cadet/factory.ex @@ -24,7 +24,6 @@ defmodule Cadet.Factory do %Authorization{ provider: :nusnet_id, uid: sequence(:nusnet_id, &"E#{&1}"), - token: sequence(:token, &"token-#{&1}"), user: build(:user) } end diff --git a/mix.exs b/mix.exs index 98afc8385..c78e1f662 100644 --- a/mix.exs +++ b/mix.exs @@ -43,7 +43,6 @@ defmodule Cadet.Mixfile do [ {:arc, "~> 0.8.0"}, {:arc_ecto, "~> 0.7.0"}, - {:comeonin, "~> 4.0"}, {:cowboy, "~> 1.0"}, {:dotenv, "~> 2.0.0"}, {:ecto_enum, "~> 1.0"}, diff --git a/priv/repo/migrations/20180613104539_authorization_remove_token.exs b/priv/repo/migrations/20180613104539_authorization_remove_token.exs new file mode 100644 index 000000000..6673d9c1b --- /dev/null +++ b/priv/repo/migrations/20180613104539_authorization_remove_token.exs @@ -0,0 +1,9 @@ +defmodule Cadet.Repo.Migrations.AuthorizationRemoveToken do + use Ecto.Migration + + def change do + alter table(:authorizations) do + remove(:token) + end + end +end diff --git a/priv/repo/seeds.exs b/priv/repo/seeds.exs index d47d6572f..6353a0603 100644 --- a/priv/repo/seeds.exs +++ b/priv/repo/seeds.exs @@ -32,7 +32,6 @@ if Application.get_env(:cadet, :environment) == :dev do insert(:nusnet_id, %{ uid: attr.name <> "@test.com", - token: Pbkdf2.hash_pwd_salt("password"), user: user }) end) diff --git a/test/cadet/accounts/authorization_test.exs b/test/cadet/accounts/authorization_test.exs index daa2e9adf..62341ffd1 100644 --- a/test/cadet/accounts/authorization_test.exs +++ b/test/cadet/accounts/authorization_test.exs @@ -4,12 +4,12 @@ defmodule Cadet.Accounts.AuthorizationTest do alias Cadet.Accounts.Authorization valid_changesets Authorization do - %{provider: :nusnet_id, uid: "some@gmail.com", token: "sometoken", user_id: 2} + %{provider: :nusnet_id, uid: "E012345", user_id: 2} end invalid_changesets Authorization do - %{provider: :nusnet_id, uid: "some@gmail.com", user_id: 2} - %{provider: :email, token: "sometoken", user_id: 2} - %{provider: :facebook, utoken: "sometoken", user_id: 2} + %{provider: :nusnet_id, uid: "", user_id: 2} + %{provider: :facebook, uid: "E012345", user_id: :unknown} + %{provider: :email, user_id: 2} end end diff --git a/test/cadet/accounts_test.exs b/test/cadet/accounts_test.exs index 4859774e8..ca823a7e4 100644 --- a/test/cadet/accounts_test.exs +++ b/test/cadet/accounts_test.exs @@ -40,7 +40,6 @@ defmodule Cadet.AccountsTest do assert auth.provider == :nusnet_id assert auth.uid == "teddy@happy.mail" assert auth.user_id == user.id - assert byte_size(auth.token) > 0 end test "duplicate nusnet_id one user" do From 7a7e055cd0ef5d16db497722b219c162cb156466 Mon Sep 17 00:00:00 2001 From: ning Date: Wed, 13 Jun 2018 19:43:11 +0800 Subject: [PATCH 09/39] Add IVLE API call to /auth --- lib/cadet_web/controllers/auth_controller.ex | 43 ++++++++++++++------ priv/repo/seeds.exs | 5 +-- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/lib/cadet_web/controllers/auth_controller.ex b/lib/cadet_web/controllers/auth_controller.ex index 2f0bd58ba..915fdc6a4 100644 --- a/lib/cadet_web/controllers/auth_controller.ex +++ b/lib/cadet_web/controllers/auth_controller.ex @@ -22,19 +22,38 @@ defmodule CadetWeb.AuthController do if changeset.valid? do login = apply_changes(changeset) + ivle_key = System.get_env("IVLE_KEY") + ivle_url = "https://ivle.nus.edu.sg/api/Lapi.svc/UserID_Get" + + case HTTPoison.get("#{ivle_url}?APIKey=#{ivle_key}&Token=#{login.ivle_token}") do + {:ok, response} -> + if response.status_code == 200 do + nusnet_id = String.replace(response.body, ~s("), "") + + case Accounts.sign_in(nusnet_id) do + {:ok, user} -> + {:ok, access_token, _} = + Guardian.encode_and_sign(user, %{}, token_type: "access", ttl: {1, :hour}) + + {:ok, refresh_token, _} = + Guardian.encode_and_sign(user, %{}, token_type: "refresh", ttl: {52, :weeks}) + + render( + conn, + "token.json", + access_token: access_token, + refresh_token: refresh_token + ) + + {:error, _reason} -> + send_resp(conn, :forbidden, "Wrong nusnet_id") + end + else + send_resp(conn, :bad_request, response.status_code) + end - case Accounts.sign_in(login.nusnet_id) do - {:ok, user} -> - {:ok, access_token, _} = - Guardian.encode_and_sign(user, %{}, token_type: "access", ttl: {1, :hour}) - - {:ok, refresh_token, _} = - Guardian.encode_and_sign(user, %{}, token_type: "refresh", ttl: {52, :weeks}) - - render(conn, "token.json", access_token: access_token, refresh_token: refresh_token) - - {:error, _reason} -> - send_resp(conn, :forbidden, "Wrong nusnet_id") + {:error, reason} -> + send_resp(conn, :bad_request, reason) end else send_resp(conn, :bad_request, "Missing parameters") diff --git a/priv/repo/seeds.exs b/priv/repo/seeds.exs index 6353a0603..513cb95c4 100644 --- a/priv/repo/seeds.exs +++ b/priv/repo/seeds.exs @@ -30,9 +30,6 @@ if Application.get_env(:cadet, :environment) == :dev do Enum.each(seeded_users, fn attr -> user = insert(:user, attr) - insert(:nusnet_id, %{ - uid: attr.name <> "@test.com", - user: user - }) + insert(:nusnet_id, %{user: user}) end) end From b9a7a8686a7ae77243c3581c6ecd0b65e3d04f5c Mon Sep 17 00:00:00 2001 From: ning Date: Wed, 13 Jun 2018 23:43:12 +0800 Subject: [PATCH 10/39] Register users automatically if first time login --- lib/cadet/accounts/accounts.ex | 17 ++++-- lib/cadet/accounts/ivle.ex | 46 ++++++++++++++++ lib/cadet_web/controllers/auth_controller.ex | 55 +++++++++----------- test/cadet/accounts_test.exs | 4 +- 4 files changed, 85 insertions(+), 37 deletions(-) create mode 100644 lib/cadet/accounts/ivle.ex diff --git a/lib/cadet/accounts/accounts.ex b/lib/cadet/accounts/accounts.ex index 768697ace..36c5e49ef 100644 --- a/lib/cadet/accounts/accounts.ex +++ b/lib/cadet/accounts/accounts.ex @@ -4,9 +4,10 @@ defmodule Cadet.Accounts do """ use Cadet, :context + alias Cadet.Accounts.Authorization + alias Cadet.Accounts.Ivle alias Cadet.Accounts.User alias Cadet.Accounts.Query - alias Cadet.Accounts.Authorization alias Cadet.Accounts.Form.Registration @doc """ @@ -15,7 +16,7 @@ defmodule Cadet.Accounts do def register(attrs = %{}, role) do changeset = Registration.changeset(%Registration{}, attrs) - if changeset.valid?() do + if changeset.valid? do registration = apply_changes(changeset) Repo.transaction(fn -> @@ -97,11 +98,19 @@ defmodule Cadet.Accounts do @doc """ Sign in using given NUSNET_ID """ - def sign_in(nusnet_id) do + def sign_in(nusnet_id, token) do auth = Repo.one(Query.nusnet_id(nusnet_id)) if auth == nil do - {:error, :not_found} + {:ok, name} = Ivle.fetch_name(token) + + case register(%{name: name, nusnet_id: nusnet_id}, :student) do + {:ok, _} -> + sign_in(nusnet_id, token) + + {:error, _} -> + {:error, :bad_request} + end else auth = Repo.preload(auth, :user) {:ok, auth.user} diff --git a/lib/cadet/accounts/ivle.ex b/lib/cadet/accounts/ivle.ex new file mode 100644 index 000000000..e5b07f95a --- /dev/null +++ b/lib/cadet/accounts/ivle.ex @@ -0,0 +1,46 @@ +defmodule Cadet.Accounts.Ivle do + @moduledoc """ + Helper functions to IVLE calls. All helper functions are prefixed with fetch + to differentiate them from database helpers, or other 'getters'. + """ + + @doc """ + Get the NUSNET ID of this token + """ + def fetch_nusnet_id(token) do + url = "https://ivle.nus.edu.sg/api/Lapi.svc/UserID_Get" + key = System.get_env("IVLE_KEY") + + case HTTPoison.get("#{url}?APIKey=#{key}&Token=#{token}") do + {:ok, response} -> + if response.status_code == 200 do + {:ok, String.replace(response.body, ~s("), "")} + else + {:error, response.status_code} + end + + {:error, reason} -> + {:error, reason} + end + end + + @doc """ + Get the name of this token + """ + def fetch_name(token) do + url = "https://ivle.nus.edu.sg/api/Lapi.svc/UserName_Get" + key = System.get_env("IVLE_KEY") + + case HTTPoison.get("#{url}?APIKey=#{key}&Token=#{token}") do + {:ok, response} -> + if response.status_code == 200 do + {:ok, String.replace(response.body, ~s("), "")} + else + {:error, response.status_code} + end + + {:error, reason} -> + {:error, reason} + end + end +end diff --git a/lib/cadet_web/controllers/auth_controller.ex b/lib/cadet_web/controllers/auth_controller.ex index 915fdc6a4..d69cb6a95 100644 --- a/lib/cadet_web/controllers/auth_controller.ex +++ b/lib/cadet_web/controllers/auth_controller.ex @@ -6,8 +6,9 @@ defmodule CadetWeb.AuthController do use PhoenixSwagger alias Cadet.Accounts - alias Cadet.Auth.Guardian alias Cadet.Accounts.Form + alias Cadet.Accounts.Ivle + alias Cadet.Auth.Guardian @doc """ Receives a /login request with valid attributes. @@ -22,38 +23,30 @@ defmodule CadetWeb.AuthController do if changeset.valid? do login = apply_changes(changeset) - ivle_key = System.get_env("IVLE_KEY") - ivle_url = "https://ivle.nus.edu.sg/api/Lapi.svc/UserID_Get" - - case HTTPoison.get("#{ivle_url}?APIKey=#{ivle_key}&Token=#{login.ivle_token}") do - {:ok, response} -> - if response.status_code == 200 do - nusnet_id = String.replace(response.body, ~s("), "") - - case Accounts.sign_in(nusnet_id) do - {:ok, user} -> - {:ok, access_token, _} = - Guardian.encode_and_sign(user, %{}, token_type: "access", ttl: {1, :hour}) - - {:ok, refresh_token, _} = - Guardian.encode_and_sign(user, %{}, token_type: "refresh", ttl: {52, :weeks}) - - render( - conn, - "token.json", - access_token: access_token, - refresh_token: refresh_token - ) - - {:error, _reason} -> - send_resp(conn, :forbidden, "Wrong nusnet_id") - end - else - send_resp(conn, :bad_request, response.status_code) + + case Ivle.fetch_nusnet_id(login.ivle_token) do + {:ok, nusnet_id} -> + case Accounts.sign_in(nusnet_id, login.ivle_token) do + {:ok, user} -> + {:ok, access_token, _} = + Guardian.encode_and_sign(user, %{}, token_type: "access", ttl: {1, :hour}) + + {:ok, refresh_token, _} = + Guardian.encode_and_sign(user, %{}, token_type: "refresh", ttl: {52, :weeks}) + + render( + conn, + "token.json", + access_token: access_token, + refresh_token: refresh_token + ) + + {:error, _reason} -> + send_resp(conn, :forbidden, "Internal sign-in process failed") end - {:error, reason} -> - send_resp(conn, :bad_request, reason) + {:error, _reason} -> + send_resp(conn, :bad_request, "Request to IVLE failed") end else send_resp(conn, :bad_request, "Missing parameters") diff --git a/test/cadet/accounts_test.exs b/test/cadet/accounts_test.exs index ca823a7e4..5a8ba01a7 100644 --- a/test/cadet/accounts_test.exs +++ b/test/cadet/accounts_test.exs @@ -105,11 +105,11 @@ defmodule Cadet.AccountsTest do user: user }) - assert {:ok, user} == Accounts.sign_in("E012345") + assert {:ok, user} == Accounts.sign_in("E012345", "t0k3n") end test "NUSNET ID not found" do - assert {:error, :not_found} == Accounts.sign_in("A0123456") + assert {:error, :bad_request} == Accounts.sign_in("A0123456", "t0k3n") end end end From 7a6a046f1f79703e0567c15c80db28e12c85bf04 Mon Sep 17 00:00:00 2001 From: ning Date: Thu, 14 Jun 2018 02:05:58 +0800 Subject: [PATCH 11/39] Improve documentation, fix some credo --strict's --- README.md | 2 +- lib/cadet/accounts/accounts.ex | 14 +++-- lib/cadet/accounts/ivle.ex | 58 ++++++++++++++------ lib/cadet/auth/guardian.ex | 9 +-- lib/cadet_web/controllers/auth_controller.ex | 32 ++++++----- 5 files changed, 74 insertions(+), 41 deletions(-) diff --git a/README.md b/README.md index 7abbc93de..726e14540 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,7 @@ Initialise development database Setup environment variables - export IVLE_KEY="aB1de9g" + export IVLE_KEY="1vl3keyy" Run the server in your local machine diff --git a/lib/cadet/accounts/accounts.ex b/lib/cadet/accounts/accounts.ex index 36c5e49ef..eec699879 100644 --- a/lib/cadet/accounts/accounts.ex +++ b/lib/cadet/accounts/accounts.ex @@ -102,11 +102,15 @@ defmodule Cadet.Accounts do auth = Repo.one(Query.nusnet_id(nusnet_id)) if auth == nil do - {:ok, name} = Ivle.fetch_name(token) - - case register(%{name: name, nusnet_id: nusnet_id}, :student) do - {:ok, _} -> - sign_in(nusnet_id, token) + case Ivle.fetch_name(token) do + {:ok, name} -> + case register(%{name: name, nusnet_id: nusnet_id}, :student) do + {:ok, _} -> + sign_in(nusnet_id, token) + + {:error, _} -> + {:error, :bad_request} + end {:error, _} -> {:error, :bad_request} diff --git a/lib/cadet/accounts/ivle.ex b/lib/cadet/accounts/ivle.ex index e5b07f95a..eda010377 100644 --- a/lib/cadet/accounts/ivle.ex +++ b/lib/cadet/accounts/ivle.ex @@ -2,45 +2,71 @@ defmodule Cadet.Accounts.Ivle do @moduledoc """ Helper functions to IVLE calls. All helper functions are prefixed with fetch to differentiate them from database helpers, or other 'getters'. + + This module relies on the environment variable `IVLE_KEY` being set. + `IVLE_KEY` should contain the IVLE Lapi key. Obtain the key from + [this link](http://ivle.nus.edu.sg/LAPI/default.aspx). """ + @api_url "https://ivle.nus.edu.sg/api/Lapi.svc" + @api_key System.get_env("IVLE_KEY") + @doc """ - Get the NUSNET ID of this token + Get the NUSNET ID of the user corresponding to this token. + + ## Parameters + + - token: String, the IVLE authentication token + + ## Examples + + iex> Cadet.Accounts.Ivle.fetch_nusnet_id("T0K3N...") + {:ok, "e012345"} + """ def fetch_nusnet_id(token) do - url = "https://ivle.nus.edu.sg/api/Lapi.svc/UserID_Get" - key = System.get_env("IVLE_KEY") - - case HTTPoison.get("#{url}?APIKey=#{key}&Token=#{token}") do + case HTTPoison.get(api_url("UserID_Get", token)) do {:ok, response} -> if response.status_code == 200 do {:ok, String.replace(response.body, ~s("), "")} else - {:error, response.status_code} + {:error, :bad_request} end - {:error, reason} -> - {:error, reason} + {:error, _} -> + {:error, :bad_request} end end @doc """ - Get the name of this token + Get the full name of the user corresponding to this token. + + ## Parameters + + - token: String, the IVLE authentication token + + ## Examples + + iex> Cadet.Accounts.Ivle.fetch_name("T0K3N...") + {:ok, "LEE NING YUAN"} + """ def fetch_name(token) do - url = "https://ivle.nus.edu.sg/api/Lapi.svc/UserName_Get" - key = System.get_env("IVLE_KEY") - - case HTTPoison.get("#{url}?APIKey=#{key}&Token=#{token}") do + case HTTPoison.get(api_url("UserName_get", token)) do {:ok, response} -> if response.status_code == 200 do {:ok, String.replace(response.body, ~s("), "")} else - {:error, response.status_code} + {:error, :bad_request} end - {:error, reason} -> - {:error, reason} + {:error, _} -> + {:error, :bad_request} end end + + defp api_url(path, token) do + # construct a valid URL with the module attributes, and given params + "#{@api_url}/#{path}?APIKey=#{@api_key}&Token=#{token}" + end end diff --git a/lib/cadet/auth/guardian.ex b/lib/cadet/auth/guardian.ex index 02fba77d1..362b10349 100644 --- a/lib/cadet/auth/guardian.ex +++ b/lib/cadet/auth/guardian.ex @@ -5,6 +5,7 @@ defmodule Cadet.Auth.Guardian do use Guardian, otp_app: :cadet alias Cadet.Accounts + alias Guardian.DB def subject_for_token(user, _claims) do {:ok, to_string(user.id)} @@ -21,25 +22,25 @@ defmodule Cadet.Auth.Guardian do end def after_encode_and_sign(resource, claims, token, _options) do - with {:ok, _} <- Guardian.DB.after_encode_and_sign(resource, claims["typ"], claims, token) do + with {:ok, _} <- DB.after_encode_and_sign(resource, claims["typ"], claims, token) do {:ok, token} end end def on_verify(claims, token, _options) do - with {:ok, _} <- Guardian.DB.on_verify(claims, token) do + with {:ok, _} <- DB.on_verify(claims, token) do {:ok, claims} end end def on_refresh({old_token, old_claims}, {new_token, new_claims}, _options) do - with {:ok, _} <- Guardian.DB.on_refresh({old_token, old_claims}, {new_token, new_claims}) do + with {:ok, _} <- DB.on_refresh({old_token, old_claims}, {new_token, new_claims}) do {:ok, {old_token, old_claims}, {new_token, new_claims}} end end def on_revoke(claims, token, _options) do - with {:ok, _} <- Guardian.DB.on_revoke(claims, token) do + with {:ok, _} <- DB.on_revoke(claims, token) do {:ok, claims} end end diff --git a/lib/cadet_web/controllers/auth_controller.ex b/lib/cadet_web/controllers/auth_controller.ex index d69cb6a95..a56c84dc1 100644 --- a/lib/cadet_web/controllers/auth_controller.ex +++ b/lib/cadet_web/controllers/auth_controller.ex @@ -6,20 +6,19 @@ defmodule CadetWeb.AuthController do use PhoenixSwagger alias Cadet.Accounts - alias Cadet.Accounts.Form + alias Cadet.Accounts.Form.Login alias Cadet.Accounts.Ivle alias Cadet.Auth.Guardian @doc """ - Receives a /login request with valid attributes. + Receives a /login request with valid attributes (`Login form`). - If the user is already registered in our database, simply return tokens. - - If the user has not been registered before, register the user, then return the - tokens. + If the user is already registered in our database, simply return `Tokens`. If + the user has not been registered before, register the user, then return the + `Tokens`. """ def create(conn, %{"login" => attrs}) do - changeset = Form.Login.changeset(%Form.Login{}, attrs) + changeset = Login.changeset(%Login{}, attrs) if changeset.valid? do login = apply_changes(changeset) @@ -41,20 +40,23 @@ defmodule CadetWeb.AuthController do refresh_token: refresh_token ) - {:error, _reason} -> - send_resp(conn, :forbidden, "Internal sign-in process failed") + {:error, :bad_request} -> + send_resp(conn, :bad_request, "Invalid token") end - {:error, _reason} -> - send_resp(conn, :bad_request, "Request to IVLE failed") + {:error, reason} -> + send_resp(conn, :service_unavailable, "Authentication call to IVLE failed: #{reason}") end else - send_resp(conn, :bad_request, "Missing parameters") + send_resp(conn, :bad_request, "Missing parameter") end end + @doc """ + Receives a /login request with invalid attributes. + """ def create(conn, _params) do - send_resp(conn, :bad_request, "Missing parameters") + send_resp(conn, :bad_request, "Missing parameter") end def refresh(conn, %{"refresh_token" => refresh_token}) do @@ -109,8 +111,8 @@ defmodule CadetWeb.AuthController do end response(200, "OK", Schema.ref(:Tokens)) - response(400, "Missing parameter(s)") - response(403, "Wrong login attributes") + response(400, "Missing parameter/Invalid token") + response(503, "Authentication call to IVLE failed") end swagger_path :refresh do From 263296b0422e7d28ec74058a39f4f4a9a97736ba Mon Sep 17 00:00:00 2001 From: ning Date: Thu, 14 Jun 2018 02:16:53 +0800 Subject: [PATCH 12/39] Squash migrations into a single file --- .../migrations/20180612084218_add_ivle_to_accounts.exs | 4 ++++ .../20180613104539_authorization_remove_token.exs | 9 --------- 2 files changed, 4 insertions(+), 9 deletions(-) delete mode 100644 priv/repo/migrations/20180613104539_authorization_remove_token.exs diff --git a/priv/repo/migrations/20180612084218_add_ivle_to_accounts.exs b/priv/repo/migrations/20180612084218_add_ivle_to_accounts.exs index bb8f767f4..9434aeeb3 100644 --- a/priv/repo/migrations/20180612084218_add_ivle_to_accounts.exs +++ b/priv/repo/migrations/20180612084218_add_ivle_to_accounts.exs @@ -7,5 +7,9 @@ defmodule Cadet.Repo.Migrations.AddIvleToAccounts do remove(:last_name) add(:name, :string) end + + alter table(:authorizations) do + remove(:token) + end end end diff --git a/priv/repo/migrations/20180613104539_authorization_remove_token.exs b/priv/repo/migrations/20180613104539_authorization_remove_token.exs deleted file mode 100644 index 6673d9c1b..000000000 --- a/priv/repo/migrations/20180613104539_authorization_remove_token.exs +++ /dev/null @@ -1,9 +0,0 @@ -defmodule Cadet.Repo.Migrations.AuthorizationRemoveToken do - use Ecto.Migration - - def change do - alter table(:authorizations) do - remove(:token) - end - end -end From a9925f83a9fe623a3bee4438b8bf44359ec819cb Mon Sep 17 00:00:00 2001 From: ning Date: Thu, 14 Jun 2018 11:50:00 +0800 Subject: [PATCH 13/39] Add tests for Ivle module via ExVCR --- config/test.exs | 3 ++ .../vcr_cassettes/ivle/fetch_nusnet_id#1.json | 29 ++++++++++++++ .../vcr_cassettes/ivle/fetch_nusnet_id#2.json | 29 ++++++++++++++ lib/cadet/accounts/ivle.ex | 12 +++++- mix.exs | 1 + mix.lock | 6 ++- test/cadet/{ => accounts}/accounts_test.exs | 0 test/cadet/accounts/ivle_test.exs | 38 +++++++++++++++++++ 8 files changed, 115 insertions(+), 3 deletions(-) create mode 100644 fixture/vcr_cassettes/ivle/fetch_nusnet_id#1.json create mode 100644 fixture/vcr_cassettes/ivle/fetch_nusnet_id#2.json rename test/cadet/{ => accounts}/accounts_test.exs (100%) create mode 100644 test/cadet/accounts/ivle_test.exs diff --git a/config/test.exs b/config/test.exs index 6d114bc5d..8e08ba547 100644 --- a/config/test.exs +++ b/config/test.exs @@ -14,6 +14,9 @@ config :pbkdf2_elixir, :rounds, 1 # Print only warnings and errors during test config :logger, level: :warn +# Don't save secret keys in ExVCR cassettes +config :exvcr, filter_url_params: true + # Configure your database config :cadet, Cadet.Repo, adapter: Ecto.Adapters.Postgres, diff --git a/fixture/vcr_cassettes/ivle/fetch_nusnet_id#1.json b/fixture/vcr_cassettes/ivle/fetch_nusnet_id#1.json new file mode 100644 index 000000000..e1d86eb90 --- /dev/null +++ b/fixture/vcr_cassettes/ivle/fetch_nusnet_id#1.json @@ -0,0 +1,29 @@ +[ + { + "request": { + "body": "", + "headers": [], + "method": "get", + "options": [], + "request_body": "", + "url": "https://ivle.nus.edu.sg/api/Lapi.svc/UserID_Get" + }, + "response": { + "binary": false, + "body": "\"e0175719\"", + "headers": { + "Cache-Control": "private", + "Content-Type": "application/json", + "Server": "Microsoft-IIS/8.5", + "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", + "Set-Cookie": "ASP.NET_SessionId=lfsylbddbogkoekdb1fmvhyy; path=/; HttpOnly", + "X-AspNet-Version": "4.0.30319", + "X-Powered-By": "ASP.NET", + "Date": "Thu, 14 Jun 2018 03:48:00 GMT", + "Content-Length": "10" + }, + "status_code": 200, + "type": "ok" + } + } +] \ No newline at end of file diff --git a/fixture/vcr_cassettes/ivle/fetch_nusnet_id#2.json b/fixture/vcr_cassettes/ivle/fetch_nusnet_id#2.json new file mode 100644 index 000000000..93fa27cee --- /dev/null +++ b/fixture/vcr_cassettes/ivle/fetch_nusnet_id#2.json @@ -0,0 +1,29 @@ +[ + { + "request": { + "body": "", + "headers": [], + "method": "get", + "options": [], + "request_body": "", + "url": "https://ivle.nus.edu.sg/api/Lapi.svc/UserID_Get" + }, + "response": { + "binary": false, + "body": "\"\"", + "headers": { + "Cache-Control": "private", + "Content-Type": "application/json", + "Server": "Microsoft-IIS/8.5", + "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", + "Set-Cookie": "ASP.NET_SessionId=cfeb2olnqqvw0bppw21mpqjq; path=/; HttpOnly", + "X-AspNet-Version": "4.0.30319", + "X-Powered-By": "ASP.NET", + "Date": "Thu, 14 Jun 2018 03:48:00 GMT", + "Content-Length": "2" + }, + "status_code": 200, + "type": "ok" + } + } +] \ No newline at end of file diff --git a/lib/cadet/accounts/ivle.ex b/lib/cadet/accounts/ivle.ex index eda010377..3d063ad83 100644 --- a/lib/cadet/accounts/ivle.ex +++ b/lib/cadet/accounts/ivle.ex @@ -14,6 +14,14 @@ defmodule Cadet.Accounts.Ivle do @doc """ Get the NUSNET ID of the user corresponding to this token. + IVLE responses + + - 200 with non-empty body: valid key & valid token + - 500: invalid key + - 200 with 'empty' body: valid key but invalid token + + Note: an 'empty' body is a string with two literal double quotes, i.e. "\"\"" + ## Parameters - token: String, the IVLE authentication token @@ -27,14 +35,14 @@ defmodule Cadet.Accounts.Ivle do def fetch_nusnet_id(token) do case HTTPoison.get(api_url("UserID_Get", token)) do {:ok, response} -> - if response.status_code == 200 do + if String.length(response.body) > 2 do {:ok, String.replace(response.body, ~s("), "")} else {:error, :bad_request} end {:error, _} -> - {:error, :bad_request} + {:error, :internal_server_error} end end diff --git a/mix.exs b/mix.exs index c78e1f662..ae6357444 100644 --- a/mix.exs +++ b/mix.exs @@ -64,6 +64,7 @@ defmodule Cadet.Mixfile do {:credo, "~> 0.8", only: [:dev, :test], runtime: false}, {:dialyxir, "~> 1.0.0-rc.2", only: [:dev, :test], runtime: false}, {:excoveralls, "~> 0.8", only: :test}, + {:exvcr, "~> 0.10", only: :test}, {:phoenix_live_reload, "~> 1.0", only: :dev}, {:pre_commit, "~> 0.3.4", only: [:dev, :test]} ] diff --git a/mix.lock b/mix.lock index a102934cf..507fabe73 100644 --- a/mix.lock +++ b/mix.lock @@ -5,7 +5,6 @@ "bunt": {:hex, :bunt, "0.2.0", "951c6e801e8b1d2cbe58ebbd3e616a869061ddadcc4863d0a2182541acae9a38", [:mix], [], "hexpm"}, "certifi": {:hex, :certifi, "2.3.1", "d0f424232390bf47d82da8478022301c561cf6445b5b5fb6a84d49a9e76d2639", [:rebar3], [{:parse_trans, "3.2.0", [hex: :parse_trans, repo: "hexpm", optional: false]}], "hexpm"}, "combine": {:hex, :combine, "0.10.0", "eff8224eeb56498a2af13011d142c5e7997a80c8f5b97c499f84c841032e429f", [:mix], [], "hexpm"}, - "comeonin": {:hex, :comeonin, "4.1.1", "c7304fc29b45b897b34142a91122bc72757bc0c295e9e824999d5179ffc08416", [:mix], [{:argon2_elixir, "~> 1.2", [hex: :argon2_elixir, repo: "hexpm", optional: true]}, {:bcrypt_elixir, "~> 0.12.1 or ~> 1.0", [hex: :bcrypt_elixir, repo: "hexpm", optional: true]}, {:pbkdf2_elixir, "~> 0.12", [hex: :pbkdf2_elixir, repo: "hexpm", optional: true]}], "hexpm"}, "connection": {:hex, :connection, "1.0.4", "a1cae72211f0eef17705aaededacac3eb30e6625b04a6117c1b2db6ace7d5976", [:mix], [], "hexpm"}, "cowboy": {:hex, :cowboy, "1.1.2", "61ac29ea970389a88eca5a65601460162d370a70018afe6f949a29dca91f3bb0", [:rebar3], [{:cowlib, "~> 1.0.2", [hex: :cowlib, repo: "hexpm", optional: false]}, {:ranch, "~> 1.3.2", [hex: :ranch, repo: "hexpm", optional: false]}], "hexpm"}, "cowlib": {:hex, :cowlib, "1.0.2", "9d769a1d062c9c3ac753096f868ca121e2730b9a377de23dec0f7e08b1df84ee", [:make], [], "hexpm"}, @@ -18,7 +17,10 @@ "ecto_enum": {:hex, :ecto_enum, "1.1.0", "d44fe2ce6e1c0e907e7c3b6456a69e0f1d662348d8b4e2a662ba312223d8ff62", [:mix], [{:ecto, ">= 2.0.0", [hex: :ecto, repo: "hexpm", optional: false]}, {:mariaex, ">= 0.0.0", [hex: :mariaex, repo: "hexpm", optional: true]}, {:postgrex, ">= 0.0.0", [hex: :postgrex, repo: "hexpm", optional: true]}], "hexpm"}, "ex_json_schema": {:hex, :ex_json_schema, "0.5.7", "14a1bcd716e432badb19e5f54fd0f3f0be47b34d1b5957944702be90d66a6cf6", [:mix], [], "hexpm"}, "ex_machina": {:hex, :ex_machina, "2.2.0", "fec496331e04fc2db2a1a24fe317c12c0c4a50d2beb8ebb3531ed1f0d84be0ed", [:mix], [{:ecto, "~> 2.1", [hex: :ecto, repo: "hexpm", optional: true]}], "hexpm"}, + "exactor": {:hex, :exactor, "2.2.4", "5efb4ddeb2c48d9a1d7c9b465a6fffdd82300eb9618ece5d34c3334d5d7245b1", [:mix], [], "hexpm"}, "excoveralls": {:hex, :excoveralls, "0.9.1", "14fd20fac51ab98d8e79615814cc9811888d2d7b28e85aa90ff2e30dcf3191d6", [:mix], [{:hackney, ">= 0.12.0", [hex: :hackney, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm"}, + "exjsx": {:hex, :exjsx, "4.0.0", "60548841e0212df401e38e63c0078ec57b33e7ea49b032c796ccad8cde794b5c", [:mix], [{:jsx, "~> 2.8.0", [hex: :jsx, repo: "hexpm", optional: false]}], "hexpm"}, + "exvcr": {:hex, :exvcr, "0.10.2", "a66a0fa86d03153e5c21e38b1320d10b537038d7bc7b10dcc1ab7f0343569822", [:mix], [{:exactor, "~> 2.2", [hex: :exactor, repo: "hexpm", optional: false]}, {:exjsx, "~> 4.0", [hex: :exjsx, repo: "hexpm", optional: false]}, {:httpoison, "~> 1.0", [hex: :httpoison, repo: "hexpm", optional: true]}, {:httpotion, "~> 3.1", [hex: :httpotion, repo: "hexpm", optional: true]}, {:ibrowse, "~> 4.4", [hex: :ibrowse, repo: "hexpm", optional: true]}, {:meck, "~> 0.8", [hex: :meck, repo: "hexpm", optional: false]}], "hexpm"}, "file_system": {:hex, :file_system, "0.2.5", "a3060f063b116daf56c044c273f65202e36f75ec42e678dc10653056d3366054", [:mix], [], "hexpm"}, "gettext": {:hex, :gettext, "0.15.0", "40a2b8ce33a80ced7727e36768499fc9286881c43ebafccae6bab731e2b2b8ce", [:mix], [], "hexpm"}, "guardian": {:hex, :guardian, "1.1.0", "36c1ea356a1bac02bc120c3f91f4f0259c5aa0ee92cee0efe8def5d7401f1921", [:mix], [{:jose, "~> 1.8", [hex: :jose, repo: "hexpm", optional: false]}, {:phoenix, "~> 1.0 or ~> 1.2 or ~> 1.3", [hex: :phoenix, repo: "hexpm", optional: true]}, {:plug, "~> 1.3.3 or ~> 1.4", [hex: :plug, repo: "hexpm", optional: true]}, {:poison, "~> 2.2 or ~> 3.0", [hex: :poison, repo: "hexpm", optional: false]}, {:uuid, ">= 1.1.1", [hex: :uuid, repo: "hexpm", optional: false]}], "hexpm"}, @@ -28,6 +30,8 @@ "idna": {:hex, :idna, "5.1.1", "cbc3b2fa1645113267cc59c760bafa64b2ea0334635ef06dbac8801e42f7279c", [:rebar3], [{:unicode_util_compat, "0.3.1", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm"}, "jason": {:hex, :jason, "1.0.0", "0f7cfa9bdb23fed721ec05419bcee2b2c21a77e926bce0deda029b5adc716fe2", [:mix], [{:decimal, "~> 1.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm"}, "jose": {:hex, :jose, "1.8.4", "7946d1e5c03a76ac9ef42a6e6a20001d35987afd68c2107bcd8f01a84e75aa73", [:mix, :rebar3], [{:base64url, "~> 0.0.1", [hex: :base64url, repo: "hexpm", optional: false]}], "hexpm"}, + "jsx": {:hex, :jsx, "2.8.3", "a05252d381885240744d955fbe3cf810504eb2567164824e19303ea59eef62cf", [:mix, :rebar3], [], "hexpm"}, + "meck": {:hex, :meck, "0.8.9", "64c5c0bd8bcca3a180b44196265c8ed7594e16bcc845d0698ec6b4e577f48188", [:rebar3], [], "hexpm"}, "metrics": {:hex, :metrics, "1.0.1", "25f094dea2cda98213cecc3aeff09e940299d950904393b2a29d191c346a8486", [:rebar3], [], "hexpm"}, "mime": {:hex, :mime, "1.3.0", "5e8d45a39e95c650900d03f897fbf99ae04f60ab1daa4a34c7a20a5151b7a5fe", [:mix], [], "hexpm"}, "mimerl": {:hex, :mimerl, "1.0.2", "993f9b0e084083405ed8252b99460c4f0563e41729ab42d9074fd5e52439be88", [:rebar3], [], "hexpm"}, diff --git a/test/cadet/accounts_test.exs b/test/cadet/accounts/accounts_test.exs similarity index 100% rename from test/cadet/accounts_test.exs rename to test/cadet/accounts/accounts_test.exs diff --git a/test/cadet/accounts/ivle_test.exs b/test/cadet/accounts/ivle_test.exs new file mode 100644 index 000000000..6bfcd6b2f --- /dev/null +++ b/test/cadet/accounts/ivle_test.exs @@ -0,0 +1,38 @@ +defmodule Cadet.Accounts.IvleTest do + @moduledoc """ + This test module uses pre-recoreded HTTP responses saved by ExVCR. This + allows testing without actual API calls. + + In the case that you need to change the recorded responses, you will need + to set the two environment variables IVLE_KEY (used as a module attribute in + `Cadet.Accounts.IVLE`) and TOKEN (used here). Don't forget to delete the + cassette files, otherwise ExVCR will not override the cassettes. + """ + + use ExUnit.Case, async: false + use ExVCR.Mock, adapter: ExVCR.Adapter.Hackney + + alias Cadet.Accounts.Ivle + + @token String.replace(inspect(System.get_env("TOKEN")), ~s("), "") + + setup_all do + HTTPoison.start() + end + + describe "Fetch an NUSNET ID" do + test "Using a valid token" do + use_cassette "ivle/fetch_nusnet_id#1" do + {:ok, resp} = Ivle.fetch_nusnet_id(@token) + assert String.length(resp) > 0 + end + end + + test "Using an invalid token" do + use_cassette "ivle/fetch_nusnet_id#2" do + {:error, resp} = Ivle.fetch_nusnet_id(@token <> "Z") + assert resp == :bad_request + end + end + end +end From 68c56c8c7581bc871f207089e72155607b98fd88 Mon Sep 17 00:00:00 2001 From: ning Date: Thu, 14 Jun 2018 12:18:43 +0800 Subject: [PATCH 14/39] Add test for fetch_name in Cadet.Accounts.Ivle --- fixture/vcr_cassettes/ivle/fetch_name#1.json | 29 +++++++++++++ fixture/vcr_cassettes/ivle/fetch_name#2.json | 29 +++++++++++++ lib/cadet/accounts/ivle.ex | 43 +++++++++----------- test/cadet/accounts/ivle_test.exs | 16 ++++++++ 4 files changed, 94 insertions(+), 23 deletions(-) create mode 100644 fixture/vcr_cassettes/ivle/fetch_name#1.json create mode 100644 fixture/vcr_cassettes/ivle/fetch_name#2.json diff --git a/fixture/vcr_cassettes/ivle/fetch_name#1.json b/fixture/vcr_cassettes/ivle/fetch_name#1.json new file mode 100644 index 000000000..57c8cdb40 --- /dev/null +++ b/fixture/vcr_cassettes/ivle/fetch_name#1.json @@ -0,0 +1,29 @@ +[ + { + "request": { + "body": "", + "headers": [], + "method": "get", + "options": [], + "request_body": "", + "url": "https://ivle.nus.edu.sg/api/Lapi.svc/UserName_Get" + }, + "response": { + "binary": false, + "body": "\"LEE NING YUAN\"", + "headers": { + "Cache-Control": "private", + "Content-Type": "application/json", + "Server": "Microsoft-IIS/8.5", + "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", + "Set-Cookie": "ASP.NET_SessionId=d0qbwr4kfx1dt40nqpkce1wh; path=/; HttpOnly", + "X-AspNet-Version": "4.0.30319", + "X-Powered-By": "ASP.NET", + "Date": "Thu, 14 Jun 2018 04:18:14 GMT", + "Content-Length": "15" + }, + "status_code": 200, + "type": "ok" + } + } +] \ No newline at end of file diff --git a/fixture/vcr_cassettes/ivle/fetch_name#2.json b/fixture/vcr_cassettes/ivle/fetch_name#2.json new file mode 100644 index 000000000..6e81a2522 --- /dev/null +++ b/fixture/vcr_cassettes/ivle/fetch_name#2.json @@ -0,0 +1,29 @@ +[ + { + "request": { + "body": "", + "headers": [], + "method": "get", + "options": [], + "request_body": "", + "url": "https://ivle.nus.edu.sg/api/Lapi.svc/UserName_Get" + }, + "response": { + "binary": false, + "body": "\"\"", + "headers": { + "Cache-Control": "private", + "Content-Type": "application/json", + "Server": "Microsoft-IIS/8.5", + "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", + "Set-Cookie": "ASP.NET_SessionId=uajdaozgjmfr3sbbxqe5ltls; path=/; HttpOnly", + "X-AspNet-Version": "4.0.30319", + "X-Powered-By": "ASP.NET", + "Date": "Thu, 14 Jun 2018 04:18:14 GMT", + "Content-Length": "2" + }, + "status_code": 200, + "type": "ok" + } + } +] \ No newline at end of file diff --git a/lib/cadet/accounts/ivle.ex b/lib/cadet/accounts/ivle.ex index 3d063ad83..a6710619c 100644 --- a/lib/cadet/accounts/ivle.ex +++ b/lib/cadet/accounts/ivle.ex @@ -14,13 +14,11 @@ defmodule Cadet.Accounts.Ivle do @doc """ Get the NUSNET ID of the user corresponding to this token. - IVLE responses + returns... - - 200 with non-empty body: valid key & valid token - - 500: invalid key - - 200 with 'empty' body: valid key but invalid token - - Note: an 'empty' body is a string with two literal double quotes, i.e. "\"\"" + - {:ok, nusnet_id} - valid token, nusnet_id is a string + - {:error, :bad_request} - invalid token + - {:error, :internal_server_error} - the ivle_key is invalid ## Parameters @@ -32,23 +30,17 @@ defmodule Cadet.Accounts.Ivle do {:ok, "e012345"} """ - def fetch_nusnet_id(token) do - case HTTPoison.get(api_url("UserID_Get", token)) do - {:ok, response} -> - if String.length(response.body) > 2 do - {:ok, String.replace(response.body, ~s("), "")} - else - {:error, :bad_request} - end - - {:error, _} -> - {:error, :internal_server_error} - end - end + def fetch_nusnet_id(token), do: api_fetch("UserID_Get", token) @doc """ Get the full name of the user corresponding to this token. + returns... + + - {:ok, username} - valid token, username is a string + - {:error, :bad_request} - invalid token + - {:error, :internal_server_error} - the ivle_key is invalid + ## Parameters - token: String, the IVLE authentication token @@ -59,17 +51,22 @@ defmodule Cadet.Accounts.Ivle do {:ok, "LEE NING YUAN"} """ - def fetch_name(token) do - case HTTPoison.get(api_url("UserName_get", token)) do + def fetch_name(token), do: api_fetch("UserName_Get", token) + + defp api_fetch(path, token) do + # IVLE responds with 200 (:ok) if key and token are valid + # If key is invalid, IVLE responds with 500 (:internal_server_error) + # If token is invalid, IVLE returns an 'empty' string (i.e. "\"\"") + case HTTPoison.get(api_url(path, token)) do {:ok, response} -> - if response.status_code == 200 do + if String.length(response.body) > 2 do {:ok, String.replace(response.body, ~s("), "")} else {:error, :bad_request} end {:error, _} -> - {:error, :bad_request} + {:error, :internal_server_error} end end diff --git a/test/cadet/accounts/ivle_test.exs b/test/cadet/accounts/ivle_test.exs index 6bfcd6b2f..4f3fed6e3 100644 --- a/test/cadet/accounts/ivle_test.exs +++ b/test/cadet/accounts/ivle_test.exs @@ -35,4 +35,20 @@ defmodule Cadet.Accounts.IvleTest do end end end + + describe "Fetch a name" do + test "Using a valid token" do + use_cassette "ivle/fetch_name#1" do + {:ok, resp} = Ivle.fetch_name(@token) + assert String.length(resp) > 0 + end + end + + test "Using an invalid token" do + use_cassette "ivle/fetch_name#2" do + {:error, resp} = Ivle.fetch_name(@token <> "Z") + assert resp == :bad_request + end + end + end end From edad9d32aed7e417e5e9683dafa63fbe02e30185 Mon Sep 17 00:00:00 2001 From: ning Date: Thu, 14 Jun 2018 12:31:40 +0800 Subject: [PATCH 15/39] Fine tune /auth error codes --- lib/cadet/accounts/accounts.ex | 9 ++++++--- lib/cadet_web/controllers/auth_controller.ex | 12 +++++++----- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/lib/cadet/accounts/accounts.ex b/lib/cadet/accounts/accounts.ex index eec699879..fa3ef0a0a 100644 --- a/lib/cadet/accounts/accounts.ex +++ b/lib/cadet/accounts/accounts.ex @@ -12,6 +12,8 @@ defmodule Cadet.Accounts do @doc """ Register new User entity using Cadet.Accounts.Form.Registration + + Returns {:ok, user} on success, otherwise {:error, changeset} """ def register(attrs = %{}, role) do changeset = Registration.changeset(%Registration{}, attrs) @@ -109,11 +111,12 @@ defmodule Cadet.Accounts do sign_in(nusnet_id, token) {:error, _} -> - {:error, :bad_request} + {:error, :internal_server_error} end - {:error, _} -> - {:error, :bad_request} + {:error, reason} -> + # reason can be :bad_request or :internal_server_error + {:error, reason} end else auth = Repo.preload(auth, :user) diff --git a/lib/cadet_web/controllers/auth_controller.ex b/lib/cadet_web/controllers/auth_controller.ex index a56c84dc1..d3602b119 100644 --- a/lib/cadet_web/controllers/auth_controller.ex +++ b/lib/cadet_web/controllers/auth_controller.ex @@ -40,12 +40,14 @@ defmodule CadetWeb.AuthController do refresh_token: refresh_token ) - {:error, :bad_request} -> - send_resp(conn, :bad_request, "Invalid token") + {:error, reason} -> + # reason can be :bad_request or :internal_server_error + send_resp(conn, reason, "Unable to retrieve user") end {:error, reason} -> - send_resp(conn, :service_unavailable, "Authentication call to IVLE failed: #{reason}") + # reason can be :bad_request or :internal_server_error + send_resp(conn, reason, "Unable to fetch NUSNET ID from IVLE.") end else send_resp(conn, :bad_request, "Missing parameter") @@ -111,8 +113,8 @@ defmodule CadetWeb.AuthController do end response(200, "OK", Schema.ref(:Tokens)) - response(400, "Missing parameter/Invalid token") - response(503, "Authentication call to IVLE failed") + response(400, "Missing or invalid parameter") + response(500, "Internal server error") end swagger_path :refresh do From 0eef9d06d23361e975a32887dbed61dd464fea11 Mon Sep 17 00:00:00 2001 From: ning Date: Thu, 14 Jun 2018 13:18:20 +0800 Subject: [PATCH 16/39] Add tests for sign_in in Cadet.Accounts --- fixture/vcr_cassettes/accounts/sign_in#1.json | 29 +++++++++++++ fixture/vcr_cassettes/accounts/sign_in#2.json | 29 +++++++++++++ test/cadet/accounts/accounts_test.exs | 42 ++++++++++++++++--- test/cadet/accounts/ivle_test.exs | 2 +- 4 files changed, 96 insertions(+), 6 deletions(-) create mode 100644 fixture/vcr_cassettes/accounts/sign_in#1.json create mode 100644 fixture/vcr_cassettes/accounts/sign_in#2.json diff --git a/fixture/vcr_cassettes/accounts/sign_in#1.json b/fixture/vcr_cassettes/accounts/sign_in#1.json new file mode 100644 index 000000000..12ac009c0 --- /dev/null +++ b/fixture/vcr_cassettes/accounts/sign_in#1.json @@ -0,0 +1,29 @@ +[ + { + "request": { + "body": "", + "headers": [], + "method": "get", + "options": [], + "request_body": "", + "url": "https://ivle.nus.edu.sg/api/Lapi.svc/UserName_Get" + }, + "response": { + "binary": false, + "body": "\"LEE NING YUAN\"", + "headers": { + "Cache-Control": "private", + "Content-Type": "application/json", + "Server": "Microsoft-IIS/8.5", + "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", + "Set-Cookie": "ASP.NET_SessionId=givaibq0nnctlxkzsgf20qs3; path=/; HttpOnly", + "X-AspNet-Version": "4.0.30319", + "X-Powered-By": "ASP.NET", + "Date": "Thu, 14 Jun 2018 05:17:33 GMT", + "Content-Length": "15" + }, + "status_code": 200, + "type": "ok" + } + } +] \ No newline at end of file diff --git a/fixture/vcr_cassettes/accounts/sign_in#2.json b/fixture/vcr_cassettes/accounts/sign_in#2.json new file mode 100644 index 000000000..9b2d9e7e4 --- /dev/null +++ b/fixture/vcr_cassettes/accounts/sign_in#2.json @@ -0,0 +1,29 @@ +[ + { + "request": { + "body": "", + "headers": [], + "method": "get", + "options": [], + "request_body": "", + "url": "https://ivle.nus.edu.sg/api/Lapi.svc/UserName_Get" + }, + "response": { + "binary": false, + "body": "\"\"", + "headers": { + "Cache-Control": "private", + "Content-Type": "application/json", + "Server": "Microsoft-IIS/8.5", + "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", + "Set-Cookie": "ASP.NET_SessionId=wcz2rmzelxkjsblxmogmnxot; path=/; HttpOnly", + "X-AspNet-Version": "4.0.30319", + "X-Powered-By": "ASP.NET", + "Date": "Thu, 14 Jun 2018 05:17:33 GMT", + "Content-Length": "2" + }, + "status_code": 200, + "type": "ok" + } + } +] \ No newline at end of file diff --git a/test/cadet/accounts/accounts_test.exs b/test/cadet/accounts/accounts_test.exs index 5a8ba01a7..65cca8bf4 100644 --- a/test/cadet/accounts/accounts_test.exs +++ b/test/cadet/accounts/accounts_test.exs @@ -1,7 +1,26 @@ defmodule Cadet.AccountsTest do + @moduledoc """ + Some tests in this module use pre-recorded HTTP responses saved by ExVCR. + this allows testing without the use of actual external IVLE API calls. + + In the case that you need to change the recorded responses, you will need + to set the two environment variables IVLE_KEY (used as a module attribute in + `Cadet.Accounts.IVLE`) and TOKEN (used here). Don't forget to delete the + cassette files, otherwise ExVCR will not override the cassettes. + """ + use Cadet.DataCase + use ExVCR.Mock, adapter: ExVCR.Adapter.Hackney alias Cadet.Accounts + alias Cadet.Repo + alias Cadet.Accounts.Query + + @token String.replace(inspect(System.get_env("TOKEN")), ~s("), "") + + setup_all do + HTTPoison.start() + end test "create user" do {:ok, user} = @@ -97,19 +116,32 @@ defmodule Cadet.AccountsTest do end describe "sign in using nusnet_id" do - test "success" do + test "unregistered user" do + use_cassette "accounts/sign_in#1" do + {:ok, _} = Accounts.sign_in("e012345", @token) + assert Repo.one(Query.nusnet_id("e012345")).uid == "e012345" + end + end + + test "registered user" do user = insert(:user) insert(:nusnet_id, %{ - uid: "E012345", + uid: "e012345", user: user }) - assert {:ok, user} == Accounts.sign_in("E012345", "t0k3n") + assert {:ok, user} == Accounts.sign_in("e012345", @token) + end + + test "invalid token" do + use_cassette "accounts/sign_in#2" do + assert {:error, :bad_request} == Accounts.sign_in("A0123456", "t0k3n") + end end - test "NUSNET ID not found" do - assert {:error, :bad_request} == Accounts.sign_in("A0123456", "t0k3n") + test "invalid nusnet id" do + assert {:error, :internal_server_error} == Accounts.sign_in("", @token) end end end diff --git a/test/cadet/accounts/ivle_test.exs b/test/cadet/accounts/ivle_test.exs index 4f3fed6e3..cc6ba5099 100644 --- a/test/cadet/accounts/ivle_test.exs +++ b/test/cadet/accounts/ivle_test.exs @@ -1,7 +1,7 @@ defmodule Cadet.Accounts.IvleTest do @moduledoc """ This test module uses pre-recoreded HTTP responses saved by ExVCR. This - allows testing without actual API calls. + allows testing without actual external IVLE API calls. In the case that you need to change the recorded responses, you will need to set the two environment variables IVLE_KEY (used as a module attribute in From 20c465e0e4ffca2d64fcd9c629d417b212b65f6f Mon Sep 17 00:00:00 2001 From: ning Date: Thu, 14 Jun 2018 14:59:40 +0800 Subject: [PATCH 17/39] Add tests for AuthController using ExVCR --- .../auth_controller/v1/auth#1.json | 56 +++++++++++++++++++ .../controllers/auth_controller_test.exs | 39 ++++++++++++- 2 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 fixture/vcr_cassettes/auth_controller/v1/auth#1.json diff --git a/fixture/vcr_cassettes/auth_controller/v1/auth#1.json b/fixture/vcr_cassettes/auth_controller/v1/auth#1.json new file mode 100644 index 000000000..d657923bd --- /dev/null +++ b/fixture/vcr_cassettes/auth_controller/v1/auth#1.json @@ -0,0 +1,56 @@ +[ + { + "request": { + "body": "", + "headers": [], + "method": "get", + "options": [], + "request_body": "", + "url": "https://ivle.nus.edu.sg/api/Lapi.svc/UserID_Get" + }, + "response": { + "binary": false, + "body": "\"e0175719\"", + "headers": { + "Cache-Control": "private", + "Content-Type": "application/json", + "Server": "Microsoft-IIS/8.5", + "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", + "Set-Cookie": "ASP.NET_SessionId=jwly1z0b1n2pe00eeaq3bggh; path=/; HttpOnly", + "X-AspNet-Version": "4.0.30319", + "X-Powered-By": "ASP.NET", + "Date": "Thu, 14 Jun 2018 06:58:46 GMT", + "Content-Length": "10" + }, + "status_code": 200, + "type": "ok" + } + }, + { + "request": { + "body": "", + "headers": [], + "method": "get", + "options": [], + "request_body": "", + "url": "https://ivle.nus.edu.sg/api/Lapi.svc/UserName_Get" + }, + "response": { + "binary": false, + "body": "\"LEE NING YUAN\"", + "headers": { + "Cache-Control": "private", + "Content-Type": "application/json", + "Server": "Microsoft-IIS/8.5", + "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", + "Set-Cookie": "ASP.NET_SessionId=i23dg0wwtwugerj5ckaldtmk; path=/; HttpOnly", + "X-AspNet-Version": "4.0.30319", + "X-Powered-By": "ASP.NET", + "Date": "Thu, 14 Jun 2018 06:58:46 GMT", + "Content-Length": "15" + }, + "status_code": 200, + "type": "ok" + } + } +] \ No newline at end of file diff --git a/test/cadet_web/controllers/auth_controller_test.exs b/test/cadet_web/controllers/auth_controller_test.exs index da46898da..c4e436558 100644 --- a/test/cadet_web/controllers/auth_controller_test.exs +++ b/test/cadet_web/controllers/auth_controller_test.exs @@ -1,11 +1,28 @@ defmodule CadetWeb.AuthControllerTest do + @moduledoc """ + Some tests in this module use pre-recorded HTTP responses saved by ExVCR. + this allows testing without the use of actual external IVLE API calls. + + In the case that you need to change the recorded responses, you will need + to set the two environment variables IVLE_KEY (used as a module attribute in + `Cadet.Accounts.IVLE`) and TOKEN (used here). Don't forget to delete the + cassette files, otherwise ExVCR will not override the cassettes. + """ + use CadetWeb.ConnCase + use ExVCR.Mock, adapter: ExVCR.Adapter.Hackney import Cadet.Factory alias CadetWeb.AuthController alias Cadet.Auth.Guardian + @token String.replace(inspect(System.get_env("TOKEN")), ~s("), "") + + setup_all do + HTTPoison.start() + end + test "swagger" do AuthController.swagger_definitions() AuthController.swagger_path_create(nil) @@ -13,7 +30,18 @@ defmodule CadetWeb.AuthControllerTest do AuthController.swagger_path_logout(nil) end - describe "POST /v1/auth" do + describe "POST /auth" do + test "success", %{conn: conn} do + use_cassette "auth_controller/v1/auth#1" do + conn = + post(conn, "/v1/auth", %{ + "login" => %{"ivle_token" => @token} + }) + + assert response(conn, 200) + end + end + test "missing parameter", %{conn: conn} do conn = post(conn, "/v1/auth", %{}) @@ -28,6 +56,15 @@ defmodule CadetWeb.AuthControllerTest do assert response(conn, 400) end + + test "invalid token", %{conn: conn} do + conn = + post(conn, "/v1/auth", %{ + "login" => %{"ivle_token" => @token <> "Z"} + }) + + assert response(conn, 400) + end end describe "POST /auth/refresh" do From 5267c28ad0458d9a9520df71a88a27210585794c Mon Sep 17 00:00:00 2001 From: ning Date: Thu, 14 Jun 2018 15:10:26 +0800 Subject: [PATCH 18/39] Fix failing travis.ci tests --- .../auth_controller/v1/auth#2.json | 29 +++++++++++++++++++ .../controllers/auth_controller_test.exs | 12 ++++---- 2 files changed, 36 insertions(+), 5 deletions(-) create mode 100644 fixture/vcr_cassettes/auth_controller/v1/auth#2.json diff --git a/fixture/vcr_cassettes/auth_controller/v1/auth#2.json b/fixture/vcr_cassettes/auth_controller/v1/auth#2.json new file mode 100644 index 000000000..f60f9cf8f --- /dev/null +++ b/fixture/vcr_cassettes/auth_controller/v1/auth#2.json @@ -0,0 +1,29 @@ +[ + { + "request": { + "body": "", + "headers": [], + "method": "get", + "options": [], + "request_body": "", + "url": "https://ivle.nus.edu.sg/api/Lapi.svc/UserID_Get" + }, + "response": { + "binary": false, + "body": "\"\"", + "headers": { + "Cache-Control": "private", + "Content-Type": "application/json", + "Server": "Microsoft-IIS/8.5", + "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", + "Set-Cookie": "ASP.NET_SessionId=4nn21p3coilmdgmlfl4lh1rc; path=/; HttpOnly", + "X-AspNet-Version": "4.0.30319", + "X-Powered-By": "ASP.NET", + "Date": "Thu, 14 Jun 2018 07:10:13 GMT", + "Content-Length": "2" + }, + "status_code": 200, + "type": "ok" + } + } +] \ No newline at end of file diff --git a/test/cadet_web/controllers/auth_controller_test.exs b/test/cadet_web/controllers/auth_controller_test.exs index c4e436558..9c53fd25f 100644 --- a/test/cadet_web/controllers/auth_controller_test.exs +++ b/test/cadet_web/controllers/auth_controller_test.exs @@ -58,12 +58,14 @@ defmodule CadetWeb.AuthControllerTest do end test "invalid token", %{conn: conn} do - conn = - post(conn, "/v1/auth", %{ - "login" => %{"ivle_token" => @token <> "Z"} - }) + use_cassette "auth_controller/v1/auth#2" do + conn = + post(conn, "/v1/auth", %{ + "login" => %{"ivle_token" => @token <> "Z"} + }) - assert response(conn, 400) + assert response(conn, 400) + end end end From dab8eb539c7136e8e993ccb78670d29de736baea Mon Sep 17 00:00:00 2001 From: ning Date: Fri, 15 Jun 2018 15:14:41 +0800 Subject: [PATCH 19/39] Remove password fields from registration test --- .../cadet/accounts/form/registration_test.exs | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/test/cadet/accounts/form/registration_test.exs b/test/cadet/accounts/form/registration_test.exs index 583ba5d22..12d0ca83d 100644 --- a/test/cadet/accounts/form/registration_test.exs +++ b/test/cadet/accounts/form/registration_test.exs @@ -6,34 +6,34 @@ defmodule Cadet.Accounts.Form.RegistrationTest do valid_changesets Registration do %{ name: "happy", - nusnet_id: "e853820", - password: "mypassword", - password_confirmation: "mypassword" + nusnet_id: "e853820" } end invalid_changesets Registration do + %{} + %{ - password: "mypassword", - password_confirmation: "mypassword" + name: "happy" } %{ - first_name: "happy", - password: "mypassword", - password_confirmation: "doesnotmatch" + nusnet_id: "e853820" } %{ - first_name: "happy", - password: "mypassword", - password_confirmation: "mypassword" + name: "", + nusnet_id: "" } %{ - first_name: "happy", - password: "passwor", - password_confirmation: "passwor" + name: "", + nusnet_id: "e853820" + } + + %{ + name: "happy", + nusnet_id: "" } end end From 753d82aecf0fdbf96a8a3fd019d0708adf8df7ab Mon Sep 17 00:00:00 2001 From: ning Date: Sat, 16 Jun 2018 20:16:32 +0800 Subject: [PATCH 20/39] Set IVLE_KEY in .env rather than environment var --- .env | 3 +++ README.md | 5 +++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/.env b/.env index 3c802e43c..8f3b3f8e7 100644 --- a/.env +++ b/.env @@ -15,3 +15,6 @@ export CADET_WEBPACK_PORT=4001 # Webpack entry filename export CADET_WEBPACK_ENTRY=app + +# IVLE LAPI Key +export IVLE_KEY=example_ivle_key diff --git a/README.md b/README.md index 726e14540..ad1641208 100644 --- a/README.md +++ b/README.md @@ -25,9 +25,10 @@ Initialise development database mix ecto.setup -Setup environment variables +Replace the value of `IVLE_KEY` in the `.env` file with your [IVLE LAPI Key +](https://ivle.nus.edu.sg/LAPI/default.aspx) - export IVLE_KEY="1vl3keyy" + export IVLE_KEY=your_ivle_lapi_key Run the server in your local machine From 66808a23bbed82cd98ba5aabefb9de7153c27ad9 Mon Sep 17 00:00:00 2001 From: ning Date: Sat, 16 Jun 2018 20:32:49 +0800 Subject: [PATCH 21/39] Move ExVCR fixture to test/fixture --- README.md | 4 +++- config/test.exs | 4 +++- .../fixture}/vcr_cassettes/accounts/sign_in#1.json | 4 ++-- .../fixture}/vcr_cassettes/accounts/sign_in#2.json | 4 ++-- .../fixture}/vcr_cassettes/auth_controller/v1/auth#1.json | 8 ++++---- .../fixture}/vcr_cassettes/auth_controller/v1/auth#2.json | 4 ++-- .../fixture}/vcr_cassettes/ivle/fetch_name#1.json | 4 ++-- .../fixture}/vcr_cassettes/ivle/fetch_name#2.json | 4 ++-- .../fixture}/vcr_cassettes/ivle/fetch_nusnet_id#1.json | 4 ++-- .../fixture}/vcr_cassettes/ivle/fetch_nusnet_id#2.json | 4 ++-- 10 files changed, 24 insertions(+), 20 deletions(-) rename {fixture => test/fixture}/vcr_cassettes/accounts/sign_in#1.json (83%) rename {fixture => test/fixture}/vcr_cassettes/accounts/sign_in#2.json (85%) rename {fixture => test/fixture}/vcr_cassettes/auth_controller/v1/auth#1.json (83%) rename {fixture => test/fixture}/vcr_cassettes/auth_controller/v1/auth#2.json (82%) rename {fixture => test/fixture}/vcr_cassettes/ivle/fetch_name#1.json (83%) rename {fixture => test/fixture}/vcr_cassettes/ivle/fetch_name#2.json (82%) rename {fixture => test/fixture}/vcr_cassettes/ivle/fetch_nusnet_id#1.json (83%) rename {fixture => test/fixture}/vcr_cassettes/ivle/fetch_nusnet_id#2.json (82%) diff --git a/README.md b/README.md index ad1641208..cae0d050c 100644 --- a/README.md +++ b/README.md @@ -26,7 +26,9 @@ Initialise development database mix ecto.setup Replace the value of `IVLE_KEY` in the `.env` file with your [IVLE LAPI Key -](https://ivle.nus.edu.sg/LAPI/default.aspx) +](https://ivle.nus.edu.sg/LAPI/default.aspx). If you've compiled the +application before setting a valid value, you must force a recompilation with +`mix clean && mix`. export IVLE_KEY=your_ivle_lapi_key diff --git a/config/test.exs b/config/test.exs index 8e08ba547..f6d19ca3d 100644 --- a/config/test.exs +++ b/config/test.exs @@ -15,7 +15,9 @@ config :pbkdf2_elixir, :rounds, 1 config :logger, level: :warn # Don't save secret keys in ExVCR cassettes -config :exvcr, filter_url_params: true +config :exvcr, + filter_url_params: true, + vcr_cassette_library_dir: "test/fixture/vcr_cassettes" # Configure your database config :cadet, Cadet.Repo, diff --git a/fixture/vcr_cassettes/accounts/sign_in#1.json b/test/fixture/vcr_cassettes/accounts/sign_in#1.json similarity index 83% rename from fixture/vcr_cassettes/accounts/sign_in#1.json rename to test/fixture/vcr_cassettes/accounts/sign_in#1.json index 12ac009c0..50e2ea617 100644 --- a/fixture/vcr_cassettes/accounts/sign_in#1.json +++ b/test/fixture/vcr_cassettes/accounts/sign_in#1.json @@ -16,10 +16,10 @@ "Content-Type": "application/json", "Server": "Microsoft-IIS/8.5", "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", - "Set-Cookie": "ASP.NET_SessionId=givaibq0nnctlxkzsgf20qs3; path=/; HttpOnly", + "Set-Cookie": "ASP.NET_SessionId=ozaipr2wleccflp10toymiti; path=/; HttpOnly", "X-AspNet-Version": "4.0.30319", "X-Powered-By": "ASP.NET", - "Date": "Thu, 14 Jun 2018 05:17:33 GMT", + "Date": "Sat, 16 Jun 2018 12:28:47 GMT", "Content-Length": "15" }, "status_code": 200, diff --git a/fixture/vcr_cassettes/accounts/sign_in#2.json b/test/fixture/vcr_cassettes/accounts/sign_in#2.json similarity index 85% rename from fixture/vcr_cassettes/accounts/sign_in#2.json rename to test/fixture/vcr_cassettes/accounts/sign_in#2.json index 9b2d9e7e4..8af446729 100644 --- a/fixture/vcr_cassettes/accounts/sign_in#2.json +++ b/test/fixture/vcr_cassettes/accounts/sign_in#2.json @@ -16,10 +16,10 @@ "Content-Type": "application/json", "Server": "Microsoft-IIS/8.5", "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", - "Set-Cookie": "ASP.NET_SessionId=wcz2rmzelxkjsblxmogmnxot; path=/; HttpOnly", + "Set-Cookie": "ASP.NET_SessionId=ixih50bscs3up2pvx5fkypjt; path=/; HttpOnly", "X-AspNet-Version": "4.0.30319", "X-Powered-By": "ASP.NET", - "Date": "Thu, 14 Jun 2018 05:17:33 GMT", + "Date": "Sat, 16 Jun 2018 12:28:47 GMT", "Content-Length": "2" }, "status_code": 200, diff --git a/fixture/vcr_cassettes/auth_controller/v1/auth#1.json b/test/fixture/vcr_cassettes/auth_controller/v1/auth#1.json similarity index 83% rename from fixture/vcr_cassettes/auth_controller/v1/auth#1.json rename to test/fixture/vcr_cassettes/auth_controller/v1/auth#1.json index d657923bd..480da0f72 100644 --- a/fixture/vcr_cassettes/auth_controller/v1/auth#1.json +++ b/test/fixture/vcr_cassettes/auth_controller/v1/auth#1.json @@ -16,10 +16,10 @@ "Content-Type": "application/json", "Server": "Microsoft-IIS/8.5", "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", - "Set-Cookie": "ASP.NET_SessionId=jwly1z0b1n2pe00eeaq3bggh; path=/; HttpOnly", + "Set-Cookie": "ASP.NET_SessionId=zbfe1ecow0gautunvm0vl11i; path=/; HttpOnly", "X-AspNet-Version": "4.0.30319", "X-Powered-By": "ASP.NET", - "Date": "Thu, 14 Jun 2018 06:58:46 GMT", + "Date": "Sat, 16 Jun 2018 12:28:46 GMT", "Content-Length": "10" }, "status_code": 200, @@ -43,10 +43,10 @@ "Content-Type": "application/json", "Server": "Microsoft-IIS/8.5", "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", - "Set-Cookie": "ASP.NET_SessionId=i23dg0wwtwugerj5ckaldtmk; path=/; HttpOnly", + "Set-Cookie": "ASP.NET_SessionId=bprx04t4lfglt31tytffzmqj; path=/; HttpOnly", "X-AspNet-Version": "4.0.30319", "X-Powered-By": "ASP.NET", - "Date": "Thu, 14 Jun 2018 06:58:46 GMT", + "Date": "Sat, 16 Jun 2018 12:28:46 GMT", "Content-Length": "15" }, "status_code": 200, diff --git a/fixture/vcr_cassettes/auth_controller/v1/auth#2.json b/test/fixture/vcr_cassettes/auth_controller/v1/auth#2.json similarity index 82% rename from fixture/vcr_cassettes/auth_controller/v1/auth#2.json rename to test/fixture/vcr_cassettes/auth_controller/v1/auth#2.json index f60f9cf8f..6143cbe2f 100644 --- a/fixture/vcr_cassettes/auth_controller/v1/auth#2.json +++ b/test/fixture/vcr_cassettes/auth_controller/v1/auth#2.json @@ -16,10 +16,10 @@ "Content-Type": "application/json", "Server": "Microsoft-IIS/8.5", "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", - "Set-Cookie": "ASP.NET_SessionId=4nn21p3coilmdgmlfl4lh1rc; path=/; HttpOnly", + "Set-Cookie": "ASP.NET_SessionId=xtrx1om05w05hdosdfrzk1t1; path=/; HttpOnly", "X-AspNet-Version": "4.0.30319", "X-Powered-By": "ASP.NET", - "Date": "Thu, 14 Jun 2018 07:10:13 GMT", + "Date": "Sat, 16 Jun 2018 12:28:46 GMT", "Content-Length": "2" }, "status_code": 200, diff --git a/fixture/vcr_cassettes/ivle/fetch_name#1.json b/test/fixture/vcr_cassettes/ivle/fetch_name#1.json similarity index 83% rename from fixture/vcr_cassettes/ivle/fetch_name#1.json rename to test/fixture/vcr_cassettes/ivle/fetch_name#1.json index 57c8cdb40..580904507 100644 --- a/fixture/vcr_cassettes/ivle/fetch_name#1.json +++ b/test/fixture/vcr_cassettes/ivle/fetch_name#1.json @@ -16,10 +16,10 @@ "Content-Type": "application/json", "Server": "Microsoft-IIS/8.5", "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", - "Set-Cookie": "ASP.NET_SessionId=d0qbwr4kfx1dt40nqpkce1wh; path=/; HttpOnly", + "Set-Cookie": "ASP.NET_SessionId=qtyp2glxtn14vk3v0rmbhrne; path=/; HttpOnly", "X-AspNet-Version": "4.0.30319", "X-Powered-By": "ASP.NET", - "Date": "Thu, 14 Jun 2018 04:18:14 GMT", + "Date": "Sat, 16 Jun 2018 12:28:47 GMT", "Content-Length": "15" }, "status_code": 200, diff --git a/fixture/vcr_cassettes/ivle/fetch_name#2.json b/test/fixture/vcr_cassettes/ivle/fetch_name#2.json similarity index 82% rename from fixture/vcr_cassettes/ivle/fetch_name#2.json rename to test/fixture/vcr_cassettes/ivle/fetch_name#2.json index 6e81a2522..6320adf7f 100644 --- a/fixture/vcr_cassettes/ivle/fetch_name#2.json +++ b/test/fixture/vcr_cassettes/ivle/fetch_name#2.json @@ -16,10 +16,10 @@ "Content-Type": "application/json", "Server": "Microsoft-IIS/8.5", "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", - "Set-Cookie": "ASP.NET_SessionId=uajdaozgjmfr3sbbxqe5ltls; path=/; HttpOnly", + "Set-Cookie": "ASP.NET_SessionId=jcrp1b2fjtq02h3iw1jm51f1; path=/; HttpOnly", "X-AspNet-Version": "4.0.30319", "X-Powered-By": "ASP.NET", - "Date": "Thu, 14 Jun 2018 04:18:14 GMT", + "Date": "Sat, 16 Jun 2018 12:28:47 GMT", "Content-Length": "2" }, "status_code": 200, diff --git a/fixture/vcr_cassettes/ivle/fetch_nusnet_id#1.json b/test/fixture/vcr_cassettes/ivle/fetch_nusnet_id#1.json similarity index 83% rename from fixture/vcr_cassettes/ivle/fetch_nusnet_id#1.json rename to test/fixture/vcr_cassettes/ivle/fetch_nusnet_id#1.json index e1d86eb90..541026b76 100644 --- a/fixture/vcr_cassettes/ivle/fetch_nusnet_id#1.json +++ b/test/fixture/vcr_cassettes/ivle/fetch_nusnet_id#1.json @@ -16,10 +16,10 @@ "Content-Type": "application/json", "Server": "Microsoft-IIS/8.5", "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", - "Set-Cookie": "ASP.NET_SessionId=lfsylbddbogkoekdb1fmvhyy; path=/; HttpOnly", + "Set-Cookie": "ASP.NET_SessionId=g33tyvk4f4qcxs0o0og3caow; path=/; HttpOnly", "X-AspNet-Version": "4.0.30319", "X-Powered-By": "ASP.NET", - "Date": "Thu, 14 Jun 2018 03:48:00 GMT", + "Date": "Sat, 16 Jun 2018 12:28:47 GMT", "Content-Length": "10" }, "status_code": 200, diff --git a/fixture/vcr_cassettes/ivle/fetch_nusnet_id#2.json b/test/fixture/vcr_cassettes/ivle/fetch_nusnet_id#2.json similarity index 82% rename from fixture/vcr_cassettes/ivle/fetch_nusnet_id#2.json rename to test/fixture/vcr_cassettes/ivle/fetch_nusnet_id#2.json index 93fa27cee..877c76192 100644 --- a/fixture/vcr_cassettes/ivle/fetch_nusnet_id#2.json +++ b/test/fixture/vcr_cassettes/ivle/fetch_nusnet_id#2.json @@ -16,10 +16,10 @@ "Content-Type": "application/json", "Server": "Microsoft-IIS/8.5", "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", - "Set-Cookie": "ASP.NET_SessionId=cfeb2olnqqvw0bppw21mpqjq; path=/; HttpOnly", + "Set-Cookie": "ASP.NET_SessionId=cmnryzexfr2unm452fbqcel4; path=/; HttpOnly", "X-AspNet-Version": "4.0.30319", "X-Powered-By": "ASP.NET", - "Date": "Thu, 14 Jun 2018 03:48:00 GMT", + "Date": "Sat, 16 Jun 2018 12:28:46 GMT", "Content-Length": "2" }, "status_code": 200, From 0b0e77e39ebe4cc723f2a35487756ea15cde1a91 Mon Sep 17 00:00:00 2001 From: ning Date: Sat, 16 Jun 2018 20:55:09 +0800 Subject: [PATCH 22/39] Fix Ivle module using env var instead of dot env --- lib/cadet/accounts/ivle.ex | 2 +- test/fixture/vcr_cassettes/accounts/sign_in#1.json | 4 ++-- test/fixture/vcr_cassettes/accounts/sign_in#2.json | 4 ++-- test/fixture/vcr_cassettes/auth_controller/v1/auth#1.json | 8 ++++---- test/fixture/vcr_cassettes/auth_controller/v1/auth#2.json | 4 ++-- test/fixture/vcr_cassettes/ivle/fetch_name#1.json | 4 ++-- test/fixture/vcr_cassettes/ivle/fetch_name#2.json | 4 ++-- test/fixture/vcr_cassettes/ivle/fetch_nusnet_id#1.json | 4 ++-- test/fixture/vcr_cassettes/ivle/fetch_nusnet_id#2.json | 4 ++-- 9 files changed, 19 insertions(+), 19 deletions(-) diff --git a/lib/cadet/accounts/ivle.ex b/lib/cadet/accounts/ivle.ex index a6710619c..466ca4e18 100644 --- a/lib/cadet/accounts/ivle.ex +++ b/lib/cadet/accounts/ivle.ex @@ -9,7 +9,7 @@ defmodule Cadet.Accounts.Ivle do """ @api_url "https://ivle.nus.edu.sg/api/Lapi.svc" - @api_key System.get_env("IVLE_KEY") + @api_key Dotenv.load!().values["IVLE_KEY"] @doc """ Get the NUSNET ID of the user corresponding to this token. diff --git a/test/fixture/vcr_cassettes/accounts/sign_in#1.json b/test/fixture/vcr_cassettes/accounts/sign_in#1.json index 50e2ea617..029e93eca 100644 --- a/test/fixture/vcr_cassettes/accounts/sign_in#1.json +++ b/test/fixture/vcr_cassettes/accounts/sign_in#1.json @@ -16,10 +16,10 @@ "Content-Type": "application/json", "Server": "Microsoft-IIS/8.5", "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", - "Set-Cookie": "ASP.NET_SessionId=ozaipr2wleccflp10toymiti; path=/; HttpOnly", + "Set-Cookie": "ASP.NET_SessionId=sddum5n5awd4wymyfl2bil20; path=/; HttpOnly", "X-AspNet-Version": "4.0.30319", "X-Powered-By": "ASP.NET", - "Date": "Sat, 16 Jun 2018 12:28:47 GMT", + "Date": "Sat, 16 Jun 2018 12:52:48 GMT", "Content-Length": "15" }, "status_code": 200, diff --git a/test/fixture/vcr_cassettes/accounts/sign_in#2.json b/test/fixture/vcr_cassettes/accounts/sign_in#2.json index 8af446729..5a9e56898 100644 --- a/test/fixture/vcr_cassettes/accounts/sign_in#2.json +++ b/test/fixture/vcr_cassettes/accounts/sign_in#2.json @@ -16,10 +16,10 @@ "Content-Type": "application/json", "Server": "Microsoft-IIS/8.5", "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", - "Set-Cookie": "ASP.NET_SessionId=ixih50bscs3up2pvx5fkypjt; path=/; HttpOnly", + "Set-Cookie": "ASP.NET_SessionId=0rtlaytdbxur0q2ebmlsmylp; path=/; HttpOnly", "X-AspNet-Version": "4.0.30319", "X-Powered-By": "ASP.NET", - "Date": "Sat, 16 Jun 2018 12:28:47 GMT", + "Date": "Sat, 16 Jun 2018 12:52:48 GMT", "Content-Length": "2" }, "status_code": 200, diff --git a/test/fixture/vcr_cassettes/auth_controller/v1/auth#1.json b/test/fixture/vcr_cassettes/auth_controller/v1/auth#1.json index 480da0f72..f09b834b2 100644 --- a/test/fixture/vcr_cassettes/auth_controller/v1/auth#1.json +++ b/test/fixture/vcr_cassettes/auth_controller/v1/auth#1.json @@ -16,10 +16,10 @@ "Content-Type": "application/json", "Server": "Microsoft-IIS/8.5", "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", - "Set-Cookie": "ASP.NET_SessionId=zbfe1ecow0gautunvm0vl11i; path=/; HttpOnly", + "Set-Cookie": "ASP.NET_SessionId=mbhwacij0jlwn21st5oma2z3; path=/; HttpOnly", "X-AspNet-Version": "4.0.30319", "X-Powered-By": "ASP.NET", - "Date": "Sat, 16 Jun 2018 12:28:46 GMT", + "Date": "Sat, 16 Jun 2018 12:52:48 GMT", "Content-Length": "10" }, "status_code": 200, @@ -43,10 +43,10 @@ "Content-Type": "application/json", "Server": "Microsoft-IIS/8.5", "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", - "Set-Cookie": "ASP.NET_SessionId=bprx04t4lfglt31tytffzmqj; path=/; HttpOnly", + "Set-Cookie": "ASP.NET_SessionId=jlm0hrgcz5xqb2pc251f0ksm; path=/; HttpOnly", "X-AspNet-Version": "4.0.30319", "X-Powered-By": "ASP.NET", - "Date": "Sat, 16 Jun 2018 12:28:46 GMT", + "Date": "Sat, 16 Jun 2018 12:52:48 GMT", "Content-Length": "15" }, "status_code": 200, diff --git a/test/fixture/vcr_cassettes/auth_controller/v1/auth#2.json b/test/fixture/vcr_cassettes/auth_controller/v1/auth#2.json index 6143cbe2f..e3f9c544a 100644 --- a/test/fixture/vcr_cassettes/auth_controller/v1/auth#2.json +++ b/test/fixture/vcr_cassettes/auth_controller/v1/auth#2.json @@ -16,10 +16,10 @@ "Content-Type": "application/json", "Server": "Microsoft-IIS/8.5", "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", - "Set-Cookie": "ASP.NET_SessionId=xtrx1om05w05hdosdfrzk1t1; path=/; HttpOnly", + "Set-Cookie": "ASP.NET_SessionId=lyg23quppiybqyvips3pshgv; path=/; HttpOnly", "X-AspNet-Version": "4.0.30319", "X-Powered-By": "ASP.NET", - "Date": "Sat, 16 Jun 2018 12:28:46 GMT", + "Date": "Sat, 16 Jun 2018 12:52:48 GMT", "Content-Length": "2" }, "status_code": 200, diff --git a/test/fixture/vcr_cassettes/ivle/fetch_name#1.json b/test/fixture/vcr_cassettes/ivle/fetch_name#1.json index 580904507..128fb18af 100644 --- a/test/fixture/vcr_cassettes/ivle/fetch_name#1.json +++ b/test/fixture/vcr_cassettes/ivle/fetch_name#1.json @@ -16,10 +16,10 @@ "Content-Type": "application/json", "Server": "Microsoft-IIS/8.5", "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", - "Set-Cookie": "ASP.NET_SessionId=qtyp2glxtn14vk3v0rmbhrne; path=/; HttpOnly", + "Set-Cookie": "ASP.NET_SessionId=xe5hsey2kwxaxzfmd5poiziu; path=/; HttpOnly", "X-AspNet-Version": "4.0.30319", "X-Powered-By": "ASP.NET", - "Date": "Sat, 16 Jun 2018 12:28:47 GMT", + "Date": "Sat, 16 Jun 2018 12:52:50 GMT", "Content-Length": "15" }, "status_code": 200, diff --git a/test/fixture/vcr_cassettes/ivle/fetch_name#2.json b/test/fixture/vcr_cassettes/ivle/fetch_name#2.json index 6320adf7f..360276d71 100644 --- a/test/fixture/vcr_cassettes/ivle/fetch_name#2.json +++ b/test/fixture/vcr_cassettes/ivle/fetch_name#2.json @@ -16,10 +16,10 @@ "Content-Type": "application/json", "Server": "Microsoft-IIS/8.5", "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", - "Set-Cookie": "ASP.NET_SessionId=jcrp1b2fjtq02h3iw1jm51f1; path=/; HttpOnly", + "Set-Cookie": "ASP.NET_SessionId=tstdeihouy4q1p010aqajcgs; path=/; HttpOnly", "X-AspNet-Version": "4.0.30319", "X-Powered-By": "ASP.NET", - "Date": "Sat, 16 Jun 2018 12:28:47 GMT", + "Date": "Sat, 16 Jun 2018 12:52:51 GMT", "Content-Length": "2" }, "status_code": 200, diff --git a/test/fixture/vcr_cassettes/ivle/fetch_nusnet_id#1.json b/test/fixture/vcr_cassettes/ivle/fetch_nusnet_id#1.json index 541026b76..2471897ce 100644 --- a/test/fixture/vcr_cassettes/ivle/fetch_nusnet_id#1.json +++ b/test/fixture/vcr_cassettes/ivle/fetch_nusnet_id#1.json @@ -16,10 +16,10 @@ "Content-Type": "application/json", "Server": "Microsoft-IIS/8.5", "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", - "Set-Cookie": "ASP.NET_SessionId=g33tyvk4f4qcxs0o0og3caow; path=/; HttpOnly", + "Set-Cookie": "ASP.NET_SessionId=rzcurbvtleepdvmxegpkvwaq; path=/; HttpOnly", "X-AspNet-Version": "4.0.30319", "X-Powered-By": "ASP.NET", - "Date": "Sat, 16 Jun 2018 12:28:47 GMT", + "Date": "Sat, 16 Jun 2018 12:52:50 GMT", "Content-Length": "10" }, "status_code": 200, diff --git a/test/fixture/vcr_cassettes/ivle/fetch_nusnet_id#2.json b/test/fixture/vcr_cassettes/ivle/fetch_nusnet_id#2.json index 877c76192..13320c265 100644 --- a/test/fixture/vcr_cassettes/ivle/fetch_nusnet_id#2.json +++ b/test/fixture/vcr_cassettes/ivle/fetch_nusnet_id#2.json @@ -16,10 +16,10 @@ "Content-Type": "application/json", "Server": "Microsoft-IIS/8.5", "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", - "Set-Cookie": "ASP.NET_SessionId=cmnryzexfr2unm452fbqcel4; path=/; HttpOnly", + "Set-Cookie": "ASP.NET_SessionId=ip4wxy11wya20ibjah0brqq3; path=/; HttpOnly", "X-AspNet-Version": "4.0.30319", "X-Powered-By": "ASP.NET", - "Date": "Sat, 16 Jun 2018 12:28:46 GMT", + "Date": "Sat, 16 Jun 2018 12:52:50 GMT", "Content-Length": "2" }, "status_code": 200, From cbe625ee0a8882dc76a088fa011c69b0fc33cf4e Mon Sep 17 00:00:00 2001 From: ning Date: Sat, 16 Jun 2018 21:02:13 +0800 Subject: [PATCH 23/39] Reverse conditional for readability --- lib/cadet/accounts/accounts.ex | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/cadet/accounts/accounts.ex b/lib/cadet/accounts/accounts.ex index fa3ef0a0a..80842dbbd 100644 --- a/lib/cadet/accounts/accounts.ex +++ b/lib/cadet/accounts/accounts.ex @@ -103,7 +103,10 @@ defmodule Cadet.Accounts do def sign_in(nusnet_id, token) do auth = Repo.one(Query.nusnet_id(nusnet_id)) - if auth == nil do + if auth do + auth = Repo.preload(auth, :user) + {:ok, auth.user} + else case Ivle.fetch_name(token) do {:ok, name} -> case register(%{name: name, nusnet_id: nusnet_id}, :student) do @@ -118,9 +121,6 @@ defmodule Cadet.Accounts do # reason can be :bad_request or :internal_server_error {:error, reason} end - else - auth = Repo.preload(auth, :user) - {:ok, auth.user} end end end From b21dbdb26955429fb6fbac815ddb2b79549a1d43 Mon Sep 17 00:00:00 2001 From: ning Date: Sat, 16 Jun 2018 23:17:11 +0800 Subject: [PATCH 24/39] Improve readability for Ivle's api_fetch/2 --- lib/cadet/accounts/ivle.ex | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/lib/cadet/accounts/ivle.ex b/lib/cadet/accounts/ivle.ex index 466ca4e18..06e474129 100644 --- a/lib/cadet/accounts/ivle.ex +++ b/lib/cadet/accounts/ivle.ex @@ -54,19 +54,17 @@ defmodule Cadet.Accounts.Ivle do def fetch_name(token), do: api_fetch("UserName_Get", token) defp api_fetch(path, token) do - # IVLE responds with 200 (:ok) if key and token are valid - # If key is invalid, IVLE responds with 500 (:internal_server_error) - # If token is invalid, IVLE returns an 'empty' string (i.e. "\"\"") case HTTPoison.get(api_url(path, token)) do - {:ok, response} -> - if String.length(response.body) > 2 do - {:ok, String.replace(response.body, ~s("), "")} - else - {:error, :bad_request} - end - - {:error, _} -> + {:ok, %{body: body, status_code: 200}} when body != ~s("") -> + {:ok, Poison.decode!(body)} + + {:ok, %{status_code: 500}} -> + # IVLE responds with 500 if APIKey is invalid {:error, :internal_server_error} + + {:ok, %{body: ~s(""), status_code: 200}} -> + # IVLE responsed 200 with body == ~s("") if token is invalid + {:error, :bad_request} end end From 9ee39a196b8aad2a5acf18fc02454faa0e26571f Mon Sep 17 00:00:00 2001 From: ning Date: Sun, 17 Jun 2018 01:14:41 +0800 Subject: [PATCH 25/39] Improve readability for AuthController's create/2 --- lib/cadet_web/controllers/auth_controller.ex | 59 +++++++++----------- 1 file changed, 27 insertions(+), 32 deletions(-) diff --git a/lib/cadet_web/controllers/auth_controller.ex b/lib/cadet_web/controllers/auth_controller.ex index d3602b119..450289850 100644 --- a/lib/cadet_web/controllers/auth_controller.ex +++ b/lib/cadet_web/controllers/auth_controller.ex @@ -1,10 +1,9 @@ defmodule CadetWeb.AuthController do use CadetWeb, :controller + use PhoenixSwagger import Ecto.Changeset - use PhoenixSwagger - alias Cadet.Accounts alias Cadet.Accounts.Form.Login alias Cadet.Accounts.Ivle @@ -20,37 +19,33 @@ defmodule CadetWeb.AuthController do def create(conn, %{"login" => attrs}) do changeset = Login.changeset(%Login{}, attrs) - if changeset.valid? do - login = apply_changes(changeset) - - case Ivle.fetch_nusnet_id(login.ivle_token) do - {:ok, nusnet_id} -> - case Accounts.sign_in(nusnet_id, login.ivle_token) do - {:ok, user} -> - {:ok, access_token, _} = - Guardian.encode_and_sign(user, %{}, token_type: "access", ttl: {1, :hour}) - - {:ok, refresh_token, _} = - Guardian.encode_and_sign(user, %{}, token_type: "refresh", ttl: {52, :weeks}) - - render( - conn, - "token.json", - access_token: access_token, - refresh_token: refresh_token - ) - - {:error, reason} -> - # reason can be :bad_request or :internal_server_error - send_resp(conn, reason, "Unable to retrieve user") - end - - {:error, reason} -> - # reason can be :bad_request or :internal_server_error - send_resp(conn, reason, "Unable to fetch NUSNET ID from IVLE.") - end + with valid = changeset.valid?, + {:changes, login} when valid <- {:changes, apply_changes(changeset)}, + {:fetch, {:ok, nusnet_id}} <- {:fetch, Ivle.fetch_nusnet_id(login.ivle_token)}, + {:signin, {:ok, user}} <- {:signin, Accounts.sign_in(nusnet_id, login.ivle_token)} do + {:ok, access_token, _} = + Guardian.encode_and_sign(user, %{}, token_type: "access", ttl: {1, :hour}) + + {:ok, refresh_token, _} = + Guardian.encode_and_sign(user, %{}, token_type: "refresh", ttl: {52, :weeks}) + + render( + conn, + "token.json", + access_token: access_token, + refresh_token: refresh_token + ) else - send_resp(conn, :bad_request, "Missing parameter") + {:changes, _} -> + send_resp(conn, :bad_request, "Missing parameter") + + {:fetch, {:error, reason}} -> + # reason can be :bad_request or :internal_server_error + send_resp(conn, reason, "Unable to fetch NUSNET ID from IVLE.") + + {:signin, {:error, reason}} -> + # reason can be :bad_request or :internal_server_error + send_resp(conn, reason, "Unable to retrieve user") end end From 1d823db949f6a8ceae1378780abfbf125576c73f Mon Sep 17 00:00:00 2001 From: ning Date: Sun, 17 Jun 2018 01:44:06 +0800 Subject: [PATCH 26/39] Improve readability for Account's sign_in/2 --- lib/cadet/accounts/accounts.ex | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/lib/cadet/accounts/accounts.ex b/lib/cadet/accounts/accounts.ex index 80842dbbd..30b8c9b72 100644 --- a/lib/cadet/accounts/accounts.ex +++ b/lib/cadet/accounts/accounts.ex @@ -107,19 +107,22 @@ defmodule Cadet.Accounts do auth = Repo.preload(auth, :user) {:ok, auth.user} else - case Ivle.fetch_name(token) do - {:ok, name} -> - case register(%{name: name, nusnet_id: nusnet_id}, :student) do - {:ok, _} -> - sign_in(nusnet_id, token) - - {:error, _} -> - {:error, :internal_server_error} - end - - {:error, reason} -> - # reason can be :bad_request or :internal_server_error - {:error, reason} + # user is not registered in our database + with {:ok, name} <- Ivle.fetch_name(token), + {:ok, _} <- register(%{name: name, nusnet_id: nusnet_id}, :student) do + sign_in(nusnet_id, token) + else + {:error, :bad_request} -> + # Ivle.fetch_name/1 responds with :bad_request if token is invalid + {:error, :bad_request} + + {:error, :internal_server_error} -> + # Ivle.fetch_name/1 responds with :internal_server_error if API key is invalid + {:error, :internal_server_error} + + {:error, _} -> + # register/2 returns {:error, changeset} if changeset is invalid + {:error, :bad_request} end end end From 797b7b1b91a273c3cc4c73d8671ef8159501869a Mon Sep 17 00:00:00 2001 From: ning Date: Sun, 17 Jun 2018 02:42:12 +0800 Subject: [PATCH 27/39] Improve tests with assert pattern match --- test/cadet/accounts/accounts_test.exs | 20 ++++++++------------ test/cadet/accounts/ivle_test.exs | 8 ++++---- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/test/cadet/accounts/accounts_test.exs b/test/cadet/accounts/accounts_test.exs index 65cca8bf4..cfeaa2132 100644 --- a/test/cadet/accounts/accounts_test.exs +++ b/test/cadet/accounts/accounts_test.exs @@ -29,8 +29,7 @@ defmodule Cadet.AccountsTest do role: :student }) - assert user.name == "happy user" - assert user.role == :student + assert %{name: "happy user", role: :student} = user end test "invalid create user" do @@ -46,7 +45,7 @@ defmodule Cadet.AccountsTest do test "get existing user" do user = insert(:user, name: "Teddy") result = Accounts.get_user(user.id) - assert result.name == "Teddy" + assert %{name: "Teddy", role: :student} = result end test "get unknown user" do @@ -55,10 +54,8 @@ defmodule Cadet.AccountsTest do test "associate nusnet_id to user" do user = insert(:user) - {:ok, auth} = Accounts.add_nusnet_id(user, "teddy@happy.mail") - assert auth.provider == :nusnet_id - assert auth.uid == "teddy@happy.mail" - assert auth.user_id == user.id + {:ok, auth} = Accounts.add_nusnet_id(user, "e012345") + assert %{uid: "e012345", provider: :nusnet_id, user: ^user} = auth end test "duplicate nusnet_id one user" do @@ -96,23 +93,22 @@ defmodule Cadet.AccountsTest do attrs = %{ provider: :nusnet_id, - uid: "test@gmail.com", + uid: "e012345", token: "hahaha" } assert {:ok, auth} = Accounts.create_authorization(attrs, user) - assert auth.user_id == user.id - assert auth.uid == "test@gmail.com" + assert %{uid: "e012345", provider: :nusnet_id, user: ^user} = auth end test "valid registration" do attrs = %{ name: "Test Name", - nusnet_id: "E012345" + nusnet_id: "e012345" } assert {:ok, user} = Accounts.register(attrs, :student) - assert user.name == "Test Name" + assert %{name: "Test Name", role: :student} = user end describe "sign in using nusnet_id" do diff --git a/test/cadet/accounts/ivle_test.exs b/test/cadet/accounts/ivle_test.exs index cc6ba5099..d35a11f19 100644 --- a/test/cadet/accounts/ivle_test.exs +++ b/test/cadet/accounts/ivle_test.exs @@ -23,14 +23,14 @@ defmodule Cadet.Accounts.IvleTest do describe "Fetch an NUSNET ID" do test "Using a valid token" do use_cassette "ivle/fetch_nusnet_id#1" do - {:ok, resp} = Ivle.fetch_nusnet_id(@token) + assert {:ok, resp} = Ivle.fetch_nusnet_id(@token) assert String.length(resp) > 0 end end test "Using an invalid token" do use_cassette "ivle/fetch_nusnet_id#2" do - {:error, resp} = Ivle.fetch_nusnet_id(@token <> "Z") + assert {:error, resp} = Ivle.fetch_nusnet_id(@token <> "Z") assert resp == :bad_request end end @@ -39,14 +39,14 @@ defmodule Cadet.Accounts.IvleTest do describe "Fetch a name" do test "Using a valid token" do use_cassette "ivle/fetch_name#1" do - {:ok, resp} = Ivle.fetch_name(@token) + assert {:ok, resp} = Ivle.fetch_name(@token) assert String.length(resp) > 0 end end test "Using an invalid token" do use_cassette "ivle/fetch_name#2" do - {:error, resp} = Ivle.fetch_name(@token <> "Z") + assert {:error, resp} = Ivle.fetch_name(@token <> "Z") assert resp == :bad_request end end From 73de001d96aa2833fe0cde73c14773fde4e95eda Mon Sep 17 00:00:00 2001 From: ning Date: Sun, 17 Jun 2018 03:03:25 +0800 Subject: [PATCH 28/39] Improve readability and docs for @token in tests --- lib/cadet/accounts/accounts.ex | 2 +- test/cadet/accounts/accounts_test.exs | 12 ++++++-- test/cadet/accounts/ivle_test.exs | 8 ++++- .../controllers/auth_controller_test.exs | 8 ++++- .../vcr_cassettes/accounts/sign_in#3.json | 29 +++++++++++++++++++ 5 files changed, 54 insertions(+), 5 deletions(-) create mode 100644 test/fixture/vcr_cassettes/accounts/sign_in#3.json diff --git a/lib/cadet/accounts/accounts.ex b/lib/cadet/accounts/accounts.ex index 30b8c9b72..e662bd109 100644 --- a/lib/cadet/accounts/accounts.ex +++ b/lib/cadet/accounts/accounts.ex @@ -122,7 +122,7 @@ defmodule Cadet.Accounts do {:error, _} -> # register/2 returns {:error, changeset} if changeset is invalid - {:error, :bad_request} + {:error, :internal_server_error} end end end diff --git a/test/cadet/accounts/accounts_test.exs b/test/cadet/accounts/accounts_test.exs index cfeaa2132..91d21e8d4 100644 --- a/test/cadet/accounts/accounts_test.exs +++ b/test/cadet/accounts/accounts_test.exs @@ -7,6 +7,12 @@ defmodule Cadet.AccountsTest do to set the two environment variables IVLE_KEY (used as a module attribute in `Cadet.Accounts.IVLE`) and TOKEN (used here). Don't forget to delete the cassette files, otherwise ExVCR will not override the cassettes. + + Token refers to the user's authentication token. Please see the IVLE API docs: + https://wiki.nus.edu.sg/display/ivlelapi/Getting+Started + To quickly obtain a token, simply supply a dummy url to a login call: + https://ivle.nus.edu.sg/api/login/?apikey=YOUR_API_KEY&url=http://localhost + then copy down the token from your browser's address bar. """ use Cadet.DataCase @@ -16,7 +22,7 @@ defmodule Cadet.AccountsTest do alias Cadet.Repo alias Cadet.Accounts.Query - @token String.replace(inspect(System.get_env("TOKEN")), ~s("), "") + @token if System.get_env("TOKEN"), do: System.get_env("TOKEN"), else: "" setup_all do HTTPoison.start() @@ -137,7 +143,9 @@ defmodule Cadet.AccountsTest do end test "invalid nusnet id" do - assert {:error, :internal_server_error} == Accounts.sign_in("", @token) + use_cassette "accounts/sign_in#3" do + assert {:error, :internal_server_error} == Accounts.sign_in("", @token) + end end end end diff --git a/test/cadet/accounts/ivle_test.exs b/test/cadet/accounts/ivle_test.exs index d35a11f19..2b68e0743 100644 --- a/test/cadet/accounts/ivle_test.exs +++ b/test/cadet/accounts/ivle_test.exs @@ -7,6 +7,12 @@ defmodule Cadet.Accounts.IvleTest do to set the two environment variables IVLE_KEY (used as a module attribute in `Cadet.Accounts.IVLE`) and TOKEN (used here). Don't forget to delete the cassette files, otherwise ExVCR will not override the cassettes. + + Token refers to the user's authentication token. Please see the IVLE API docs: + https://wiki.nus.edu.sg/display/ivlelapi/Getting+Started + To quickly obtain a token, simply supply a dummy url to a login call: + https://ivle.nus.edu.sg/api/login/?apikey=YOUR_API_KEY&url=http://localhost + then copy down the token from your browser's address bar. """ use ExUnit.Case, async: false @@ -14,7 +20,7 @@ defmodule Cadet.Accounts.IvleTest do alias Cadet.Accounts.Ivle - @token String.replace(inspect(System.get_env("TOKEN")), ~s("), "") + @token if System.get_env("TOKEN"), do: System.get_env("TOKEN"), else: "" setup_all do HTTPoison.start() diff --git a/test/cadet_web/controllers/auth_controller_test.exs b/test/cadet_web/controllers/auth_controller_test.exs index 9c53fd25f..5fc2b7291 100644 --- a/test/cadet_web/controllers/auth_controller_test.exs +++ b/test/cadet_web/controllers/auth_controller_test.exs @@ -7,6 +7,12 @@ defmodule CadetWeb.AuthControllerTest do to set the two environment variables IVLE_KEY (used as a module attribute in `Cadet.Accounts.IVLE`) and TOKEN (used here). Don't forget to delete the cassette files, otherwise ExVCR will not override the cassettes. + + Token refers to the user's authentication token. Please see the IVLE API docs: + https://wiki.nus.edu.sg/display/ivlelapi/Getting+Started + To quickly obtain a token, simply supply a dummy url to a login call: + https://ivle.nus.edu.sg/api/login/?apikey=YOUR_API_KEY&url=http://localhost + then copy down the token from your browser's address bar. """ use CadetWeb.ConnCase @@ -17,7 +23,7 @@ defmodule CadetWeb.AuthControllerTest do alias CadetWeb.AuthController alias Cadet.Auth.Guardian - @token String.replace(inspect(System.get_env("TOKEN")), ~s("), "") + @token if System.get_env("TOKEN"), do: System.get_env("TOKEN"), else: "" setup_all do HTTPoison.start() diff --git a/test/fixture/vcr_cassettes/accounts/sign_in#3.json b/test/fixture/vcr_cassettes/accounts/sign_in#3.json new file mode 100644 index 000000000..26fda2639 --- /dev/null +++ b/test/fixture/vcr_cassettes/accounts/sign_in#3.json @@ -0,0 +1,29 @@ +[ + { + "request": { + "body": "", + "headers": [], + "method": "get", + "options": [], + "request_body": "", + "url": "https://ivle.nus.edu.sg/api/Lapi.svc/UserName_Get" + }, + "response": { + "binary": false, + "body": "\"LEE NING YUAN\"", + "headers": { + "Cache-Control": "private", + "Content-Type": "application/json", + "Server": "Microsoft-IIS/8.5", + "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", + "Set-Cookie": "ASP.NET_SessionId=yehhmhnjrvni2aha2twm5u2l; path=/; HttpOnly", + "X-AspNet-Version": "4.0.30319", + "X-Powered-By": "ASP.NET", + "Date": "Sat, 16 Jun 2018 18:57:26 GMT", + "Content-Length": "15" + }, + "status_code": 200, + "type": "ok" + } + } +] \ No newline at end of file From 634ccd71c8c044ff9e7903acac608a60db187ea3 Mon Sep 17 00:00:00 2001 From: ning Date: Sun, 17 Jun 2018 03:12:38 +0800 Subject: [PATCH 29/39] Fix credo offences due to ff89294 --- test/cadet/accounts/accounts_test.exs | 1 - 1 file changed, 1 deletion(-) diff --git a/test/cadet/accounts/accounts_test.exs b/test/cadet/accounts/accounts_test.exs index 91d21e8d4..3d9cf27f5 100644 --- a/test/cadet/accounts/accounts_test.exs +++ b/test/cadet/accounts/accounts_test.exs @@ -84,7 +84,6 @@ defmodule Cadet.AccountsTest do assert {:ok, []} = Accounts.set_nusnet_id(user, "E012345") end - # TODO: A user may not have multiple NUSNET_IDs? test "setting user nusnet_id with multiple e-mails" do user = insert(:user) insert(:nusnet_id, user: user) From f11f519554d9398a98553cdb4e0a9052f2381ce3 Mon Sep 17 00:00:00 2001 From: ning Date: Sun, 17 Jun 2018 03:36:53 +0800 Subject: [PATCH 30/39] Fix failing credo tests because of bad cassettes --- test/cadet/accounts/accounts_test.exs | 2 +- test/cadet/accounts/ivle_test.exs | 2 +- test/cadet_web/controllers/auth_controller_test.exs | 2 +- test/fixture/vcr_cassettes/auth_controller/v1/auth#1.json | 8 ++++---- test/fixture/vcr_cassettes/auth_controller/v1/auth#2.json | 4 ++-- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/test/cadet/accounts/accounts_test.exs b/test/cadet/accounts/accounts_test.exs index 3d9cf27f5..1efac908d 100644 --- a/test/cadet/accounts/accounts_test.exs +++ b/test/cadet/accounts/accounts_test.exs @@ -22,7 +22,7 @@ defmodule Cadet.AccountsTest do alias Cadet.Repo alias Cadet.Accounts.Query - @token if System.get_env("TOKEN"), do: System.get_env("TOKEN"), else: "" + @token if System.get_env("TOKEN"), do: System.get_env("TOKEN"), else: "token" setup_all do HTTPoison.start() diff --git a/test/cadet/accounts/ivle_test.exs b/test/cadet/accounts/ivle_test.exs index 2b68e0743..f0c26a09f 100644 --- a/test/cadet/accounts/ivle_test.exs +++ b/test/cadet/accounts/ivle_test.exs @@ -20,7 +20,7 @@ defmodule Cadet.Accounts.IvleTest do alias Cadet.Accounts.Ivle - @token if System.get_env("TOKEN"), do: System.get_env("TOKEN"), else: "" + @token if System.get_env("TOKEN"), do: System.get_env("TOKEN"), else: "token" setup_all do HTTPoison.start() diff --git a/test/cadet_web/controllers/auth_controller_test.exs b/test/cadet_web/controllers/auth_controller_test.exs index 5fc2b7291..832f2715c 100644 --- a/test/cadet_web/controllers/auth_controller_test.exs +++ b/test/cadet_web/controllers/auth_controller_test.exs @@ -23,7 +23,7 @@ defmodule CadetWeb.AuthControllerTest do alias CadetWeb.AuthController alias Cadet.Auth.Guardian - @token if System.get_env("TOKEN"), do: System.get_env("TOKEN"), else: "" + @token if System.get_env("TOKEN"), do: System.get_env("TOKEN"), else: "token" setup_all do HTTPoison.start() diff --git a/test/fixture/vcr_cassettes/auth_controller/v1/auth#1.json b/test/fixture/vcr_cassettes/auth_controller/v1/auth#1.json index f09b834b2..840d8c45c 100644 --- a/test/fixture/vcr_cassettes/auth_controller/v1/auth#1.json +++ b/test/fixture/vcr_cassettes/auth_controller/v1/auth#1.json @@ -16,10 +16,10 @@ "Content-Type": "application/json", "Server": "Microsoft-IIS/8.5", "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", - "Set-Cookie": "ASP.NET_SessionId=mbhwacij0jlwn21st5oma2z3; path=/; HttpOnly", + "Set-Cookie": "ASP.NET_SessionId=aux2k13w3tgb0bydyntgs3mv; path=/; HttpOnly", "X-AspNet-Version": "4.0.30319", "X-Powered-By": "ASP.NET", - "Date": "Sat, 16 Jun 2018 12:52:48 GMT", + "Date": "Sat, 16 Jun 2018 19:36:33 GMT", "Content-Length": "10" }, "status_code": 200, @@ -43,10 +43,10 @@ "Content-Type": "application/json", "Server": "Microsoft-IIS/8.5", "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", - "Set-Cookie": "ASP.NET_SessionId=jlm0hrgcz5xqb2pc251f0ksm; path=/; HttpOnly", + "Set-Cookie": "ASP.NET_SessionId=oymxocc04xjq2mwispu422cz; path=/; HttpOnly", "X-AspNet-Version": "4.0.30319", "X-Powered-By": "ASP.NET", - "Date": "Sat, 16 Jun 2018 12:52:48 GMT", + "Date": "Sat, 16 Jun 2018 19:36:33 GMT", "Content-Length": "15" }, "status_code": 200, diff --git a/test/fixture/vcr_cassettes/auth_controller/v1/auth#2.json b/test/fixture/vcr_cassettes/auth_controller/v1/auth#2.json index e3f9c544a..63b8f9d84 100644 --- a/test/fixture/vcr_cassettes/auth_controller/v1/auth#2.json +++ b/test/fixture/vcr_cassettes/auth_controller/v1/auth#2.json @@ -16,10 +16,10 @@ "Content-Type": "application/json", "Server": "Microsoft-IIS/8.5", "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", - "Set-Cookie": "ASP.NET_SessionId=lyg23quppiybqyvips3pshgv; path=/; HttpOnly", + "Set-Cookie": "ASP.NET_SessionId=34mwmc4gh5fexb3bcqipacar; path=/; HttpOnly", "X-AspNet-Version": "4.0.30319", "X-Powered-By": "ASP.NET", - "Date": "Sat, 16 Jun 2018 12:52:48 GMT", + "Date": "Sat, 16 Jun 2018 19:36:33 GMT", "Content-Length": "2" }, "status_code": 200, From 62987993f9e53afa6859c915c6532775cdc29a5e Mon Sep 17 00:00:00 2001 From: ning Date: Mon, 18 Jun 2018 10:30:33 +0800 Subject: [PATCH 31/39] Move .env file as .env.example, update README --- .env => .env.example | 12 ++++++------ README.md | 10 +++++----- 2 files changed, 11 insertions(+), 11 deletions(-) rename .env => .env.example (60%) diff --git a/.env b/.env.example similarity index 60% rename from .env rename to .env.example index 8f3b3f8e7..2f1a9de9a 100644 --- a/.env +++ b/.env.example @@ -2,19 +2,19 @@ # and building production tarball. # Host used in development -export CADET_HOST=0.0.0.0 +CADET_HOST=0.0.0.0 # Hostname of production server -export CADET_PROD_HOST=https://sourceacademy.comp.nus.edu.sg/ +CADET_PROD_HOST=https://sourceacademy.comp.nus.edu.sg/ # Port which Phoenix should be serving from in development -export CADET_SERVER_PORT=4000 +CADET_SERVER_PORT=4000 # Port which Webpack should be serving from in development -export CADET_WEBPACK_PORT=4001 +CADET_WEBPACK_PORT=4001 # Webpack entry filename -export CADET_WEBPACK_ENTRY=app +CADET_WEBPACK_ENTRY=app # IVLE LAPI Key -export IVLE_KEY=example_ivle_key +IVLE_KEY=your_ivle_lapi_key diff --git a/README.md b/README.md index cae0d050c..a28db4544 100644 --- a/README.md +++ b/README.md @@ -25,12 +25,12 @@ Initialise development database mix ecto.setup -Replace the value of `IVLE_KEY` in the `.env` file with your [IVLE LAPI Key -](https://ivle.nus.edu.sg/LAPI/default.aspx). If you've compiled the -application before setting a valid value, you must force a recompilation with -`mix clean && mix`. +Copy the file `.env.example` as `.env` in the project root, and replace the +value of `IVLE_KEY` in with your [IVLE LAPI Key](https://ivle.nus.edu.sg/LAPI/default.aspx). +If you've compiled the application before setting a valid value, you must force +a recompilation with `mix clean && mix`. - export IVLE_KEY=your_ivle_lapi_key + IVLE_KEY=your_ivle_lapi_key Run the server in your local machine From 3508e833a74800d3f18e3fc9267d34d076381cfc Mon Sep 17 00:00:00 2001 From: ning Date: Mon, 18 Jun 2018 10:32:58 +0800 Subject: [PATCH 32/39] Fix redundant pattern matching --- lib/cadet/accounts/accounts.ex | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/cadet/accounts/accounts.ex b/lib/cadet/accounts/accounts.ex index e662bd109..525f79f8a 100644 --- a/lib/cadet/accounts/accounts.ex +++ b/lib/cadet/accounts/accounts.ex @@ -116,11 +116,8 @@ defmodule Cadet.Accounts do # Ivle.fetch_name/1 responds with :bad_request if token is invalid {:error, :bad_request} - {:error, :internal_server_error} -> - # Ivle.fetch_name/1 responds with :internal_server_error if API key is invalid - {:error, :internal_server_error} - {:error, _} -> + # Ivle.fetch_name/1 responds with :internal_server_error if API key is invalid # register/2 returns {:error, changeset} if changeset is invalid {:error, :internal_server_error} end From 2c2cedee78fda0c87ca4911cfef5aeb0693b6e28 Mon Sep 17 00:00:00 2001 From: ning Date: Mon, 18 Jun 2018 10:38:47 +0800 Subject: [PATCH 33/39] Rename module Ivle -> IVLE --- lib/cadet/accounts/accounts.ex | 8 ++++---- lib/cadet/accounts/ivle.ex | 6 +++--- lib/cadet_web/controllers/auth_controller.ex | 4 ++-- test/cadet/accounts/ivle_test.exs | 12 ++++++------ 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/cadet/accounts/accounts.ex b/lib/cadet/accounts/accounts.ex index 525f79f8a..319c9d5cf 100644 --- a/lib/cadet/accounts/accounts.ex +++ b/lib/cadet/accounts/accounts.ex @@ -5,7 +5,7 @@ defmodule Cadet.Accounts do use Cadet, :context alias Cadet.Accounts.Authorization - alias Cadet.Accounts.Ivle + alias Cadet.Accounts.IVLE alias Cadet.Accounts.User alias Cadet.Accounts.Query alias Cadet.Accounts.Form.Registration @@ -108,16 +108,16 @@ defmodule Cadet.Accounts do {:ok, auth.user} else # user is not registered in our database - with {:ok, name} <- Ivle.fetch_name(token), + with {:ok, name} <- IVLE.fetch_name(token), {:ok, _} <- register(%{name: name, nusnet_id: nusnet_id}, :student) do sign_in(nusnet_id, token) else {:error, :bad_request} -> - # Ivle.fetch_name/1 responds with :bad_request if token is invalid + # IVLE.fetch_name/1 responds with :bad_request if token is invalid {:error, :bad_request} {:error, _} -> - # Ivle.fetch_name/1 responds with :internal_server_error if API key is invalid + # IVLE.fetch_name/1 responds with :internal_server_error if API key is invalid # register/2 returns {:error, changeset} if changeset is invalid {:error, :internal_server_error} end diff --git a/lib/cadet/accounts/ivle.ex b/lib/cadet/accounts/ivle.ex index 06e474129..7b4ff1563 100644 --- a/lib/cadet/accounts/ivle.ex +++ b/lib/cadet/accounts/ivle.ex @@ -1,4 +1,4 @@ -defmodule Cadet.Accounts.Ivle do +defmodule Cadet.Accounts.IVLE do @moduledoc """ Helper functions to IVLE calls. All helper functions are prefixed with fetch to differentiate them from database helpers, or other 'getters'. @@ -26,7 +26,7 @@ defmodule Cadet.Accounts.Ivle do ## Examples - iex> Cadet.Accounts.Ivle.fetch_nusnet_id("T0K3N...") + iex> Cadet.Accounts.IVLE.fetch_nusnet_id("T0K3N...") {:ok, "e012345"} """ @@ -47,7 +47,7 @@ defmodule Cadet.Accounts.Ivle do ## Examples - iex> Cadet.Accounts.Ivle.fetch_name("T0K3N...") + iex> Cadet.Accounts.IVLE.fetch_name("T0K3N...") {:ok, "LEE NING YUAN"} """ diff --git a/lib/cadet_web/controllers/auth_controller.ex b/lib/cadet_web/controllers/auth_controller.ex index 450289850..2233bc44f 100644 --- a/lib/cadet_web/controllers/auth_controller.ex +++ b/lib/cadet_web/controllers/auth_controller.ex @@ -6,7 +6,7 @@ defmodule CadetWeb.AuthController do alias Cadet.Accounts alias Cadet.Accounts.Form.Login - alias Cadet.Accounts.Ivle + alias Cadet.Accounts.IVLE alias Cadet.Auth.Guardian @doc """ @@ -21,7 +21,7 @@ defmodule CadetWeb.AuthController do with valid = changeset.valid?, {:changes, login} when valid <- {:changes, apply_changes(changeset)}, - {:fetch, {:ok, nusnet_id}} <- {:fetch, Ivle.fetch_nusnet_id(login.ivle_token)}, + {:fetch, {:ok, nusnet_id}} <- {:fetch, IVLE.fetch_nusnet_id(login.ivle_token)}, {:signin, {:ok, user}} <- {:signin, Accounts.sign_in(nusnet_id, login.ivle_token)} do {:ok, access_token, _} = Guardian.encode_and_sign(user, %{}, token_type: "access", ttl: {1, :hour}) diff --git a/test/cadet/accounts/ivle_test.exs b/test/cadet/accounts/ivle_test.exs index f0c26a09f..56ed79e59 100644 --- a/test/cadet/accounts/ivle_test.exs +++ b/test/cadet/accounts/ivle_test.exs @@ -1,4 +1,4 @@ -defmodule Cadet.Accounts.IvleTest do +defmodule Cadet.Accounts.IVLETest do @moduledoc """ This test module uses pre-recoreded HTTP responses saved by ExVCR. This allows testing without actual external IVLE API calls. @@ -18,7 +18,7 @@ defmodule Cadet.Accounts.IvleTest do use ExUnit.Case, async: false use ExVCR.Mock, adapter: ExVCR.Adapter.Hackney - alias Cadet.Accounts.Ivle + alias Cadet.Accounts.IVLE @token if System.get_env("TOKEN"), do: System.get_env("TOKEN"), else: "token" @@ -29,14 +29,14 @@ defmodule Cadet.Accounts.IvleTest do describe "Fetch an NUSNET ID" do test "Using a valid token" do use_cassette "ivle/fetch_nusnet_id#1" do - assert {:ok, resp} = Ivle.fetch_nusnet_id(@token) + assert {:ok, resp} = IVLE.fetch_nusnet_id(@token) assert String.length(resp) > 0 end end test "Using an invalid token" do use_cassette "ivle/fetch_nusnet_id#2" do - assert {:error, resp} = Ivle.fetch_nusnet_id(@token <> "Z") + assert {:error, resp} = IVLE.fetch_nusnet_id(@token <> "Z") assert resp == :bad_request end end @@ -45,14 +45,14 @@ defmodule Cadet.Accounts.IvleTest do describe "Fetch a name" do test "Using a valid token" do use_cassette "ivle/fetch_name#1" do - assert {:ok, resp} = Ivle.fetch_name(@token) + assert {:ok, resp} = IVLE.fetch_name(@token) assert String.length(resp) > 0 end end test "Using an invalid token" do use_cassette "ivle/fetch_name#2" do - assert {:error, resp} = Ivle.fetch_name(@token <> "Z") + assert {:error, resp} = IVLE.fetch_name(@token <> "Z") assert resp == :bad_request end end From f1e09aa05c17f2411a6caa0d51e313d0ca1154b6 Mon Sep 17 00:00:00 2001 From: ning Date: Mon, 18 Jun 2018 10:40:30 +0800 Subject: [PATCH 34/39] Move test/fixture -> test/fixtures --- config/test.exs | 2 +- .../{fixture => fixtures}/vcr_cassettes/accounts/sign_in#1.json | 0 .../{fixture => fixtures}/vcr_cassettes/accounts/sign_in#2.json | 0 .../{fixture => fixtures}/vcr_cassettes/accounts/sign_in#3.json | 0 .../vcr_cassettes/auth_controller/v1/auth#1.json | 0 .../vcr_cassettes/auth_controller/v1/auth#2.json | 0 test/{fixture => fixtures}/vcr_cassettes/ivle/fetch_name#1.json | 0 test/{fixture => fixtures}/vcr_cassettes/ivle/fetch_name#2.json | 0 .../vcr_cassettes/ivle/fetch_nusnet_id#1.json | 0 .../vcr_cassettes/ivle/fetch_nusnet_id#2.json | 0 10 files changed, 1 insertion(+), 1 deletion(-) rename test/{fixture => fixtures}/vcr_cassettes/accounts/sign_in#1.json (100%) rename test/{fixture => fixtures}/vcr_cassettes/accounts/sign_in#2.json (100%) rename test/{fixture => fixtures}/vcr_cassettes/accounts/sign_in#3.json (100%) rename test/{fixture => fixtures}/vcr_cassettes/auth_controller/v1/auth#1.json (100%) rename test/{fixture => fixtures}/vcr_cassettes/auth_controller/v1/auth#2.json (100%) rename test/{fixture => fixtures}/vcr_cassettes/ivle/fetch_name#1.json (100%) rename test/{fixture => fixtures}/vcr_cassettes/ivle/fetch_name#2.json (100%) rename test/{fixture => fixtures}/vcr_cassettes/ivle/fetch_nusnet_id#1.json (100%) rename test/{fixture => fixtures}/vcr_cassettes/ivle/fetch_nusnet_id#2.json (100%) diff --git a/config/test.exs b/config/test.exs index f6d19ca3d..d5494f27e 100644 --- a/config/test.exs +++ b/config/test.exs @@ -17,7 +17,7 @@ config :logger, level: :warn # Don't save secret keys in ExVCR cassettes config :exvcr, filter_url_params: true, - vcr_cassette_library_dir: "test/fixture/vcr_cassettes" + vcr_cassette_library_dir: "test/fixtures/vcr_cassettes" # Configure your database config :cadet, Cadet.Repo, diff --git a/test/fixture/vcr_cassettes/accounts/sign_in#1.json b/test/fixtures/vcr_cassettes/accounts/sign_in#1.json similarity index 100% rename from test/fixture/vcr_cassettes/accounts/sign_in#1.json rename to test/fixtures/vcr_cassettes/accounts/sign_in#1.json diff --git a/test/fixture/vcr_cassettes/accounts/sign_in#2.json b/test/fixtures/vcr_cassettes/accounts/sign_in#2.json similarity index 100% rename from test/fixture/vcr_cassettes/accounts/sign_in#2.json rename to test/fixtures/vcr_cassettes/accounts/sign_in#2.json diff --git a/test/fixture/vcr_cassettes/accounts/sign_in#3.json b/test/fixtures/vcr_cassettes/accounts/sign_in#3.json similarity index 100% rename from test/fixture/vcr_cassettes/accounts/sign_in#3.json rename to test/fixtures/vcr_cassettes/accounts/sign_in#3.json diff --git a/test/fixture/vcr_cassettes/auth_controller/v1/auth#1.json b/test/fixtures/vcr_cassettes/auth_controller/v1/auth#1.json similarity index 100% rename from test/fixture/vcr_cassettes/auth_controller/v1/auth#1.json rename to test/fixtures/vcr_cassettes/auth_controller/v1/auth#1.json diff --git a/test/fixture/vcr_cassettes/auth_controller/v1/auth#2.json b/test/fixtures/vcr_cassettes/auth_controller/v1/auth#2.json similarity index 100% rename from test/fixture/vcr_cassettes/auth_controller/v1/auth#2.json rename to test/fixtures/vcr_cassettes/auth_controller/v1/auth#2.json diff --git a/test/fixture/vcr_cassettes/ivle/fetch_name#1.json b/test/fixtures/vcr_cassettes/ivle/fetch_name#1.json similarity index 100% rename from test/fixture/vcr_cassettes/ivle/fetch_name#1.json rename to test/fixtures/vcr_cassettes/ivle/fetch_name#1.json diff --git a/test/fixture/vcr_cassettes/ivle/fetch_name#2.json b/test/fixtures/vcr_cassettes/ivle/fetch_name#2.json similarity index 100% rename from test/fixture/vcr_cassettes/ivle/fetch_name#2.json rename to test/fixtures/vcr_cassettes/ivle/fetch_name#2.json diff --git a/test/fixture/vcr_cassettes/ivle/fetch_nusnet_id#1.json b/test/fixtures/vcr_cassettes/ivle/fetch_nusnet_id#1.json similarity index 100% rename from test/fixture/vcr_cassettes/ivle/fetch_nusnet_id#1.json rename to test/fixtures/vcr_cassettes/ivle/fetch_nusnet_id#1.json diff --git a/test/fixture/vcr_cassettes/ivle/fetch_nusnet_id#2.json b/test/fixtures/vcr_cassettes/ivle/fetch_nusnet_id#2.json similarity index 100% rename from test/fixture/vcr_cassettes/ivle/fetch_nusnet_id#2.json rename to test/fixtures/vcr_cassettes/ivle/fetch_nusnet_id#2.json From 2a5329adbd6793eaaa6b04792118926be9055d57 Mon Sep 17 00:00:00 2001 From: ning Date: Mon, 18 Jun 2018 10:43:45 +0800 Subject: [PATCH 35/39] Change Dotenv.load! -> Dotenv.load --- lib/cadet/accounts/ivle.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cadet/accounts/ivle.ex b/lib/cadet/accounts/ivle.ex index 7b4ff1563..94af406f4 100644 --- a/lib/cadet/accounts/ivle.ex +++ b/lib/cadet/accounts/ivle.ex @@ -9,7 +9,7 @@ defmodule Cadet.Accounts.IVLE do """ @api_url "https://ivle.nus.edu.sg/api/Lapi.svc" - @api_key Dotenv.load!().values["IVLE_KEY"] + @api_key Dotenv.load().values["IVLE_KEY"] @doc """ Get the NUSNET ID of the user corresponding to this token. From eaa77fce358a708856953558b0e1d27ea9c75a48 Mon Sep 17 00:00:00 2001 From: ning Date: Mon, 18 Jun 2018 12:11:34 +0800 Subject: [PATCH 36/39] Add test for bad auth_controller create/2 sign_in --- config/test.exs | 3 +- .../controllers/auth_controller_test.exs | 13 +++++++++ .../auth_controller/v1/auth#1.json | 29 +++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 test/fixtures/custom_cassettes/auth_controller/v1/auth#1.json diff --git a/config/test.exs b/config/test.exs index d5494f27e..b3e3d9cb7 100644 --- a/config/test.exs +++ b/config/test.exs @@ -17,7 +17,8 @@ config :logger, level: :warn # Don't save secret keys in ExVCR cassettes config :exvcr, filter_url_params: true, - vcr_cassette_library_dir: "test/fixtures/vcr_cassettes" + vcr_cassette_library_dir: "test/fixtures/vcr_cassettes", + custom_cassette_library_dir: "test/fixtures/custom_cassettes" # Configure your database config :cadet, Cadet.Repo, diff --git a/test/cadet_web/controllers/auth_controller_test.exs b/test/cadet_web/controllers/auth_controller_test.exs index 832f2715c..7c717b735 100644 --- a/test/cadet_web/controllers/auth_controller_test.exs +++ b/test/cadet_web/controllers/auth_controller_test.exs @@ -73,6 +73,19 @@ defmodule CadetWeb.AuthControllerTest do assert response(conn, 400) end end + + test "invalid nusnet id", %{conn: conn} do + # an invalid nusnet id == ~s("") is typically caught by IVLE.fetch_nusnet_id + # the custom cassette skips this step so that we can test Accounts.sign_in + use_cassette "auth_controller/v1/auth#1", custom: true do + conn = + post(conn, "/v1/auth", %{ + "login" => %{"ivle_token" => 'token'} + }) + + assert response(conn, 400) + end + end end describe "POST /auth/refresh" do diff --git a/test/fixtures/custom_cassettes/auth_controller/v1/auth#1.json b/test/fixtures/custom_cassettes/auth_controller/v1/auth#1.json new file mode 100644 index 000000000..30ec2967c --- /dev/null +++ b/test/fixtures/custom_cassettes/auth_controller/v1/auth#1.json @@ -0,0 +1,29 @@ +[ + { + "request": { + "body": "", + "headers": [], + "method": "get", + "options": [], + "request_body": "", + "url": "https://ivle.nus.edu.sg/api/Lapi.svc/UserID_Get?APIKey=&Token=token" + }, + "response": { + "binary": false, + "body": "\"\"", + "headers": { + "Cache-Control": "private", + "Content-Type": "application/json", + "Server": "Microsoft-IIS/8.5", + "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", + "Set-Cookie": "ASP.NET_SessionId=34mwmc4gh5fexb3bcqipacar; path=/; HttpOnly", + "X-AspNet-Version": "4.0.30319", + "X-Powered-By": "ASP.NET", + "Date": "Sat, 16 Jun 2018 19:36:33 GMT", + "Content-Length": "2" + }, + "status_code": 200, + "type": "ok" + } + } +] From 0a9c58938aef743bacd54404bcca0a4f781ab505 Mon Sep 17 00:00:00 2001 From: ning Date: Mon, 18 Jun 2018 17:29:19 +0800 Subject: [PATCH 37/39] Catch all :errors for api_fetch --- lib/cadet/accounts/ivle.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cadet/accounts/ivle.ex b/lib/cadet/accounts/ivle.ex index 94af406f4..ffd67c589 100644 --- a/lib/cadet/accounts/ivle.ex +++ b/lib/cadet/accounts/ivle.ex @@ -62,8 +62,8 @@ defmodule Cadet.Accounts.IVLE do # IVLE responds with 500 if APIKey is invalid {:error, :internal_server_error} - {:ok, %{body: ~s(""), status_code: 200}} -> - # IVLE responsed 200 with body == ~s("") if token is invalid + {:ok, _} -> + # IVLE responds 200 with body == ~s("") if token is invalid {:error, :bad_request} end end From 59a315feaef0f0f9a2a4f4168a91ece0dbbb720d Mon Sep 17 00:00:00 2001 From: ning Date: Mon, 18 Jun 2018 17:48:42 +0800 Subject: [PATCH 38/39] Fix missing coverage for signin error --- .../controllers/auth_controller_test.exs | 2 +- .../auth_controller/v1/auth#1.json | 29 ++++++++++++++++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/test/cadet_web/controllers/auth_controller_test.exs b/test/cadet_web/controllers/auth_controller_test.exs index 7c717b735..0f92d9c47 100644 --- a/test/cadet_web/controllers/auth_controller_test.exs +++ b/test/cadet_web/controllers/auth_controller_test.exs @@ -80,7 +80,7 @@ defmodule CadetWeb.AuthControllerTest do use_cassette "auth_controller/v1/auth#1", custom: true do conn = post(conn, "/v1/auth", %{ - "login" => %{"ivle_token" => 'token'} + "login" => %{"ivle_token" => "token"} }) assert response(conn, 400) diff --git a/test/fixtures/custom_cassettes/auth_controller/v1/auth#1.json b/test/fixtures/custom_cassettes/auth_controller/v1/auth#1.json index 30ec2967c..c615bc99f 100644 --- a/test/fixtures/custom_cassettes/auth_controller/v1/auth#1.json +++ b/test/fixtures/custom_cassettes/auth_controller/v1/auth#1.json @@ -10,7 +10,7 @@ }, "response": { "binary": false, - "body": "\"\"", + "body": "\"e012345\"", "headers": { "Cache-Control": "private", "Content-Type": "application/json", @@ -25,5 +25,32 @@ "status_code": 200, "type": "ok" } + }, + { + "request": { + "body": "", + "headers": [], + "method": "get", + "options": [], + "request_body": "", + "url": "https://ivle.nus.edu.sg/api/Lapi.svc/UserName_Get?APIKey=&Token=token" + }, + "response": { + "binary": false, + "body": "\"\"", + "headers": { + "Cache-Control": "private", + "Content-Type": "application/json", + "Server": "Microsoft-IIS/8.5", + "Request-Context": "appId=cid-v1:9bebd252-0be2-48b7-a1db-8e2b70524944", + "Set-Cookie": "ASP.NET_SessionId=oymxocc04xjq2mwispu422cz; path=/; HttpOnly", + "X-AspNet-Version": "4.0.30319", + "X-Powered-By": "ASP.NET", + "Date": "Sat, 16 Jun 2018 19:36:33 GMT", + "Content-Length": "15" + }, + "status_code": 200, + "type": "ok" + } } ] From ae97a8a1bcaf7bbecd0e416e482e1e983311684a Mon Sep 17 00:00:00 2001 From: ning Date: Mon, 18 Jun 2018 18:26:11 +0800 Subject: [PATCH 39/39] Revert "Catch all :errors for api_fetch" This reverts commit 0a9c58938aef743bacd54404bcca0a4f781ab505. --- lib/cadet/accounts/ivle.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cadet/accounts/ivle.ex b/lib/cadet/accounts/ivle.ex index ffd67c589..94af406f4 100644 --- a/lib/cadet/accounts/ivle.ex +++ b/lib/cadet/accounts/ivle.ex @@ -62,8 +62,8 @@ defmodule Cadet.Accounts.IVLE do # IVLE responds with 500 if APIKey is invalid {:error, :internal_server_error} - {:ok, _} -> - # IVLE responds 200 with body == ~s("") if token is invalid + {:ok, %{body: ~s(""), status_code: 200}} -> + # IVLE responsed 200 with body == ~s("") if token is invalid {:error, :bad_request} end end