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

Feature/totp default admin #126

Merged
merged 4 commits into from
Jun 4, 2021
Merged

Feature/totp default admin #126

merged 4 commits into from
Jun 4, 2021

Conversation

agonbar
Copy link
Collaborator

@agonbar agonbar commented Jul 17, 2020

to:
cc: @subspacecommunity/subspace-maintainers
related to: #111
resolves: #104

Background

NEW: This fixes a typo in the last commited change, I'll copy all the contents from the original PR

Wanted to secure the default user of subspace with TOTP. Now it is possible to turn on TOTP for the default user by visiting /settings and scanning the QR code with your phone (or putting the secret key into something else)

Changes

  • Configure TOTP via /settings
  • Reset TOTP via /settings (only visible if TOTP is already configured)
  • Can only be done by an Admin (This means both SAML Signed in Admin and the default Admin)

Pics

Initial Setup of MFA
Setup MFA

Reset MFA (will then result in the above page)
reset totp

Sign in page for default admin account if TOTP has been saved
Sign in with MFA Configured

Testing

NEW: Run from a clean file, but still needs deep testing.

Ran this locally multiple times on my Droplet in Digital Ocean (i'm using rsync to sync changes to my box and then running docker-compose up --build -d whenever changes are made to go files. For changes related static files i simply specify --debug in the build step for go-bindata and mount web/ into the running container. go-bindata then reads these files live on each request :D)

@agonbar agonbar added the enhancement New feature or request label Jul 17, 2020
@jack1902 jack1902 requested review from jack1902, elliotwestlake and a team July 17, 2020 15:48
@jack1902
Copy link

Can you fix the commits to credit myself on this?

@agonbar agonbar force-pushed the feature/totp_default_admin branch 2 times, most recently from 09cce90 to c63437d Compare July 24, 2020 22:24
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@agonbar
Copy link
Collaborator Author

agonbar commented Jul 24, 2020

@jack1902 It should be already changed, maybe not the cleanest way, but I was on the phone. I also tested, the fix needs the email to be something, but that something can be any string, as soon as the user sets up the default account, its overwritten. I tested this branch on clean installs in my machine, in my server and in a raspberry pi and its working with clean installs, please, give it a try and tell me how it went.

@gavinelder
Copy link

Can this just be rebased onto master and PR'd from there ?

@agonbar
Copy link
Collaborator Author

agonbar commented Aug 14, 2020

It just needs to be reviewed by someone else in order to merge and close this once and for all. Or, seeing that there is a lot of traction lost, I'll wait until next week, if there is no new activity, force merge this and release a version (didn't want to but seems the only reasonable option), then move to the next PR.

Copy link

@jack1902 jack1902 left a comment

Choose a reason for hiding this comment

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

I no longer work on this project, so leaving this review to remove it from my list of PRs on Github

@gchamon
Copy link

gchamon commented May 4, 2021

@agonbar I applied this PR to my forked repo and have been using it without issues. Just so you know your work wasn't for nothing.

@gchamon gchamon requested a review from a team May 27, 2021 15:57
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 3, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 6 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@gchamon
Copy link

gchamon commented Jun 3, 2021

@agonbar @subspacecommunity/subspace-maintainers fixed the conflicts with version 1.4.1. This is ready to merge.

@agonbar
Copy link
Collaborator Author

agonbar commented Jun 4, 2021

I was waiting for someone to approve, as I'm the one credited in this PR so I was breaking the rule of 2 approvals, but, there it goes

@agonbar agonbar merged commit 728e0eb into master Jun 4, 2021
@agonbar agonbar deleted the feature/totp_default_admin branch June 4, 2021 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TOTP Support for the default Admin User
4 participants