search

web2py / py4web

Other
258 stars 127 forks source link

Improvements to Auth #902

Closed kszys closed 2 months ago

kszys commented 2 months ago

When trying to migrate my old web2py projects into py4web, I run into a couple of issues with the new implementation of Auth in py4web.

Issue 1: While Template fixture in py4web allows for flexible choice of delimiters through a dedicated parameter, this does not currently extend to the templates used by the Auth. There, there is no possibility of choosing the delimeters.

Issue 2: Because the way the templates are rendered for Auth actions, there is no way to apply additional fixtures. This is an issue, if I generally use a layout.html template that expects certain variables. In regular templates I can use the Inject fixture to provide them, but there is no such mechanism for templates used by the Auth actions.

In this PR I propose two commits - one solving issue 1 and the second solving issue 2. They introduce only minimal changes to auth.py by adding additional parameters to the Auth.enable() function and then further modifying slightly how the template for the Auth action is used - explicitley calling Template() and providing the delimiters parameter. Additionally, more fixtures can be passed as well allowing to (for instance) inject addtional variables into the Auth templates.

mdipierro commented 2 months ago

I agree with your analysis of the problem. I have a slightly different solution (which I already merged in master but I am happy to discuss further). Basically passing a dict of template_args to the Auth(...template_args={}) constructor which is plumbed through the template instance. I also do not think we need optional since we can already pass extra fixtures via enable(... uses=()).

Please let me know if you agree/disagree.

kszys commented 2 months ago

Thanks Massimo for a quick reaction! I just tested the solution you merged, and it completely satisfies my needs. Your approach is probably even more elegant/flexible.

As for the enable(..., uses=()) - I completely missed it - this is why I invented yet another parameter 😊 To my defence - this is not very well documented 😇

So - sumarising - your changes ensure that all the functionality I needed is there - so just keep it as is 😄

mdipierro commented 2 months ago

Thanks for checking!