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

Authentication API cleanup #13764

Merged
merged 12 commits into from Oct 23, 2017

Conversation

Projects
None yet
1 participant
@nijel
Member

nijel commented Oct 20, 2017

Cleanup authentication API, move some helper code from common.inc.php to authentication classes.

Before submitting pull request, please check that every commit:

  • Has proper Signed-Off-By
  • Has commit message which describes it
  • Is needed on it's own, if you have just minor fixes to previous commits, you can squash them
  • Any new functionality is covered by tests

@nijel nijel self-assigned this Oct 20, 2017

nijel added some commits Oct 20, 2017

Share base code for AuthenticationPlugin::authSetUser
Signed-off-by: Michal Čihař <michal@cihar.com>
Consistely use no return value for AuthenticationPlugin::authFails
It really does not return in the end, so make the docs consistent
with the code.

Signed-off-by: Michal Čihař <michal@cihar.com>
Rationalize AuthenticationPlugin API
Make the API more consistent, remove not needed auth prefix and stop
talking about advanced authentication which has been there about 10
years ago.

API changed:

- authCheck is now readCredentials
- authSetUser is now storeCredentials
- auth is now showLoginForm
- authFails is now showFailure
- storeUserCredentials is now rememberCredentials

Signed-off-by: Michal Čihař <michal@cihar.com>
Move authentication logic to AuthenticationPlugin
Issue #11731

Signed-off-by: Michal Čihař <michal@cihar.com>
Move allow/deny rules check to AuthenticationPlugin
Issue #11731

Signed-off-by: Michal Čihař <michal@cihar.com>
@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Oct 20, 2017

Codecov Report

Merging #13764 into master will increase coverage by 0.08%.
The diff coverage is 89.79%.

@@            Coverage Diff             @@
##           master   #13764      +/-   ##
==========================================
+ Coverage   53.78%   53.87%   +0.08%     
==========================================
  Files         485      485              
  Lines       82302    82290      -12     
==========================================
+ Hits        44268    44333      +65     
+ Misses      38034    37957      -77

codecov bot commented Oct 20, 2017

Codecov Report

Merging #13764 into master will increase coverage by 0.08%.
The diff coverage is 89.79%.

@@            Coverage Diff             @@
##           master   #13764      +/-   ##
==========================================
+ Coverage   53.78%   53.87%   +0.08%     
==========================================
  Files         485      485              
  Lines       82302    82290      -12     
==========================================
+ Hits        44268    44333      +65     
+ Misses      38034    37957      -77

nijel added some commits Oct 20, 2017

Pass failure reason to showFailure
This way we can avoid relying on global variables to check it.

Signed-off-by: Michal Čihař <michal@cihar.com>
Avoid using PHP_AUTH_USER and PHP_AUTH_PW globals for auth
Use object attributes to store the actual credentials and avoid
messing up with global variables.

Signed-off-by: Michal Čihař <michal@cihar.com>
Remove not needed access through globals
Signed-off-by: Michal Čihař <michal@cihar.com>
Add test for AuthenticationPlugin::authenticate
Signed-off-by: Michal Čihař <michal@cihar.com>
Add tests for AuthenticationPlugin::checkRules
Signed-off-by: Michal Čihař <michal@cihar.com>
Fix createIV documentation
Signed-off-by: Michal Čihař <michal@cihar.com>
Check return value from cookieDecrypt
This can return false and we should fail early once this happens.

Signed-off-by: Michal Čihař <michal@cihar.com>

@nijel nijel merged commit 41c55cc into phpmyadmin:master Oct 23, 2017

5 of 6 checks passed

codacy/pr Not so good... This pull request quality could be better.
Details
DCO All commits have a DCO sign-off from the author
Scrutinizer Analysis: 34 updated code elements – Tests: passed
Details
codecov/patch 89.79% of diff hit (target 53.78%)
Details
codecov/project 53.87% (+0.08%) compared to 01a5133
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@nijel nijel deleted the nijel:auth branch Oct 23, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment