-
Notifications
You must be signed in to change notification settings - Fork 5
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
User login #196
User login #196
Conversation
@mbodeantor nice! I think this needs a docs update and ideally |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me!
@josh-chamberlain Docs are updated here: https://app.gitbook.com/o/-MXypK5ySzExtEzQU6se/s/-MXyolqTg_voOhFyAcr-/~/changes/464/api/endpoints/admin Here are the new tests I added for the new endpoints to try them out: data-sources-app/regular_api_checks.py Line 156 in e4c5577
|
Put this on hold so the front end work can be added here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've started looking into the client work for this @mbodeantor and just have a question for you on the password reset endpoint.
@mbodeantor Needed to make a few updates to |
* Add last cached value to data source query * test: update snapshots * fix(assets): favicon not displaying * feat(pages): update DataSourceStaticView Fix url breaking, add support for last archived, add button to go to Internet Archive * test(pages): update tests and snapshots Update test for DataSourceStaticView, update various snapshots * chore(lint): reformat files with lint errors using black * chore(lint): attempt re-formatting again with actual VS 'black' extension * chore: update .gitignore * refactor(util): update formatDate to return undefined for invalid date passed * test(util): update test for formatDate * get_data_source_by_id fix * chore(deps): add eslint-config from design-system * test(scripts): update test scripts * docs(readme): document lint/test script updates * refactor: update miscellaneous files with linting errors * test(snapshots): update outdated snapshots * ci(client): add client scripts to pull workflow * chore(config): remove stray eslint config from package.json * ci: move working-directory to top level defaults * ci: add cache-dependency-path to setup-node action * ci: add cache-dependency-path to all steps * chore(linting): remove extraneous rules in linting config update files per update * chore(scripts): update ci script to use proper flag * Revert "test(snapshots): update outdated snapshots" This reverts commit 386d09b. * test: update snapshots again * ci: use exact node version used locally * Revert "ci: use exact node version used locally" This reverts commit e1c7b73. * chore(deps): re-install deps with node v20 * ci: use node v20 * test: update snapshots * ci: add time zone setter to test script * feature: add tertiary button from design-system * test(pages): update DataSourceStaticView test * chore(deps): bump design-system -> 2.2.0 * fix: miscellaneous styling issues * test: update snapshots * chore(deps): bump design-system -> 2.3.0 * chore(cleanup): remove logs and miscellaneous updates * test: update snapshots * remove agencies join from archives endpoint, change url_status when updating broken url * update test columns * standardized error codes --------- Co-authored-by: kalenluciano <[email protected]> Co-authored-by: Joshua Graber <[email protected]> Co-authored-by: Joshua Graber <[email protected]> Co-authored-by: Marty Bode <[email protected]>
One other thing I'm wondering here @josh-chamberlain @mbodeantor (a next-week question 😅 ): how are we handling sessions? Are we handling this entirely client-side? Or do we want to use a time-bound access-token solution returned from the backend? FWIW, I vote for the latter, as it's much more secure (particularly since we'll eventually be adding the ability for users to modify records in the db). |
@joshuagraber @mbodeantor I advocate for the latter, too—it's what we've talked about, and supported by auth libraries if needed |
@josh-chamberlain I think it makes sense to use a trusted lib rather than building ourselves, but I defer to @mbodeantor on that. I pushed the initial |
Hey @mbodeantor I'm working on writing some tests for the change/reset password routes, and in the process I noticed that login is failing. Behavior is the same from browser or Request:
Response (
|
@joshuagraber can you verify SECRET_KEY is in your environment locally? I had this issue in GH tests but they're passing locally for me |
@mbodeantor Yes it is. Did that value change recently? |
No, but it wasn't being used recently. |
So does this need to be passed to the API somewhere? It's exported via the shell alias, but the client app doesn't access it in code right now.
Are you able to run the client app locally and log in via the browser with your current configuration? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
definitely good enough to merge into dev!
Adding password reset endpoint, splitting out queries into functions for unit tests, splitting out API key generation to its own endpoint
To test login:
signing up and / or logging in via the /login route
To test: navigate to /login and perform whichever operation. (Sign up a user, or you can use [email protected] | Password1! to log in)
Gating protected routes.
To test: If not logged in, navigate to /change-password (a blank page presently), and you should be redirected to /login. Log in, and you should be redirected back to /change-password.
Automatically refresh user's access tokens if they are active within 1 minute of current token expiration.
To test: Log in with a user. Wait 4 minutes. Click, press keys, or scroll anywhere in the UI (doesn't have to be a button or anything). Wait another minute and navigate to /change-password. You should not be redirected back to /login.
Automatically log a user out on user action when the access token has expired
To test: Log in with a user and navigate to /change-password. Wait 5+ minutes and click/press/scroll anywhere in the UI. You should be redirected to /login (if not on a protected route, there will be no navigation, so it won't interrupt users who are using /, /search, etc.
To test changing password:
/login
and log in with any user./change-password
To test reseting password
/login
, clickClick here to reset it
Click here to log in