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

UI: needs to authenticate every time in 1.10.2 #15311

Closed
zerkms opened this issue May 6, 2022 · 17 comments · Fixed by #15769
Closed

UI: needs to authenticate every time in 1.10.2 #15311

zerkms opened this issue May 6, 2022 · 17 comments · Fixed by #15769
Labels

Comments

@zerkms
Copy link
Contributor

zerkms commented May 6, 2022

Describe the bug

I recently upgraded from 1.9.3 to 1.10.2

And immediately discovered that now you cannot open vault UI in a different tab without authentication.

My authentication is configured via LDAP

Previously the UI was using local storage, but now it uses session storage.

To Reproduce
Steps to reproduce the behavior:

  1. Open vault UI
  2. Login via LDAP
  3. Open vault UI in the new tab

Expected behavior

It's expected that the current authentication is retained in the different tab too

Environment:

  • Vault Server Version (retrieve with vault status): 1.10.2
  • Vault CLI Version (retrieve with vault version): n/a
  • Server Operating System/Architecture: Ubuntu focal

Vault server configuration file(s):

# Paste your Vault config here.
# Be sure to scrub any sensitive values

Additional context
Add any other context about the problem here.

@maxb
Copy link
Contributor

maxb commented May 6, 2022

Please please revert this change. I'm not on 1.10 just yet, so this issue is the first I'd heard of this, but a web application which logs you out just because you open it in a different tab (not browser), is not normal behaviour at all, and will significantly impact my day to day use of Vault.

@heatherezell heatherezell added the ui label May 6, 2022
@mttradebyte
Copy link

mttradebyte commented May 24, 2022

Can confirm this also happens in 1.10.3. Horrible UX change if intended. Highly doubt it was intended, though.

@heatherezell
Copy link
Contributor

Hi folks, we hear you. We will be working on a way to choose a flag to utilize either localStorage or sessionStorage. I'm not sure when that will be released, but we understand that this is not a good experience for folks and are looking at ways to make it better. Thanks for your patience!

@zerkms
Copy link
Contributor Author

zerkms commented May 24, 2022

@hsimon-hashicorp is there any publically available document/page that discusses the original reasons behind the change in the very first place?

@zerkms
Copy link
Contributor Author

zerkms commented May 24, 2022

@mttradebyte it was an intended change: #14054 (with no explanation why though)

@mttradebyte
Copy link

mttradebyte commented May 24, 2022

Then I am utterly staggered as to why they'd make such a poor decision. We access ours using Okta and MFA, so having to log in and verify MFA every time you close and reopen the tab is really quite inconvenient. Where is the architectural decision record for this change? Seems to have just been done on a whim.

@danmasta
Copy link

FYI - found a note on the release blog specifying this change was made to improve security:
https://www.vaultproject.io/docs/release-notes/1.10.0#using-sessionstorage-instead-of-localstorage-for-the-vault-ui

@zerkms
Copy link
Contributor Author

zerkms commented May 29, 2022

"The data in localStorage was persisted in browsers and removed only on demand" --- that's why people carefully choose the token's TTL when they configure their instances.

It's not clear what the attack vector is that is being fixed (secured): is it when somebody gets a full control over a user's browser? In that case even using sessionStorage won't help either 🤷

@danmasta thanks!

@heatherezell
Copy link
Contributor

In an upcoming release, we will be allowing this to be configured via an environment variable. We're targeting this for 1.11. I apologize again that this has been such a negative experience for folks.

@iniinikoski
Copy link

Thank you @hsimon-hashicorp! Just a "side note": I'd kindly ask you to consider how to bring this kind of "feature-switching" to HCP Vault customers as well. As we cannot address the underlying setup "at all"...

@heatherezell
Copy link
Contributor

I will definitely take that back to the team for discussion. Thanks a lot! :)

@maxb
Copy link
Contributor

maxb commented May 31, 2022

In an upcoming release, we will be allowing this to be configured via an environment variable. We're targeting this for 1.11. I apologize again that this has been such a negative experience for folks.

@hsimon-hashicorp It's bothered me for a while that many things in Vault are configured via the config file, but a random assortment of things are only accessible via environment variables.

Please could this be made a proper first class option in the config file instead?

Also, I would like to put the case that the default should be the 1.9 behaviour. In support of that, please note that "login per tab" is a very unexpected behaviour, as all webapps implementing login via cookies don't do that - its unique to the rather rarely used sessionstorage.

@heatherezell
Copy link
Contributor

In an upcoming release, we will be allowing this to be configured via an environment variable. We're targeting this for 1.11. I apologize again that this has been such a negative experience for folks.

@hsimon-hashicorp It's bothered me for a while that many things in Vault are configured via the config file, but a random assortment of things are only accessible via environment variables.

Please could this be made a proper first class option in the config file instead?

Also, I would like to put the case that the default should be the 1.9 behaviour. In support of that, please note that "login per tab" is a very unexpected behaviour, as all webapps implementing login via cookies don't do that - its unique to the rather rarely used sessionstorage.

An environment variable would have been easier to implement with less lifting on the backend code. However, the point about HCP Vault not being able to use an environment variable is a good one, so we'll be reconsidering the approach we want to take. This was originally implemented as a customer request, to ensure that a bad actor couldn't re-open a closed tab / browser window without re-authenticating, but as we see here the negative impact on day-to-day workflows is significant and worth considering how to balance security and usability.

@tgmatt
Copy link

tgmatt commented Jun 3, 2022

Thanks for listening to the community and reverting the change in favour of a more optional approach in the future. Any idea when you might cut a release with this change?

@heatherezell
Copy link
Contributor

Thanks for listening to the community and reverting the change in favour of a more optional approach in the future. Any idea when you might cut a release with this change?

I can't make any guarantees, but I believe it will be a little bit later this month. Thanks in advance for your patience. <3

@maxb
Copy link
Contributor

maxb commented Jun 11, 2022

There is no mention of this in the 1.10.4 changelog... What happened?

@tgmatt
Copy link

tgmatt commented Jun 15, 2022

There is no mention of this in the 1.10.4 changelog... What happened?

Looks like it's in the 1.11.0-rc1 branch, so should be coming soon 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants