Skip to content

Enhancement: Add login failure detection#23

Merged
j-andrews7 merged 1 commit intoj-andrews7:masterfrom
esqew:login-failure-detection
Nov 8, 2022
Merged

Enhancement: Add login failure detection#23
j-andrews7 merged 1 commit intoj-andrews7:masterfrom
esqew:login-failure-detection

Conversation

@esqew
Copy link
Collaborator

@esqew esqew commented Aug 5, 2022

Logging into KenPom with MechanicalSoup is a bit wonky in that the service seems to always return 200 OK status code even on a credential-related failure. This leads to confusion like in #15 where one might assume a login was successful as no exception was thrown, but downstream encounters issues trying to pull data requiring a valid session.

On further inspection, it seems the difference the response for a successful login and an unsuccessful one (due to incorrect credentials) is that the Set-Cookie response header returns a PHPSESSID for successful logins, and cookie invalidation information otherwise. As such, I've updated utils.py to look for the string "PHPSESSID=" in the response header value for Set-Cookie and, in its absence, throw the Logging in to kenpom.com failed Exception. I've also added logic into tests/conftest.py to ensure that pytest aborts when the login fails (as most tests will begin failing without a successful login anyway).

If you can, I could use some help testing this in verifying that everyone does indeed get a Set-Cookie: "PHPSESSID=..." header on successful login before this gets merged in.

@j-andrews7
Copy link
Owner

Missed this, thanks for the PR. I'll take try to test soon.

@esqew esqew changed the title Add login failure detection Enhancement: Add login failure detection Nov 1, 2022
@j-andrews7 j-andrews7 merged commit 4cfcc57 into j-andrews7:master Nov 8, 2022
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.

2 participants