Skip to content
This repository has been archived by the owner on Jun 30, 2021. It is now read-only.

Add auth token expiration #1001

Merged
merged 33 commits into from
Jun 5, 2019
Merged

Add auth token expiration #1001

merged 33 commits into from
Jun 5, 2019

Conversation

ripzery
Copy link
Contributor

@ripzery ripzery commented May 10, 2019

Issue/Task Number: #953

Closes #953

Overview

This PR implements the authentication token expiration.

The default behavior of this feature is: the authentication token will never be expired unless the settings (:pre_auth_token_lifetime and :auth_token_lifetime) are set to the integer which more than zero.

Changes

  • Added expire_at to AuthToken and PreAuthToken
  • Added :pre_auth_token_lifetime in the setting to specify how long the PreAuthToken can be used.
  • Added :auth_token_lifetime in the setting to specify how long the AuthToken can be used.
  • Added an ability to expire either PreAuthToken or AuthToken or both.

Note: :pre_auth_token_lifetime and :auth_token_lifetime can be positive integer or 0. By set it to 0, meaning that the auth_token or pre_auth_token will never be expired.

Implementation Details

Every authenticated apis call AuthToken.authenticate. This PR adds some logic around there to check whether the authentication token's expire_at has lapsed.

So the main logic was implemented pretty much like this:

if lapsed do
  expire()
else
  refresh() 
end

That means If it has lapsed, then set expire: true, otherwise refresh the authentication token by advance an expire_at to the next pre_auth_token_lifetime or auth_token_lifetime seconds.

How to test?

1.mix do ecto.migrate, omg.server

  1. Set the :auth_token_lifetime (seconds) with the second you want to wait for expire.
curl http://localhost:4000/api/admin/configuration.update \
-H "Accept: application/vnd.omisego.v1+json" \
-H "Authorization: OMGAdmin `echo -n "user_id:authentication_token" | base64`" \
-H "Content-Type: application/json" \
-d '{
  "auth_token_lifetime": 20
}' \
-w "\n" | jq
  1. Open the admin panel and login

  2. Try to wait for x seconds (the value you've set at the previous step) to see if the auth token is expired or keep refreshing the webpage within x seconds to see if auth token is refreshed.

Note: You can also test the PreAuthToken but it will be applied only to the/admin.login_2fa which you've to enable two-factor authentication of the user first.

Impact

This PR requires a re-seeding of the settings using mix seed --settings. The steps after a deploy are therefore:

  1. mix ecto.migrate
  2. mix seed --settings

@ripzery ripzery self-assigned this May 10, 2019
@ripzery ripzery changed the base branch from master to 533-implement-2fa-authentication May 10, 2019 06:18
@ripzery ripzery force-pushed the 533-implement-2fa-authentication branch from a9b076b to e45c865 Compare May 17, 2019 09:57
@ripzery ripzery force-pushed the 953-add-auth-token-expiration branch from f95ceee to 6a0ef45 Compare May 17, 2019 12:05
@ripzery ripzery force-pushed the 533-implement-2fa-authentication branch from 56d0679 to 52e97a7 Compare May 21, 2019 06:48
@ripzery ripzery force-pushed the 953-add-auth-token-expiration branch 3 times, most recently from 1269fd8 to 4b7988c Compare May 27, 2019 08:55
@ripzery ripzery changed the base branch from 533-implement-2fa-authentication to v1.3 May 27, 2019 08:56
@ripzery ripzery requested review from unnawut and T-Dnzt and removed request for unnawut May 27, 2019 10:02
apps/ewallet_db/lib/ewallet_db/auth_token.ex Outdated Show resolved Hide resolved
apps/ewallet_db/lib/ewallet_db/auth_token.ex Outdated Show resolved Hide resolved
apps/ewallet_db/lib/ewallet_db/auth_token.ex Outdated Show resolved Hide resolved
apps/ewallet_db/lib/ewallet_db/auth_token.ex Outdated Show resolved Hide resolved
apps/ewallet_db/lib/ewallet_db/auth_token.ex Show resolved Hide resolved
apps/ewallet_db/test/ewallet_db/pre_auth_token_test.exs Outdated Show resolved Hide resolved
apps/ewallet_db/test/ewallet_db/pre_auth_token_test.exs Outdated Show resolved Hide resolved
apps/ewallet_db/test/ewallet_db/pre_auth_token_test.exs Outdated Show resolved Hide resolved
@@ -64,18 +64,32 @@ config :ewallet_config,
position: 105,
description: "The duration (in minutes) that a forget password request will be valid for."
},
"atk_lifetime" => %{
key: "atk_lifetime",
Copy link

Choose a reason for hiding this comment

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

I'm having problems with those names 😅 Can we just call it auth_token_lifetime and pre_auth_token_lifetime? more verbose, but clearer.

@@ -52,6 +53,7 @@ defmodule EWalletDB.AuthToken do
)

field(:expired, :boolean)
field(:expire_at, :naive_datetime_usec)
Copy link

Choose a reason for hiding this comment

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

Should be expired_at

@ripzery ripzery force-pushed the 953-add-auth-token-expiration branch from 2cf87c4 to 3ddf46a Compare May 29, 2019 09:24
@ripzery ripzery force-pushed the 953-add-auth-token-expiration branch from 0a60bcd to ed5eae2 Compare May 30, 2019 04:05
@ripzery ripzery requested review from unnawut and T-Dnzt May 31, 2019 01:39
apps/ewallet_db/lib/ewallet_db/auth_token.ex Outdated Show resolved Hide resolved
apps/ewallet_db/lib/ewallet_db/auth_token.ex Outdated Show resolved Hide resolved
apps/ewallet_db/lib/ewallet_db/auth_token.ex Outdated Show resolved Hide resolved
apps/ewallet_db/lib/ewallet_db/auth_token.ex Outdated Show resolved Hide resolved
@ripzery ripzery requested review from unnawut and sirn May 31, 2019 08:12
Copy link
Contributor

@unnawut unnawut left a comment

Choose a reason for hiding this comment

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

Lovely! 👏👏👏❤️❤️❤️

@T-Dnzt T-Dnzt added the flag/needs upgrade path 🛠️ This requires an upgrade path e.g. running an extra command or settings change label Jun 5, 2019
Copy link

@T-Dnzt T-Dnzt left a comment

Choose a reason for hiding this comment

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

Approving, but can you fix the PR description to reflect the changes you've made (name of fields, etc.). Another small thing to add to the description of the configurations would be what 0 means - it never expires, right?

@ripzery ripzery force-pushed the 953-add-auth-token-expiration branch from a916dda to 7478905 Compare June 5, 2019 08:24
@ripzery ripzery merged commit 0216d40 into v1.3 Jun 5, 2019
@ripzery ripzery deleted the 953-add-auth-token-expiration branch June 5, 2019 08:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
flag/needs upgrade path 🛠️ This requires an upgrade path e.g. running an extra command or settings change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants