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

Don't show Service Account JSON contents when set by a constant #320

Closed
dchenk opened this issue Nov 7, 2018 · 8 comments
Closed

Don't show Service Account JSON contents when set by a constant #320

dchenk opened this issue Nov 7, 2018 · 8 comments
Assignees
Milestone

Comments

@dchenk
Copy link
Contributor

dchenk commented Nov 7, 2018

I think an important security feature that should be added is to not display the Service Account JSON field value in the settings panel when the JSON key is set with a PHP constant. What's the point of showing the JSON anyway? The user can't edit it from wp-admin, and they don't need to see it for any reason. This is especially a security threat where I'm letting a client use a bucket that I'm paying for.

I'd like to hear all your thoughts about this. Doesn't it sound like the right thing to do?

Thanks!

@dchenk
Copy link
Contributor Author

dchenk commented Dec 12, 2018

@alimuzzaman or @ewsopp will we be seeing this feature in the next release? I'd really appreciate it.

@alimuzzaman
Copy link
Contributor

@dchenk yes this will be on next release.

@alimuzzaman
Copy link
Contributor

@dchenk @ewsopp how does this look? Any suggestion?
Should I make the box empty or the message is good?
image

alimuzzaman pushed a commit that referenced this issue Dec 24, 2018
Don't show Service Account JSON contents when set by a constant #320
@dchenk
Copy link
Contributor Author

dchenk commented Dec 24, 2018

That's really great. You can even have the textbox not show up but just display the message. Thank you :)

@ewsopp
Copy link
Member

ewsopp commented Dec 27, 2018

Looks good, but adjust the text within the text box...

Currently configured via a constant.

You missed the "a" before constant.

alimuzzaman pushed a commit that referenced this issue Jan 9, 2019
alimuzzaman pushed a commit that referenced this issue Jan 9, 2019
@alimuzzaman
Copy link
Contributor

@dchenk removing isn't good idea.

@alimuzzaman
Copy link
Contributor

@antonkorotkov do you have any idea about what to do with the textarea when not showing the JSON key?

Showing Currently configured via a constant. inside textarea is redundant because it's already exists bottom.

@ewsopp
Copy link
Member

ewsopp commented Jan 10, 2019

It's fine, just leave it as is.

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

No branches or pull requests

4 participants