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

Offline Sessions not shown in users session tab #24523

Closed
1 task done
dasniko opened this issue Nov 3, 2023 · 12 comments
Closed
1 task done

Offline Sessions not shown in users session tab #24523

dasniko opened this issue Nov 3, 2023 · 12 comments
Labels
area/admin/ui area/core kind/feature Categorizes a PR related to a new feature team/ui

Comments

@dasniko
Copy link
Contributor

dasniko commented Nov 3, 2023

Before reporting an issue

  • I have read and understood the above terms for submitting issues, and I understand that my issue may be closed without action if I do not follow them.

Area

admin/ui

Describe the bug

Offline Sessions are not being displayed under users session tab, but are available under realm sessions and client sessions tab.
Bildschirmfoto 2023-11-03 um 13 22 06
Bildschirmfoto 2023-11-03 um 13 22 12
Bildschirmfoto 2023-11-03 um 13 22 21

Version

22.0.5

Expected behavior

Offline Sessions are also displayed at users sessions tab.

Actual behavior

Offline Sessions are not displayed at users sessions tab, only regular sessions are shown

How to Reproduce?

Create an offline session an inspect the sessions at realm, client and user.

Anything else?

Don't know if this is really a bug in the admin ui, or if it is intended. If intended, then why? I don't see a reason here.

@dasniko dasniko added kind/bug Categorizes a PR related to a bug status/triage labels Nov 3, 2023
@ssilvert
Copy link
Contributor

ssilvert commented Nov 3, 2023

This is a question of what the REST API should return when you ask for the user's sessions. Moving this issue to the core team.

@kaustubh-rh
Copy link
Contributor

It looks like the issue exists because only the /session endpoint is being called. We need to also call the corresponding /offline-sessions endpoint . What do you suggest @mposolda ?

@john2094
Copy link

@kaustubh-rh exist another issue related with same endpoints. When You signout a offline session the endpoint call for delete is /sessions instead /offline-sessions. These cause that the endpoint respond with a 404 and the offline session was not loged out.

@jonkoops
Copy link
Contributor

We could add a tab that shows the offline sessions in the same manner the current session overview is implemented. @ssilvert @xianli123 WDYT?

Since a REST API exists for this data, I will re-label this to the UI team.

@jonkoops jonkoops added kind/feature Categorizes a PR related to a new feature area/admin/ui team/ui and removed kind/bug Categorizes a PR related to a bug team/core labels Nov 14, 2023
@ssilvert
Copy link
Contributor

We could add a tab that shows the offline sessions in the same manner the current session overview is implemented. @ssilvert @xianli123 WDYT?

Since a REST API exists for this data, I will re-label this to the UI team.

I think two tabs might be overkill. I don't think a user typically has offline sessions. If the user does have an offline session, you probably want to see it along with the others. So a single tab would give a better overview of activity.

I don't remember how the old admin console handled it or if it did what the issue author wants. But I suppose we could implement it with two REST calls.

@ssilvert
Copy link
Contributor

@kaustubh-rh exist another issue related with same endpoints. When You signout a offline session the endpoint call for delete is /sessions instead /offline-sessions. These cause that the endpoint respond with a 404 and the offline session was not loged out.

Created issue for this one. #24763

@dasniko
Copy link
Contributor Author

dasniko commented Nov 15, 2023

I think two tabs might be overkill. I don't think a user typically has offline sessions. If the user does have an offline session, you probably want to see it along with the others. So a single tab would give a better overview of activity.

I also don't think that we do need two tabs, one combined tab with regular and offline sessions should be sufficient. The same way, as the realm sessions and the client sessions views work.

@jonkoops
Copy link
Contributor

I think two tabs might be overkill. I don't think a user typically has offline sessions. If the user does have an offline session, you probably want to see it along with the others. So a single tab would give a better overview of activity.

Yeah, I am only proposing this as it's the least frictional approach. Having to call two APIs and handle their pagination on the client together is always problematic (see the amount of issues reported around breaking pagination). If this needs to be a single overview, then we need to have a single REST endpoint that handles that for us.

Basically, we either show offline sessions, or online sessions, but never both at the same time. Unless changes to the REST API are made to handle pagination there.

@hmlnarik hmlnarik assigned hmlnarik and unassigned hmlnarik Nov 15, 2023
@ssilvert
Copy link
Contributor

@dasniko We have looked into this further and, unfortunately, a proper implementation would require a lot of changes to the back end. So we are unlikely to devote resources to this..

Since Keycloak has never supported this feature and nobody has asked for it until now, it's probably not a high priority feature for the majority of installations.

I'm going to close this issue now, but if you think there is a truly compelling use case, I suggest that you open a discussion and see what the rest of the community thinks. We can always reconsider.

@ssilvert ssilvert closed this as not planned Won't fix, can't repro, duplicate, stale Nov 15, 2023
@ghost ghost removed the status/triage label Nov 15, 2023
@xianli123
Copy link
Contributor

Hi @jonkoops I also agree with combining the offline sessions and regular sessions. But if we want to consider this feature in the future, it works for me. Whenever we need to update this part, please feel free to count me in.

@dasniko
Copy link
Contributor Author

dasniko commented Nov 15, 2023

@ssilvert Thanks for your effort and explanation. It's not a crititcal feature for me and I wasn't aware of the legacy behavior any more. I just thought it was a "bug" because of the inconsistency between realm, client and user tabs.

@jonkoops
Copy link
Contributor

Personally, I think we could still land this as a separate overview if it's a useful feature for users 🤷‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/admin/ui area/core kind/feature Categorizes a PR related to a new feature team/ui
Projects
None yet
Development

No branches or pull requests

7 participants