search

monterail / rails_sso

Single Sign On solution for Ruby on Rails
MIT License
30 stars 3 forks source link

Extract strategy eval #22

Open jandudulski opened 9 years ago

jandudulski commented 9 years ago

Right now we have to inject some modifications into omniauth strategy class in use (as done here). Hackish.

I'm thinking about creating base omniauth-sso strategy which finally should be used to develop your own strategy.

It would make things simpler, cleaner but require changes (or implementation of new ones) in currently existing strategies.

jandudulski commented 9 years ago

@teamon WDYT?

teamon commented 9 years ago

Why do we need to do this? Shouldn't RailsSSO be able to use any omniauth-xyz?

jandudulski commented 9 years ago

Not any. Only those which implements oauth2 (in most cases that means they inherit from omniauth-oauth2). Actually we're using slightly hacked oauth2 so it seems to me natural to create general oauth2-sso omniauth strategy.

teamon commented 9 years ago

Why hacked oauth?

(But yes it makes sense to stick to oauth for rails sso) On Thu 27 Aug 2015 at 11:11 Jan Dudulski notifications@github.com wrote:

Not any. Only those which implements oauth2 (in most cases that means they inherit from omniauth-oauth2). Actually we're using slightly hacked oauth2 so it seems to me natural to create general oauth2-sso omniauth strategy.

— Reply to this email directly or view it on GitHub https://github.com/monterail/rails_sso/issues/22#issuecomment-135354231.

jandudulski commented 9 years ago

Maybe not hacked, but with a lot of assumptions. The main one is that granting person is the resource owner which is not mandatory true in OAuth2.

The second (and main) thing is that we're doing very specific job around omniauth and question is - should that be explicit (so then go with general omniauth strategy to inherit from) or is implicit ok (so stay as it is right now)?