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

Add simple HTTP authentication that can be used via configuration or ENV #171

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

Conversation

krtschmr
Copy link

@krtschmr krtschmr commented Oct 7, 2024

We needed a very simple easy way to add http_auth. inheriting from own controller seems overkill.

fixes #166

image

@rosa
Copy link
Member

rosa commented Oct 31, 2024

@krtschmr thanks for this! We need to have some default authentication built-in before we can propose this as default for Rails, but I'm not sure yet how it's going to look, so I'll keep this PR on hold until then.

@krtschmr
Copy link
Author

Hi @rosa i'm unsure what you mean with DefaultAuthentication.
Doesn't this act like it somehow?
right now everybody needs to implement their own logic, this would be a very quick way to at least have it protected from the start. (we use it in production already)

@rosa
Copy link
Member

rosa commented Oct 31, 2024

@krtschmr, I mean some authentication solution that comes enabled by default, so yes, this would work, but I'm not sure if we'll go with basic auth or with something else (such as a simple cookie-based authentication). It also needs to be closed by default in production and be easy to configure. I haven't decided yet what it'll look like.

ENV["MISSION_CONTROL_JOBS_HTTP_AUTH_USER"]=captain
ENV["MISSION_CONTROL_JOBS_HTTP_AUTH_password"]=topsecret
```
If no value is provided (`nil`, `""` or `false`), authentication is skipped.
Copy link
Member

Choose a reason for hiding this comment

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

Would you be up for changing this so that in production, if no configuration is set, it fails? The idea is that nobody could install Mission Control, forget to configure this, ship it to production and have this open to everyone by default.

Copy link
Author

Choose a reason for hiding this comment

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

Hi @rosa , that is indeed a fantastic situation.
So by default, mission-control is default secured by HTTP_AUTH unless you disable it explicitly, or roll out your own ControllerInheritance (then you're on your own anyways)

Copy link
Member

Choose a reason for hiding this comment

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

Yes, exactly! 🙏 Thank you so much!

@@ -4,7 +4,15 @@ class MissionControl::Jobs::ApplicationController < MissionControl::Jobs.base_co
include MissionControl::Jobs::ApplicationScoped, MissionControl::Jobs::NotFoundRedirections
include MissionControl::Jobs::AdapterFeatures

before_action :http_auth
Copy link
Member

Choose a reason for hiding this comment

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

I'd rename this to authenticate_by_basic_auth and then make it conditional on another property http_auth_enabled (similar to http_auth_user), which would default to true, just to disable it easily.

Copy link
Author

Choose a reason for hiding this comment

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

Will take care on it

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.

I want to contribute and implement http_auth but need guidance on best practice/syntax
2 participants