Some errors with namespaced classes. about simple_token_authentication HOT 18 CLOSED

Hi @joshblour!

Many thanks for raising this issue and sharing your workaround. I see at least two bugs here, and I believe we should review all the case transformations with something like Module::AnotherModule::ClassName in mind. I tag this as a bug for further review.

from simple_token_authentication.

I have the same issue with namespaces (acts_as_token_authentication_handler_for User::User). I dont get the workaround of @joshblour .

@gonzalo-bulnes is there any easy workaround for this problem?

from simple_token_authentication.

Hello @fwidtmann,

The issue with namespaces takes its origin in the way we are lowcasing/capitalizing/underscoring/manipulating the class name to generate the headers names, dynamic methods names and so on.
There are several places in the code where the names amnipulation occurs, and one way to solve the issue is finding and fixing them. I believe the fastest way to do that is writting a series of small tests with known cases (e.g. Core::User should lead to X-CoreUser-Token, SuperAdmin to X-SuperAdmin-Token, etc.) and then using them to find the right conversion formula. If you want to tackle the issue, your help or feedback will be very welcome.

Independently of that, the v2.0.0 milestone consists in modifying the gem so it provides an additional strategy to Devise instead of only being a close-but-not-so-close Devise companion (see #57 ).
The work in that direction is being led by the @adamniedzielski fork: see #69

The @joshblour workaround consists in using that fork and adding some configuration to the Devise initializer config/initializers/devise.rb to get the naming work done. (One advantage of being a Devise pal is that you can delegate some responsibilities to it.) I don't know which exact configuration it is, but @joshblour may be willing to share it, could you ask him? That would certainly be an useful thing to do, and such an example would be a great documented workaround to have.

I hope this explanation helped in some way, keep me updated ; )
Regards!

from simple_token_authentication.

I stuck this in my config/initializers/devise.rb

  config.token_header_names = {:'core/user' => {authentication_token: 'X-User-Token', email: 'X-User-Email'}}

You'll need to change the key from 'core/user' to whatever your namespaced class is ('user/user'?)

from simple_token_authentication.

Same bug here, but different perspective: I'm heavily namespacing my devise_for

namespace 'api' do
    namespace 'v1' do
      devise_for :managers, 
...

so, in eval, I get error:

 syntax error, unexpected '/', expecting ';' or '\n'
      def authenticate_api/v1/manager_from_token

because line

entity_underscored = entity_class.name.singularize.underscore

returns api/v1/manager for my Api::V1::Manager class.

Quick workaround: change line to

entity_underscored = entity_class.name.singularize.underscore.gsub('/', '_')

so all the slashes will be changed to underscores.

But what is also strucking me as odd, that

<some variable holding class here>.name.singularize.underscore

can be found 9 (!!) times in that file. Maybe, we can DRY it a little bit? =)

from simple_token_authentication.

Actually, #113 is an attempt to fix what I've stumbled upon. =)

from simple_token_authentication.

Hi Ivan,

the slahes problem occurs when dow "underscores" a namespaced name. The result can't mske a valid symbol.

If wanting to contribute code, do it from the master branch, but I think the solution still has to be defined. I don't know if ignoring all namespaces as proposed in #113 would work for you, but I would try not to implement a sub-optimal solution.

If you want to take a look at the advantages and drawbacks of a few alternatives, I'll be reading the discussion. The refactoring in progress makes implementation easy, do not worry for that.

Ivan Kolmychek [email protected] wrote:

Same bug here, but different perspective: I'm heavily namespacing my devise_for, so, in eval, I get error:

syntax error, unexpected '/', expecting ';' or '\n'
def authenticate_api/v1/manager_from_token

because line

entity_underscored = entity_class.name.singularize.underscore

returns api/v1/manager for my Api::V1::Manager class.

Quick workaround: change line to

entity_underscored = entity_class.name.singularize.underscore.gsub('/', '_')

so all the slashes will be changed to underscores.

But what is also strucking me as odd, that

.name.singularize.underscore

repeats 9 (!!) times in that file. Maybe, we can DRY it a little bit? =)

—
Reply to this email directly or view it on GitHub.

{"@context":"http://schema.org","@type":"EmailMessage","description":"View this Issue on GitHub","action":{"@type":"ViewAction","url":"https://github.com/gonzalo-bulnes/simple_token_authentication/issues/83#issuecomment-57607931","name":"View Issue"}}

from simple_token_authentication.

Hi, @gonzalo-bulnes, thank you for the reply. =)

Yes, ignoring namespaces works for me too and I think, we can safely use that approach by-default. Actually, I've tried both on the copy, plugged in my app, and everything works fine in both variants.

I've not encountered cases where you would like to authenticate models with same name, but different namespaces, in the same controller - if there will be a problem with it, then, I think, someone will eventualy report it. =)

from simple_token_authentication.

I've discovered some side-effect with solution from #113 - you cannot specify different headers for models with different namespaces.
Looks like, when ignoring namespaces, you cannot write something like

config.header_names = { 
  api_v1_manager: { 
    authentication_token: 'X-Manager-Token', 
    email: 'X-Manager-Email' 
  },
  api_v2_manager: { 
    authentication_token: 'X-Manager-Token', 
    email: 'X-Manager-Username' 
  }
}

in config/initializers/simple_token_auth.rb - you'll have to use one set of fields.

UPD
Also, I think, in my case, using manager as a key in config.header_names hash and api_v1_manager everywhere else is counter-intuitive, but it may be caused by lazy dezign of my app. =)

from simple_token_authentication.

So, 11 days gone and issue is still alive, even with pull-request submitted. Can you, please, accept #113, so, namespaces will, at least, work, and then we can make other discussion around namespaced keys in initializers/simple_token_auth.rb and handling namespaces at all? =)

UPD: Even if I would just fork and fix keys in initializer for my project, it will be easier to do so and keep fork in sync with that pull-request accepted. =)

from simple_token_authentication.

@ivan-kolmychek,

Both the possible solutions we've considered until now have serious limitations.

Part of the time taken to consider alternatives and real use cases before implementing any enhancement is, in fact, a feature. If you're in a hurry, if the submitted code fits your use case and if you are willing to impement the feature without the corresponding specs or documentation, I suggest you update a fork with it. That's not something I can afford to do without compromising the gem stability or usefullness.

from simple_token_authentication.

@gonzalo-bulnes

Ok, thank you for answer. You're right, it's not good to incorporate such solutions into a gem version, considered stable.

I still have to try current master branch behavior and look into a code - maybe, we can figure out better, optimal solution.

Still, I'll consider forking and laying some changes on top of current (1.5.1) version - it may help, for a while.

from simple_token_authentication.

In the master branch, you may want to look at SimpleTokenAuthentication::Entity#name_underscore and other *_name methods for patching.

I use to keep an eye on forks and I'll keep an eye on yours. Of course, feedback on issues you may run into will be valued in this thread!

Best regards!

from simple_token_authentication.

Master still does not work, but code is much cleaner, yes. And, as for now, everything about conversion is done in Entity#name_underscore:

def name_underscore
  name.underscore
end

Let's gahter pros and cons list of splitting the entity name by :: and...

1. drop everything except the last token
   - all models with same name across all namespaces share the header name
   + more convinient helpers (current_manager vs current_api_v1_manager)

2. join all the tokens by _
   - long header lookup symbol name
   - long helper methods names
   - doesn't allow to differentiate SomeAppStore::Manager from SomeApp::StoreManager.
   + each model headers can be customized separatedly

Feel free to add more.

from simple_token_authentication.

Add to 2) that it doesn't allow to differentiate SomeAppStore::Manager from SomeApp::StoreManager.

from simple_token_authentication.

@gonzalo-bulnes Added. Maybe, change _ to other separator?

from simple_token_authentication.

Hi @joshblour, @fwidtmann, @ivan-kolmychek!

How have you been? Support for namespaced models may be at sight. In the spike-add-models-aliases-option branch I added an option to define model aliases. What's the idea?

In your controllers, when you want to handle token authentication for a namespaced model (let's say MobileApp::User), you can now define an alias for it:

# app/controllers/application_controller.rb

class ApplicationController < ActionController::Base # or ActionController::API
  # ...

  acts_as_token_authentication_handler_for MobileApp::User, as: :mobile_user

  # ...
end

Once that done, you should be able to authenticate them using the mobile_user_token and mobile_user_email or the X-MobileUser-Token and X-MobileUser-Email headers. (Tadaa!)

Could anyone (or two or the three) of you give it a try please? I would love to cross-check that everything is fine in a real application context before shipping it. I look forward for news!

Best regards!

from simple_token_authentication.

Hi people,

The v1.9.0 release is now available, with aliases support for token authenticatable models.

A few words about this solution to the issue:

I didn't really liked the idea of replacing the problematic slashes / by underscores _, because slashes and underscores have conventionally distinct meanings in Rails. Of course, there is nothing wrong in deciding not to follow one convention or another in our applications as long as that produces no name clashes. As developers, we evaluate the context we work in and following or not that kind of conventions is our call.

What I didn't liked was letting Simple Token Authentication make the decision for us. Often the _ solution would be just good, but sometimes it wouldn't.

Aliases allow us to decide, and be explicit about the choices we make, I hope you'll agree.

Explicit is better than implicit. -- Tim Peters

Best regards!

from simple_token_authentication.