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

Settings for identity policy #309

Closed
faassen opened this Issue Apr 13, 2015 · 42 comments

Comments

Projects
None yet
3 participants
@faassen
Copy link
Member

faassen commented Apr 13, 2015

@href and @henri-hulski, prompted by @seyhbold, were having a discussion about how to pass settings into identity policies. morepath/more.itsdangerous#1

There were three general possibilities:

  • the identity policy evolves some kind of ad-hoc settings system, such as the cookie_settings method
    proposed initially. I think we should try to avoid this.
  • the identity policy is configured through the Morepath settings mechanism. It defines a section
    and various settings it inspects.
  • the identity policy is configured by passing parameters to its constructor. These parameters could be
    retrieved from Morepath settings if the application author chooses to do so.

The question is which of two latter approaches is most appropriate. The parameter to constructor approach is more general and simpler. Tests are easy to write. The settings system allows a standard way to configure an identity policy independently from the application. But the settings system is not always available (you need a Morepath lookup), and makes testing more difficult.

I introduced the Morepath settings originally when dealing with more.transaction. In the way that things are set up now, you have to use the settings mechanism to configure the transaction tween. There is no hook like for identity_policy where you can do without settings.

You can do both for identity policy: have an application class you inherit from that reads the settings the Morepath way and then passes them into a specific identity policy, and also a way to set up the identity policy by using the identity_policy directive directly. I am attracted to this approach.

This would imply we need to refactor more.transaction so that you have the option to use transaction_tween without relying on settings. Only when you subclass TransactionApp do you
get to use settings.

So perhaps the rule for an extension should be:

  • make available an API for those who want to use the directives themselves that doesn't rely on settings. (so the tween_factory directive, or the identity_policy directive). If no appropriate Morepath core directives exist to make this easy, this is a clue to us Morepath developers to make it exist.
  • optionally, also make available a base class that sets this up using Morepath's settings mechanism.

Please discuss!

@henri-hulski

This comment has been minimized.

Copy link
Member

henri-hulski commented Apr 13, 2015

Just an idea.

Is there a possibility to enhance the morepath settings mechanism, so you can use it also outside the app scope?
Just a unified way to achieve what you are proposing.

@href

This comment has been minimized.

Copy link
Member

href commented Apr 13, 2015

You can do both for identity policy: have an application class you inherit from that reads the settings the Morepath way and then passes them into a specific identity policy, and also a way to set up the identity policy by using the identity_policy directive directly. I am attracted to this approach.

I feel this is the best approach as well. I know already that I will use a customized IdentityPolicy based on more.itsdangerous and I don't need hand-holding, so I just inherit from the policy class and set it up.

Since it might be useful for others, I don't mind adding an application set up with the right directives/settings to more.itsdangerous though. If not for anything else it would serve as documentation.

As a rule I would agree that this separation is optional, however I would strongly encourage it!

@henri-hulski

This comment has been minimized.

Copy link
Member

henri-hulski commented Apr 13, 2015

Actually more.jwtauth not only provides an identity policy, but also a function to create the JWT auth header, which can be called on response to a successful login request as a callback function.
Most settings are used for both, decoding JWT for the identity policy and encoding JWT for creating the authentication header on login.
So I think in this case I need to use the morepath settings mechanism.

@faassen

This comment has been minimized.

Copy link
Member

faassen commented Apr 13, 2015

Could you elaborate on what you mean by 'outside the app scope'? If the requirement is to have settings information available no matter where you are in the code, the settings information has to reside in some kind of global singleton object. This inevitably makes the code more tightly coupled and makes testing more cumbersome. The Morepath settings mechanism actually does allow this, as Reg is set in implicit mode and it uses a single global singleton (the current lookup).

Could not the function to create the JWT auth header be passed into the identity policy? I can't judge this without understanding this function better -- what is this function and how it is used? Is it set_jwt_auth_header? Why is the identity policy remember not the right place to call this?

I'm trying to understand the doc string:

https://github.com/morepath/more.jwtauth/blob/master/more/jwtauth/main.py#L278

does this docstring miss a comma after the word 'processed'?

I think it makes sense from a factoring point of view to decouple at least your identity policy from the settings mechanism by letting people pass in these values by the constructor. Maybe there's a way to deal with set_jwt_auth_header too in this manner but I don't understand enough yet about what's going on to judge this. (shouldn't the README docs describe that it exists? shouldn't it be exposed through the __init__.py?)

@henri-hulski

This comment has been minimized.

Copy link
Member

henri-hulski commented Apr 13, 2015

Could you elaborate on what you mean by 'outside the app scope'?

I think this was just a naive idea and when I think more about it it makes not to much sense.

Could not the function to create the JWT auth header be passed into the identity policy? I can't judge this without understanding this function better -- what is this function and how it is used? Is it set_jwt_auth_header? Why is the identity policy remember not the right place to call this?

Yes it's set_jwt_auth_header and you use it on login to create the JWT token and send it to the client in the Authentication header.
After the client stores it in a local storage and send it with every request.
Then within the identity policy more.jwtauth validates the authenticity of the claimset using the signature which all is inside the token sent by the client.
The token is stored on the client not the server, so I think identity policy remember is not the right place.

I'm trying to understand the doc string:
https://github.com/morepath/more.jwtauth/blob/master/more/jwtauth/main.py#L278
does this docstring miss a comma after the word 'processed'?

Yes that's right.

(shouldn't the README docs describe that it exists? shouldn't it be exposed through the init.py?)

Sure, I still didn't write the Usage part of the README. So I will do it after we decide how to deal with this. And of cause it should be exposed (as I told I'm new to Python).

I think it makes sense from a factoring point of view to decouple at least your identity policy from the settings mechanism by letting people pass in these values by the constructor. Maybe there's a way to deal with set_jwt_auth_header too in this manner.

The settings make only sense if they are the same for the identity policy and set_jwt_auth_header. So I don't know how to achieve this.

@henri-hulski

This comment has been minimized.

Copy link
Member

henri-hulski commented Apr 13, 2015

You can find an example how login works in jwtauth/tests/test_jwtauth.py:

def test_login():
    config = morepath.setup()
    config.scan(more.jwtauth)

    class App(JwtApp):
        testing_config = config

    @App.setting_section(section="jwtauth")
    def get_jwtauth_settings():
        return {
            'master_secret': 'secret',
            'expiration_delta': None,
        }

    class Login(object):
        pass

    @App.path(model=Login, path='login')
    def get_login():
        return Login()

    @App.json(model=Login, request_method='POST')
    def login(self, request):
        username = request.POST['username']
        password = request.POST['password']
        if not user_has_password(username, password):
            raise HTTPProxyAuthenticationRequired('Invalid username/password')
        @request.after
        def set_auth_header(response):
            auth_header = more.jwtauth.main.set_jwt_auth_header(request, username)
            response.headers['Authorization'] = auth_header
        return {
            'username': username,
        }

    def user_has_password(username, password):
        return username == 'user' and password == 'password'

    config.commit()
    lookup = App().registry.lookup
    c = Client(App())
    r = c.post('/login', 'username=user&password=false', status=407)
    r = c.post('/login', 'username=not_exists&password=password', status=407)
    r = c.post('/login', 'username=user&password=password')

    assert r.json == {
        'username': 'user',
    }

    claims_set = {
        'sub': 'user'
    }
    expected_token = more.jwtauth.main.encode_jwt(claims_set, settings(lookup=lookup).jwtauth)
    assert r.headers['Authorization'] == '%s %s' % ('JWT', expected_token)

    authtype, token = r.headers['Authorization'].split(' ', 1)
    claims_set_decoded = more.jwtauth.main.decode_jwt(token, settings(lookup=lookup).jwtauth)

    assert more.jwtauth.main.get_userid(claims_set_decoded, settings(lookup=lookup).jwtauth) == 'user'
@faassen

This comment has been minimized.

Copy link
Member

faassen commented Apr 13, 2015

Yes it's set_jwt_auth_header and you use it on login to create the JWT token and send it to the client in
the Authentication header. After the client stores it in a local storage and send it with every request.

This sounds like what remember should do, but you seem to think it is not, so I must not be getting it. Why is that so? I mean, why not something like:

@request.after
def remember(response):
    identity = morepath.Identity(username, password=password)
    morepath.remember_identity(response, request, identity)

in login?

and if it is done by remember, then it would be easy to just get the information from self and leave the settings handling to the code that instantiates your policy.

@henri-hulski

This comment has been minimized.

Copy link
Member

henri-hulski commented Apr 13, 2015

So maybe I didn't understand what function has remember_identity. As I understand it now it's just for passing the identity to the response on login. That's right?
If it's like this, I should implement it this way, what makes it a lot easier to pass the settings.
(I think in this case the password should not be passed to morepath.Identity.)

@faassen

This comment has been minimized.

Copy link
Member

faassen commented Apr 13, 2015

Yes, the intent of remember is to do to the response what is needed to remember the login info. Glad that this seems to apply here! Hopefully this means we can organize things as I suggested.

@faassen

This comment has been minimized.

Copy link
Member

faassen commented Apr 13, 2015

And I think you are right about the password indeed.

@henri-hulski

This comment has been minimized.

Copy link
Member

henri-hulski commented Apr 13, 2015

I would propose to parse the settings to the identity policy as a dictionary, so we can use the same dictionary for direct parsing and for the morepath settings.

@faassen

This comment has been minimized.

Copy link
Member

faassen commented Apr 13, 2015

Parameters with defaults where they make sense and then ** when you have a dict should be okay?

@henri-hulski

This comment has been minimized.

Copy link
Member

henri-hulski commented Apr 13, 2015

Another question is the priority.
I don't know if there will be a use case where you inherit from the auth app and use morepath settings and after overwrite them by parsing different settings directly to the identity policy. I think then directly passed arguments should have priority.

And another one how to pass default values.
When I use **kwargs for the identity policy and after retrieve the default values for arguments which are not given from morepath settings it does't work when I use the identity policy outside of the app scope.
On the other hand, when I set the default values directly in the arguments of the identity policy they will overwrite the morepath settings.
Neither way is what we want.

@henri-hulski

This comment has been minimized.

Copy link
Member

henri-hulski commented Apr 13, 2015

One solution could be to give the morepath settings priority.
So if if we inherit from e.g. JWTApp the identity policy will always use the morepath settings and it's not possible to pass different settings directly.
Actually I think this would be ok and at the moment I have no other solution.

@faassen

This comment has been minimized.

Copy link
Member

faassen commented Apr 14, 2015

I don't understand your comment about priority: if you use settings you can override the settings in a subclass of the app, just like for anything else. I don't think we need something more.

There are two layers which we should separate:

  • the identity policy which should know nothing directly about Morepath settings at all. It just receives arguments to its constructors to configure it. These keyword arguments should have sensible defaults.
  • the integration code that defines a special app class that sets identity_policy and instantiates your identity policy. This retrieves information from the Morepath settings. It can do this as a dictionary and then use ** to apply it -- see for instance what I do in more.jinja2.

https://github.com/morepath/more.jinja2/blob/master/more/jinja2/main.py#L19

@henri-hulski

This comment has been minimized.

Copy link
Member

henri-hulski commented Apr 14, 2015

Thanks. This is the information I need. Switching the settings to arguments went smoothly, but I didn't find out who to handle now the morepath settings.

@faassen

This comment has been minimized.

Copy link
Member

faassen commented Apr 14, 2015

Hm, I just realized that the identity_policy directive does not get app as an argument, and it looks like it should, because otherwise there is no explicit way to retrieve the settings through app.registry.settings. (you can still implicitly retrieve the settings but framework code should not rely on this). I think the prepare of the IdentityPolicyDirective needs to be modified so it passes self.app into the obj() call there. But this will break compatibility with existing identity policies, unless we use Reg's abilities to inspect the function arguments first and only pass in app when we define such an argument.

Is anyone willing to look into making this modification? We want the existing tests to pass and have a new test that demonstrate that 'app.registry.settings' is indeed accessible at this point.

@henri-hulski

This comment has been minimized.

Copy link
Member

henri-hulski commented Apr 14, 2015

I think this is where I had a problem. When moving the settings to the
constructor where I don't have access to the request object, I was not able
to make a lookup. The app scope was not accessible.
Am 14.04.2015 11:45 schrieb "Martijn Faassen" notifications@github.com:

Hm, I just realized that the identity_policy directive does not get app
as an argument, and it looks like it should, because otherwise there is no
explicit way to retrieve the settings through app.registry.settings. (you
can still implicitly retrieve the settings but framework code should not
rely on this). I think the prepare of the IdentityPolicyDirective needs
to be modified so it passes self.app into the obj() call there. But this
will break compatibility with existing identity policies, unless we use
Reg's abilities to inspect the function arguments first and only pass in
app when we define such an argument.

Is anyone willing to look into making this modification? We want the
existing tests to pass and have a new test that demonstrate that
'app.registry.settings' is indeed accessible at this point.


Reply to this email directly or view it on GitHub
#309 (comment).

@href

This comment has been minimized.

Copy link
Member

href commented Apr 14, 2015

Is anyone willing to look into making this modification? We want the existing tests to pass and have a new test that demonstrate that 'app.registry.settings' is indeed accessible at this point.

I have to pass on this at the moment. It seems useful and not too hard to implement, but I have no need for it personally.

@faassen

This comment has been minimized.

Copy link
Member

faassen commented Apr 14, 2015

@henri-hulski Are you willing to give it a shot? Just adding the 'app' argument should be easy enough - you can use reg.mapply to make sure it stays backwards compatible. The trickiest bit is adjusting the docs and writing a new test for this.

@henri-hulski

This comment has been minimized.

Copy link
Member

henri-hulski commented Apr 14, 2015

Ok, i will try.
Am 14.04.2015 12:20 schrieb "Martijn Faassen" notifications@github.com:

@henri-hulski https://github.com/henri-hulski Are you willing to give
it a shot? Just adding the 'app' argument should be easy enough - you can
use reg.mapply to make sure it stays backwards compatible. The trickiest
bit is adjusting the docs and writing a new test for this.


Reply to this email directly or view it on GitHub
#309 (comment).

@faassen

This comment has been minimized.

Copy link
Member

faassen commented Apr 14, 2015

@henri-hulski, great! I'm there to help should you need feedback.

@henri-hulski

This comment has been minimized.

Copy link
Member

henri-hulski commented Apr 14, 2015

Should I add the app argument to IdentityPolicy and BasicAuthIdentityPolicy?
This will brake existing implementations of BasicAuthIdentityPolicy.

@henri-hulski

This comment has been minimized.

Copy link
Member

henri-hulski commented Apr 14, 2015

Just writing a test (part):

    @app.setting_section(section="test")
    def get_jwtauth_settings():
        return {'username': 'testidentity'}

    @app.identity_policy()
    def get_identity_policy(app):
        lookup = app.lookup
        test_settings = settings(lookup).test.__dict__.copy()
        return IdentityPolicy(**test_settings)

    class IdentityPolicy(object):
        def __init__(self, username):
            self.username = username

        def identify(self, request):
            return Identity(self.username)

        def remember(self, response, request, identity):
            pass

        def forget(self, response, request):
            pass

And I get this error:

morepath/config.py:534: in commit
    for action, obj in self.prepared():
morepath/config.py:505: in prepared
    for prepared, prepared_obj in action.prepare(obj):
morepath/directive.py:837: in prepare
    policy = mapply(obj, app=app)
src/reg/reg/mapply.py:24: in mapply
    return func(*args, **new_kw)
morepath/tests/test_security.py:560: in get_identity_policy
    test_settings = settings(lookup).test.__dict__.copy()
src/reg/reg/dispatch.py:54: in __call__
    lookup = get_lookup(kw)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

kw = {}

    def get_lookup(kw):
        """Find lookup to use.

        First inspects ``kw``, a dictionary of keyword arguments given for
        an argument called ``lookup``. If that cannot be found, fall back
        on a global ``implicit.lookup``. If no such lookup is available,
        raise a ``NoImplicitLookupError``.
        """
        lookup = kw.pop('lookup', implicit.lookup)
        if lookup is None:
            raise NoImplicitLookupError(
>               "Cannot lookup without explicit lookup argument "
                "because no implicit lookup was configured.")
E           NoImplicitLookupError: Cannot lookup without explicit lookup argument because no implicit lookup was configured.

src/reg/reg/dispatch.py:84: NoImplicitLookupError

This is IdentityPolicyDirective prepare():

    def prepare(self, obj):
        app = self.app
        policy = mapply(obj, app=app)
        yield app.function(generic.identify), policy.identify
        yield app.function(generic.remember_identity), policy.remember
        yield app.function(generic.forget_identity), policy.forget
@faassen

This comment has been minimized.

Copy link
Member

faassen commented Apr 15, 2015

It shouldn't break existing identity policies, as you'd have an app argument added to the function that returns the IdentityPolicy, not the policy itself.

@faassen

This comment has been minimized.

Copy link
Member

faassen commented Apr 15, 2015

Instead of writing this:
lookup = app.lookup test_settings = settings(lookup).test.__dict__.copy()

You can instead write this:

test_settings = app.registry.settings.test.__dict__.copy()

This is a more direct way to get at the settings.

@faassen

This comment has been minimized.

Copy link
Member

faassen commented Apr 15, 2015

Concerning your test code, it's a bit odd that the example setting you use is 'username'. I'd expected something more settingy as username is usually what you get from the browser, not from settings.

@faassen

This comment has been minimized.

Copy link
Member

faassen commented Apr 15, 2015

You cannot write:

   settings(lookup)

Lookup is special and not actually a declared argument. It's fished out of the keyword arguments if it's passed in, so you have to make this explicit:

    settings(lookup=lookup)
@henri-hulski

This comment has been minimized.

Copy link
Member

henri-hulski commented Apr 15, 2015

When using

    @App.setting_section(section="test")
    def get_jwtauth_settings():
        return {'username': 'testidentity'}

    @App.identity_policy()
    def get_identity_policy(app):
        test_settings = app.registry.settings.test.__dict__.copy()
        return IdentityPolicy(**test_settings)

I'm getting

    @App.identity_policy()
    def get_identity_policy(app):
>       test_settings = app().registry.settings.test.__dict__.copy()
E       AttributeError: 'SettingSectionContainer' object has no attribute 'test'

morepath/tests/test_security.py:553: AttributeError

Using

        lookup = app.lookup
        test_settings = settings(lookup=lookup).test.__dict__.copy()

I get

morepath/config.py:534: in commit
    for action, obj in self.prepared():
morepath/config.py:505: in prepared
    for prepared, prepared_obj in action.prepare(obj):
morepath/directive.py:837: in prepare
    policy = mapply(obj, app=app)
src/reg/reg/mapply.py:24: in mapply
    return func(*args, **new_kw)
morepath/tests/test_security.py:554: in get_identity_policy
    test_settings = settings(lookup=lookup).test.__dict__.copy()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <function settings at 0x7f4e7032daa0>, args = (), kw = {}, lookup = <morepath.reify.reify object at 0x7f4e6f90cf90>

    def __call__(self, *args, **kw):
        lookup = get_lookup(kw)
>       return lookup.call(self.wrapped_func, *args, **kw)
E       AttributeError: 'reify' object has no attribute 'call'

src/reg/reg/dispatch.py:55: AttributeError

(I will use some faked cryptic key instead of username.)

@faassen

This comment has been minimized.

Copy link
Member

faassen commented Apr 15, 2015

The second example with the reify error looks the same as the other one, perhaps a copy and paste error? Anyway, the 'reify' error indicates that we have the class not the instance passed in and try to get the 'lookup' from that. The lookup is a special property that uses the reify decorator which means it only gets called once, and after that the result is put in its place - this is done for performance reasons.

You can get to the real lookup even on the app class by doing App.registry.lookup. But more fundamentally the function to get the identity policy is executed during the configuration process, before it has been committed fully.

To avoid all this confusion let's pass in settings into the identity policy function, not app.

But more fundamentally I think your first error might mean that settings isn't actually fully baked yet at the time the prepare call executes. Unlike perform, prepare is executed before the settings are necessarily done. So this is a harder problem than I anticipated.

We could modify the IdentityPolicyDirective so that it actually does its stuff in perform. We can't then use the trick we use in prepare which essentially translates IdentityPolicyDirective into three FunctionDirectives. Instead, we'd have to use registry.register_function, which is what the function directive really does. This means you can't register the individual methods as functions separately anymore, nor would we see configuration conflicts, but for the identity policy that doesn't make sense anyway.

So you'd rewrite the prepare to a perform, which does three register_function calls for the given methods. In the perform you will have access to an app object that has settings available.

This is all getting a lot more involved than I anticipated though, so if you want to give up on this I will understand. We'll have this issue for someone else to pick up later.

On my wishlist is a project to rewrite and document the configuration engine given all the things I've learned so far, but who knows when that will happen. And it might still not make this issue go away.

@henri-hulski

This comment has been minimized.

Copy link
Member

henri-hulski commented Apr 15, 2015

I've corrected the code above.

To avoid all this confusion let's pass in settings into the identity policy function, not app.

I do not really understand, where this should happen.

So you'd rewrite the prepare to a perform, which does three register_function calls for the given methods. In the perform you will have access to an app object that has settings available.

I will work on. Actually I already read much of your source code related to this problem and it helps me to understand morepath more deeply.
So I will not yet give up. (Maybe later.)

@henri-hulski

This comment has been minimized.

Copy link
Member

henri-hulski commented Apr 15, 2015

You think like this it's ok:

    def identifier(self, registry):
        return self.app

    def perform(self, registry, obj):
        app = self.app
        policy = mapply(obj, app=app)
        registry.register_function(generic.identify, policy.identify)
        registry.register_function(generic.remember_identity, policy.remember)
        registry.register_function(generic.forget_identity, policy.forget)

I'm not sure about using self.app as identifier, but it seems to work.

@henri-hulski

This comment has been minimized.

Copy link
Member

henri-hulski commented Apr 15, 2015

After having another look I see, that you sometimes use

    def identifier(self, registry):
        return ()

If this is enough I will use it like this.

@faassen

This comment has been minimized.

Copy link
Member

faassen commented Apr 15, 2015

That looks good. Except I'm wondering whether 'app' here isn't a class instead of an instance. So I think it makes more sense to pass 'app.registry.settings' directly instead of the app class.

() for the identifier is enough for the identifier in this case, as you can have only a single identity policy per application.

@faassen

This comment has been minimized.

Copy link
Member

faassen commented Apr 15, 2015

Okay, thanks @henri-hulski, this was great work!

The one bit I'm wondering about is documentation for this pattern so that future implementers of identity policy know what to do with settings. This pattern of not using settings within the extension point directly but at the integration point can probably also be generalized to other configurations that may require settings -- more.transaction perhaps, possibly the template language integrations.

@henri-hulski

This comment has been minimized.

Copy link
Member

henri-hulski commented Apr 15, 2015

There should be some documentation. But I'm not sure where to put it.

@faassen

This comment has been minimized.

Copy link
Member

faassen commented Apr 15, 2015

Perhaps we should start with a sidebar in the security.rst document. Something that explains that if you want to use settings in the identity policy, this is how to do it with a small code sample just showing the identity_policy directive that extracts settings and passes it into a real identity policy (which we don't need to show).

@henri-hulski

This comment has been minimized.

Copy link
Member

henri-hulski commented Apr 16, 2015

I just finished the refactoring of more.jwtauth and uploaded it.
It's much cleaner now.
I also added an Introduction and Usage section to README.rst.
Details in CHANGES.txt.

There are two layers which we should separate:
[...]

  • the integration code that defines a special app class that sets identity_policy and instantiates your identity policy. This retrieves information from the Morepath settings. [...]

With the new feature we don't need to setup a special app anymore, as we can use the Morepath settings right away.

So somehow we achieved what was my initial idea at least for the identity policy.
To get rid of the App context and use Morepath settings without inheriting from an app.

Awesome!

@href

This comment has been minimized.

Copy link
Member

href commented Apr 17, 2015

Go Team Morepath!

@henri-hulski

This comment has been minimized.

Copy link
Member

henri-hulski commented Apr 17, 2015

Also interesting that we have here a clear lookup path for the settings through the app without passing it as an argument even in explicit mode.
Actually I still not entirely understand the lookup system, but I think that for the settings we don't need the flexibility to choose the lookup mechanism.
As I understand in morepath you use app.lookup and request.lookup but the settings should be rather bond to the current app.
Please tell me if I'm wrong.
Now with the new feature it seems, that we do exactly that. Binding the settings for the identity policy to the current app.

I wonder if something like this would be possible for the entire setting system or if there are use cases, where we need different lookup mechanism for settings.

@faassen

This comment has been minimized.

Copy link
Member

faassen commented Apr 4, 2016

I think the original point of the issue was resolved long ago so closing this one.

@faassen faassen closed this Apr 4, 2016

@faassen

This comment has been minimized.

Copy link
Member

faassen commented Apr 4, 2016

@henri-hulski if you're still interested open another issue discussing your idea for accessing settings through the app or request everywhere. It may indeed be possible to be explicit throughout, I'm not sure whether there are exceptions.

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