public
/
pyceo
27
1
Fork 3
Code Issues 25 Pull Requests 5 Projects Releases 1 Wiki Activity

Add web UI for password resets #123

Merged
merenber merged 9 commits from web into master 2024-03-23 19:26:33 -04:00
Conversation 2 Commits 9 Files Changed 37 +2270
merenber commented 2024-02-05 01:59:06 -05:00
Owner
Copy Link
No description provided.
merenber added 3 commits 2024-02-05 01:59:08 -05:00
continuous-integration/drone/pr Build is passing Details
31f5f950dd change cookie name
merenber added 1 commit 2024-02-12 00:56:52 -05:00
continuous-integration/drone/pr Build is passing Details
cab636dc35 add 'Return home' link to pwreset.html
n4chung reviewed 2024-02-12 01:06:16 -05:00
web/internal/views/base.html
@ -0,0 +14,4 @@
</header>
<main>{{ block "main" . }}{{ end }}</main>
<footer>
Copyright &copy; 2024 Computer Science Club of the University of Waterloo
n4chung commented 2024-02-12 01:06:16 -05:00
Owner
Copy Link

2024 might change later is fine?

2024 might change later is fine?
merenber commented 2024-02-18 10:29:13 -05:00
Author
Owner
Copy Link

I think it should be fine, but we can insert the current year if we want to.

I think it should be fine, but we can insert the current year if we want to.
n4chung marked this conversation as resolved
n4chung reviewed 2024-02-12 01:11:08 -05:00
web/tests/test.json
@ -0,0 +1,7 @@
{
n4chung commented 2024-02-12 01:11:08 -05:00
Owner
Copy Link

what's the difference between this and dev.json besides the port?

what's the difference between this and dev.json besides the port?
merenber commented 2024-02-18 10:30:12 -05:00
Author
Owner
Copy Link

The hostname needs to be exactly "127.0.0.1" in the test.json because that's httptest uses.

The hostname needs to be exactly "127.0.0.1" in the test.json because that's httptest uses.
👍 1
n4chung marked this conversation as resolved
n4chung reviewed 2024-02-12 01:11:53 -05:00
n4chung left a comment
Owner
Copy Link

most of the core logic, seems a bit harder to review unless I try testing it ;)

most of the core logic, seems a bit harder to review unless I try testing it ;)
merenber added 1 commit 2024-02-13 05:27:03 -05:00
continuous-integration/drone/pr Build is passing Details
68cb9d4600 use margin-top
n4chung commented 2024-03-03 17:54:50 -05:00
Owner
Copy Link

Just tested in a development environment.

However, I wasn't able to test the following situations:

  • user not logged in?
  • testing reset LDAP password

Some potential suggestions/considerations (in order of importance):

  1. should syscom-alerts be cc'ed on password resets? resets should be rather infrequent, so shouldn't be very "spammy" (hopefully); this would be nice for traceability
  2. notice on reset page home, stating that users could reset password using the passwd utility (if they could still access SSH)
  3. require exec 2fa code or some other form of validation/"sponsorship" when resetting accounts with staff group?
  • ie. another staff user could say this person is who they claim themselves to be
  1. "confirm action" button when user presses "Reset Password"
  2. update CSC main website (and wiki) about this new self-serve reset password feature
Just tested in a development environment. However, I wasn't able to test the following situations: - user not logged in? - testing reset LDAP password Some potential suggestions/considerations (in order of importance): 1. should syscom-alerts be cc'ed on password resets? resets should be rather infrequent, so shouldn't be very "spammy" (hopefully); this would be nice for traceability 2. notice on reset page home, stating that users could reset password using the `passwd` utility (if they could still access SSH) 3. require exec 2fa code or some other form of validation/"sponsorship" when resetting accounts with `staff` group? - ie. another `staff` user could say this person is who they claim themselves to be 4. "confirm action" button when user presses "Reset Password" 5. update CSC main website (and wiki) about this new *self-serve reset password* feature
merenber commented 2024-03-03 18:12:19 -05:00
Author
Owner
Copy Link

user not logged in?

This can be tested by updating the arguments to the proxy program to a user which does not exist, e.g. go run scripts/proxy.go -s app.sock -u jdoe -f John.

testing reset LDAP password

This can be tested by running the login program as root in the phosphoric-acid container, then logging in as ctdalek.

should syscom-alerts be cc'ed on password resets?

syscom-alerts probably shouldn't be CC'd, since the email contains the raw password, but I agree that we should get notified. I'll add this.

notice on reset page home, stating that users could reset password using the passwd utility (if they could still access SSH)

Good idea; I'll add this.

require exec 2fa code or some other form of validation/"sponsorship" when resetting accounts with staff group?

Hm ... I'm not sure how helpful that would be? The whole point of this website is to avoid the hassle of having to email syscom to reset one's password, so if somebody has to email a staff member, we lose the main benefit...

"confirm action" button when user presses "Reset Password"

I don't think this is necessary - the button itself is supposed to be for confirmation. The text above it also explains very clearly what it does.

> user not logged in? This can be tested by updating the arguments to the `proxy` program to a user which does not exist, e.g. `go run scripts/proxy.go -s app.sock -u jdoe -f John`. > testing reset LDAP password This can be tested by running the `login` program as root in the phosphoric-acid container, then logging in as ctdalek. > should syscom-alerts be cc'ed on password resets? syscom-alerts probably shouldn't be CC'd, since the email contains the raw password, but I agree that we should get notified. I'll add this. > notice on reset page home, stating that users could reset password using the passwd utility (if they could still access SSH) Good idea; I'll add this. > require exec 2fa code or some other form of validation/"sponsorship" when resetting accounts with staff group? Hm ... I'm not sure how helpful that would be? The whole point of this website is to avoid the hassle of having to email syscom to reset one's password, so if somebody has to email a staff member, we lose the main benefit... > "confirm action" button when user presses "Reset Password" I don't think this is necessary - the button itself is supposed to be for confirmation. The text above it also explains very clearly what it does.
merenber added 4 commits 2024-03-23 19:11:44 -04:00
continuous-integration/drone/pr Build is passing Details
0a74c04e84 add app_url documentation to README
merenber merged commit 7716f7bd10 into master 2024-03-23 19:26:33 -04:00
merenber deleted branch web 2024-03-23 19:26:33 -04:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
priority
high
High priority

  
priority
low
Low priority

  
priority
medium
Medium priority

  
priority
very high
Very high priority

  
BUG

   
Feature

   
High Priority

   
Low Priority

   
Medium Priority
No Label
priority
high
priority
low
priority
medium
priority
very high
BUG
Feature
High Priority
Low Priority
Medium Priority
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: public/pyceo#123
No description provided.
Delete Branch "web"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?