Revamp remember/forget API

[luhn]

One wishlist item for Pyramid 2.0 is to revamp remember and forget in the authentication API. This issue is to continue discussion started in #3422.

@mmerickel says:

The second major goal I have is that remember / forget should be more useful for non-cookie-based authentication. I'd like to come up with some way to define them so they can return the token or body instead of only headers. For example a dict with headers, token, and body, content-type keys. There is not a standard way to construct a response using the token key, but headers or body+content-type could be used to define a response object.

[luhn]

Is it crazy to suggest that Pyramid doesn't enforce a forget/remember API? We could expose the identity policy to the user and allow it to provide whatever it thinks is best. e.g. implement pyramid.security.get_identity_policy(request) then the user could do get_identity_policy(request).remember(userid), which could do and return whatever the policy sees fit.

[mmerickel]

I don't think it's the responsibility of the policy directly but I would like remember/forget to handle session invalidation if a session factory is registered. I can't see an argument why it shouldn't do it, even if the user is already doing something else themselves. The idea would be that pyramid.security.remember() would invoke request.session.invalidate() and document it as such, so if there's anything the user wants to store on the session across the boundary they should pull it out and set it again after calling remember. Same with forget. The API would call it before invoking the auth policy's remember api so that any mutations to the session done by the auth policy would stick.

[luhn]

Invalidating the session is a bit draconian, but I can see it's important for security by default.

Do you still want remember/forget to return a dict?

[digitalresistor]

Invalidating the session is required for good security and to disallow the potential of session fixation. All of the projects I've built do this, and my pyramid_authsanity project does this implicitly when crossing a security boundary (logged out -> in and vice versa).

[mmerickel]

Current issues include:

1. Stateful api with better support for privilege escalation boundaries, vary headers, session invalidation, etc.

2. Support for bearer tokens and other non-header-based mechanisms in remember/forget.

[luhn]

So, taking another look at this and mulling over things, here's a brain dump:

I understand remember for headers/cookie/session and tokens, but what's the envisioned use for body+content-type? I can't think of what kind of body I'd want remember to generate for me.

To me return a dict of token/body/session still doesn't sit right. It's too many things, makes it unclear what remember is supposed to be. Different policies could have drastically different ways to use remember.

Also remember returning an access token doesn't seem right, because in that case its not remembering anything, that responsibility is now on the application. "remember" implies a modification of some persistent state, i.e. cookie/session.

That being said I like the idea of having a way to generate access tokens.

What about leaving remember as-is and adding a new method to the securitypolicy interface? generate_access_token(request, userid) or something like that. Returns a string that the application can take and do what it needs to, which works well

I think that also works well with the talk of remember+forget invalidating the session. Since remember+forget modify the state, it makes sense to wipe that state. But generate_access_token might be used for something like oauth, where you're handing the token off to another application and you definitely don't want to nuke the user's session.

Hopefully that's somewhat coherent and helpful.

[mmerickel]

I'm basically resolved to not changing this api because I can't think of anything that really solves the issue atm. The only "right" option imo is to return a response object and give the policy full control of login/logout. However I just don't like it! It's hard to test and often the responses should be more app-specific versus what a policy can do - like determining where to redirect (but that could be a kwarg to remember to help the policy construct the response). Anyone who wants to do app-specific things in their own policies atm can just tweak remember/forget to return whatever they want and be happy since pyramid itself doesn't consume the return value anywhere. I'd personally probably suggest to policy creators to support some form of kwarg like token = remember(request, userid, return_token=True) to be explicit about what return value you get. This is one reason I think we should add **kw to the forget(request, **kw) api.

[digitalresistor]

I'm okay with not changing the API, with the new policies I think it is easier than ever to create custom policies and thus change the response from remember/forget to do what users want to do.

[luhn]

Works for me.

Since the answer is to let remember/forget be whatever the user wants, will we remove "returns a list of headers" from the interface docs, or will that remain the "official" API?

[mmerickel]

I'm closing this. Please open a PR to tweak the docs if anyone wants to improve them a bit but I don't think we'll change pyramid's underlying impl.