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

fix: dont require auth for healthchecks #2350

Merged
merged 7 commits into from
Nov 6, 2023

Conversation

markphelps
Copy link
Collaborator

@markphelps markphelps commented Nov 6, 2023

fixes: #2348

  • Exclude health check endpoints from requiring authentication
  • Also adds example using a static bootstrap token
  • Adds IT to test health check endpoint for both authenticated and un-authenticated

Config

# config.yml

log:
  level: DEBUG

authentication:
  required: true
  session:
    domain: "localhost:8080"
    secure: false
    csrf:
      key: "abcdef1234567890"
  methods:
    token:
      enabled: true
      bootstrap:
        token: "secret"
        expiration: 24h
      cleanup:
        interval: 2h
        grace_period: 48h

Before

Healthcheck returns 401 Unauthorized if auth is enabled

~ » curl -v http://localhost:8080/health
*   Trying 127.0.0.1:8080...
* Connected to localhost (127.0.0.1) port 8080 (#0)
> GET /health HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/8.1.2
> Accept: */*
>
< HTTP/1.1 401 Unauthorized
< Content-Type: application/json
< Www-Authenticate: request was not authenticated
< Date: Mon, 06 Nov 2023 15:07:02 GMT
< Content-Length: 68
<
* Connection #0 to host localhost left intact
{"code":16, "message":"request was not authenticated", "details":[]}

After

Healtcheck returns 200 OK even if auth is enabled

~ » curl -v http://localhost:8080/health
*   Trying 127.0.0.1:8080...
* Connected to localhost (127.0.0.1) port 8080 (#0)
> GET /health HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/8.1.2
> Accept: */*
>
< HTTP/1.1 200 OK
< Content-Type: application/json
< Date: Mon, 06 Nov 2023 15:05:02 GMT
< Content-Length: 21
<
{"status":"SERVING"}

Copy link
Contributor

github-actions bot commented Nov 6, 2023

Uffizzi Preview deployment-40137 was deleted.

Copy link

codecov bot commented Nov 6, 2023

Codecov Report

❗ No coverage uploaded for pull request base (release/1.30@b9d7d87). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 1e011b0 differs from pull request most recent head d596362. Consider uploading reports for the commit d596362 to get more accurate results

@@               Coverage Diff               @@
##             release/1.30    #2350   +/-   ##
===============================================
  Coverage                ?   70.81%           
===============================================
  Files                   ?       78           
  Lines                   ?     7442           
  Branches                ?        0           
===============================================
  Hits                    ?     5270           
  Misses                  ?     1872           
  Partials                ?      300           

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@markphelps markphelps marked this pull request as ready for review November 6, 2023 15:16
@markphelps markphelps requested a review from a team as a code owner November 6, 2023 15:16
@markphelps markphelps changed the title fix(wip): dont require auth for healthchecks fix: dont require auth for healthchecks Nov 6, 2023
@markphelps markphelps force-pushed the http-healthcheck-fix branch from 819d085 to 37d0d5a Compare November 6, 2023 15:17
@markphelps markphelps changed the base branch from main to release/1.30 November 6, 2023 15:17
@markphelps markphelps requested a review from yquansah November 6, 2023 15:18
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.

[FLI-667] [Bug]: Latest version fails healthcheck if auth enabled
2 participants