search

gbv / login-server

Login and connect accounts with multiple identity providers
https://coli-conc.gbv.de/login/
MIT License
32 stars 32 forks source link

Show where a user came from #61

Closed nichtich closed 3 years ago

nichtich commented 4 years ago

Add a parameter return to /login and /account so a message "Return to ${url}" is shown below the "Login Server" title. The value of url can be taken from document.referrer.

Use case: applications don't have to provide their own user interface for login, just point to /account?return

Implementation: a lot of redirect can happen in between so set a session cookie with the value of document.referrer if the query contains return. This can fully be implemented with client-side JavaScript.

We might even always show "return to ..." based on document.referrer, so no special URL parameter is needed. In the end it's just like the browser's back button but going back before login server, skipping the whole session.

stefandesu commented 4 years ago

Yeah, this makes sense and shouldn't be hard to implement. It doesn't make much sense to use it in Cocoda, but definitely in other applications (I assume BARTOC).

I wouldn't enable the redirect by default though. Instead, we could have either ?return=https://... or ?return=1 (which then uses the value in document.referrer like you suggested. What do you think?

stefandesu commented 4 years ago

(By the way, I would do this all on the server. I don't see a reason to involve client-side JavaScript when it isn't necessary.)

nichtich commented 4 years ago

I wouldn't enable the redirect by default though. Instead, we could have either ?return=https://... or ?return=1 (which then uses the value in document.referrer like you suggested. What do you think?

yes except "return=back"

stefandesu commented 4 years ago

I did a little bit of research and OAuth uses the parameter name redirect_uri, but then a value like back wouldn't make sense. I'm also fine with return, I was just wondering whether there's a standard name for it.

Also, we could consider to add a JWT to the return URL so that the application can immediately start using it.

nichtich commented 4 years ago

Using the referrer is not needed.If it works with redirect_uri then take this name. The JWT is not needed, the application will need to establish a connection with login-server anyway. This issue is only about showing a hint in the user interface where a user came from, it is not about making login-server a fill OAuth identity provider by itself.

stefandesu commented 3 years ago

I started trying to implement this and it's not as trivial as I had expected. The problem is that we have two ways to deal with this:

  1. Do it server side and use a session cookie (or I'd use connect-flash since it also uses the session and we are already using it anyway).
    • Advantages: No need for client side JavaScript. The client basically doesn't have to do anything except for redirecting after successful login.
    • Disadvantages: The session persists across browser tabs. This could be an issue if it is first opened with a redirect_uri, but then instead of logging in, you close the tab. Then later, you navigate to Login Server via some other way without redirect_uri and the previous one will still be stored. This would be very confusing if it happened to someone.
  2. Do it client side in JavaScript using sessionStorage.
    • Advantages: sessionStorage is limited to a specific tab, so the described edge case can't happen.
    • Disadvantages: JavaScript needs to be enabled.

I don't think there's a solution which does not have one of the two disadvantages. But since basically every application that could ever be used with Login Server requires JavaScript, the second option might be the better one. Alternatively, we could use the first option, but limit the lifetime of the cookie to something short like 5 minutes. What do you think, @nichtich?

nichtich commented 3 years ago

Better use JavaScript.

stefandesu commented 3 years ago

There's now a first implementation in dev. It should cover everything that's needed here. Since we are using client-side JavaScript to parse the parameter, I changed /login/:provider to show an intermediary page that redirects to /login/:provider/auth (which in turn deals with the authentication). This also means that every time someone logs in with one of the providers, the message about their data is shown (if only for two seconds before the redirect).

Could you take a look at the commit, @nichtich? Since there's no dev instance of login server, we can't test it before merging, but it works locally even with GitHub authentication.

stefandesu commented 3 years ago

Here's a short video of what it looks like:

https://user-images.githubusercontent.com/3275148/104184049-26fa9400-5413-11eb-855e-0669374bfe11.mov