solidusio / solidus

🛒 Solidus, the open-source eCommerce framework for industry trailblazers.
https://solidus.io
Other
4.97k stars 1.29k forks source link

Provide standard API authentication #4871

Closed chrean closed 1 year ago

chrean commented 1 year ago

Currently, there is no way to provide an email and password to get an API key.

waiting-for-dev commented 1 year ago

I'd like to delimit the scope of this issue before starting to work on it.

Solidus has a poorly defined abstraction for authentication. That means there's no guarantee of the underlying solution or a well-defined interface. That translates into the impossibility of building a concrete implementation that makes sense. For instance, implementations could fetch users from other systems without going through the ActiveRecord interface.

The most common approach is solidus_auth_devise, which uses Devise as the underlying authentication engine. For that scenario, there's already a strategy to retrieve a user from the database from their email & password.

On the other side, there's also the solidus-jwt extension that provides a cohesive authentication through JWT, thus providing a parallel path to the built-in API authentication with the spree_api_key. However, to allow the initial email/password authentication, it also requires solidus_auth_devise.

With all of that in mind, I can think of different solutions ordered from the lowest to the highest effort required:

elia commented 1 year ago

Freely taking inspiration from the list of options above I would propose this:

  1. add a --frontend option to the installer named api (the following assumes that's the selected option)
  2. add solidus_auth_devise to the Gemfile (like we do for other options)
  3. add the glue code to make the api authenticate with solidus_auth_devise to the target application

That code can then also act as the reference for the guide if we still want to have it.

Thoughts?

kennyadsl commented 1 year ago

@elia until backend depends on api we need to still ship api no matter what frontend you select. But this is not a blocker for the plan you proposed, because selecting --frontend=api we could just add the auth layer to the already provided api dependency. In the future, when we decouple backend and api, we can add the api dependency via the installer.

So, your plan makes sense to me. In combination with Review solidus_jwt and probably update it to make it compatible with the latest Solidus versions. Optionally deprecate authentication with spree_api_key. seems like the best trade-off to me at this point.

Plus, I think the guide page is a must-have for this topic, even if we have the installer option. I imagine a lot of people adding the API "frontend" at a later stage, after they installed Solidus for the first time

waiting-for-dev commented 1 year ago

Thanks for your feedback. I guess that would require changing the --frontend option to allow multiple values, as you might want to install both solidus_starter_frontend and the api. However, at this stage is also kind of confusing as that seems to convey the idea that we're going to copy all the API controllers into the host application while we're only talking about authentication. I'll start by trying to use solidus_auth_devise to authenticate the API on a sandbox app, so we have a better picture of the required changes. I'll report back once done.

elia commented 1 year ago

I guess that would require changing the --frontend option to allow multiple values

@waiting-for-dev I'm not sure to understand what would be the use case for having both, did you have a specific one in mind?

(context: from what I know API based stores would be very happy to handle everything via the API and sometimes they keep the authentication part on Rails only out of necessity)

waiting-for-dev commented 1 year ago

@elia, I mean that stores could want to provide both HTML and JSON interfaces for their stores, for instance, to have a web frontend and mobile apps or third-party tools.

To be clear, I don't mean providing the API as another frontend is a bad idea. It's the way to go, but it's only that we aren't at that point yet, so only copying the authentication code could be confusing. Let's keep talking about it!

waiting-for-dev commented 1 year ago

It took me a while, but I think I found the bare minimum to add an authentication endpoint to the API of a Solidus store with solidus_auth_devise:

# config/routes.rb
# ...
  namespace :api do
    devise_scope :spree_user do
      post '/sign_in', to: '/spree/api/user_sessions#create', format: false, defaults: { format: :json }
    end
  end
# ...
# app/controllers/spree/api/user_sessions_controller.rb
# frozen_string_literal: true

class Spree::Api::UserSessionsController < Devise::SessionsController
  skip_before_action :verify_authenticity_token

  clear_respond_to && respond_to(:json)

  def after_sign_in_path_for(resource)
    nil
  end
end

Then, you can authenticate like:

curl -X POST -v -H 'Content-Type: application/json' http://localhost:3000/api/sign_in  -d '{"spree_user": {"email": "admin@example.com", "password": "test123" }}'

Getting a response like the following:

{"id":1,"email":"admin@example.com","persistence_token":null,"perishable_token":null,"last_request_at":null,"login":"admin@example.com","ship_address_id":null,"bill_address_id":null,"created_at":"2023-02-02T04:54:14.867Z","updated_at":"2023-02-02T12:52:27.606Z","spree_api_key":"fbfd90eb1b323fbcdebf59fe9280917b4e2c80569e2d4aed","authentication_token":null,"deleted_at":null}

As we said, the minimal action would be creating a how-to guide with that code. Another option would be adding that to solidus_auth_devise. However, as we don't want to push more controller code into libraries, that is probably not a good idea. I also think it doesn't pay off automatically generating that code. Users might want to customize it often, plus it'd be good to wait before adding more auth stuff into the core until we revisit the topic and come up with a robust solution (a good abstraction or flexible implementation).

The needed code could be further simplified in the case of stores using devise but not solidus_auth_devise. For instance, we could use the devise_for route helper, and I recall I once managed to get rid of the after_sign_in_path_for method (although it probably was an API-only Rails app). However, I'm not sure if that scenario would be too niche to be worthy of consideration.

Thoughts?

kennyadsl commented 1 year ago

Thanks @waiting-for-dev, I also think it's not worth adding this code to solidus_auth_devise or auto-generating this code. The REST API documentation main page might be a good place for this small guide, where we are also covering other usual needs with the REST API. Alternatively, we could add this how-to to the main guides, because it's something more generic that we probably need even for GraphQL for example.

elia commented 1 year ago

@elia, I mean that stores could want to provide both HTML and JSON interfaces for their stores, for instance, to have a web frontend and mobile apps or third-party tools.

I agree it's a realistic scenario, maybe not so common tho. In most cases if you have a mobile app using REST APIs you probably would pick a frontend tech that uses the same APIs (e.g. React + ReactNative).

So, as a starting point, having a headless option that supports authentication out of the box still seems worth doing even now.

@waiting-for-dev what would say we need before we can offer a headless option in the installer?

waiting-for-dev commented 1 year ago

@elia, I agree with your point, but I think it's beyond the scope of this ticket. We would like a quick solution to show how the initial authentication can be added to the API. Taking all the rest of the API is something that we should consider in the future, so maybe we can add it to the OST?

chrean commented 1 year ago

My 2 cents: I think we may want to rework the API at some point this year, starting from decoupling them from the backend. For this reason, I'd stick to the quickest solution for this issue.

waiting-for-dev commented 1 year ago

@kennyadsl, I think that creating a complete how-to guide and linking from the REST API documentation is the best solution. Please, see https://github.com/solidusio/edgeguides/pull/91.

I still had to add a workaround for the failure path on Rails 7, plus documented how to customize both the success & failure responses.