appsmithorg / appsmith

Platform to build admin panels, internal tools, and dashboards. Integrates with 25+ databases and any API.
https://www.appsmith.com
Apache License 2.0
34.63k stars 3.74k forks source link

[Bug SSO]: Logging out of Appsmith does not log the user out of the IdP #14759

Open trishaanand opened 2 years ago

trishaanand commented 2 years ago

Is there an existing issue for this?

Description

When the user logs out of Appsmith, the instance configuration should support adding a flag which logs the user out of IAM as well.

Steps To Reproduce

  1. Login using SSO
  2. Logout from Appsmith
  3. When logging in again, you will observe that you are still logged into the identity provider and would get redirected to Appsmith with successful login. The expectation is that if configured, you should need to put in your credentials on the identity provider again before logging into Appsmith.

Public Sample App

No response

Version

Prod

Front logo Front conversations

pranavkanade commented 1 year ago

@sharat87, can we please prioritize this issue? An EE user has been waiting for a month.

infinitetrooper commented 1 year ago

@pranavkanade - This for me is intended behavior. We should not be logging users out of the IDP if they log out of Appsmith. We are a service provider and users might be using IDP with other service providers, we should take that as the primary behavior. This is also the common implementation across similar tools in the industry.

I've replied the same in the A-Force channel conversation as well last month. We can provide this as a secondary configurable behavior but, we need more feedback/requests for us to do that.

sharat87 commented 1 year ago

@infinitetrooper, I'd disagree with that stance strongly. The whole problem we're solving here is SSO, single-sign-on. Basically meaning that you sign-in with a username/password or other forms of authentication in one place, and they don't have to it on every service they use. This is the point and purpose of SSO in an organization. But the way this manifests is that the user attempts to login to an application, which then asks the IdP, and then the user logs into their IdP. But once this happens, they're practically logged-in to all their internal services that use this SSO. This is where and how the problem is solved.

Now this translates to, I, as a user, logged in to Appsmith, and that logged me in to all these other services too. So, naturally, if I logout of Appsmith, I'll be logged out of all the other services too, right? That's the assumption a user would have, and that's what I'd have.

Both SAML and OIDC schemes for SSO have specific provisions to do single-logout, to address this exact problem. In fact, in some sense, this isn't even a feature request, it's a bug that our SAML and OIDC implementations are incomplete. We don't support logout.

However, all that said, I do agree with you, if we're talking about OAuth login methods, like Google or GitHub. There, if I logout of Appsmith, I don't expect to be logged out of Google, and OAuth is okay with that. That's how it works. It's not branded as an SSO solution, and it's not one.

But for SSO, logout has to be single too.

infinitetrooper commented 1 year ago

Understood @sharat87. I see two areas we need to improve.

  1. Logout users from Appsmith if they log out from the IdP. I think this is a requirement(bug) as folks using SSO would assume all service providers will be logged out if they log out from their identity provider.
  2. Logout the user from IdP if they log out from Appsmith. I understand your arguments around it and I think enterprise users(especially users who use Microsoft suite) are used to this flow. What are your thoughts around supporting this through a variable optionally as I believe there will be orgs that expect OAuth-type implementation and don't want this automatic logout flow?
sharat87 commented 1 year ago

Thanks. Very good questions.

  1. This, yes, but let's do this along with IdP initiated login. So that, we don't have to say things like "IdP initiated logout works, but IdP initiated login doesnt". So just, let's do them togather. 🙂
  2. I'd say no. I'll defer to your team's judgement on this, but my preference it to just logout, and not make it configurable. If someone wants an OAuth-like experience, they're using a wrong feature. OIDC and SAML aren't built to be used that way. If this request does come, my suggestion would be to add support for configuring arbitrary OAuth providers, in addition to the currently supported Google and GitHub. But that's a whole separate feature/project. Again, more of an opinion, feel free to take a call. As long as the default behaviour is to do the IdP-logout, we're super.
snadorp commented 1 year ago

Just the confirm @sharat87 summary here, we ran into exact this scenario and our QA team flagged the behavior as non compliant. We're using auth0 for authentication, and each app is expected to be able to do a global log out of the user. So it would be great if this issue can be tackled.

sharat87 commented 1 year ago

Just noticed that this is assigned to me. I'm switching that to @infinitetrooper and @trishaanand, so it can be prioritized and picked up in the team. 🙏

Jarrah-libremfg commented 7 months ago

Currently facing this issue when integrating with Keycloak as IdP. The QA team are quite confused when they log out on shared machines only to have the next user come along, not be asked to provide a username/password and take over the old session.

Thanks,