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

Have handlers be @web.authenticated by default ? #389

Closed
Carreau opened this issue Jan 18, 2021 · 2 comments · Fixed by #1392
Closed

Have handlers be @web.authenticated by default ? #389

Carreau opened this issue Jan 18, 2021 · 2 comments · Fixed by #1392

Comments

@Carreau
Copy link
Contributor

Carreau commented Jan 18, 2021

I've seen many extension forgetting to put @web.authenticated on handlers;
I'm tempted to think that AuthenticatedFileHandler should use init_subclass – or whatever, peak at SUPPORTED_METHODS, and autowrap any handler in @web.authenticated unless the handler is marked with a specific @public decorator.

it's likely something like

def __init_subclass__(cls):
    for verb in cls.SUPPORTED_METHODS:
        meth = getattr(cls, verb, None):
		if meth and not getattr(meth, '_public', None):
           setattr(cls, verb, web.authenticated(meth))

Hard part is likely deprecation and detecting methods that are already in @web.authenticated, though that should be not too hard as it set the __wrapped__ attribute and wrapping twice with @web.authenticated should be no op.

I think from a security standpoint its a strict gain and likely a net decrease in code size as well (I can find just on this repo at least 44 mention of @web.authenticated.)

@echarles
Copy link
Member

Having a secured by default core server, as extensions, sound like a feature we need. We just need to clearly document that extensions will require authentication unless the add a @public decorator

@krassowski
Copy link
Collaborator

@Zsailer I see that you had a commit referencing this issue, is there an associated PR or was it referenced by accident?

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

Successfully merging a pull request may close this issue.

4 participants