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

[data views] implement fallback sha1 hash for fields request #175181

Merged
merged 5 commits into from
Jan 22, 2024

Conversation

mattkime
Copy link
Contributor

@mattkime mattkime commented Jan 21, 2024

Summary

tldr; The implements a fallback sha1 method when using the browser in an insecure context.

The data views fields requests cache need uniqueness across users. This has been implemented by hashing the user object into a HTTP header using sha1. Typically we can use the browser's built in crypto objects for this HOWEVER its not available in insecure contexts - https://www.chromium.org/blink/webcrypto/#accessing-it - this PR supplies a sha1 function for insecure contexts.

How to test - when running kibana locally, it will run in a secure context via 127.0.0.1 or localhost. It will run in an insecure context at 0.0.0.0. Simply load some sample data and load a data view.

follow up to #168910

Screenshot of error resolved by this pr -
Visualize Visualize Reporting Screenshots Print PDF button becomes available whe-c57ca69f29465527c4079569a4548eb10fe0568302776500148260b299fbd5c4

@mattkime
Copy link
Contributor Author

/ci

@mattkime mattkime changed the title implement and test sha1 for insecure contexts [data views] implement fallback sha1 hash for fields request Jan 21, 2024
@mattkime mattkime self-assigned this Jan 21, 2024
@mattkime mattkime added Feature:Data Views Data Views code and UI - index patterns before 8.0 Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. release_note:skip Skip the PR/issue when compiling release notes labels Jan 21, 2024
@mattkime mattkime marked this pull request as ready for review January 21, 2024 05:56
@mattkime mattkime requested a review from a team as a code owner January 21, 2024 05:56
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

Copy link
Member

@mistic mistic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not directly involved in the code review but was in the process of that failure. This PR LGTM

package.json Outdated
@@ -1459,6 +1460,7 @@
"@types/redux-actions": "^2.6.1",
"@types/redux-logger": "^3.0.8",
"@types/resolve": "^1.20.1",
"@types/rusha": "^0.8.3",
Copy link
Member

@kertal kertal Jan 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx for taking care of this so quickly 👍 QQ: why not using kbn-crypto-browser, it's not sha1 but sha256, which should do the job, and no new dependency would be necessary (and it just this could be used, to simplify the code)
https://github.com/elastic/kibana/blob/4824d9da8c59f03522779d1b41d531f52b255dab/packages/kbn-crypto-browser/README.md#L4-L3
Maybe it would also reduce the size of the async chunk which has a plus of +12.0KB with this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that does make more sense!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea here :)

Copy link
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well, tested locally at both localhost and 0.0.0.0 (thanks btw, TIL). LGTM 👍

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
dataViews 50 53 +3

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
dataViews 1.6KB 5.0KB +3.3KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
dataViews 48.3KB 48.3KB +85.0B
Unknown metric groups

async chunk count

id before after diff
dataViews 1 2 +1

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @mattkime

@mattkime mattkime merged commit 99112c0 into elastic:main Jan 22, 2024
18 checks passed
@kibanamachine kibanamachine added v8.13.0 backport:skip This commit does not require backporting labels Jan 22, 2024
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
…#175181)

## Summary

tldr; The implements a fallback sha1 method when using the browser in an
insecure context.

The data views fields requests cache need uniqueness across users. This
has been implemented by hashing the user object into a HTTP header using
sha1. Typically we can use the browser's built in crypto objects for
this HOWEVER its not available in insecure contexts -
https://www.chromium.org/blink/webcrypto/#accessing-it - this PR
supplies a sha1 function for insecure contexts.

How to test - when running kibana locally, it will run in a secure
context via 127.0.0.1 or localhost. It will run in an insecure context
at 0.0.0.0. Simply load some sample data and load a data view.

follow up to elastic#168910

Screenshot of error resolved by this pr - 
![Visualize Visualize Reporting Screenshots Print PDF button becomes
available
whe-c57ca69f29465527c4079569a4548eb10fe0568302776500148260b299fbd5c4](https://github.com/elastic/kibana/assets/216176/d7bcec41-631f-426f-b209-87b2d6403f23)

---------

Co-authored-by: kibanamachine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Data Views Data Views code and UI - index patterns before 8.0 release_note:skip Skip the PR/issue when compiling release notes Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants