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.

Sign up for GitHub

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

Jump to bottom

multi-tenancy: handle ux for session expiration #92843

Closed
Santamaura opened this issue Dec 1, 2022 · 2 comments · Fixed by #95020
Closed

multi-tenancy: handle ux for session expiration #92843

Santamaura opened this issue Dec 1, 2022 · 2 comments · Fixed by #95020
Assignees
@Santamaura
Labels
A-kv-observability C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@Santamaura
Copy link
Contributor

Santamaura commented Dec 1, 2022 •
edited by cockroach-jira-scripts
Loading

Is your feature request related to a problem? Please describe.
Follow up from #92694. A decision should be made on what the user experience should be when a user tries to access a tenant from the admin ui while their session has expired.

Describe the solution you'd like
One potential solution is to redirect the user to the login page and have them login for that tenant again.

Jira issue: CRDB-21999
@Santamaura Santamaura added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Dec 1, 2022
@dhartunian
Copy link
Collaborator

Here's how things work on the first pass:

Login

  1. User logs in as usual
  2. Separate sessions are created on each tenant for which the given creds are valid
  3. Tenant switcher allows switching active tenant sessions

Logout

  1. Lose all active sessions on all tenants

Session expires

Haven't tested this but probably:

  1. Session remains listed in switcher
  2. Switching to it redirects to logout since session is expired
  3. (As above) All sessions are cleared.

My preference here is to keep the "all sessions just get cleared on a logout event" logic. Otherwise there's a lot of extra UI that needs to be built to manage login into a particular tenant and merge that tenant's session with the existing cookie etc etc.

I doubt the extra flows provide such an elevated user experience that it's worth thinking it through. Logging in again for the user should be easy enough and is something they'd already have to do with the existing flows on a non-UA cluster. Since by construction all sessions would be for matching credentials anyway, a single login will bring the back to the multi-tenant switcher state they had prior to logout.

cc @thtruo

@Santamaura
Copy link
Contributor Author

cc @knz

@Santamaura Santamaura mentioned this issue Jan 10, 2023
Merged
craig bot pushed a commit that referenced this issue Jan 13, 2023
@Santamaura @rafiss @nvanbenschoten
Merge #95020 #95029 #95172
6f5d89e 
95020: ui: clear tenant cookie when multitenant session expires r=Santamaura a=Santamaura

This change adds a method to clear the tenant cookie when the multitenant-session cookie has more than one tenant. The method is called on logout and if an api returns a 401 unauthorized error. This is to ensure that in a multitenant cluster when the session expires and the user is redirected to the login page, they will be able to do a multitenant login.

Fixes: #92843

Release note (ui change): clear tenant cookie on an api returning 401 error in order to allow for multitenant login again.

95029: sql: use crdb_internal for pg_proc.pronamespace when needed r=knz a=rafiss

fixes #94952

Release note (bug fix): The pronamespace column of the pg_proc table now correctly reports the crdb_internal schema for built-in functions that have the "crdb_internal" prefix.

---

comprules: support completion for qualified functions

The syntax completion engine now generates suggestion for user-defined
functions. It restricts the suggestions to only functions in the schema
name if one was provided, or else to the schemas in the search_path.

No release note since this functionality is new in v23.1.

95172: go.mod: bump Pebble to 90c810757fe6 r=nvanbenschoten a=nvanbenschoten

```
90c81075 cmd/pebble: add bench replay command
1e862340 db: scan a readState during format major version upgrades
dac60ce1 Revert "sstable: skip two known test flakes on windows"
78933282 ci: renable linux race
5bad2c1a sstable: avoid allocating up to maximum block size in valueBlockWriter
a4481627 deflake TestCompactionTombstones
06fbe6ae db: add DB.ApplyNoSyncWait for asynchronous apply
a60e75cf db: misc LazyValue tweaks, for testing, and simpler Clone invariant
ba575119 doc: fix
326549f0 db: metrics and stats for value blocks and lazy values
cc913bad db: don't validate version edits during a panic
```

Release note: None
Epic: None

Co-authored-by: Santamaura <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
@craig craig bot closed this as completed in 2f10222 Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Santamaura Santamaura

Labels
A-kv-observability C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ui: clear tenant cookie when multitenant session expires Santamaura/cockroach
3 participants
@dhartunian @thtruo @Santamaura