Implements a seamless login flow that remembers your last sign in scope

[rauhryan]

Closes #168
Closes #172
Closes huboard/huboard#624

[dahlbyk]

Why move?

[rauhryan]

I cleans up the logs for development.

Don't really need the log noise while I'm developing.

Sorry I threw that in there, just felt compeled

[dahlbyk]

Probably a trivial fix that's worth just including here, but https://huboard-rails-pr-291.herokuapp.com/dashboard throws a 500 if you're not authenticated. Should challenge for auth.

[dahlbyk]

Is there anything we can do about this?

Repro:

1. /login/private
2. /login/public
3. Note public-only warning, even though private repo is visible.

[dahlbyk]

Also, thoughts on directing the Private tab to /login/private instead of disabling it?

[dahlbyk]

A few other questions, just thinking through edge cases:

1. What's the value of keeping both /login and /welcome? Should /login redirect to /login/github?
2. Should /welcome redirect to /dashboard if a non-default scope is available?

Really just trying to think through how we could completely shut down access routes to weird states.

[discorick]

Also, thoughts on directing the Private tab to /login/private instead of disabling it?

I played with it originally, but it is a deceiving UX. I.E:

• User is in public only
• User wants to see private too so they click private tab
• GitHub upgrades Oauth to private
• User is redirected to private repo only filter... wonders what happened to their public repos

[dahlbyk]

I played with it originally, but it is a deceiving UX.

👍

[rauhryan]

@dahlbyk new push addresses your edge cases.

I think we are ready to 🚢

[discorick]

Just one minor UX nitpick if you think its worth it, we could probably move everything closer to the top of the browser on welcome:

[rauhryan]

Just one minor UX nitpick if you think its worth it, we could probably move everything closer to the top of the browser on welcome:

Nitpick noted.

I'm going to leave it for now, I'm good to ship the way it is

Anything else before we 🚢?

[discorick]

It may be worth implementing @dahlbyk 's suggestion that if there is a private or publics scope already, /welcome should redirect to /dashboard

This would effectively block users from reaching bad states from scope downgrades.

Otherwise I think it's 🚢

[dahlbyk]

I can still reproduce #291 (comment) if I have authorized private and hit /login/public (via /login).

Also noticed that if the app is revoked, /dashboard redirects to /login.

[rauhryan]

Also noticed that if the app is revoked, /dashboard redirects to /login.

This is because Ghee::Unauthorized will log you out and redirect to /login, I can easily change this behavior... ref: https://github.com/huboard/huboard-web/blob/master/app/controllers/application_controller.rb#L13-L19

I can still reproduce #291 (comment) if I have authorized private and hit /login/public (via /login).

We can still ship, knowing that "could" happen.

Currently there is only one scenario where the application redirects or links to /login

I'm not ready to completely kill /login, I'd like to keep it around in case we need to bring it back.

I think the question is:

1. What's the risk or do we care if users "url hack" the /login route?
2. Are we ok to ship, knowing there are edge cases that can get you into a weird state?

If someone gets into a funky state, the solution is...

/logout => /login/github

The user will be taken to either /welcome or /dashboard where there are links to upgrade access that will fix it.

Without adding datastore or switch auth frameworks, I don't think there is a perfect solution.

My vote is:

Make a decision on how we handle Ghee::Unauthorized and ship it

[discorick]

Make a decision on how we handle Ghee::Unauthorized

Just to send users to /login/github ?

[dahlbyk]

I'm not ready to completely kill /login, I'd like to keep it around in case we need to bring it back.

That's why we have Git. 😉

But seriously, life gets simpler if we make /login redirect to /login/github. We can easily remove the redirect if /login needs to come back.

With that plus a /welcome to /dashboard redirect I think we are more than sufficiently buttoned up. I just don't want to have to revisit this again for a long time.

[rauhryan]

OK! I've added the requested changes.

Let's 🚢 this bad boy!

[dahlbyk]

👏 ✨ 👏 Excellent work!