Closed gomezem closed 4 years ago
After pulling your branch and testing a couple of things, I noticed a few issues that relate to the overall authentication scheme that I'd like to correct. I went ahead and pushed a commit to your branch since I've tested the changes. I'll list them below:
Create Account
tab is only accessible to administrators, other users can still visit the register modal by the URL /registermodal.jsp. The permission checking logic should redirect the user back to the home page if they are not in the administrators group.After pulling your branch and testing a couple of things, I noticed a few issues that relate to the overall authentication scheme that I'd like to correct. I went ahead and pushed a commit to your branch since I've tested the changes. I'll list them below:
- Sign up modal placeholder and loading script should be removed from account.jsp and index.jsp. This prevents the register modal from being loaded in the background on every page since it should only be accessible by administrators.
- The previously mentioned sign up modal placeholder and loading script should be added to navbar.jsp. This ensures that administrators can create a new account no matter where they are at on the website. As long as the navbar is loaded, the register modal will be loaded, too.
- Permission checking logic should be added to registermodal.jsp. Currently, although the
Create Account
tab is only accessible to administrators, other users can still visit the register modal by the URL /registermodal.jsp. The permission checking logic should redirect the user back to the home page if they are not in the administrators group.
Thanks for looking into all this I really appreciate that! I thought there would be some backend stuff I missed with rearranging things.
One more thing..
I think the title for the register modal should be changed. Right now, it is listed as
Sign Up
. https://github.com/jake-hansen/nova/blob/e06d213c9da743999fc2e48c994b779b5f7a3007/novaweb/src/main/webapp/registermodal.jsp#L19Since only administrators have access to create accounts now, I think it should be changed to
Create Account
in order to match the link in the navbar. Also, the footer should be removed directing the user to login if they already have an account, since that doesn't make sense to have if an administrator is already logged in.
I'll change this right now thanks for noticing that
After I go through the process of creating an account, the "Create Account" option disappears from the navbar.
Nice catch Ben I'll take a look at that today
The strange functionality should be fixed now! Check it out and test it, please.
It's all working for me now!
This change will keep registering for a new user account only available for people who belong to the Administrator group. I created a new tab on the navbar named Create Account. I figured the functionality should have its own tab since it didn't make sense to put it inside the user account page which would display information regarding the current user's account.
This fix addresses #28
Before:
After: