Add authentication #97

bio-boris · 2025-10-17T18:03:30Z

This PR adds Basic Authentication to the Elasticsearch client, allowing the service to connect to secured clusters.

The startup script has been updated to use the new authentication variables, resolving the connection issues when pointed at the elastic-next-spike instance. I've deployed and verified this on the next environment.

Next Steps:

Populate a test Elasticsearch instance with actual data + auth to fully verify end-to-end functionality.
Create a user for elastic

Closes #90

Add authentication (#96)

Copilot

Pull Request Overview

This PR adds authentication support for Elasticsearch connections throughout the application. The changes implement both Basic authentication (username/password) and Bearer token authentication with proper priority handling.

Adds authentication configuration options for Elasticsearch (username, password, and token)
Updates all Elasticsearch client code to include authentication headers
Implements comprehensive test coverage for the new authentication functionality

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/utils/config.py	Adds auth configuration variables and `get_elasticsearch_auth_header()` function
src/utils/wait_for_service.py	Updates service connection function to accept and use auth tokens
src/server/main.py	Updates service startup to use authentication when connecting to Elasticsearch
src/es_client/query.py	Adds authentication headers to Elasticsearch search requests
tests/helpers/init_elasticsearch.py	Updates test helper to use centralized auth header function
tests/unit/utils/test_config.py	Adds comprehensive tests for authentication header generation
tests/unit/utils/test_wait_for_service.py	Adds tests for service connection with and without authentication
.github/workflows/docker-image.yml	Adds CI/CD workflow for Docker image building

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

codecov · 2025-10-17T18:08:17Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (23ec7c1) to head (ec06906).

Additional details and impacted files

@@            Coverage Diff            @@
##           develop       #97   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           21        21           
  Lines          660       685   +25     
=========================================
+ Hits           660       685   +25

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

bio-boris · 2025-10-17T18:11:40Z

src/utils/config.py

+        logger.info("Using Basic Authentication for Elasticsearch.")
+        return f"Basic {base64_credentials}"
+    elif auth_token:
+        # Bearer authentication


There is no way to test this live right now, as this requires enabling SSL communication to be able to generate API keys.

so with our current elastic search configuration we can't use auth tokens?

Correct. We would have to create and distribute SSL certs and enable HTTPS. @jbezouska-ANL were there any plans to enable HTTPS on the backend?

src/utils/config.py

MrCreosote · 2025-10-17T20:05:02Z

src/utils/config.py

+    Priority:
+    1. If username and password are set, use Basic authentication
+    2. If only auth token is set, use Bearer authentication


Why this precedence?

I picked it arbitrarily.

My thinking is that tokens are more secure than user/pwd so they should be preferred

I think if we can get tokens working, we can delete the username and password code too, but since they aren't working yet, I just got it working with what we have.

Might as well leave it if it works and just swap the preference

I looked this up the other day and AI says they aren't more secure.

TL;DR: With One Username Per Service, They're Identical If you have one username per service, then API keys and username/password are literally the same thing security-wise: Both are: Base64 encoded strings in headers Unique per service (no shared credentials) Equally vulnerable if intercepted Equally easy to revoke (just disable that one user/key) Can have identical RBAC permissions The only remaining differences are cosmetic: API Key: Authorization: ApiKey <base64(id:key)> Basic Auth: Authorization: Basic <base64(username:password)> Both decode to two strings separated by a colon. Both can be scoped, rotated, and revoked independently.

Equally easy to revoke (just disable that one user/key)

I don't think that's true, it's easier to revoke a token than change a password

How is it easier? Both will require shutting down and restarting the service?

I assumed it was easier on the ES side, like how in KBase revoking a token is trivial.

If that's not the case and you're saying that passwords and tokens are completely indistinguishable for this use case then we should just drop token support

I don't know, I haven't tested it out, I was thinking more from the side of what happens to the service. Ok I will remove it.

src/utils/wait_for_service.py

tests/unit/utils/test_wait_for_service.py

tests/unit/utils/test_config.py

src/es_client/query.py

Refactor tests for wait_for_service to reduce duplication and improve clarity.

bio-boris · 2025-10-21T01:06:53Z

@MrCreosote Can we merge this?

MrCreosote · 2025-10-21T01:40:16Z

This one is still open #97 (comment)

Updated the docstring to clarify test purpose.

Removed unused logging import and logger initialization.

Remove comment about calculating Elasticsearch auth header.

Replaced centralized auth header function with config-based authorization header retrieval.

Removed unused import of get_elasticsearch_auth_header.

Refactor authentication header retrieval in show_indexes function.

Refactor Elasticsearch authentication header retrieval.

bio-boris · 2025-10-21T19:27:12Z

src/es_client/query.py

    params = {'allow_no_indices': 'true'}

-    resp = requests.post(url, data=json.dumps(options), params=params, headers=headers)
+    resp = requests.post(url, data=json.dumps(options), params=params, headers=headers, timeout=[120, 600])


added timeouts, maybe I should instead add an annotation for bandit to ignore these and not add timeouts, since I don't know what the real timeouts should be?

bio-boris · 2025-10-21T21:54:01Z

Closing in favor of #100

Add authentication (#96)

e76a5ca

Add authentication (#96)

Copilot AI review requested due to automatic review settings October 17, 2025 18:03

Copilot AI reviewed Oct 17, 2025

View reviewed changes

bio-boris commented Oct 17, 2025

View reviewed changes

Delete .github/workflows/docker-image.yml

6f6d083

MrCreosote reviewed Oct 17, 2025

View reviewed changes

bio-boris and others added 4 commits October 17, 2025 15:59

Refactor wait_for_service tests for auth token handling

56e6b23

Refactor tests for wait_for_service to reduce duplication and improve clarity.

Fix tests

780158a

remove comments

4cc54ef

fix flake

576c841

bio-boris mentioned this pull request Oct 17, 2025

Add integration tests for auth #98

Open

bio-boris and others added 16 commits October 21, 2025 12:00

Tests

93b7b68

Revise docstring for Elasticsearch auth tests

5e81d49

Updated the docstring to clarify test purpose.

update creds

9c389aa

update creds

5934134

update creds

19ae693

Tests

688ace6

Remove token

2883d61

Update query.py

ec06906

Update config.py

d8e7cbc

Remove unused logging setup in config.py

0863e36

Removed unused logging import and logger initialization.

Remove comment on Elasticsearch auth header calculation

fd3b2d5

Remove comment about calculating Elasticsearch auth header.

Update Elasticsearch headers to use config for auth

da56683

Replaced centralized auth header function with config-based authorization header retrieval.

Update Elasticsearch auth token retrieval method

ac4c349

Clean up imports in __main__.py

25230ee

Removed unused import of get_elasticsearch_auth_header.

Update auth header retrieval in show_indexes

1971432

Refactor authentication header retrieval in show_indexes function.

Update Elasticsearch auth header handling

8b0a047

Refactor Elasticsearch authentication header retrieval.

bio-boris added 2 commits October 21, 2025 13:46

Update timeouts

01050de

update tests

02bb1da

bio-boris commented Oct 21, 2025

View reviewed changes

bio-boris added 3 commits October 21, 2025 14:33

update tests

755060d

update tests

1bbc2d4

Update timeouts

13a1d6e

bio-boris closed this Oct 21, 2025

Add authentication #97

Add authentication #97

Uh oh!

Conversation

bio-boris commented Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

codecov bot commented Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bio-boris commented Oct 21, 2025

Uh oh!

MrCreosote commented Oct 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bio-boris commented Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bio-boris commented Oct 17, 2025 •

edited

Loading

codecov bot commented Oct 17, 2025 •

edited

Loading

bio-boris commented Oct 21, 2025 •

edited

Loading