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

Rename env variables to be used in React #142

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Rename env variables to be used in React #142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1235b03
rename env variables to be used in React
lio-p Dec 21, 2023
ba41ec1
Add security disclaimer about exposing API Key
lio-p Jan 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions example-apps/internal-knowledge-search/app-ui/.env
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
SEARCH_APP_NAME=some-search-application
SEARCH_APP_API_KEY=xxxxxxxxxxxxxxxxxxx
SEARCH_APP_ENDPOINT=https://some-search-end-point.co
REACT_APP_SEARCH_APP_NAME=some-search-application
REACT_APP_SEARCH_APP_API_KEY=xxxxxxxxxxxxxxxxxxx
Copy link
Collaborator

Choose a reason for hiding this comment

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

Correct me if I'm wrong, but this would make the API key available in the browser, which is a really bad idea security-wise, and not something we should promote or recommend.

Copy link
Collaborator Author

@lio-p lio-p Dec 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yes you're correct, but actually the current app already does expose it. But this is the method documented in the Elasticsearch documentation.

This PR just fixes a bug that prevent developers to pass those parameters using an .env file. If not, those parameters can be set through the web app itself.

Maybe we should add a disclaimer in the README about the security risks and point to the Elasticsearch documentation for more details?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Interesting that we recommend this method, as it is very risky and depends on people having an understanding of what they're doing when generating their keys, which isn't always the case.

Even with a scoped API key, there is risk of a bad actor using the key to generate unexpected expenses for the application owner, or render the search functionality unusable by making enough calls to hit the rate limits, so this is still a bad idea.

At the very least there should be a very big warning in the readme explaning the risk.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added a disclaimer in the Readme file, please let me know what you think.

REACT_APP_SEARCH_APP_ENDPOINT=https://some-search-end-point.co
5 changes: 4 additions & 1 deletion example-apps/internal-knowledge-search/app-ui/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,5 +106,8 @@ To be able to use the index filtering and sorting in the UI you should update th

### Setting the environment variables

You need to set `SEARCH_APP_NAME`, `SEARCH_APP_API_KEY` and `SEARCH_APP_ENDPOINT` inside [.env](.env) to the corresponding values, which you'll get when [creating a search application](https://www.elastic.co/guide/en/enterprise-search/current/search-applications.html).
You need to set `REACT_APP_SEARCH_APP_NAME`, `REACT_APP_SEARCH_APP_API_KEY` and `REACT_APP_SEARCH_APP_ENDPOINT` inside [.env](.env) to the corresponding values, which you'll get when [creating a search application](https://www.elastic.co/guide/en/enterprise-search/current/search-applications.html).

### Security consideration

As you provide the `REACT_APP_SEARCH_APP_API_KEY`, make sure this is a restricted API Key as this will be available in the browser. More information about Search application security and alternatives approaches can be found [here](https://www.elastic.co/guide/en/elasticsearch/reference/current/search-application-security.html).
6 changes: 3 additions & 3 deletions ...-apps/internal-knowledge-search/app-ui/src/store/slices/searchApplicationSettingsSlice.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import {createSlice, PayloadAction} from '@reduxjs/toolkit';
import {SearchApplicationSettingsModel} from "../../models/SearchApplicationSettingsModel";
const initialState: SearchApplicationSettingsModel = {
appName: process.env.SEARCH_APP_NAME || "some-search-application",
apiKey: process.env.SEARCH_APP_API_KEY || "xxxxxxxxxxxxxxxxxxx",
searchEndpoint: process.env.SEARCH_APP_ENDPOINT || "https://some-search-end-point.co",
appName: process.env.REACT_APP_SEARCH_APP_NAME || "some-search-application",
apiKey: process.env.REACT_APP_SEARCH_APP_API_KEY || "xxxxxxxxxxxxxxxxxxx",
searchEndpoint: process.env.REACT_APP_SEARCH_APP_ENDPOINT || "https://some-search-end-point.co",
};

const searchApplicationSettingsSlice = createSlice({
Expand Down