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

Magic link support (for feeds etc.) #282

Closed
fluffy-critter opened this issue Sep 3, 2019 · 4 comments

Comments

@fluffy-critter
Copy link
Collaborator

@fluffy-critter fluffy-critter commented Sep 3, 2019

Expected Behavior

If a link-generating callable is given an argument like auth=True and there's a current user, it should add a session-granting request argument.

If an incoming link has the session-granting request argument, it should, for that request only, act as if the user is the one specified in the argument.

Possible Solution

The argument should probably be generated in a similar way to how Flask's signed cookies are generated, but the signature should be based on the unsigned version of the URL to avoid trivial replay attacks.

tl;dr use macaroons

Context

Would make it much easier to make friends-only feeds, and could conceivably also be a bridge to add friends-only WebSub.

e.g. <link rel="alternate" type="application/atom+xml" href="{{category.link(template='feed',auth=True)}}"> -> atom link which https://example.com/feed?_s=encodeduser.hmac

@fluffy-critter

This comment has been minimized.

Copy link
Collaborator Author

@fluffy-critter fluffy-critter commented Oct 25, 2019

macaroons are pretty heavyweight (and large!) for this. So, I'm thinking that it would make more sense to just HMAC-sign after all.

It should be fairly straightforward to just use itsdangerous to handle the signing and verification; generating an authorized link would be something like:

return Signer('secret-key').sign(entry.link(_s=user.id))

Tricky bit is figuring out if the URL is signed, and getting Flask to re-parse the URL after verifying the signature, which might actually require setting a new request object in the context or something. (There's no way to guarantee that the _s query argument comes last, after all. /blog/feed?u=bob&tag=cats for example)

Alternately: generate the URL without the _s, then append the _s ourselves with e.g.:

# sign the URI
uri = blahblahblah
if kwargs.get('auth'):
    uri += ('&' if '?' in uri else '?') + '_s=' + itsdangerous.base64_encode(user.id)
    uri = Signer('secret-key').sign(uri)

# and now in user.get_active, before checking the session
if '_s' in request.args:
    user_id = itsdangerous.base64_decode(request.args['_s'].split(.)[0]
    try:
        uri = Signer('secret-key').unsign(request.uri) # will throw if the signature fails
        return userid
    except itsdangerous.BadSignature:
        LOGGER.warning('got bad signature for user %s', user_id)
# continue on with session path
@fluffy-critter

This comment has been minimized.

Copy link
Collaborator Author

@fluffy-critter fluffy-critter commented Oct 25, 2019

for the signing side of things, maybe do utils.url_for which checks for (and removes) the auth parameter in kwargs and passes the rest to flask.url_for, like:

def url_for(endpoint, auth=False, *args, **kwargs):
    uri = flask.url_for(endpoint, *args, **kwargs)
    if auth:
        uri += ...etc...
    return uri

and then use utils.url_for everywhere that currently gets flask.url_for

@fluffy-critter

This comment has been minimized.

Copy link
Collaborator Author

@fluffy-critter fluffy-critter commented Oct 25, 2019

the number of places where url_for would make sense to take that is very small; better to just have like user.sign_url that signs a URL if there's an active user and is explicitly called from the few places where it makes sense (entry.link, entry.archive, category.link, view.link. This also makes documentation easier.

@fluffy-critter

This comment has been minimized.

Copy link
Collaborator Author

@fluffy-critter fluffy-critter commented Oct 25, 2019

But no matter how good the implementation, the risk of a private feed leaking is way too high. Sharing a non-private entry in a preauthorized atom feed will also share the feed, regardless if user behavior or intent.

This is not a reasonable path forward. Shared cookie jars are the short term solution and AutoAuth is the path forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.