search

harleylang / hydroxide-docker

Docker configuration examples for the hydroxide package
MIT License
15 stars 3 forks source link

Added two factor authentication and user security #7

Closed arichtman closed 3 years ago

arichtman commented 3 years ago

Apologies for the large PR. Happy to discuss, explain, negotiate any changes and decisions.

harleylang commented 3 years ago

@arichtman thank you for your pull request! I reviewed the list of changes, those are all awesome additions, really appreciate all the effort that went into making these changes happen :smiley: I will plan to review in more detail early this coming week and get back to you with any questions / feedback. Thanks again, these additions will greatly enhance this repo!!

harleylang commented 3 years ago

@arichtman Thanks again for your pull request. I owe you an apology for being tardy in returning a review, esp. since the quality of your PR is so high. I've been wrapped up in other coding projects and work, gah! Anyways, I took a quick look, and all of your commits look great. I want a chance to test before merging. I'll aim to do that over the weekend and get back to you soon!

arichtman commented 3 years ago

It's fine I'm still finding time to sort reverse proxy anyhow

harleylang commented 3 years ago

Alright, I gave this spin without 2FA. I am following the updated readme.md and performed the following operations:

1) I updated .env with a fresh protonmail username / password pair. I changed the 2FA token so that it is blank. 2) From the project root, I ran: source .env 3) Then I ran docker build . --tag hydroxide:${HYDROXIDE_VERSION} --tag hydroxide:latest After build, I tried: 4) Running as per the docs:

docker run \
  --volume "/opt/hydro:/home/hydroxide/.config/hydroxide" --user=1000:1000 \
  --env USERNAME="${PROTONMAIL_LOGON}" --env PASSWORD="${PROTONMAIL_PASSWORD}" --env TOKEN="${PROTONMAIL_EXTRA_2FA}" --env DEBUG="${HYDROXIDE_DEBUG}" --env INTERPOD="${HYDROXIDE_DEBUG}" \
  --network host \
  --name hydroxide -d \
  "hydroxide:${HYDROXIDE_VERSION}"

I received a hash, but the container exited after 2-3 seconds on each of my tests.

I may have missed a config option above, and if so let me know and I'll re-test. In either case, I think we need to provide a way to test what is up with hydroxide so we can easily debug additional features that are added to this repo.

5) In a further attempt to debug the above, I attempted the following command:

docker run -it "hydroxide:${HYDROXIDE_VERSION}" --env USERNAME="${PROTONMAIL_LOGON}" --env PASSWORD="${PROTONMAIL_PASSWORD}"

And received the following error:

harley@desktop:~/Development/hydroxide-docker$ docker run -it "hydroxide:${HYDROXIDE_VERSION}" --env USERNAME="${PROTONMAIL_LOGON}" --env PASSWORD="${PROTONMAIL_PASSWORD}"
Incorrect argument count.
Please provide:
1) Username
2) Password
3) Two factor token [Optional]

Hmm. If I follow, it looks like my image did not generate a auth.json file. I'll look into this a little bit more this coming week, but if you see a clear error in my reproduction, let me know and I'll test!

arichtman commented 3 years ago

Thanks for testing and providing plenty of details. I'll disable 2FA on my account, do a restart and a fresh clone and see if I can resolve the issues. Likely next weekend or later though

harleylang commented 3 years ago

@arichtman awesome, and no rush! I really appreciate having your eyes on this repo, and look forward to seeing your changes.

I'm not sure if this helps at all, but if there is any value to pairing up to work through these changes (even if having a human sounding board would at all help!), let me know and I can try to make some time to meet.

harleylang commented 3 years ago

Hey @arichtman :wave:

What are your thoughts on next steps?

I've been thinking on this. This PR tackles a lot. Are you interested in breaking this PR down into separate branches and making these changes incrementally? This may enable us to more quickly test and roll out these changes.

Let me know either way, it would be cool to include these updates!

arichtman commented 3 years ago

I agree, it'll be easier to manage if broken down.