Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Logout with OIDC not working #8369

Open
BramMeerten opened this issue Jul 5, 2023 · 29 comments
Open

Logout with OIDC not working #8369

BramMeerten opened this issue Jul 5, 2023 · 29 comments
Assignees
Labels
bug Bug report product PR or Issue related to the DataHub UI/UX stale

Comments

@BramMeerten
Copy link

Describe the bug
I configured OIDC following the datahub guide and I disabled JAAS. I used keycloak as an identity provider.
Login works fine. When I try to logout I'm redirected to https://<my-datahub>/login*, but I'm nog logged out with my identity provider. So when I go back to datahub I'm automatically logged in again and can't switch accounts.

*This is also weird because I get a username/password form (+ sso button), but I have disabled JAAS and can't login with username/password.

To Reproduce
Steps to reproduce the behavior:

  1. Configure OIDC
  2. Click on logout
  3. Go back to datahub homepage
  4. You are no longer logged in

Expected behavior
I'm also logged out with my identity provider so I can switch accounts.

Desktop

  • OS: Mac
  • Browser: Brave (chromium)
  • Version: Brave 1.52.129 Chromium: 114.0.5735.198 (Official Build)

Additional context
I suspect this pull request (play framework upgrade) has introduced the problem:
https://github.com/datahub-project/datahub/pull/6626/files#diff-7cbee1cea8c44e4aa618564185bfcffbb23b1dd42e83c2c8bb3f381cc9b77cf5

The CentralLogoutController calls the setCentralLogout(true) method of its parent (LogoutController). This should make sure your also logged out with your identity provider.
But on line 39 the logout method of LogoutController is no longer called. It seems it just clears the session and redirects to /login

public class CentralLogoutController extends LogoutController {
    // ...
    @Inject
    public CentralLogoutController(Config config) {
        _isOidcEnabled = config.hasPath("auth.oidc.enabled") && config.getBoolean("auth.oidc.enabled");
        setDefaultUrl(DEFAULT_BASE_URL_PATH);
        setLogoutUrlPattern(DEFAULT_BASE_URL_PATH + ".*");
        setLocalLogout(true);
        setCentralLogout(true);
    }

    // ...

-  public Result executeLogout() throws ExecutionException, InterruptedException {
+  public Result executeLogout(Http.Request request) {
        if (_isOidcEnabled) {
            try {
-              return logout().toCompletableFuture().get().withNewSession();
+              return Results.redirect(DEFAULT_BASE_URL_PATH)
+                    .removingFromSession(request);
// ...
@BramMeerten BramMeerten added the bug Bug report label Jul 5, 2023
@BramMeerten
Copy link
Author

There is at least one other user experiencing this issue according to the slack thread:

I have same issue. The session does not end with logout. It is still logged in.

@RyanHolstien RyanHolstien added the product PR or Issue related to the DataHub UI/UX label Jul 11, 2023
@github-actions
Copy link

This issue is stale because it has been open for 30 days with no activity. If you believe this is still an issue on the latest DataHub release please leave a comment with the version that you tested it with. If this is a question/discussion please head to https://slack.datahubproject.io. For feature requests please use https://feature-requests.datahubproject.io

@github-actions github-actions bot added the stale label Aug 11, 2023
@BramMeerten
Copy link
Author

This is still an issue, and also confirmed by other people in the the slack thread.

@BramMeerten
Copy link
Author

Tested on 0.10.4

@BramMeerten
Copy link
Author

BramMeerten commented Sep 7, 2023

The pull request did not fix the issue for me, has anyone tried it?

By debugging I can see that it's in the right direction: The central logout logic is triggered (in DefaultLogoutLogic::perform) but it doesn't do anything if no UserProfile is found in the session or request.

For some reason the UserProfile can't be found in the request/session. We can see that it gets set, but in the following requests it's no longer set.

Maybe the this issue is related, but I enabled the PlayCacheSessionStore and it still didn't work.

@FirKys this PR worked for you? Do you have some special settings regarding sessions/cookies/...?

@github-actions
Copy link

github-actions bot commented Oct 8, 2023

This issue is stale because it has been open for 30 days with no activity. If you believe this is still an issue on the latest DataHub release please leave a comment with the version that you tested it with. If this is a question/discussion please head to https://slack.datahubproject.io. For feature requests please use https://feature-requests.datahubproject.io

@github-actions github-actions bot added the stale label Oct 8, 2023
@BramMeerten
Copy link
Author

Still an issue, tested on 0.10.4

@github-actions github-actions bot removed the stale label Oct 10, 2023
Copy link

github-actions bot commented Nov 9, 2023

This issue is stale because it has been open for 30 days with no activity. If you believe this is still an issue on the latest DataHub release please leave a comment with the version that you tested it with. If this is a question/discussion please head to https://slack.datahubproject.io. For feature requests please use https://feature-requests.datahubproject.io

@github-actions github-actions bot added the stale label Nov 9, 2023
@BramMeerten
Copy link
Author

Still no response, still an issue, please keep open

@rubensancor
Copy link

I also have this issue in my deployment

@github-actions github-actions bot removed the stale label Nov 10, 2023
Copy link

This issue is stale because it has been open for 30 days with no activity. If you believe this is still an issue on the latest DataHub release please leave a comment with the version that you tested it with. If this is a question/discussion please head to https://slack.datahubproject.io. For feature requests please use https://feature-requests.datahubproject.io

@github-actions github-actions bot added the stale label Dec 10, 2023
@BramMeerten
Copy link
Author

still an issue, please keep open

@github-actions github-actions bot removed the stale label Dec 12, 2023
Copy link

This issue is stale because it has been open for 30 days with no activity. If you believe this is still an issue on the latest DataHub release please leave a comment with the version that you tested it with. If this is a question/discussion please head to https://slack.datahubproject.io. For feature requests please use https://feature-requests.datahubproject.io

@github-actions github-actions bot added the stale label Jan 11, 2024
@kzgrzendek
Copy link

kzgrzendek commented Jan 15, 2024

Still an issue ; I'm still logged in even after logging out from the UI - tested on 0.12.1

@github-actions github-actions bot removed the stale label Jan 16, 2024
@bhaveshvasandani
Copy link

I am seeing this issue on a datahub deployed on k8s with version 0.12.1

Copy link

github-actions bot commented Mar 2, 2024

This issue is stale because it has been open for 30 days with no activity. If you believe this is still an issue on the latest DataHub release please leave a comment with the version that you tested it with. If this is a question/discussion please head to https://slack.datahubproject.io. For feature requests please use https://feature-requests.datahubproject.io

@github-actions github-actions bot added the stale label Mar 2, 2024
@kzgrzendek
Copy link

Still an issue ; if you want any specific details about the setup/scenario don't hesitate to reach :)

@github-actions github-actions bot removed the stale label Mar 6, 2024
Copy link

github-actions bot commented Apr 5, 2024

This issue is stale because it has been open for 30 days with no activity. If you believe this is still an issue on the latest DataHub release please leave a comment with the version that you tested it with. If this is a question/discussion please head to https://slack.datahubproject.io. For feature requests please use https://feature-requests.datahubproject.io

Copy link

This issue is stale because it has been open for 30 days with no activity. If you believe this is still an issue on the latest DataHub release please leave a comment with the version that you tested it with. If this is a question/discussion please head to https://slack.datahubproject.io. For feature requests please use https://feature-requests.datahubproject.io

@github-actions github-actions bot added the stale label May 10, 2024
Copy link

github-actions bot commented Jun 9, 2024

This issue was closed because it has been inactive for 30 days since being marked as stale.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 9, 2024
@LucaDorinAnton
Copy link

Still experiencing this issue - can we please re-open this? Thanks.

@maggiehays maggiehays reopened this Jul 9, 2024
@david-leifker
Copy link
Collaborator

@LucaDorinAnton - Can you confirm that the logout url is configured in your oidc provider? It should point to the login page <hostname>/login? If the logout url is set to any other location, the oidc login flow will be triggered and log you back in.

@LucaDorinAnton
Copy link

@david-leifker Just to confirm, do you mean backchannel_logout_uri ?

https://openid.net/specs/openid-connect-backchannel-1_0.html#BCRegistration

@david-leifker
Copy link
Collaborator

david-leifker commented Jul 10, 2024

From okta, the configuration is post_logout_redirect_uri. The underlying logout when oidc is enabled is using the pac4j library which is following configuration from the idp. I believe post_logout_redirect_uri is also the name of the query parameter in the flow and is part of the docs here: https://openid.net/specs/openid-connect-rpinitiated-1_0.html#RPLogout

@LucaDorinAnton

@BramMeerten
Copy link
Author

@david-leifker ,
Shouldn't the post_logout_direct_uri be set by datahub (Relying Party) and not the Oidc Provider? Because the Relying Party should indicate where it wants to be redirected after the logout.
In my Oidc Provider (Keycloak), I have configured the client to allow any redirect url.

It's been a while since I tried to debug this issue, so I don't remember very well. But I think datahub only clears the session, and doesn't call the logout endpoint of the Oidc Provider at all (See my original post and this comment).

@LucaDorinAnton
Copy link

Similarly to @BramMeerten, we're also using KeyCloak and we allow any redirect URI. I tried inspecting the network traffic and I haven't seen DataHub attempting to use the post_logout_direct_uri parameter.

@georgeleeru
Copy link

I'm having this same issue in v0.13.3. I'm taken back to the landing page after I click the /logOut url. I can however logout in two steps - by accessing the "end_session_endpoint" url in my .well-known configuration and then the datahub /logOut url.

@minsql
Copy link

minsql commented Sep 5, 2024

I have also been through this issue in v0.14.0.2.
What I changed follows,

  public Result executeLogout(Http.Request request) {
    if (_isOidcEnabled) {
      try {
        // return logout(request).toCompletableFuture().get().withNewSession();
       logout(request).toCompletableFuture().get();
      } catch (Exception e) {
.. skipped ..
      }
    }
    return Results.redirect(AUTH_URL_CONFIG_PATH).withNewSession();
  }

I skipped to open new session when oidc enabled, then the session will be redirect to "/login" at the final line.
This change would avoid the infinite re-authorization.
Hope it helps you.

@luizhsalazar
Copy link

@minsql I've tried your solution but didn't work as expected in my case: the user was redirected to login page (as expected) but didn't invalidate his application access - if he goes to home page again, he will be logged.

Then, I write some code and opened a PR to fix this. In a nutshel, now there's a piece of code that grabs the logout URL (end_session_endpoint) from the auth.oidc.discoveryUri environment variable and redirect the user properly. We validated with Keycloak solution.

PR: #11388

Hope it helps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug report product PR or Issue related to the DataHub UI/UX stale
Projects
None yet
Development

No branches or pull requests