Fix #77 Add API controller with token_authenticate_user method

[mrjoshua22]

Добавлен метод token_authenticate_user, тесты к нему и гем 'rails-controller-testing'. Метод тестируется через создание анонимного контроллера и использовании в нем метода через before_action. Для проверки значения возвращаемой инстанс переменной @current_user добавлен гем 'rails-controller-testing'. Тесты написаны для запроса с корректным токеном, токена с некорректным payload и запроса без токена вообще.

[mrjoshua22]

Если @current_user == nil, то нужно возвращать status :unauthorized. Добавлю следующим коммитом.

[mrjoshua22]

Добавлена обработка ActiveRecord::RecordNotFound и JWT::DecodeError

[XaoGao]

@mrjoshua22 указал замечания

[XaoGao]

Вот хороший способ избавиться от JSON.parse(response.body) https://stackoverflow.com/questions/5159882/how-to-check-for-a-json-response-using-rspec

module ApiHelpers
  def json_body
    JSON.parse(response.body)
  end
end

RSpec.configure do |config| 
  config.include ApiHelpers, type: :request
end

[XaoGao]

@mrjoshua22 все выгрлядит правильно, но можно еще немного улучшить код

 def token_authenticate_user
  # В будущем будут другие статусы ответа
  if authorization_header.blank?
    render status: :unauthorized, json: { error: t("api.errors.unauthorized") } and return
  end

  if authenticate_scheme_bearer?
    render status: :unauthorized, json: { error: t("Wrong scheme, you have to use a Bearer") } and return
  end

  if token.nil? || token.empty?
    render status: :unauthorized, json: { error: t("Token is nil or empty") } and return
  end

  if token_expired?
    render status: :unauthorized, json: { error: t("Token is expired") } and return
  end

  begin
    set_user
  rescue ActiveRecord::RecordNotFound
    render status: :unauthorized, json: { error: t("api.errors.not_found") }
  rescue JWT::DecodeError
    render status: :unauthorized, json: { error: t("unexpected error") }
  end
end

private

# приватные методы не тестируются

def authorization_header
  request.headers["Authorization"]
end

def token
  authorization_header.split.last
end

def authenticate_scheme_bearer?
  authenticate_scheme != "Bearer"
end

def authenticate_scheme
  authorization_header.split.first 
end

def decoded
  JwtService.decode(token)
end

def set_user
  @current_user ||= User.find(decoded[:id])
end

# эту проверку забыл указать в задаче
def token_expired?
  DateTime.now.to_i > decoded[:exp]
end

[mrjoshua22]

Вопросы по замечаниям:

1. Почему именно проверка токена идет через token.nil? || token.empty? если можно сделать token.blank?
2. Название метода authenticate_scheme_bearer? несколько сбивает с толку. Из названия кажется, что он должен возвращать true если header.split.first == 'Bearer', а он наоборот возвращает true при header.split.first != 'Bearer'. Может стоит назвать что-то вроде authenticate_scheme_wrong?
3. Все переменные определяются внутри приватных методов. Как определить когда стоит напрямую присваивать значения переменным, как в JwtService или SessionsController, а когда стоит все построить на приватных методах?

[XaoGao]

@mrjoshua22 1) Можно использовать blank? 2) authenticate_scheme_wrong - мне тут не нравиться wrong малоинформативно, давай просто сделаем

unless authenticate_scheme_bearer?
  render status: :unauthorized, json: { error: t("Wrong scheme, you have to use a Bearer") } and return
end
def authenticate_scheme_bearer?
  authenticate_scheme == "Bearer"
end

3) Нет четких критерии, нужно попробовать написать в обоих стилях и посмотреть что лучше читается

[mrjoshua22]

По последним замечаниям:

1. Метод token.blank? (nil?, empty?) использовать нельзя, так как token возвращает последний элемент массива authorization_header, соответственно в случае пустого токена будет возвращаться Bearer, и проверки не проходят. Для проверки добавил метод token_blank? где сравнивается token и authenticate_scheme - последний и первый элемент массива. Можно было прописать через authorization_header.size == 1, но сделал вот так. По этой же причине в тестах прописан только вариант token blank - на token nil нет смысла писать, получится одно и то же.
2. Проверку token_expired? убрал вообще вместе с приватным методом, так как JWT сам проверяет срок действия токена и возвращает JWT::ExpiredSignature в случае просрочки. Поэтому добавил еще один блок rescue, иначе токен не может быть декодирован в случае истечения срока действия.
3. Оказывается метод current_user изначально доступен благодаря Devise через ApplicationController, поэтому отдельно прописывать его для проверки значения @current_user в тестах не нужно.
4. В api_helper добавил type: :controller после config.include ApiHelpers, потому что так правильней.