Omniauth developer strategy #41

KitaitiMakoto · 2012-11-08T06:00:44Z

@kentaro san said "I want you to do with omniauth developer mode." to me at issue #29.
I'v done it at RubyWorld Conference 2012.

Usage:

Create a user anyway.
Click "Dev sign in" at /caveat.
Enter the user's name and uid, then push "sign in" button.

Required Considerations

POST request to /auth/developer/callback is accepted. Is it OK?
No test. Is it needed even though developer strategy is activated only when RAILS_ENV=development?

Thanks!

…eters

kentaro · 2012-11-08T08:49:32Z

spec/support/signin.rb

@@ -1,5 +1,5 @@
 def auth_params_for (user)
-  {
+  Hashie::Mash.new({


Gemfileに依存かかかなくてもだいじょうぶですか?

omniauthの依存で入ってくるんですねー

そうなんです。
実装の方でも使っていて、OmuniAuthが使うのをやめた時はこけてほしいので書かないままがいいと思います。
便利なので他で使うなら勿論入れちゃっていいと思いますが。

了解ですー

何かのライブラリで依存しているものを暗黙的に利用すると見通しが悪くなる気がするのですが、そうでもないのですかね？

そうかもしれぬ

ここ、auth_params_for(:foo).uid みたいにすることが無いなら Hash でも良いような。

Hasie::Mashを使いたいという強い意図を感じたのでそのままにしてありますｗ

あ、俺か。遅れてすんません。

Hashie::Mashを使いたいという強い意図のみなもとはここです。
https://github.com/kentaro/triglav/pull/41/files#L0R14

developer strategyではauth_paramsにupdate_image(auth_params['extra']['raw_info']['avatar_url'])という物が入ってこないので、

update_image(auth_params['extra'] && auth_params['extra']['raw_info'] && auth_params['extra']['raw_info']['avatar_url'])

といったことをしなくちゃいけなくてだるいなあと思ってドットでつなげてtryとかpresent?とかを使えるようにしたかったです。
（その他の場所でドットを使うようにしたのはほとんどおまけです）

で、

何かのライブラリで依存しているものを暗黙的に利用すると見通しが悪くなる気がするのですが、

なんですが、OmuniAuthが渡してくれるrequest.env['omniauth.auth']（auth_paramsはこれを返すだけのメソッド）が
そもそもHashie::Mashなのでテストデータの方でそれに揃えるのは妥当か、（だから↑のように楽しても許されるよね）と思って使いました。

ただまあ、こんな長い説明を要するのを見るに、すっきりしたストラテジーを自作するべきかも知れません……。

kentaro · 2012-11-08T08:56:27Z

POST request to /auth/developer/callback is accepted. Is it OK?

Seems OK. Go ahead.

No test. Is it needed even though developer strategy is activated only when RAILS_ENV=development?

Do you have any idea how you can test that?

hsbt · 2012-11-08T12:27:09Z

No test. Is it needed even though developer strategy is activated only when RAILS_ENV=development?

Do you have any idea how you can test that?

To use Rails.stub(:env) { "development" } when running test.

KitaitiMakoto · 2012-11-08T12:40:07Z

Naruhodo! Thank you.
The rest of my worry is about database. I'll research and try to resolve it if problems there.

kentaro · 2012-11-08T14:55:25Z

Rails.stub(:env) { "development" }

I never came up with such an idea!

"Sign in as a developer" sounds like "Sign in as a developer who has additional privilegeds."

KitaitiMakoto · 2012-11-10T05:52:10Z

@kentaro san
I think this branch is ready to be merged.
Could you review it?

hsbt · 2012-11-10T06:18:01Z

app/contexts/session_context.rb

-  def update_privilege
-    if Settings.github.try(:organizations).present?
+  def update_privilege(provider)
+    if provider == 'github' && Settings.github.try(:organizations).present?


To use Settings.github.organizations.presence instead of `Settings.github.try(:organizations).present?``

確かにtryは全然いりませんね。
if節の中では述語のpresent?の方がしっくりくるのですがどうでしょう。

kentaro · 2012-11-11T09:18:13Z

@KitaitiMakoto
I'll merge this pull request after you do with the points @hsbt said

kentaro · 2012-11-12T05:33:05Z

Merge origin/master to this branch, plz.

hsbt · 2012-11-12T14:36:21Z

コメントが消えていた! try(:attr).present? でも許容出来る範囲なのですが、try で存在するかどうかをチェックしてなおかつ present? でチェックというのは何かむずむずするなあと思った次第です。

KitaitiMakoto · 2012-11-12T14:48:16Z

コメントが消えていた!

ソースから該当行が無くなったので「hsbt discussed an outdated diff 2 days ago Show outdated diff」というところに隠れてしまいました。
http://gyazo.com/6560535af460f3231856d27ff9147ce0
（何日か前の @glidenote さんと @kyanny さんのやり取りで知りました）

何かむずむずするなあ

指摘されて僕もそう思いました。
24c0dcf
↑でtryだけ外しました。

Omniauth developer strategy

kentaro · 2012-11-12T16:05:42Z

Thanks!!1 Great job!!1

KitaitiMakoto added 6 commits November 7, 2012 02:44

[REFACTORING]Use dot accessor instead of hash accessor for auth param…

0530299

…eters

WIP: add developer strategy of OmniAuth

8ab13f0

Change fields for developer strategy: name -> nickname, add uid

8e07f04

Add links to sign in at development environment

4c94345

[BUG FIX]:github -> "github"

817c9ad

Change spec after implementation

1c29353

kentaro reviewed Nov 8, 2012
View reviewed changes

Dev sign in -> Sign in as a developer

e796c69

KitaitiMakoto added 4 commits November 9, 2012 00:13

Add spec for developer strategy of OmniAuth

7e32b1a

Sign in as a developer -> Sign in with developer strategy

ec47eef

"Sign in as a developer" sounds like "Sign in as a developer who has additional privilegeds."

Fix spec following change of implementation

fdcf33a

Test all about Signin/Signout for development environment

eda72ec

hsbt reviewed Nov 10, 2012
View reviewed changes

Gather routes for dev environment at the same place

9997bd1

KitaitiMakoto added 3 commits November 12, 2012 23:01

Manage OAuth providers in config file

2029b01

Remove unneccessary #try

24c0dcf

Resolve conflict

4649b3e

kentaro added a commit that referenced this pull request Nov 12, 2012

Merge pull request #41 from KitaitiMakoto/omniauth-developer

36f6527

Omniauth developer strategy

kentaro merged commit 36f6527 into kentaro:master Nov 12, 2012

kentaro mentioned this pull request Nov 14, 2012

Developer sign in mode doesn't work? #53

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Omniauth developer strategy #41

Omniauth developer strategy #41

KitaitiMakoto commented Nov 8, 2012

kentaro Nov 8, 2012

kentaro Nov 8, 2012

KitaitiMakoto Nov 8, 2012

kentaro Nov 8, 2012

banyan Nov 26, 2012

kentaro Nov 26, 2012

hsbt Nov 26, 2012

kentaro Nov 26, 2012

KitaitiMakoto Nov 26, 2012

kentaro commented Nov 8, 2012

hsbt commented Nov 8, 2012

KitaitiMakoto commented Nov 8, 2012

kentaro commented Nov 8, 2012

KitaitiMakoto commented Nov 10, 2012

hsbt Nov 10, 2012

KitaitiMakoto Nov 12, 2012

kentaro commented Nov 11, 2012

kentaro commented Nov 12, 2012

hsbt commented Nov 12, 2012

KitaitiMakoto commented Nov 12, 2012

kentaro commented Nov 12, 2012

Omniauth developer strategy #41

Omniauth developer strategy #41

Conversation

KitaitiMakoto commented Nov 8, 2012

Usage:

Required Considerations

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kentaro commented Nov 8, 2012

hsbt commented Nov 8, 2012

KitaitiMakoto commented Nov 8, 2012

kentaro commented Nov 8, 2012

KitaitiMakoto commented Nov 10, 2012

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kentaro commented Nov 11, 2012

kentaro commented Nov 12, 2012

hsbt commented Nov 12, 2012

KitaitiMakoto commented Nov 12, 2012

kentaro commented Nov 12, 2012