Charcoal-SE / metasmoke

Web dashboard for SmokeDetector.
https://metasmoke.erwaysoftware.com
Creative Commons Zero v1.0 Universal
43 stars 34 forks source link

Redirected to wrong page after successful login with Stack Exchange #247

Closed SulphurDioxide closed 7 years ago

SulphurDioxide commented 7 years ago

When you login with Stack Exchange you get redirected to the following URL: https://github.com/Charcoal-SE/metasmoke/wiki/API-Documentation#log-in

This doesn't happen when you login with a MS account.

angussidney commented 7 years ago

I have a feeling this may be some incorrect configuration with our app over on SE's side. @Undo1 could you check to make sure that the app URL isn't set to the API documentation?

ArtOfCode- commented 7 years ago

That would make sense, actually, given how much searching the code hasn't revealed anything wrong.

Undo1 commented 7 years ago

Nothing on the Stack Apps page pointing to docs; the redirect URI is defined on the metasmoke side.

https://stackexchange.com/oauth?client_id=6315&redirect_uri=https%3a%2f%2fmetasmoke.erwaysoftware.com%2fauthentication%2flogin_redirect_target&scope=&response_type=&state=&returnurl=%2foauth%3fclient_id%3d6315%26redirect_uri%3dhttps%253a%252f%252fmetasmoke.erwaysoftware.com%252fauthentication%252flogin_redirect_target%26scope%3d%26response_type%3d%26state%3d

Our redirect target is /authentication/login_redirect_target - that's where we'd look. My bet is on some Devise redirect trickery.

ArtOfCode- commented 7 years ago

I can only find the docs URL in one place in the code, and I can't find any links between Devise redirections - including sign_in_and_redirect - and that code. Me = stumped.

Undo1 commented 7 years ago

Yeah, it's a bizarre issue. Might end up just force redirecting to homepage.

Undo1 commented 7 years ago

Found it. Apparently root_path in https://github.com/Charcoal-SE/metasmoke/blob/b6b4a6cdf7c9ab47733bc700b2ed1ca2227dfba6/app/controllers/application_controller.rb#L34 only counts route root entries that don't have the as: parameter. The API docs one was the only root entry to match that, so that's where we got redirected.

ArtOfCode- commented 7 years ago

WHAT.

Undo1 commented 7 years ago

I know, right? Debating calling it a bug in Rails - root should be /, always... unless I'm missing something

ArtOfCode- commented 7 years ago

Either it's a bug or it's just a dumb idea... root is root, not scope-root.