Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Move api authentication into Devise Strategy #3131

Closed
skukx opened this issue Mar 4, 2019 · 2 comments
Closed

RFC: Move api authentication into Devise Strategy #3131

skukx opened this issue Mar 4, 2019 · 2 comments
Labels
changelog:solidus_api Changes to the solidus_api gem type:enhancement Proposed or newly added feature

Comments

@skukx
Copy link
Contributor

skukx commented Mar 4, 2019

Is your feature request related to a problem? Please describe.

Currently the api authentication has no ties to Devise and is self implemented. This is great but devise provides some plugins like lockable, confirmable etc. By Implementing the auth logic in a Devise strategy we can piggy back off devise.

Describe the solution you'd like

Create a devise strategy which inherits from Devise::Strategies::Base

class Spree::AuthStrategies::SpreeToken < Devise::Strategies::Base
  def valid?
    api_key.present?
  end

  def authenticate!
    resource = Spree::User.find_by(spree_api_key: api_key)
    return fail!(:not_found_in_database) if resource.nil?

    if resource.valid_for_authentication?
      success!(resource)
    else
      fail!(:invalid)
    end
  end

  private

  def api_key
    bearer_token || spree_token || params[:token]
  end

  def bearer_token
    pattern = /^Bearer /
    header = request.headers["Authorization"]
    header.gsub(pattern, '') if header.present? && header.match(pattern)
  end

  def spree_token
    token = request.headers["X-Spree-Token"]
    return unless token.present?

    Spree::Deprecation.warn(
      'The custom X-Spree-Token request header is deprecated and will be removed in the next release.' \
      ' Please use bearer token authorization header instead.'
    )
    token
  end
end

Warden::Strategies.add(:spree_token, Spree::AuthStrategies::SpreeToken)

The key is valid_for_authentication? method. If the user is locked out, then they will also be locked out from using the api.

@jacobherrington jacobherrington added changelog:solidus_api Changes to the solidus_api gem type:enhancement Proposed or newly added feature labels Mar 6, 2019
@jacobherrington jacobherrington changed the title Move api authentication into Devise Strategy RFC: Move api authentication into Devise Strategy Mar 6, 2019
@kennyadsl
Copy link
Member

@skukx I think this is a good idea, but what about stores that are not using Devise (if any)?

@skukx
Copy link
Contributor Author

skukx commented Apr 1, 2019

I've thought about that as well and don't really have a definitive answer for it. If we moved the api authentication to solidus_auth_devise then that leaves an open api. If solidus_auth_devise gets moved into core then stores can no longer control what they use for authentication.

One possible choice, (if this was something wanted), is to pull in warden as a dependency. Warden is a very core part of any authentication system for ruby server applications.

Another option is that we create an adapter around authentication, however I don't think that'd be worth the work since I would assume most stores are using devise for their authentication.

TLDR
I'm not sure how to handle that without having to require the solidus_auth_devise gem which would then need to contain the api authentication logic. This however leaves the api unprotected by default if the gem isn't used.

@solidusio solidusio locked and limited conversation to collaborators Sep 5, 2022
@kennyadsl kennyadsl converted this issue into discussion #4550 Sep 5, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
changelog:solidus_api Changes to the solidus_api gem type:enhancement Proposed or newly added feature
Projects
None yet
Development

No branches or pull requests

3 participants