Skip to content

Add API key authentication to ApiServer - #13

Merged
thechibuikem merged 11 commits into
hyphae:vertex-4from
thechibuikem:vertex-4
Aug 5, 2026
Merged

Add API key authentication to ApiServer#13
thechibuikem merged 11 commits into
hyphae:vertex-4from
thechibuikem:vertex-4

Conversation

@thechibuikem

@thechibuikem thechibuikem commented Jul 3, 2026

Copy link
Copy Markdown
Member

Addresses hyphae/APIS#91 (item 1: Unauthenticated Control APIs)

Adds timing-safe X-API-Key header check to ApiServer's HTTP request handler, covering all three handlers (ErrorGeneration, DealGeneration, LogConfiguration) since the check runs before dispatch. Returns 500 if apiKey unconfigured, 401 on missing/invalid key.

Tests

Unit Tests

ApiServerAuthTest: missing key, empty key, missing header, wrong
header, valid header.

Manual Testing: ApiServer X-API-Key Authentication

Covers checkAuth() in ApiServer — the shared auth gate for /error, /deal, and /log. Tested against /log, but behavior is identical for the other two endpoints since auth runs before any handler-specific code.

Prerequisites

  • apis-web running (Docker or local)
  • .env configured with DEV_INTERNAL_API_KEY
  • curl available
    Set once per session for convenience:
HOST=localhost
KEY="<your configured DEV_INTERNAL_API_KEY>"

Manual Testing: ApiServer X-API-Key Authentication

Covers checkAuth() in ApiServer — the shared auth gate for /error, /deal, and /log. Tested against /log, but behavior is identical for the other two endpoints since auth runs before any handler-specific code.

Prerequisites

  • apis-web running (Docker or local)
  • .env configured with DEV_INTERNAL_API_KEY
  • curl available

Set once per session for convenience:

HOST=localhost
KEY="<your configured DEV_INTERNAL_API_KEY>"

# Case Command Expected
1 Valid key curl -i http://$HOST:9999/log -H "X-API-Key: $KEY" 200, HTML form body
2 Missing key curl -i http://$HOST:9999/log 401, unauthorized
3 Empty key curl -i http://$HOST:9999/log -H "X-API-Key: " 401, unauthorized
4 Wrong key curl -i http://$HOST:9999/log -H "X-API-Key: wrong-value" 401, unauthorized
5 No key configured server-side Unset DEV_INTERNAL_API_KEY, restart server, then curl -i http://$HOST:9999/log -H "X-API-Key: anything" 500, server misconfigured

Screenshots

Valid key (200):
Screenshot from 2026-07-21 09-10-10

Missing key (401):
Screenshot from 2026-07-21 09-11-00

Empty key (401):
Screenshot from 2026-07-21 09-11-00

Wrong key (401):
Screenshot from 2026-07-21 09-11-38

Unconfigured server (500):
Screenshot from 2026-07-21 09-01-34

Notes

  • checkAuth() uses MessageDigest.isEqual for constant-time comparison — prevents key-length/prefix leakage via response-time analysis. No need to test this behaviorally; it's a code-level guarantee.
  • Auth runs once in ApiServer before path dispatch, so this behavior applies identically to /error and /deal — no need to repeat these cases per-endpoint.
  • Fixed: initial misconfiguration check ran before the DEV_INTERNAL_* env-var resolution, so an unset DEV_INTERNAL_API_KEY left apiKey null past that guard — causing an uncaught NPE instead of a 500. Added a second check after the swap.

@FlawzyByte @axmsoftware @subhramit

Follow-up (separate PRs)

@FlawzyByte

Copy link
Copy Markdown
Member

Interesting insight, I provided my answer through zulip
@subhramit Hi subha again, I appreciate your feedback on this. Thanks!

Comment thread Makefile

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please revert the formatting changes to this file

when(req.getHeader("X-API-Key")).thenReturn("secret");
assertNull(ApiServer.checkAuth(req, "secret"));
}
} No newline at end of file

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add a newline here. This can also be covered by Checkstyle eventually.

@thechibuikem thechibuikem Jul 14, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hi @subhramit , thank you for the feedback!
I added a new-line in the most recent changes.

Comment on lines +5 to +6
import static org.junit.Assert.*;
import static org.mockito.Mockito.*;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Don't import *, just import the required methods

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hi Subhramit.
I appreciate your feedback! I addressed this in the most recent changes.

@subhramit subhramit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hey, can you provide some steps to manually test this?
Also can you look into if assertEquals needs explicit boxing for comparison? (Integer.valueOf)

@thechibuikem

thechibuikem commented Jul 14, 2026

Copy link
Copy Markdown
Member Author

Hi @subhramit.
I hope you're doing good.

Why assertEquals needs explicit boxing for comparison.

The build was failing with a compilation error: "reference to assertEquals is ambiguous". I researched and found out it happened because org.junit.Assert.assertEquals has overloaded method signatures for both primitives, assertEquals(long, long) and objects assertEquals(Object, Object).

Because the test was mixing a primitive type (like int) and a wrapper object type (like Long or Integer), Java's automatic type conversion (autoboxing) made both method signatures equally valid. The compiler couldn't determine which version to invoke, making the build fail.

Solution:
So I resolved the ambiguity by explicitly wrapping the primitive value using Integer.valueOf(). Forcing both arguments to be evaluated as objects, cleanly routing the call to the assertEquals(Object, Object) method and fixing the compilation error.

As to the steps to the manual testing, I updated the description of this PR to include it.
Thank you!

@axmsoftware axmsoftware left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@thechibuikem Please trace calls from apis-main_controller on the client side to apis-web, and how apis-web calls apis-main. Use debugger on the Java side with break points, and the Developer tools on the Python web app. Thanks!

@thechibuikem

thechibuikem commented Jul 20, 2026

Copy link
Copy Markdown
Member Author

@thechibuikem Please trace calls from apis-main_controller on the client side to apis-web, and how apis-web calls apis-main. Use debugger on the Java side with break points, and the Developer tools on the Python web app. Thanks!

Hi @axmsoftware.

According to my findings from tracing the calls sent by apis-main_controller to apis-web. I discovered that Apis-main_controller interacts with apis-web via the:

  • apis_emul_port (43900), and
  • apis_budo_port (43830)
Screenshot from 2026-07-20 14-26-25

Which are at jp.co.sony.csl.dcoes.apis.tools.web.EmulatorEmulator.java and jp.co.sony.csl.dcoes.apis.tools.web.BudoEmulator.java respectively.

The third port being exposed by apis-web, via the ApiServer (port 9999) at ApiServer.java . I couldn't find any caller for it inside apis-main, apis-main_controller, or apis-web — no self-calls, no client code anywhere in these three repos. Based on the code (each handler serves an HTML

on GET), it looks built for manual/external use e.g a human via browser or curl, likely for injecting test errors/deals — not part of the normal inter-service dataflow.

@subhramit subhramit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Lgtm

Comment thread Makefile Outdated
clean:
mvn clean
rm -f *.log *.err
rm -f *.log *.err No newline at end of file

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add the newline back

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks for the feedback @subhramit, I just resolved this in the latest changes.

@thechibuikem
thechibuikem merged commit 6fc9acd into hyphae:vertex-4 Aug 5, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants