From 00bd7da6a7772ae73c3709184c90b3fbd86acd96 Mon Sep 17 00:00:00 2001
From: aidewoode <aidewoode@gmail.com>
Date: Wed, 1 Jun 2022 17:12:59 +0800
Subject: [PATCH 1/2] Add basic structure of API

---
 app/controllers/api/v1/api_controller.rb      | 28 +++++++++++++++++++
 app/controllers/application_controller.rb     | 11 ++++++++
 app/controllers/sessions_controller.rb        |  5 +---
 app/models/user.rb                            |  1 +
 config/routes.rb                              |  6 ++++
 .../20220531070546_add_api_token_to_users.rb  |  6 ++++
 db/schema.rb                                  | 25 +++++++++--------
 test/controllers/sessions_controller_test.rb  | 10 +++++++
 test/test_helper.rb                           |  8 ++++++
 9 files changed, 84 insertions(+), 16 deletions(-)
 create mode 100644 app/controllers/api/v1/api_controller.rb
 create mode 100644 db/migrate/20220531070546_add_api_token_to_users.rb

diff --git a/app/controllers/api/v1/api_controller.rb b/app/controllers/api/v1/api_controller.rb
new file mode 100644
index 00000000..fe640525
--- /dev/null
+++ b/app/controllers/api/v1/api_controller.rb
@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+module Api
+  module V1
+    class ApiController < ApplicationController
+      skip_before_action :verify_authenticity_token
+
+      private
+
+      def find_current_user
+        Current.user = UserSession.find&.user
+
+        return if logged_in?
+
+        Current.user = authenticate_with_http_token do |token, options|
+          user = User.find_by(api_token: token)
+
+          return unless user.present?
+          return user if ActiveSupport::SecurityUtils.secure_compare(user.api_token, token)
+        end
+      end
+
+      def require_login
+        head :unauthorized unless logged_in?
+      end
+    end
+  end
+end
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index fe16b448..0cbf1d13 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -22,6 +22,10 @@ class ApplicationController < ActionController::Base
     end
   end
 
+  rescue_from ActionController::InvalidAuthenticityToken do
+    logout_current_user
+  end
+
   def browser
     @browser ||= Browser.new(
       request.headers["User-Agent"],
@@ -64,4 +68,11 @@ def require_login
   def require_admin
     raise BlackCandyError::Forbidden unless is_admin?
   end
+
+  def logout_current_user
+    UserSession.find&.destroy
+    cookies.delete(:user_id)
+
+    redirect_to new_session_path
+  end
 end
diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index 48a22713..5f152f9c 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -21,10 +21,7 @@ def create
 
   def destroy
     return unless logged_in?
-
-    UserSession.find.destroy
-    cookies.delete(:user_id)
-    redirect_to new_session_path
+    logout_current_user
   end
 
   private
diff --git a/app/models/user.rb b/app/models/user.rb
index cbe12048..0dd194fd 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -6,6 +6,7 @@ class User < ApplicationRecord
 
   include ScopedSetting
 
+  has_secure_token :api_token
   has_setting :theme, default: DEFAULT_THEME
 
   before_create :downcase_email
diff --git a/config/routes.rb b/config/routes.rb
index 38520b11..91767a1e 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -49,4 +49,10 @@
   get "/404", to: "errors#not_found", as: :not_found
   get "/422", to: "errors#unprocessable_entity", as: :unprocessable_entity
   get "/500", to: "errors#internal_server_error", as: :internal_server_error
+
+  namespace :api do
+    namespace :v1 do
+      resource :authentication, only: [:create]
+    end
+  end
 end
diff --git a/db/migrate/20220531070546_add_api_token_to_users.rb b/db/migrate/20220531070546_add_api_token_to_users.rb
new file mode 100644
index 00000000..5c5dc9be
--- /dev/null
+++ b/db/migrate/20220531070546_add_api_token_to_users.rb
@@ -0,0 +1,6 @@
+class AddApiTokenToUsers < ActiveRecord::Migration[7.0]
+  def change
+    add_column :users, :api_token, :string
+    add_index :users, :api_token, unique: true
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 646f7e43..6756ac79 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,8 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2021_12_21_081317) do
-
+ActiveRecord::Schema[7.0].define(version: 2022_05_31_070546) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "hstore"
   enable_extension "pg_trgm"
@@ -20,8 +19,8 @@
   create_table "albums", force: :cascade do |t|
     t.string "name"
     t.string "image"
-    t.datetime "created_at", precision: 6, null: false
-    t.datetime "updated_at", precision: 6, null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
     t.bigint "artist_id"
     t.index ["artist_id", "name"], name: "index_albums_on_artist_id_and_name", unique: true
     t.index ["artist_id"], name: "index_albums_on_artist_id"
@@ -31,8 +30,8 @@
   create_table "artists", force: :cascade do |t|
     t.string "name"
     t.string "image"
-    t.datetime "created_at", precision: 6, null: false
-    t.datetime "updated_at", precision: 6, null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
     t.boolean "is_various", default: false
     t.index ["name"], name: "index_artists_on_name", unique: true
   end
@@ -40,8 +39,8 @@
   create_table "playlists", force: :cascade do |t|
     t.string "name"
     t.string "type"
-    t.datetime "created_at", precision: 6, null: false
-    t.datetime "updated_at", precision: 6, null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
     t.bigint "user_id"
     t.index ["user_id"], name: "index_playlists_on_user_id"
   end
@@ -65,8 +64,8 @@
     t.string "md5_hash", null: false
     t.float "duration", default: 0.0, null: false
     t.integer "tracknum"
-    t.datetime "created_at", precision: 6, null: false
-    t.datetime "updated_at", precision: 6, null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
     t.bigint "album_id"
     t.bigint "artist_id"
     t.index ["album_id"], name: "index_songs_on_album_id"
@@ -78,11 +77,13 @@
     t.string "email", null: false
     t.string "crypted_password", null: false
     t.boolean "is_admin", default: false
-    t.datetime "created_at", precision: 6, null: false
-    t.datetime "updated_at", precision: 6, null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
     t.hstore "settings"
     t.string "password_salt"
     t.string "persistence_token"
+    t.string "api_token"
+    t.index ["api_token"], name: "index_users_on_api_token", unique: true
     t.index ["email"], name: "index_users_on_email", unique: true
     t.index ["persistence_token"], name: "index_users_on_persistence_token", unique: true
   end
diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb
index c6fb56b4..819652c2 100644
--- a/test/controllers/sessions_controller_test.rb
+++ b/test/controllers/sessions_controller_test.rb
@@ -35,4 +35,14 @@ class SessionsControllerTest < ActionDispatch::IntegrationTest
     assert_empty cookies[:user_id]
     assert_redirected_to new_session_url
   end
+
+  test "should have forgery protection" do
+    with_forgery_protection do
+      login @user
+
+      assert_nil session[:user_credentials]
+      assert_nil cookies[:user_id]
+      assert_redirected_to new_session_url
+    end
+  end
 end
diff --git a/test/test_helper.rb b/test/test_helper.rb
index cbd11d2f..79c5c820 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -92,6 +92,14 @@ def media_file_info_stub(file_path, attributes = {})
   def flush_redis
     Redis::Objects.redis.flushdb
   end
+
+  def with_forgery_protection
+    old = ActionController::Base.allow_forgery_protection
+    ActionController::Base.allow_forgery_protection = true
+    yield
+  ensure
+    ActionController::Base.allow_forgery_protection = old
+  end
 end
 
 class ActionDispatch::IntegrationTest

From 282ac1fcdc6ad9d83c69ad5e48a80e814a6f71d8 Mon Sep 17 00:00:00 2001
From: aidewoode <aidewoode@gmail.com>
Date: Thu, 2 Jun 2022 16:38:26 +0800
Subject: [PATCH 2/2] Fix #159, add API for user authentication

---
 app/controllers/api/v1/api_controller.rb      |  9 ++-
 .../api/v1/authentications_controller.rb      | 31 +++++++++
 app/controllers/api/v1/songs_controller.rb    | 12 ++++
 app/controllers/songs_controller.rb           |  5 --
 app/javascript/player.js                      |  2 +-
 .../v1/authentications/create.json.jbuilder   |  1 +
 .../{ => api/v1}/songs/show.json.jbuilder     |  2 -
 config/routes.rb                              |  3 +-
 .../controllers/api/v1/api_controller_test.rb | 29 ++++++++
 .../api/v1/authentications_controller_test.rb | 67 +++++++++++++++++++
 .../api/v1/songs_controller_test.rb           | 17 +++++
 test/controllers/songs_controller_test.rb     |  6 --
 test/fixtures/users.yml                       |  3 +
 test/test_helper.rb                           |  4 ++
 14 files changed, 173 insertions(+), 18 deletions(-)
 create mode 100644 app/controllers/api/v1/authentications_controller.rb
 create mode 100644 app/controllers/api/v1/songs_controller.rb
 create mode 100644 app/views/api/v1/authentications/create.json.jbuilder
 rename app/views/{ => api/v1}/songs/show.json.jbuilder (93%)
 create mode 100644 test/controllers/api/v1/api_controller_test.rb
 create mode 100644 test/controllers/api/v1/authentications_controller_test.rb
 create mode 100644 test/controllers/api/v1/songs_controller_test.rb

diff --git a/app/controllers/api/v1/api_controller.rb b/app/controllers/api/v1/api_controller.rb
index fe640525..b71d08e0 100644
--- a/app/controllers/api/v1/api_controller.rb
+++ b/app/controllers/api/v1/api_controller.rb
@@ -7,16 +7,19 @@ class ApiController < ApplicationController
 
       private
 
+      # If user already has logged in then authenticate with current session,
+      # otherwise authenticate with api token.
       def find_current_user
         Current.user = UserSession.find&.user
 
         return if logged_in?
 
-        Current.user = authenticate_with_http_token do |token, options|
+        authenticate_with_http_token do |token, options|
           user = User.find_by(api_token: token)
-
           return unless user.present?
-          return user if ActiveSupport::SecurityUtils.secure_compare(user.api_token, token)
+
+          # Compare the tokens in a time-constant manner, to mitigate timing attacks.
+          Current.user = user if ActiveSupport::SecurityUtils.secure_compare(user.api_token, token)
         end
       end
 
diff --git a/app/controllers/api/v1/authentications_controller.rb b/app/controllers/api/v1/authentications_controller.rb
new file mode 100644
index 00000000..b35630f6
--- /dev/null
+++ b/app/controllers/api/v1/authentications_controller.rb
@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+module Api
+  module V1
+    class AuthenticationsController < ApiController
+      skip_before_action :find_current_user
+      skip_before_action :require_login
+
+      def create
+        session = UserSession.new(session_params.merge({remember_me: true}).to_h)
+
+        if params[:with_session]
+          head :unauthorized unless session.save
+        else
+          head :unauthorized unless session.valid?
+        end
+
+        @current_user = User.find_by(email: session_params[:email])
+        head :unauthorized unless @current_user.present?
+
+        @current_user.regenerate_api_token if @current_user.api_token.blank?
+      end
+
+      private
+
+      def session_params
+        params.require(:user_session).permit(:email, :password)
+      end
+    end
+  end
+end
diff --git a/app/controllers/api/v1/songs_controller.rb b/app/controllers/api/v1/songs_controller.rb
new file mode 100644
index 00000000..ef1df032
--- /dev/null
+++ b/app/controllers/api/v1/songs_controller.rb
@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+module Api
+  module V1
+    class SongsController < ApiController
+      def show
+        @song = Song.find(params[:id])
+        @song_format = need_transcode?(@song.format) ? Stream::TRANSCODE_FORMAT : @song.format
+      end
+    end
+  end
+end
diff --git a/app/controllers/songs_controller.rb b/app/controllers/songs_controller.rb
index 612862eb..0c30dfa0 100644
--- a/app/controllers/songs_controller.rb
+++ b/app/controllers/songs_controller.rb
@@ -7,9 +7,4 @@ def index
     records = Song.includes(:artist, :album).order(:name)
     @pagy, @songs = pagy(records)
   end
-
-  def show
-    @song = Song.find(params[:id])
-    @song_format = need_transcode?(@song.format) ? Stream::TRANSCODE_FORMAT : @song.format
-  end
 end
diff --git a/app/javascript/player.js b/app/javascript/player.js
index 3a3caa86..6b793bc1 100644
--- a/app/javascript/player.js
+++ b/app/javascript/player.js
@@ -19,7 +19,7 @@ class Player {
     this.isPlaying = true;
 
     if (!song.howl) {
-      fetchRequest(`/songs/${song.id}`)
+      fetchRequest(`/api/v1/songs/${song.id}`)
         .then((response) => {
           return response.json();
         })
diff --git a/app/views/api/v1/authentications/create.json.jbuilder b/app/views/api/v1/authentications/create.json.jbuilder
new file mode 100644
index 00000000..348c6ac5
--- /dev/null
+++ b/app/views/api/v1/authentications/create.json.jbuilder
@@ -0,0 +1 @@
+json.call(@current_user, :api_token)
diff --git a/app/views/songs/show.json.jbuilder b/app/views/api/v1/songs/show.json.jbuilder
similarity index 93%
rename from app/views/songs/show.json.jbuilder
rename to app/views/api/v1/songs/show.json.jbuilder
index bd267161..d0959f60 100644
--- a/app/views/songs/show.json.jbuilder
+++ b/app/views/api/v1/songs/show.json.jbuilder
@@ -1,5 +1,3 @@
-# frozen_string_literal: true
-
 json.call(@song, :id, :name, :duration)
 json.url new_stream_path(song_id: @song.id)
 json.album_name @song.album.title
diff --git a/config/routes.rb b/config/routes.rb
index 91767a1e..7d1c2f01 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -14,7 +14,7 @@
   resources :stream, only: [:new]
   resources :transcoded_stream, only: [:new]
   resources :cached_transcoded_stream, only: [:new]
-  resources :songs, only: [:index, :show]
+  resources :songs, only: [:index]
   resources :albums, only: [:index, :show, :edit, :update], concerns: :playable
 
   resources :users, except: [:show] do
@@ -53,6 +53,7 @@
   namespace :api do
     namespace :v1 do
       resource :authentication, only: [:create]
+      resources :songs, only: [:show]
     end
   end
 end
diff --git a/test/controllers/api/v1/api_controller_test.rb b/test/controllers/api/v1/api_controller_test.rb
new file mode 100644
index 00000000..480c5ea5
--- /dev/null
+++ b/test/controllers/api/v1/api_controller_test.rb
@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+require "test_helper"
+
+class Api::V1::ApiControllerTest < ActionDispatch::IntegrationTest
+  setup do
+    @user = users(:visitor1)
+    @song = songs(:mp3_sample)
+  end
+
+  test "should authenticate when have user session" do
+    login(@user)
+    get api_v1_song_url(@song), as: :json
+
+    assert_response :success
+  end
+
+  test "should authenticate when have api token" do
+    get api_v1_song_url(@song), as: :json, headers: api_token_header(@user)
+
+    assert_response :success
+  end
+
+  test "should not authenticate when do not have user seesion or api token" do
+    get api_v1_song_url(@song), as: :json
+
+    assert_response :unauthorized
+  end
+end
diff --git a/test/controllers/api/v1/authentications_controller_test.rb b/test/controllers/api/v1/authentications_controller_test.rb
new file mode 100644
index 00000000..07b83375
--- /dev/null
+++ b/test/controllers/api/v1/authentications_controller_test.rb
@@ -0,0 +1,67 @@
+# frozen_string_literal: true
+
+require "test_helper"
+
+class Api::V1::AuthenticationsControllerTest < ActionDispatch::IntegrationTest
+  setup do
+    @user = users(:visitor1)
+  end
+
+  test "should create authentication without session" do
+    post api_v1_authentication_url, as: :json, params: {
+      user_session: {
+        email: @user.email,
+        password: "foobar"
+      }
+    }
+
+    response = @response.parsed_body
+
+    assert_response :success
+    assert_nil session[:user_credentials]
+    assert_equal @user.reload.api_token, response["api_token"]
+  end
+
+  test "should create authentication with session" do
+    post api_v1_authentication_url, as: :json, params: {
+      with_session: true,
+      user_session: {
+        email: @user.email,
+        password: "foobar"
+      }
+    }
+
+    response = @response.parsed_body
+
+    assert_response :success
+    assert_not_nil session[:user_credentials]
+    assert_equal @user.reload.api_token, response["api_token"]
+  end
+
+  test "should not create authentication with wrong credential" do
+    post api_v1_authentication_url, as: :json, params: {
+      user_session: {
+        email: @user.email,
+        password: "fake"
+      }
+    }
+
+    assert_response :unauthorized
+    assert_nil session[:user_credentials]
+    assert_empty @response.body
+  end
+
+  test "should not create authentication and session with wrong credential" do
+    post api_v1_authentication_url, as: :json, params: {
+      with_session: true,
+      user_session: {
+        email: @user.email,
+        password: "fake"
+      }
+    }
+
+    assert_response :unauthorized
+    assert_nil session[:user_credentials]
+    assert_empty @response.body
+  end
+end
diff --git a/test/controllers/api/v1/songs_controller_test.rb b/test/controllers/api/v1/songs_controller_test.rb
new file mode 100644
index 00000000..afaf836f
--- /dev/null
+++ b/test/controllers/api/v1/songs_controller_test.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+require "test_helper"
+
+class Api::V1::SongsControllerTest < ActionDispatch::IntegrationTest
+  setup do
+    @song = songs(:mp3_sample)
+  end
+
+  test "should show song" do
+    get api_v1_song_url(@song), as: :json, headers: api_token_header(users(:visitor1))
+    response = @response.parsed_body
+
+    assert_response :success
+    assert_equal @song.name, response["name"]
+  end
+end
diff --git a/test/controllers/songs_controller_test.rb b/test/controllers/songs_controller_test.rb
index c85a53cd..3e23c7fc 100644
--- a/test/controllers/songs_controller_test.rb
+++ b/test/controllers/songs_controller_test.rb
@@ -8,10 +8,4 @@ class SongsControllerTest < ActionDispatch::IntegrationTest
       assert_response :success
     end
   end
-
-  test "should show song" do
-    assert_login_access(url: song_url(songs(:mp3_sample)), xhr: true) do
-      assert_response :success
-    end
-  end
 end
diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml
index 2abf76a7..b07c54bd 100644
--- a/test/fixtures/users.yml
+++ b/test/fixtures/users.yml
@@ -4,15 +4,18 @@ admin:
   password_salt: <%= salt = Authlogic::Random.hex_token %>
   crypted_password: <%= Authlogic::CryptoProviders::BCrypt.encrypt("foobar" + salt) %>
   persistence_token: <%= Authlogic::Random.hex_token %>
+  api_token: <%= SecureRandom.base58 %>
 
 visitor1:
   email: 'visitor1@blackcandy.com'
   password_salt: <%= salt = Authlogic::Random.hex_token %>
   crypted_password: <%= Authlogic::CryptoProviders::BCrypt.encrypt("foobar" + salt) %>
   persistence_token: <%= Authlogic::Random.hex_token %>
+  api_token: <%= SecureRandom.base58 %>
 
 visitor2:
   email: 'visitor2@blackcandy.com'
   password_salt: <%= salt = Authlogic::Random.hex_token %>
   crypted_password: <%= Authlogic::CryptoProviders::BCrypt.encrypt("foobar" + salt) %>
   persistence_token: <%= Authlogic::Random.hex_token %>
+  api_token: <%= SecureRandom.base58 %>
diff --git a/test/test_helper.rb b/test/test_helper.rb
index 79c5c820..587adb98 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -66,6 +66,10 @@ def login(user)
     post session_url, params: {user_session: {email: user.email, password: "foobar"}}
   end
 
+  def api_token_header(user)
+    {authorization: ActionController::HttpAuthentication::Token.encode_credentials(user.api_token)}
+  end
+
   def logout
     delete session_url
   end