Support for resource identifiers different than email (example: login, uuid...)? about simple_token_authentication HOT 31 CLOSED

Hi @aried3r & @gonzalo-bulnes,

I ended up writing a warden strategy to use, but I should still have the code somewhere. Give me a day or so to find it and check it against the current master. I never did finish writing the tests before switching my application to the warden strategy, so someone would have to do that, but when I left it, it was fully complete and working very well.

from simple_token_authentication.

Hello @bchase,

The anti-timing attacks attribute is hardcoded as :email, but it could be any other attribute as long as it identifies the user (or any token authenticatable model) you're working with.

There are a few forks that customize that attribute, either hardcoding their own, or through a configuration option. Take a look at them!

A PR on that topic would be very welcome and I think an initializer option would fit perfectly. A real bonus would be to allow the option value to be a hash so different token authenticatable models could use their own custom attributes (just as the header_names option does; see the initializer comments in the README for examples).

Regards!

from simple_token_authentication.

Thanks for your reply, Gonzalo. And sorry for my delayed response!

I'm not very familiar with Devise internals, but in poking around I think I may have found an introspective solution that might be even more convenient when combined with the configuration approach you mentioned.

So far, I've got the functionality I want by changing this line to:

auth_key_sym = entity_class.authentication_keys.first
auth_key_str = auth_key_sym.to_s.camelize
"X-#{entity_class.name.singularize.camelize}-#{auth_key_str}"

This partially accomplishes custom attributes for individual models by way of asking entity_class what its authentication_keys are. These can be set with devise ... authentication_keys: [:foo] in a model, and Devise even conveniently defaults this to [:email]. ...an array, hence the .first used above, which is the part about this exact solution that I don't like at all.

Which leads to the greater point that my workaround here completely ignores the case where there are SimpleTokenAuthentication.header_names, but as authentication_keys will be an array, going this configuration route would probably require a larger reworking of the singular header_email_name to something that is aware of the possible many authentication keys.

So that's what I've come up with so far and I wanted to get some feedback from you before I started moving everything around and submitting a PR that nobody liked. 😄 So please let me know if you like taking the auth keys this direction and I'll be happy to take a stab at making it happen!

Thanks,
Brad

from simple_token_authentication.

Ah, just realized I was still authenticating with an email address (I'm aiming for X-User-Login, which for me is email or username).

Looks like authenticate_entity_from_token! would need some similar adjustments.

from simple_token_authentication.

I'm running into a similar issue. I'd like to use phone_number instead of email to log people in.

@gonzalo-bulnes do I have to fork the repository and edit the code to get this to work?

Is using a phone number to authenticate vs email cause any security issues with timing attacks?

from simple_token_authentication.

@nicolo, for now yes you should fork the repo to make the change. As long as the phone number is unique among users, it will play the role of the email without problem. What's important here is to tie the authentication token to one specific user.

from simple_token_authentication.

@gonzalo-bulnes if I created a PR for a configurable authentication field, would it be something you'd consider merging into the main repository?

from simple_token_authentication.

Yes.

The mid-term plans for Simple Token Authentication include modifying the gem to provide a Devise strategy (see #57). That will change the gem behaviour significatively (milestone: v2.0.0), but that's not for the next few days (despite a lot of help from @adamniedzielski, my side of the contribution is going slowly) and based on the gem forks, I think that a configurable authentication field would be greatly appreciated among the v1.x.x users. @nicolo

from simple_token_authentication.

Until @gonzalo-bulnes will fix this, you can use my fork:
https://github.com/ProGM/simple_token_authentication

You can configure the parameter this way:

SimpleTokenAuthentication.configure do |config|
  config.header_names = { user: 'your_parameter_name' }
end

It's not a great solution, but it seems to work.

from simple_token_authentication.

@ProGM
Should that be?

SimpleTokenAuthentication.configure do |config|
  config.header_names = { user: { auth_parameter_name: 'your_parameter_name' } }
end

from simple_token_authentication.

@speed-of-light
Sorry, the correct one is:

SimpleTokenAuthentication.configure do |config|
  config.auth_parameter_name = { user: 'your_parameter_name' }
end

Look here for details: https://github.com/ProGM/simple_token_authentication#configuration

from simple_token_authentication.

Hi @ProGM,

Your fork looks great. Thanks for helping @nicolo! If you want to make it a PR, I'm sure we could work with @nicolo on it. : ) For now, I tagged this discussion to make more visible your contribution.

EDIT: Sorry for the spam, I got confused ; )

from simple_token_authentication.

Hello @speed-of-light

If you had any feedback on using the @ProGM code, I would be glad to read it.
Please feel free to share it here if you want to : )

Regards,

from simple_token_authentication.

@gonzalo-bulnes opened :)

from simple_token_authentication.

@ProGM Great!
@nicolo I believe your comments would be welcome in #90 if you have any thoughts about it : )

from simple_token_authentication.

@ProGM This is great. Thanks for doing this.

Are you working on the tests now? I may be able to help on this next week.
I also wrote one small comment on the pull request about header formatting.

from simple_token_authentication.

There are some example unit tests at the very top of the Travis build : )

from simple_token_authentication.

Hi @ProGM, @bchase, @nicolo, @speed-of-light,

The needs testing issue is in good progress (the test suite is ~30% faster at this point), see #104 for updates : )

I just did mention here of a new PR: @wielinde proposed it's own solution in #119. A good thing the implementation has is that it partly takes benefit of the refactoring in progress in #104. I'm not sure however if I like the resulting hash of options, and I believe it may be worth reconsidering it focusing on ease of use and coherence. Since you already have used the feature, I'll be glad to hear you opinion on that.

Regards!

from simple_token_authentication.

Hi there, I do not know what was wrong with me but for some reason I developed this PR without properly searching through existing PRs. Please excuse my ignorance.

Regarding the naming in the configuration I see potential for improvements. Separating header_names from identifier_field sounds good. I can change it if you like.

Regarding testing I have to admit that it still my weak point. I wired up some tests put in terms of elegence there might be some potential for improvements.

from simple_token_authentication.

@RevoHoffman,

Is there a reason that it shouldn't be finding users based on their email and token in a single step, instead of finding the first user that matches the email, and then comparing the tokens?

There is no obligation to identify resources (users, admins, whichever their names are) with their email. But you should not look for the resources using their authentication tokens (see the original gist, and particularly this comment).

I would also avoid using a parameter that's not unique (because it should be indexed IMHO). Since two users never have the same autentication token at the same time, however, that last has to do with performance, not security.

from simple_token_authentication.

@RevoHoffman,

Maybe there is a simpler method to accomplish this?

Forking a you did is the usual way to change the identifier that's used to find resources. With proper care to the security implications described before, the implementation would look fine. : )

from simple_token_authentication.

Simple Token Authentication v1.8.0 is released with support for custom identifiers : )

Thanks again @bchase, @ProGM, @wielinde, @nicolo and @speed-of-light for your contributions and support, it's great help!

from simple_token_authentication.

I had a requirement to use multiple identifiers to authenticate (:email and :application_id - used by an API that supports multiple mobile apps, but the users of the apps don't know they are related so will use the same email address if they have more than one installed, and also allows access via a website scoped by subdomains).

simple_token_authentication did everything I required except this, so I created a fork that supports the Devise authentication_keys and request_keys, in addition to the authentication headers supported by simple_token_authentication. This allows one or multiple identifiers to be used for authentication, as well as request properties (such as subdomain, etc.).

I think this provides better integration with devise so you can define the authentication identifiers in only one place, if for example you are authenticating using database_authenticatable via a website, and simple_token_authentication via an API. Currently to use a different identifier you would have to change the Devise authentication_keys to use with database_authenticatable and the config.identifiers to use with simple_token_authentication. With this fork you only need to use the Devise authentication_keys.

Would you be interested in this as a PR?

from simple_token_authentication.

Hello @trentclowater!

That's great! I'm moving, and may not react very quickly these days, but I think that enhancing the Devise integration is an excellent thing to do, and I would be very interested in your PR.

Best regards!

from simple_token_authentication.

@gonzalo-bulnes, just need to finish some of the tests (most are done, have a few left) and add some text to README.md . Here's what I have:

• If you do not set config.identifiers, authorization_keys, or request_keys, it will use the default value from Devise (which is :email). This results in the same behaviour as currently, it just gets the default directly from Devise instead of from simple_token_authentication.
• If you set config.identifiers and it has a key that matches the model currently being used for authentication, it will use that and operate exactly as it does now. This results in the same behaviour as currently.
• If you set config.identifiers, but there is no key that matches the model currently being used for authentication, it will use authentication_keys and request_keys, or if they also don't exist it will use the default value from Devise (which is :email).

So basically, it will get the value from Devise (thus allowing for multiple authentication keys) unless you explicitly set a value for config.identifiers for the model that is being authenticated.

Since all users who are currently using simple_token_authentication will either have a value for config.identifiers set, or be using :email, they should see no change at all. New users can use authentication_keys and request_keys, allowing them to use multiple authentication keys if they want, and to not have to worry about keeping the Devise and simple_token_authentication configurations synchronized.

Headers are still used exactly the same way (you can use custom header names, header values are given priority over the param values).

I have also included a deprecation warning when using config.identifers, since using authentication_keys and request_keys basically makes its use redundant and provides some benefits (but nobody wants to have their application break for a minor gem update). All of the config.identifiers code should be pretty easy to remove in the future.

from simple_token_authentication.

Hello @trentclowater,

That sounds great!

Note that you can open the PR before it's done, so we can discuss implementation or testing details there. I could also help you with the tests if necessary.

I'm also thinking that a wiki page explaining how to setup a typical case could be great to have. We can probably write it afterwards from the content of you comments.

I look forward to your PR!
Best!

from simple_token_authentication.

Still coming! I have some free time in about a week, so I will try to write the final tests and open the PR then (or just open the PR and ask for some help finishing the tests if my free time runs out).

from simple_token_authentication.

I still don't know how to do it? is there anyone make a conclusion

from simple_token_authentication.

@trentclowater, do you have your code online somewhere? I could not find it in your fork of this repository. I also have the need for multiple identifiers, like devise already supports, but in simple_token_authentication.

@gonzalo-bulnes, also, since a lot of time has passed, is this somehow already possible? I tried different options in config.header_names and config.identifiers but it seems there's still only the possibility for one identifier.

For support of old API clients I can't turn on X-USER-EMAIL and switch to X-USER-LOGIN but have to support both until old API versions are being turned off.

from simple_token_authentication.

Hi @aried3r,

No news on this front, however, the use case makes a lot of sense, and I'm still happy to help with any PR : )

Indeed, if you feel like opening a new issue to support multiple different identifiers, please feel free to do it - it would probably be better than discussing in this closed issue IMHO.

from simple_token_authentication.

My devise authentication keys are an array. How can I configure that ?
@trentclowater

from simple_token_authentication.