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

feat: add BasicAuth Support to Atlantis ServeHTTP #1777

Merged
merged 2 commits into from Oct 21, 2021

Conversation

fblgit
Copy link
Contributor

@fblgit fblgit commented Aug 28, 2021

This PR:

  1. Introduces the following flags:
    --web-basic-auth=bool (false by default) to enable/disable basic-auth on the webserver
    --web-username=string (atlantis by default) to set the webserver username
    --web-password=string (atlantis by default) to set the webserver password

  2. BasicAuth feature over middleware HTTP server of Atlantis

  • The URLPath /events is filtered out of the basic-auth requirement, as this element can be protected already by webhook secrets.
  • The rest of the routes (lock, unlock, etc), if enabled, will request a basic-auth header in order to access the webserver.

Probably RequestLogger should be relabeled to something else that now identifies both the authentication, serving, and logging scenario for HTTP requests.

Motivation:
Despite being able to apply different mechanisms with a layer in front of Atlantis, like ingress, I believe that is reasonable the capacity of scope the access to the HTTP server directly within Atlantis.

@fblgit fblgit requested a review from a team as a code owner August 28, 2021 16:19
@fblgit
Copy link
Contributor Author

fblgit commented Sep 3, 2021

hey @nishkrishnan can u please give a hand and review this PR please ?

@xavipanda
Copy link
Contributor

@chenrui333 there is any missing thing for this PR to go in ?

Copy link
Contributor

@acastle acastle left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

Copy link
Member

@chenrui333 chenrui333 left a comment

Choose a reason for hiding this comment

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

LGTM, nice addition

@chenrui333 chenrui333 changed the title Add BasicAuth Support to Atlantis ServeHTTP feat: add BasicAuth Support to Atlantis ServeHTTP Oct 21, 2021
@acastle acastle merged commit cbf35ca into runatlantis:master Oct 21, 2021
@wendtek
Copy link
Contributor

wendtek commented Nov 12, 2021

If putting Atlantis somewhere that requires healthchecks for availability, this now makes the /healthz endpoint require auth as well.

@wendtek
Copy link
Contributor

wendtek commented Nov 12, 2021

I made a PR to address the issues this causes for using /healthz here: #1896
I'm not sure how others feel about leaving the healthcheck endpoint unprotected, but it is a fairly standard practice in many other authenticated web services.

@jamengual
Copy link
Contributor

@fblgit the section https://www.runatlantis.io/docs/server-configuration.html was not updated with the new flags, could you please create another pr to add the new flags in that doc?

thanks.

krrrr38 pushed a commit to krrrr38/atlantis that referenced this pull request Dec 16, 2022
* Add BasicAuth Support to Atlantis ServeHTTP

* Added Security notes

Co-authored-by: xmurias <xmurias@gmail.com>
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.

None yet

7 participants