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

Fix SIP Quickstart login issues #330

Open
wants to merge 27 commits into
base: main
Choose a base branch
from
Open

Conversation

bld010
Copy link
Contributor

@bld010 bld010 commented Feb 15, 2022

Description

This should resolve issues #245 and #295

Admin password requirements are made obvious and require a change from "default" password before accessing the rest of the admin area.
Show and hide login errors on DOM.

Checklist

  • I ran npm test locally and it passed without errors.
  • I acknowledge that all my contributions will be made under the project's license.

Related issues

#245 and #295

@bld010 bld010 marked this pull request as draft February 23, 2022 02:14
@bld010 bld010 marked this pull request as ready for review April 12, 2022 22:14
@bld010
Copy link
Contributor Author

bld010 commented Apr 13, 2022

Admin Page Error Handling/Messaging:

CLI / Serverless plugin messaging:

Password not changed from default:

Screen Shot 2022-04-13 at 1 37 37 PM

Password doesn't meet complexity requirements

Screen Shot 2022-04-13 at 1 38 20 PM

Functions / Console messaging:

Password not changed from default

Screen Shot 2022-04-13 at 1 40 25 PM

Password doesn't meet complexity requirements

Screen Shot 2022-04-13 at 1 41 22 PM

Incorrect Password Error (403: Not authorized)

Screen Shot 2022-04-13 at 1 36 46 PM

Copy link
Contributor

@craigsdennis craigsdennis left a comment

Choose a reason for hiding this comment

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

Just some tiny tweaks! Thank you so much for diving into this! Awesome work! 🎉

sip-quickstart/assets/admin/shared.private.js Outdated Show resolved Hide resolved
sip-quickstart/assets/admin/admin-client.js Show resolved Hide resolved
sip-quickstart/assets/admin/admin-client.js Outdated Show resolved Hide resolved
sip-quickstart/assets/admin/shared.private.js Outdated Show resolved Hide resolved
sip-quickstart/assets/admin/admin-client.js Show resolved Hide resolved
sip-quickstart/assets/admin/index.html Outdated Show resolved Hide resolved
@bld010
Copy link
Contributor Author

bld010 commented May 3, 2022

@craigsdennis Ok! Made changes from your suggestions + added tests to add coverage to new admin-password-related functions!

@cmsunu28
Copy link
Contributor

Hey @bld010 ! We're in the process of going through the backlog of PRs. Did you still need to merge this one? Looks like there's a conflict in package-lock.json.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants