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

Authentication refactoring #582

Merged
merged 8 commits into from
Dec 8, 2014
Merged

Authentication refactoring #582

merged 8 commits into from
Dec 8, 2014

Conversation

evert
Copy link
Member

@evert evert commented Dec 7, 2014

This PR has the following changes:

  1. The authentication plugin now supports multiple cooperative authentication backends. This allows for scenarios where for example both Basic as well as OAuth is supported.
  2. The authentication backends no longer have state. They act purely on incoming Request and Response objects.
  3. The authentication plugins and backend now provide information about the currently logged in principal. This was before in a very clumsy way handled by the ACL plugin, but this caused a lot of issues. The side-effect is that even for sabredav servers that don't use RFC3744 (most fileservers) the terminology 'principal' creeps in. This could be a bit confusing at first for people who write their own authentication backends.

Fixes #191

*/
function authenticate(DAV\Server $server, $realm) {
function check(RequestInterface $request, ResponseInterface $response) {
Copy link
Member

Choose a reason for hiding this comment

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

New methods in this class miss visibility indicator

Copy link
Member

Choose a reason for hiding this comment

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

Should be used in all subclasses but the interface

Copy link
Member Author

Choose a reason for hiding this comment

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

@staabm I've actually removed public everywhere in all the master branches.

I think when public/private/protected was a thing for the first time when PHP 5.0 first came out, I wanted to specify it everywhere.. but now I find that it looks cleaner and clearer without :P The entire public keyword is pretty much has no purpose :)

evert added a commit that referenced this pull request Dec 8, 2014
Authentication refactoring
@evert evert merged commit 231d0a8 into master Dec 8, 2014
@@ -17,7 +17,7 @@
"php": ">=5.4.1",
"sabre/vobject": "~3.3.4",
"sabre/event" : "~2.0.0",
"sabre/http" : "~3.0.0",
"sabre/http" : "dev-master",
Copy link
Member

Choose a reason for hiding this comment

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

Why?

Copy link
Member Author

Choose a reason for hiding this comment

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

Needed the latest (unreleased) bugfix from that branch. Will switch back to a stable version as soon as a release happened on that side.

Copy link
Member

Choose a reason for hiding this comment

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

Ok.

@evert evert deleted the multi-auth branch October 20, 2015 21:10
tcitworld added a commit to nextcloud/server that referenced this pull request Feb 14, 2022
Since Sabre 3.0.6 this is no longer possible.

@see sabre-io/dav#582

Signed-off-by: Thomas Citharel <tcit@tcit.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support multiple authentication backends
3 participants