add support for multiple omniauthable models

[ivandenysov]

AFAIK devise doesn't support omniauthable for multiple models because long time ago omniauth did not support custom path_prefix for each strategy. But according to this comment from @josevalim omniauth solved that issue at least 9 years ago.

I implemented a sample rails application that shows how omniauthable can be used for multiple devise models.
main commit of that app: ivandenysov/showcase-multiple-omniauthable-models@7b1f237
installation instructions: https://github.com/john-denisov/showcase-multiple-omniauthable-models/blob/master/README.md
I followed this guide to implement omniauth sign in for users and managers in my demo app

I will also update https://github.com/plataformatec/devise/wiki/OmniAuth-with-multiple-models if this PR gets approved

Will appreciate any ideas how to improve this feature. Theoretically we can automatically duplicate all registered providers and set correct path_prefix for them if devise :omniauthable is specified in more than 1 model. That will reduce amount of configuration required but it won't allow to have separate sets of providers for different models 🤔Or we could also add new arg to Devise.omniauth called scopes:. Default would be to add this strategy to all omniauthable scopes.

[sourcelevel-bot]

Hello, @john-denisov! This is your first Pull Request that will be reviewed by SourceLevel, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information.

[sourcelevel-bot]

Prefer single-quoted strings when you don't need string interpolation or special symbols.

Read more about it here.

[sourcelevel-bot]

SourceLevel has finished reviewing this Pull Request and has found:

• 4 possible new issues (including those that may have been commented here).
• 2 fixed issues! 🎉

See more details about this review.

[tommyalvarez]

+1 for this one @ivan-denysov . Can I help in anyway to move this forward?

[fractaledmind]

@ivan-denysov: This current setup doesn't quite cover the possible situations thoroughly.

Firstly, you might have more providers defined on the model than in the configuration (if you use the :developer provider in a test env, for example). If that is the case, then mapping.to.omniauth_providers will have an entry not present in Devise.omniauth_configs.keys and the code will throw an error since Devise.omniauth_configs[provider] will be Nil.

Secondly, you might have an additional provider defined in addition to the provider used on multiple models. In that case, you will likely have a configuration without a path_prefix set and your code will return false forcing the setter and thus throwing the error we are looking to avoid.

Thirdly, you might have multiple providers used by multiple models.

Given these possibilities, I think the better articulation of the desired logic is:

For every provider that has multiple configurations set, ensure that each configuration for each provider supplies a path_prefix; however, for any provider with only one configuration, set the path_prefix.

Here is my attempt to capture this logic:

single_provider_with_multiple_configs = Devise.omniauth_configs.group_by { |_,config| config.provider }.select { |_,configs| configs.count > 1 }
all_configs_for_each_provider_with_multiple_configs_has_path_prefix_set = single_provider_with_multiple_configs.all? { |_,configs| configs.all? { |_, config| config.options[:path_prefix].present? } }

if single_provider_with_multiple_configs.empty? || !all_configs_for_each_provider_with_multiple_configs_has_path_prefix_set
  set_omniauth_path_prefix!(path_prefix)
end