Instagram added to providers (Oauth2) #622
Conversation
- registered as external submodule - implemenation in providers directory added mapping to create_user functionality
|
if I see you get back to me, I'll go ahead and write the generators/railties/what have you for the config initializer and move the comments over there |
|
whoops, substitute ^see you get back to me^see the CI server working when I look back at it |
|
Why would there be any issue with CI here? You added no tests |
|
Ha ha, great point. Definitely (really) was more wondering whether this was a feature you were interested in e.g. what I'd originally said. So I'll go ahead and assume that it passes that test, and go ahead and change the actual tests. Thanks, good note |
|
Ah, ok, I didn't see it as a question. Sure, we're open for new external integrations! |
- in internal app, named routes/methods for instagram - stubbed/tested changes in controller_oauth_2_spec - (note some reformatting changes, not logic changed) - in instagram provider, change logic for testing strategy
Conflicts: spec/active_record/controller_oauth_spec.rb spec/rails_app/app/controllers/sorcery_controller.rb spec/rails_app/config/routes.rb Conflicts: spec/rails_app/app/controllers/sorcery_controller.rb
Currently we store in session `user.id` which in some cases is integer, but in some cases (like Mongoid) is an object. What's the problem with storing object? First thing, it might break compatibility between different ORM versions (Mongoid changed class of this object from v2 to v3). Second, the bigger issue, is that new serializer in Rails uses JSON instead of YAML, so it can store only selected types, and it parses custom objects incorrectly, which may break the authentication feature. This commit changes the way we store user id to plain string. This will make it being stored correctly no matter what serializer is used.
|
oh hm I didn't think about that merge before I started these changes so how about this: @arnvald I'll make a new branch and open new PR there, check the tests work, and then this would be something you'd merge to main? |
|
I'm closing this one in favour of #625 |
added mapping to create_user functionality