Closed Bug 614716 Opened 14 years ago Closed 14 years ago

Make synccore.auth behave more like a library

Categories

(Cloud Services :: Server: Other, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: telliott, Assigned: tarek)

Details

(Whiteboard: [qa-])

Right now, just dropping our auth into a program is quite complex. It should work something like:

from synccore.auth import SyncAuth
auth = SyncAuth(config)

or, alternately (This approach seems less 'clean', though)

from synccore.auth import get_auth
auth = get_auth(config)

and then stuff like auth.validate_user(username, password) just works.

It would also be nice if passing in a config file was optional. If synccore has been set up and is working on a system, I'm likely just to want to use its own internal configuration. If config isn't passed in, it should look for /etc/mozilla-services/synccore.cfg, or whatever was used to set it up from the original make and use whatever's there. That way, I can make use of the functionality without having to know a single thing about the implementation.
Assignee: nobody → tarek
Yes, the current design makes the assumption that you use the auth through the  wsgi application factory the lib provides. 

I'll move the registry so all auth core backend gets registered without having to use the base wsgi application. That will be much better because it'll allow any Python script to use it.

But I don't think you'll want to use it in your web app this way. We need to make sure your app uses the wsgi app factory I provide in core. If it does you do not have do anything to initialize the auth.  (I feel ashamed there's no detailed doc yet for this, this is getting in the top of the pile next week)

Here's how it works:

The "set_app" function is a "make_app" factory that let you create a wsgi application that contains the initialised auth, and you do not need to write any code for this, just pass the URL rules and your controllers, like how I did in server-reg:

  http://hg.mozilla.org/services/server-reg/file/5bb3c32414ee/syncreg/wsgiapp.py

Then, the make_app() is ready to be used by Paste, and returns a valid wsgi application that contains an "authtool" variable you can use in your controllers.

See how I use it in my User controller:  http://hg.mozilla.org/services/server-reg/file/5bb3c32414ee/syncreg/controllers/user.py

To summarise, creating a web app with an access to auth is done by writing a wsgiapp file that:

1. defines controllers. Controllers are simple classes which methods receive a request object.
2. defines the URL mapping for the app - each entry defines a dispatch rule that will pass a request to a specific controller method.
3. uses the synccore.baseapp.set_app() factory to provide a "make_app" function, by passing to it the controllers and the URLs 

The factory creates the application instance automatically and uses for this its application class: SyncServerApp. For more complex apps, that need to initialise more things in the application, you can provide your own application class to set_app, as long as it inherits from SyncServerApp.

This is what I do for instance in server-storage, see : http://hg.mozilla.org/services/server-storage/file/427c0d3810b0/syncstorage/wsgiapp.py
 
The other thing I need to do is s/Sync/Services/g in server-core.
I guess I'm missing the benefit to the approach you outline above. It suggests that we require both wsgi and paste to use the auth library, and that seems like a problem. What if I want to write a non-wsgi script?

I'm not sure why a library class should need to know anything about the context in which it was called. If it's relying on things in the environment other than are being passed in via the constructor (+ the config file) or function paramaters, this seems to me to be a problem.
A ha! I've traced through all this, and I kind of understand what you're trying to do. It's pretty cool.

However, I think it's philosophically wrong, and here's why: /auth should be under /services, not under /synccore. 

It's not specific to sync, and if I want to use it with assumptions that don't match the sync ones, it's problematic. For example, your handling of routes, with the auth-required test as the last parameter, is great for sync, but problematic if I'm doing persistent authentication. Yes, I can subclass things to make it work, but that seems complicated - the barrier to writing applications on top of our auth system becomes high.
(In reply to comment #4)
> I guess I'm missing the benefit to the approach you outline above. It suggests
> that we require both wsgi and paste to use the auth library, and that seems
> like a problem. 

I think you've missed the goal of baseapp. It provides you a base wsgi application with an initialized auth. You could do a plain wsgi/paste application from scratch if you want, without using it. But if your application is an application that needs to do auth on services, then you'd end up doing something similar in your wsgiapp. That's the whole point of baseapp: avoiding to write that boiler plate code. your wsgi app is then reduced to defining your URL mappings and listing your controllers.

> What if I want to write a non-wsgi script?

That was your first request, and I answered to it by moving the registry. Look at my changes. You can now instanciate in you script the auth with a single call.

> I'm not sure why a library class should need to know anything about the context
> in which it was called. If it's relying on things in the environment other than
> are being passed in via the constructor (+ the config file) or function
> paramaters, this seems to me to be a problem.

I don't understand that part. 

Synccore is two main things:

1- auth backends
2- a base wsgi application

If you write a wsgi app with auth, you only require to use 2.
(In reply to comment #5)
> A ha! I've traced through all this, and I kind of understand what you're trying
> to do. It's pretty cool.
> 
> However, I think it's philosophically wrong, and here's why: /auth should be
> under /services, not under /synccore. 

I need to do some renaming. synccore is a bad name, should be services for all modules. There are also a few occurrences of Weave left I'll wipe.

Improving the auth-required part is next.
s/synccore/services done.

So, here's a change I am planning to do to make the authentication story simpler for your app:

The authentication part at the wsgi level will be splitted in its own class, and will look like this:

class Authentication(object):

    def __init__(self, config):
        self.config = config
        self.backend = get_auth(self.config)

    def check_request(self, request, match):
        do stuff on the request + match, like:
        - check if the request/match needs to be auth
        - call self.backend if it needs to perform an auth
        - raise Unauthorized or redirect to a login screen
        - check session data, or simply return directly...
        - etc

server-core provides a default implementation for that auth class, that instantiates and uses the auth backend, given a config, and just raises a 401 when it should. That's good enough for Sync.

The base application in server-sync instanciates that tool and uses it like this:


  class Server(object):
    
    def __init__(self, ..., auth_class=Authentication, ...):
        ... reads the config...
        self.auth = Authentication(self.config)
  
    @wsgify
    def __call__(self, request):
        ... perform the URL matching..
        self.auth.check_request(request, match)
   
        ... do the actual controller call
        return controller.method(request)


And set_app (the make_app factory) will let you define your own auth class. 
So for you app, wsgiapp.py should look like this:

  from services.baseapp import set_app, Authentication

  controllers = [my controllers..]
  urls = [myurls...]
  
  class MyAuth(Authentication):
      def check_request(self, request, match):
          ... do you cooking...
  
  make_app = set_app(urls, controllers, auth_class=MyAuth)
         

This way, you can just focus on your controllers code, and on your authentication process
Actually, I really liked where it was on Friday. It was easy for me to grab an auth class in my controller and use it however I needed it where necessary.
Well, you can still do this so I guess it's up to you :)

I'll still build this so I can use it in server-openid.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.