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: multiple user/password authentication #55

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

alexng353
Copy link

Allows user to provide multiple user/password objects as a JSON object via the PROXY_CREDENTIALS environment variable.

[
	{ "username": "username1", "password": "password123" },
	{ "username": "username2", "password": "password456" }
]

Recommended to squoosh your JSON so that it's easier to copy and paste into a command prompt. You can also provide a .env file via the --env-file flag which makes providing this JSON object a little easier.

I am unsure if this is the best way to pass credentials, but given the string-only limitations of environment variables, this is the best I could think of.

fixes: #4

@serjs
Copy link
Owner

serjs commented Jan 21, 2024

Hi, @alexng353! Long living request 👍🏻 Thank for you contribution, lets talk about userlist fetaere before merge it. What about making yaml-based format and using ENV var for using userpass file or event config-file? Most moder schedullers can generate secretfiles or configs with generating secretfiles. We can do config file at all, with userlist section.

For scaling features scenario, in future, we can have one entrypoint file for different cases, I can contribute and finalize this idea.

@alexng353
Copy link
Author

alexng353 commented Feb 25, 2024

What about making yaml-based format and using ENV var for using userpass file or event config-file?

If you would like to keep this project working without a database, the best way is probably to use a config file. Personally, if you were to do a config file for this project, I would prefer to have as many possible formats supported as possible, TOML, YAML, JSON, Apple PKL, etc all supported and parsing into one struct at start time. I'm not sure what an event config file or userpass file means, but if you can give me more context, I'll gladly contribute more to this project.

Considering how this project is primarily docker-based, I would think that continuing to support passing a multi-user configuration through an environment variable is likely a good idea. I wanted to ask you if it would be a good idea to allow passing username-password combinations as follows:

CREDENTIALS=username:password;username2:password2

And then manually parsing the string. I opted to use JSON since I wanted to support as wide a range of characters as possible.

Sidenote: Don't mind the unverified commits; I can re-sign them if you want. I accidentally lost my old GPG RSA keys when my computer died a few weeks ago.

Edit1: What are schedullers, secretfiles or configs with generating secretfiles ? I've never worked with those things before, so please pardon my ignorance about the matter.

@serjs
Copy link
Owner

serjs commented Feb 26, 2024

Yea, i think CREDENTIALS=username:password;username2:password2 env var is good balanced option, still not optimized for users with large users datasets (untill they create side-generator for env var), but i liked it. Do you need help to rewrite multi-user feature for suggested one or you can create changes in this PR to complete feature?

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.

Multiples users passwords support
2 participants