Skip to content

Implemented canLogIn check for authenticate method. FIXES #52 - #53

Open
bummzack wants to merge 1 commit into
colymba:masterfrom
bummzack:issue-52-canlogin
Open

Implemented canLogIn check for authenticate method. FIXES #52#53
bummzack wants to merge 1 commit into
colymba:masterfrom
bummzack:issue-52-canlogin

Conversation

@bummzack

Copy link
Copy Markdown
Contributor

The login method itself already checks canLogIn, deeply covered in MemberAuthenticator::authenticate, so only authenticate had to be fixed.

Added a test to RESTfulAPI_TokenAuthenticator_Test that checks both authentication methods (login & authenticate) for canLogIn.

Added tests to check `canLogIn` calls.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Would be cleaner to have this as a different file and rename the class to follow the same pattern as the other ones like RESTfulAPI_CanLogInExtension_Test.php

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's only being used for this test-case.. so moving it to an external file seems counter-intuitive as it makes understanding of the test more difficult. As for naming, feel free to rename it.

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