codeforpdx / PASS

PASS project - with HMIS module integration
MIT License
24 stars 24 forks source link

464 - Simplify Signup Flow #584

Closed timbot1789 closed 4 months ago

timbot1789 commented 4 months ago

This PR:

Resolves #464

1. Rebuilds the user registration logic using community solid server V7's automation API. This allows us to build out a user's entire pod with no action needed from the user other than username and password. This system only works on Community Solid Server V7, but that has been the case. 2. Adds a couple more states to the registration page. Now the page can show: creation form, loading page, and finish page. A future PR will need to add an error page for if there's an error in pod creation 3. Added a signup button to the home screen that takes a new user to the signup process. This won't set them up with a case manager, but it will make new volunteer onboarding significantly easier. 4. Added some code to outline the IDP logout feature that comes with inrupt's V2 client auth libraries (allows logout from both pod and app in one click). However, this is proving complicated, especially to deploy, so finishing the feature will be left to another PR. I don't want the code to sit on my computer forever, though.

Future Steps/PRs Needed to Finish This Work (optional):

We will need to improve the error handling of the signup flow in a future PR. This PR only implements the happy path. It doesn't handle any errors during pod creation.

leekahung commented 4 months ago

I've cleared out my local_temp_server_files directory to test out the new build before diving into the codebase changes. The new signup functionality seems to be running fine, I see the new pods being created from the process. One small issue I've saw pop-up was the webId not being shown (see screenshot).

Screenshot 2024-02-25 at 4 37 17 PM

I've made a few more accounts to test things out, but it looks like the problem above was related to who logged in last as it show the previous logged in user's webId (see clips).

(Previous user logged in was http://localhost:3000/newtestuser/profile/card#me)

https://github.com/codeforpdx/PASS/assets/14917816/b36ba706-c738-4f1b-ade1-80c301949ce7

(Previous user logged in was http://localhost:3000/testpod/profile/card#me)

https://github.com/codeforpdx/PASS/assets/14917816/ff3aac94-18b6-4503-adcb-40b5892b1c93

If the server is fresh and no one had logged in last, it makes sense that the webId was blank for the first pod creation, but it should have been the webId of the user creating the webId, not the previous logged in user.

Another thing I've noticed when signing up using that button was that the .acl file for /PASS/Documents failed to generate for some of the new users. It returns a 409 error instead when logging in for the first time. Without it, we can't control the permissions for "Documents".

Screenshot 2024-02-25 at 4 39 04 PM

If we were to re-login (logout and login again), the .acl file gets generated as it should if the file was missing (see screenshot), but the function for generating the .acl file should have been running correctly the first time around, so I'm not sure what happened there.

Screenshot 2024-02-25 at 4 43 28 PM

If I were to signup for a pod using the signup forms by CSS instead, the 409 error doesn't seem to appear at all (see screenshot). The .acl file gets generated the first time around.

Screenshot 2024-02-25 at 4 45 41 PM

Something not related to this PR (but an earlier one), is that user deletion fails to trigger when the first and last name are null. I'll be posting a separate bug report for that (see issue #585).

Other than the issues I've seen above, all other functionalities seem to be work correctly like document uploads/preview/deletion, adding new contacts, sending messages, etc. Sharing documents still work as long as the .acl file exists within /PASS/Documents/ inside the user's pod

timbot1789 commented 4 months ago

@leekahung Thanks for the thorough testing. Fixed the web ID display and the pod URL display (it was broken too).

Added the doclist to pod initialization. It's an empty document. The ACL doesn't need to be created at this time. The doclist inherits permissions from the PASS pod, and the document sharing feature creates an acl if one doesn't already exist.

leekahung commented 4 months ago

@leekahung Thanks for the thorough testing. Fixed the web ID display and the pod URL display (it was broken too).

Added the doclist to pod initialization. It's an empty document. The ACL doesn't need to be created at this time. The doclist inherits permissions from the PASS pod, and the document sharing feature creates an acl if one doesn't already exist.

All good, I'll give it another go through post changes.

leekahung commented 4 months ago

Alright, just gone through the changes. The previous errors I was getting before for the features are now gone. So things are looking good there. I'll take a look over the code.

timbot1789 commented 4 months ago

Outside of two minor syntax comments, the rest of the code looks good to me!

It's not too important now for development, but for the sign-up process, I was wondering if we should give a disclaimer somewhere pointing out when this feature could be used since it's limited to just servers with CSS v7 vs. when users should just use the login button to get to the sign-up page for their respective servers like with solidcommunity.net's server or Inrupt's PodSpaces to make their Pods.

With the current design, the user doesn't get to choose their pod provider during pod creation flow. They're required to use the pod provider configured in the running PASS instance. In that case, the disclaimer would need to be sysadmin/dev facing. "You can only connect this part of the app to a CSS v7 server." I've added disclaimers in the comments of the registration function code, which is sufficient unless we want to write a sysadmin guide in the wiki.

You're right though that we need to display the Pod Provider that PASS signed the user up with at the end. I added a line to the last page that shows the URL of the pod provider.

We can also add an option to let a user enroll with an existing pod. But that's a separate flow where we can expect the user to be able to handle pod creation without our help, so no disclaimer is needed. I just added an issue to the project for it. I had this feature in a demo a while back, but haven't written up tests for it yet.