symfonycorp / connect

The SymfonyConnect official API SDK
https://connect.symfony.com/
MIT License
91 stars 40 forks source link

The SensioLabs Connect connection should redirect to the current page #43

Closed stof closed 2 years ago

stof commented 10 years ago

This issue is related to the connect website and its integration in other Symfony/SensioLabs website, but there is no public issue tracker for it AFAIK, so I'm opening it on the SDK repo.

It is very annoying that each time you connect with SensioLabsConnect, you get redirected to the homepage of the website instead of staying on the current page. The redirection should go to the page where we were before the connection.

lyrixx commented 10 years ago

Fair enough. I already implement this feature on insight. I have edited your issue to add some todo.

ping @javiereguiluz @fabpot

javiereguiluz commented 10 years ago

@stof I didn't forget about this issue ;) I opened an issue in the internal SensioLabs Connect repo, but I guess that tracking the solution of this issue in public is better :)

Thanks @lyrixx for working on this!

lyrixx commented 10 years ago

@javiereguiluz can you do it on symfony.com ?

javiereguiluz commented 10 years ago

@lyrixx sure!

lyrixx commented 10 years ago

@stof I ticked more checkbox. live and symfony should be ok right now.

@sebastiensensio can you fix this issue on sl.com ? (see PR on symfony.com) Thanks

@saro0h / @hhamon can you fix this issue on training? thanks

buzzy1982 commented 10 years ago

@lyrixx done

lyrixx commented 10 years ago

@sebastiensensio thanks.

stof commented 10 years ago

another affected website: the profiler /cc @nicolas-grekas It is really annoying, as this then affect the companion, asking as to authenticate again each time we open our browser.

I think this would deserve being added in a check-list for the implementation of connect auth in SensioLabs products, to avoid forgetting it for each new website.

romainneutron commented 10 years ago

as this then affect the companion, asking as to authenticate again each time we open our browser.

@stof What do you mean? Do you have a companion issue poping when opening your browser (related to authentication)?

stof commented 10 years ago

@romainneutron I have the companion telling me all the time I need to authenticate before being able to profile a request, which is annoying. If profiler.sensiolabs.com was considered me as authenticated when I'm already authenticated on connect, it would ask it far more often

romainneutron commented 10 years ago

I never experienced it, but it's definitely something we need to fix. Thanks for the feedback.

stof commented 10 years ago

@lyrixx this issue seems to happen again on Insight

lyrixx commented 10 years ago

@stof I'm unable to reproduce it.

  1. I logout
  2. I go to https://insight.sensiolabs.com/pricing
  3. I click on connect
  4. I'm well redirected to https://insight.sensiolabs.com/pricing

Do you have a scenario to reproduce it?

stof commented 10 years ago

ah sorry, I confused this ticket with another issue. The issue happening again is not this one, but the fact that you need to click on Connect explicitly even if you are already logged on Connect (and this other issue was not tracked here on github but on support.sensiolabs.com apparently).

lyrixx commented 10 years ago

I see what support ticket you are referring. But I'm not sure to understand what you want.

But I don't understand it. Right now, If you log out, and try to access a private area (A), insight will start the authentication, and so, will redirect you to connect. Connect (if you are connected) will redirect you back to insight. Insight will log you in, then insight will redirect you the private area (A). And IIRC, you don't like this behavior? Did I miss something?

stof commented 10 years ago

@lyrixx If you access the Insight homepage, the content is different when you are logged (dashboard) or no (page showing you the features to ask you to start using Insight). But if you are connected on Connect, Insight does not display the dashboard unless you have an active Insight session (and Insight does not remember me when I close my browser, while Connect does)

lyrixx commented 10 years ago

Yes, it's the expected behavior. / cc @nicolas-grekas

nicolas-grekas commented 10 years ago

This is fixed on Blackfire (to be deployed on Monday at last).

"the need to click on Connect explicitly even if you are already logged on Connect" is a requirement, not a bug. The contrary would mean being forced to be authenticated. And equivalent IRL would be to be forced to walk in the street with your ID card on your brow. Not a usual pratice nowdays (these kind of social standards change over time, but I prefer being on the "freedom" side for now ;) )

stof commented 10 years ago

@nicolas-grekas it should not force to be authenticated to access it. But it should check whether Connect is authenticated, instead of having to let the user ask explicitly to check whether Connect is connected.

nicolas-grekas commented 10 years ago

This is what I mean by "force": you can't navigate anonymously on Insight (or whatever app) without loging out from Connect. Travis has the same behavior with respect to github login, and that's what we expect also here.

lyrixx commented 10 years ago

@stof I disagree with you. I think it's a bad oAuth practice. For instance, If I go on travis (https://travis-ci.org/) (I'm obviously connected on github right now) I'm not connected on travis. So we (sensiolabs) prefer to keep the current workflow like it work right now

lyrixx commented 10 years ago

huhu, race condition spotted with @nicolas-grekas comments ;) I should talk with him next time ;)

stof commented 10 years ago

Well, in this case, please make the session timeout less often on Insight or Blackfire. It is annoying to have to connect again and again (even though Connect will just redirect back immediately), especially for the Blackfire Companion, which requires opening a new tab to connect on Blackfire (or at least make the Companion force the authentication when opening it in case Connect is authenticated, as it cannot do anything anonymously)

lyrixx commented 10 years ago

Well, in this case, please make the session timeout less often on Insight

IIRC, the session timeout after 12H on insight. I could increase it.

nicolas-grekas commented 10 years ago

Oh, btw, for the companion, the latest release (0.3.3) removes one click: a click inside the companion is enough to create the Blackfire session with Connect.

lyrixx commented 2 years ago

I think this issue can be closed?

javiereguiluz commented 2 years ago

Yes, let's close it. Thanks!