Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
89cccb5
Move context files folders with the same name
ning-y Jun 12, 2018
4a9d700
Change user fields to include NUSNET ID
ning-y Jun 12, 2018
fac9126
Change /auth to take ivle_token (was email, pw)
ning-y Jun 13, 2018
2a2d8ba
Add HTTPoison dependency for requests to IVLE
ning-y Jun 13, 2018
28fe00d
Change email in authorizations to nusnet_id
ning-y Jun 13, 2018
62c5817
Remove NUSNET ID from users
ning-y Jun 13, 2018
4190763
Remove password from registration, authorization
ning-y Jun 13, 2018
3e8a27d
Remove comeonin tokens
ning-y Jun 13, 2018
7a7e055
Add IVLE API call to /auth
ning-y Jun 13, 2018
b9a7a86
Register users automatically if first time login
ning-y Jun 13, 2018
7a6a046
Improve documentation, fix some credo --strict's
ning-y Jun 13, 2018
263296b
Squash migrations into a single file
ning-y Jun 13, 2018
a9925f8
Add tests for Ivle module via ExVCR
ning-y Jun 14, 2018
68c56c8
Add test for fetch_name in Cadet.Accounts.Ivle
ning-y Jun 14, 2018
edad9d3
Fine tune /auth error codes
ning-y Jun 14, 2018
0eef9d0
Add tests for sign_in in Cadet.Accounts
ning-y Jun 14, 2018
20c465e
Add tests for AuthController using ExVCR
ning-y Jun 14, 2018
5267c28
Fix failing travis.ci tests
ning-y Jun 14, 2018
dab8eb5
Remove password fields from registration test
ning-y Jun 15, 2018
753d82a
Set IVLE_KEY in .env rather than environment var
ning-y Jun 16, 2018
66808a2
Move ExVCR fixture to test/fixture
ning-y Jun 16, 2018
0b0e77e
Fix Ivle module using env var instead of dot env
ning-y Jun 16, 2018
cbe625e
Reverse conditional for readability
ning-y Jun 16, 2018
b21dbdb
Improve readability for Ivle's api_fetch/2
ning-y Jun 16, 2018
9ee39a1
Improve readability for AuthController's create/2
ning-y Jun 16, 2018
1d823db
Improve readability for Account's sign_in/2
ning-y Jun 16, 2018
797b7b1
Improve tests with assert pattern match
ning-y Jun 16, 2018
73de001
Improve readability and docs for @token in tests
ning-y Jun 16, 2018
634ccd7
Fix credo offences due to ff89294
ning-y Jun 16, 2018
f11f519
Fix failing credo tests because of bad cassettes
ning-y Jun 16, 2018
6298799
Move .env file as .env.example, update README
ning-y Jun 18, 2018
3508e83
Fix redundant pattern matching
ning-y Jun 18, 2018
2c2cede
Rename module Ivle -> IVLE
ning-y Jun 18, 2018
f1e09aa
Move test/fixture -> test/fixtures
ning-y Jun 18, 2018
2a5329a
Change Dotenv.load! -> Dotenv.load
ning-y Jun 18, 2018
eaa77fc
Add test for bad auth_controller create/2 sign_in
ning-y Jun 18, 2018
0a9c589
Catch all :errors for api_fetch
ning-y Jun 18, 2018
59a315f
Fix missing coverage for signin error
ning-y Jun 18, 2018
ae97a8a
Revert "Catch all :errors for api_fetch"
ning-y Jun 18, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions .env → .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to rename this to .env.example and add instruction in README to rename this to .env and modify the content accordingly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done with 8d017f9, and README updated accordingly.

# Webpack entry filename
export CADET_WEBPACK_ENTRY=app
CADET_WEBPACK_ENTRY=app

# IVLE LAPI Key
IVLE_KEY=your_ivle_lapi_key
9 changes: 8 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,17 @@ Install Elixir dependencies

mix deps.get

Initialise Development Database
Initialise development database

mix ecto.setup

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`.

IVLE_KEY=your_ivle_lapi_key

Run the server in your local machine

mix cadet.server
Expand Down
6 changes: 6 additions & 0 deletions config/test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ 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,
vcr_cassette_library_dir: "test/fixtures/vcr_cassettes",
custom_cassette_library_dir: "test/fixtures/custom_cassettes"

# Configure your database
config :cadet, Cadet.Repo,
adapter: Ecto.Adapters.Postgres,
Expand Down
95 changes: 38 additions & 57 deletions lib/cadet/accounts.ex → lib/cadet/accounts/accounts.ex
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,21 @@ defmodule Cadet.Accounts do
"""
use Cadet, :context

alias Comeonin.Pbkdf2

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 """
Register new User entity using E-mail and Password
authentication.
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)

if changeset.valid?() do
if changeset.valid? do
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@indocomsoft The new style guide in #50 says that zero-arity calls should end with parenthesis, but I think predicate functions should be excluded, because their trailing ? obviously indicates that they are predicate functions, not variables. Which do we go with? Your call.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, in the first place, changeset is a map (https://hexdocs.pm/ecto/Ecto.Changeset.html#t:t/0), so valid? is just a key in the map, so there should definitely be no paranthesis here.

registration = apply_changes(changeset)

Repo.transaction(fn ->
Expand All @@ -28,9 +28,8 @@ defmodule Cadet.Accounts do
{:ok, _} =
create_authorization(
%{
provider: :email,
uid: registration.email,
token: Pbkdf2.hashpwsalt(registration.password)
provider: :nusnet_id,
uid: registration.nusnet_id
},
user
)
Expand Down Expand Up @@ -69,77 +68,59 @@ 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_email(user = %User{}, email) do
token = get_token(:email, email) || get_random_token()

def add_nusnet_id(user = %User{}, nusnet_id) do
changeset =
%Authorization{}
|> Authorization.changeset(%{
provider: :email,
uid: email,
token: token
provider: :nusnet_id,
uid: nusnet_id
})
|> put_assoc(:user, user)

Repo.insert(changeset)
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
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
|> change(%{token: token})
for authorization <- authorizations do
authorization
|> change(%{nusnet_id: nusnet_id})
|> Repo.update!()
end
end)
end

@doc """
Sign in using given e-mail and password combination
Sign in using given NUSNET_ID
"""
def sign_in(email, password) do
auth = Repo.one(Query.email(email))

cond do
auth == nil ->
{:error, :not_found}

not Pbkdf2.checkpw(password, auth.token) ->
{:error, :invalid_password}
def sign_in(nusnet_id, token) do
auth = Repo.one(Query.nusnet_id(nusnet_id))

true ->
auth = Repo.preload(auth, :user)
{: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
if auth do
auth = Repo.preload(auth, :user)
{:ok, auth.user}
else
auth.token
# 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, _} ->
# 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
end
end

defp get_random_token() do
length = 64

:crypto.strong_rand_bytes(length)
|> Base.url_encode64()
|> binary_part(0, length)
end
end
5 changes: 2 additions & 3 deletions lib/cadet/accounts/authorization.ex
Original file line number Diff line number Diff line change
@@ -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.
"""
Expand All @@ -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
Expand Down
7 changes: 3 additions & 4 deletions lib/cadet/accounts/form/login.ex
Original file line number Diff line number Diff line change
@@ -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
Expand Down
25 changes: 8 additions & 17 deletions lib/cadet/accounts/form/registration.ex
Original file line number Diff line number Diff line change
@@ -1,33 +1,24 @@
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
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

import Ecto.Changeset

embedded_schema do
field(:first_name, :string)
field(:last_name, :string)
field(:email, :string)
field(:password, :string)
field(:password_confirmation, :string)
field(:name, :string)
field(:nusnet_id, :string)
end

@required_fields ~w(first_name email password password_confirmation)a
@optional_fields ~w(last_name)a

@email_format ~r/^[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,4}$/
@required_fields ~w(name nusnet_id)a

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)
|> validate_confirmation(:password)
end
end
75 changes: 75 additions & 0 deletions lib/cadet/accounts/ivle.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
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 Dotenv.load().values["IVLE_KEY"]

@doc """
Get the NUSNET ID of the user corresponding to this token.
returns...
- {: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
- token: String, the IVLE authentication token
## Examples
iex> Cadet.Accounts.IVLE.fetch_nusnet_id("T0K3N...")
{:ok, "e012345"}
"""
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
## Examples
iex> Cadet.Accounts.IVLE.fetch_name("T0K3N...")
{:ok, "LEE NING YUAN"}
"""
def fetch_name(token), do: api_fetch("UserName_Get", token)

defp api_fetch(path, token) do
case HTTPoison.get(api_url(path, token)) do
{: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}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should handle the case {:error, _}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, done with 0a9c589.

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
4 changes: 2 additions & 2 deletions lib/cadet/accounts/provider.ex
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import EctoEnum

defenum(Cadet.Accounts.Provider, :provider, [
# Email provides e-mail and password based authorization
:email
# An IVLE authentication token provides a NUSNET ID to track user identity
:nusnet_id
])
Loading