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

Hand RepozeWho1AuthenticationPolicy.remember kwargs to repoze.who (master) #1249 #1254

Merged
merged 1 commit into from Mar 3, 2014

Conversation

rbu
Copy link
Contributor

@rbu rbu commented Mar 3, 2014

Documentation for pyramid.security.remember supports keyword arguments
to hand over to the authentication policy. However, when using
RepozeWho1AuthenticationPolicy, all of the kw were dropped in remember.

It is my understanding that with repoze.who, additional configuration
parameters shall be stored in the identity dictionary. In our case,
setting the max_age parameter to the authtkt identifier, would be done
using an identity {'repoze.who.userid':principal, 'max_age': 23}.

It seems sensible just to hand over kw through the identity dictionary
and all users to specify max_age or other parameters such as userdata.

…ons#1249

Documentation for pyramid.security.remember supports keyword arguments
to hand over to the authentication policy. However, when using
RepozeWho1AuthenticationPolicy, all of the kw were dropped in remember.

It is my understanding that with repoze.who, additional configuration
parameters shall be stored in the identity dictionary. In our case,
setting the max_age parameter to the authtkt identifier, would be done
using an identity {'repoze.who.userid':principal, 'max_age': 23}.

It seems sensible just to hand over kw through the identity dictionary
and all users to specify max_age or other parameters such as userdata.
@rbu
Copy link
Contributor Author

rbu commented Mar 3, 2014

for issue #1249

tseaver added a commit that referenced this pull request Mar 3, 2014
Hand RepozeWho1AuthenticationPolicy.remember kwargs to repoze.who (master) #1249
@tseaver tseaver merged commit 3063246 into Pylons:master Mar 3, 2014
@tseaver
Copy link
Member

tseaver commented Mar 3, 2014

Thank you!

@rbu
Copy link
Contributor Author

rbu commented Mar 4, 2014

@tseaver thanks for merging. Any particular reason you didn't merge #1255 as well?

@tseaver
Copy link
Member

tseaver commented Mar 4, 2014

I didn't notice they were separate PRs, actually. I will merge it now (Chris usually syncs up stuff before releasing from the branch, so it likely would have been caught).

@rbu
Copy link
Contributor Author

rbu commented Mar 9, 2014

Ok, I see. Will limit myself to PRs to master only next time. Thanks a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants