server: only set default tenant if login successful #98983
Conversation
|
This change is |
dhartunian
force-pushed
the
only-use-default-tenant-in-session-if-successful
branch
from
e1ac486 to
08a1948
Compare
There was a problem hiding this comment.
LGTM, but could you also help me understand what happens (in terms of UX) if login doesn't succeed on any of the tenants at all?
Good reminder. I checked and the error messages weren't passing through so I added that in to retain the feedback in the UI.
Additionally, I changed the fanout login to always do so on login because otherwise existing sessions would make things work oddly. For instance, clicking the demo login link would overwrite a multi-tenant session with one that was just for a single tenant and the dropdown would disappear.
Reviewable status:
complete! 0 of 0 LGTMs obtained
There was a problem hiding this comment.
Reviewed 1 of 2 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status:
dhartunian
added
the
backport-23.1.x
|
bors r=knz |
|
Build failed (retrying...): |
|
Build failed (retrying...): |
|
The new test seems to be flaky (failed on bors and in extended CI). bors r- |
|
Canceled. |
dhartunian
force-pushed
the
only-use-default-tenant-in-session-if-successful
branch
3 times, most recently
from
167d18f to
5a918a7
Compare
Previously, we would always set the default tenant cookie to the default tenant cluster setting regardless of what tenants the user logged-in to successfully. This change ensures that the default tenant selection is only used when the successful logins include that tenant. Otherwise, we select the first tenant from the list of successful logins. Always fanout login requests to ensure that an existing session doesn't prevent login from proceeding. Otherwise a multi-tenant session can get clobbered by a single-tenant one. Include errors from failed login requests in the error repsonse so that users can see the error message in the Login box on DB Console. Otherwise it's blank. Epic: CRDB-12100 Release note: None
dhartunian
force-pushed
the
only-use-default-tenant-in-session-if-successful
branch
from
5a918a7 to
e883c70
Compare
|
bors r=knz |
|
Build succeeded: |
dhartunian
deleted the
only-use-default-tenant-in-session-if-successful
branch
Previously, we would always set the default tenant cookie to the default tenant cluster setting regardless of what tenants the user logged-in to successfully.
This change ensures that the default tenant selection is only used when the successful logins include that tenant. Otherwise, we select the first tenant from the list of successful logins.
Epic: CRDB-12100
Release note: None